From 72cabcab1cfb72c379c97e7d77f0e94b0698ff44 Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Wed, 8 Dec 2021 12:26:00 +0530 Subject: [PATCH 1/6] Making span as internal in presence of a span in current context --- CHANGELOG.md | 3 ++ .../instrumentation/django/middleware.py | 1 - .../instrumentation/flask/__init__.py | 20 +++++++++---- .../tests/test_programmatic.py | 29 +++++++++++++++++++ 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46f42fd58b..fbdafee17b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-tornado` Add support instrumentation for Tornado 5.1.1 ([#812](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/812)) + +- `opentelemetry-instrumentation-flask` Flask: Conditionally create SERVER spans + ([#827](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/827)) ## [1.7.1-0.26b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.7.0-0.26b0) - 2021-11-11 diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py index 91af787c28..9cdc7e682c 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -58,7 +58,6 @@ def __call__(self, request): response = self.get_response(request) return self.process_response(request, response) - else: # Django versions 1.x can use `settings.MIDDLEWARE_CLASSES` and expect # old-style middlewares, which are created by inheriting from diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index e96b005acd..ccf8581708 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -176,15 +176,23 @@ def _before_request(): return flask_request_environ = flask.request.environ span_name = get_default_span_name() - token = context.attach( - extract(flask_request_environ, getter=otel_wsgi.wsgi_getter) - ) + + token = ctx = None + span_kind = trace.SpanKind.INTERNAL + + if trace.get_current_span() is trace.INVALID_SPAN: + ctx = extract(flask_request_environ, getter=otel_wsgi.wsgi_getter) + print(ctx) + token = context.attach(ctx) + span_kind = trace.SpanKind.SERVER span = tracer.start_span( span_name, - kind=trace.SpanKind.SERVER, + ctx, + kind=span_kind, start_time=flask_request_environ.get(_ENVIRON_STARTTIME_KEY), ) + if request_hook: request_hook(span, flask_request_environ) @@ -229,7 +237,9 @@ def _teardown_request(exc): activation.__exit__( type(exc), exc, getattr(exc, "__traceback__", None) ) - context.detach(flask.request.environ.get(_ENVIRON_TOKEN)) + + if flask.request.environ.get(_ENVIRON_TOKEN, None) is not None: + context.detach(flask.request.environ.get(_ENVIRON_TOKEN)) return _teardown_request diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 50dd188af1..a3064b52e5 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -23,6 +23,7 @@ get_global_response_propagator, set_global_response_propagator, ) +from opentelemetry.instrumentation.wsgi import OpenTelemetryMiddleware from opentelemetry.sdk.resources import Resource from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.test_base import TestBase @@ -413,3 +414,31 @@ def test_custom_span_name(self): span_list[0].resource.attributes["service.name"], "flask-api-no-app", ) + + +class TestProgrammaticWrappedWithOtherFramework( + InstrumentationTest, TestBase, WsgiTestBase +): + def setUp(self): + super().setUp() + + self.app = Flask(__name__) + self.app.wsgi_app = OpenTelemetryMiddleware(self.app.wsgi_app) + FlaskInstrumentor().instrument_app(self.app) + self._common_initialization() + + def tearDown(self) -> None: + super().tearDown() + with self.disable_logging(): + FlaskInstrumentor().uninstrument_app(self.app) + + def test_mark_span_internal_in_presence_of_span_from_other_framework(self): + resp = self.client.get("/hello/123") + self.assertEqual(200, resp.status_code) + resp.get_data() + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(trace.SpanKind.INTERNAL, span_list[0].kind) + self.assertEqual(trace.SpanKind.SERVER, span_list[1].kind) + self.assertEqual( + span_list[0].parent.span_id, span_list[1].context.span_id + ) From 07e98bea3c42b50a9f215652eb817d2dec512b90 Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Wed, 8 Dec 2021 12:28:52 +0530 Subject: [PATCH 2/6] Updating changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbdafee17b..13590317ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#812](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/812)) - `opentelemetry-instrumentation-flask` Flask: Conditionally create SERVER spans - ([#827](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/827)) + ([#828](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/828)) ## [1.7.1-0.26b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.7.0-0.26b0) - 2021-11-11 From 389cc7e3c688cad166d843455edb51424e847782 Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Thu, 9 Dec 2021 21:47:54 +0530 Subject: [PATCH 3/6] Removing extra print statements --- .../src/opentelemetry/instrumentation/flask/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index ccf8581708..43ff8dc657 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -182,7 +182,6 @@ def _before_request(): if trace.get_current_span() is trace.INVALID_SPAN: ctx = extract(flask_request_environ, getter=otel_wsgi.wsgi_getter) - print(ctx) token = context.attach(ctx) span_kind = trace.SpanKind.SERVER From 6388b47a0fff251f8a70c853e17b6ec7e2976e17 Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Fri, 10 Dec 2021 00:28:50 +0530 Subject: [PATCH 4/6] Resolving comments: Setting current context as parent in its presence --- .../src/opentelemetry/instrumentation/flask/__init__.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 43ff8dc657..de403f9ea3 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -177,13 +177,15 @@ def _before_request(): flask_request_environ = flask.request.environ span_name = get_default_span_name() - token = ctx = None - span_kind = trace.SpanKind.INTERNAL + token = ctx = span_kind = None if trace.get_current_span() is trace.INVALID_SPAN: ctx = extract(flask_request_environ, getter=otel_wsgi.wsgi_getter) token = context.attach(ctx) span_kind = trace.SpanKind.SERVER + else: + ctx = context.get_current() + span_kind = trace.SpanKind.INTERNAL span = tracer.start_span( span_name, @@ -237,7 +239,7 @@ def _teardown_request(exc): type(exc), exc, getattr(exc, "__traceback__", None) ) - if flask.request.environ.get(_ENVIRON_TOKEN, None) is not None: + if flask.request.environ.get(_ENVIRON_TOKEN, None): context.detach(flask.request.environ.get(_ENVIRON_TOKEN)) return _teardown_request From 9927c0af645d88c9bfa81bf52010535b59027733 Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Fri, 10 Dec 2021 18:18:53 +0530 Subject: [PATCH 5/6] Ignoring pylint check as django.conf.urls.url is removed in django 4.0 Django release notes: https://docs.djangoproject.com/en/4.0/releases/4.0/ --- .../tests/test_middleware.py | 2 +- .../tests/test_middleware_asgi.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 473a0fb3c0..d33e1f4316 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -60,7 +60,7 @@ if DJANGO_2_0: from django.urls import re_path else: - from django.conf.urls import url as re_path + from django.conf.urls import url as re_path # pylint: disable=E0611 urlpatterns = [ re_path(r"^traced/", traced), diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py index 3d8d557bcc..508b9c2686 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py @@ -59,7 +59,7 @@ if DJANGO_2_0: from django.urls import re_path else: - from django.conf.urls import url as re_path + from django.conf.urls import url as re_path # pylint: disable=E0611 urlpatterns = [ re_path(r"^traced/", async_traced), From 8f50cacc635305fc4e7a68850d80cb1532a743d1 Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Mon, 20 Dec 2021 16:28:21 +0530 Subject: [PATCH 6/6] Removing changes in django files --- .../tests/test_middleware.py | 2 +- .../tests/test_middleware_asgi.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index ddb669bf9b..f5d31d920b 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -62,7 +62,7 @@ if DJANGO_2_0: from django.urls import re_path else: - from django.conf.urls import url as re_path # pylint: disable=E0611 + from django.conf.urls import url as re_path urlpatterns = [ re_path(r"^traced/", traced), diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py index 3dea0d200c..ecc3172e8b 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py @@ -61,7 +61,7 @@ if DJANGO_2_0: from django.urls import re_path else: - from django.conf.urls import url as re_path # pylint: disable=E0611 + from django.conf.urls import url as re_path urlpatterns = [ re_path(r"^traced/", async_traced),