From 334e014811dbfad261462ad2d1b07d76e113b718 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Wed, 19 Jan 2022 17:32:01 +0530 Subject: [PATCH 1/6] code change to resolve the bug https://github.com/open-telemetry/opentelemetry-python-contrib/issues/449 --- .../instrumentation/pyramid/callbacks.py | 32 +++++++++++++------ .../tests/test_automatic.py | 32 +++++++++++++++++++ 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index f8be4ca720..1645c6feef 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -20,13 +20,20 @@ from pyramid.tweens import EXCVIEW import opentelemetry.instrumentation.wsgi as otel_wsgi -from opentelemetry import context, trace +from opentelemetry import context from opentelemetry.instrumentation.propagators import ( get_global_response_propagator, ) from opentelemetry.instrumentation.pyramid.version import __version__ from opentelemetry.propagate import extract from opentelemetry.semconv.trace import SpanAttributes +from opentelemetry.trace import ( + INVALID_SPAN, + SpanKind, + get_current_span, + get_tracer, + use_span, +) from opentelemetry.util._time import _time_ns from opentelemetry.util.http import get_excluded_urls @@ -82,19 +89,24 @@ def _before_traversal(event): start_time = request_environ.get(_ENVIRON_STARTTIME_KEY) - token = context.attach( - extract(request_environ, getter=otel_wsgi.wsgi_getter) - ) - tracer = trace.get_tracer(__name__, __version__) + token = ctx = None + span_kind = SpanKind.INTERNAL + tracer = get_tracer(__name__, __version__) if request.matched_route: span_name = request.matched_route.pattern else: span_name = otel_wsgi.get_default_span_name(request_environ) + if get_current_span() is INVALID_SPAN: + ctx = extract(request_environ, getter=otel_wsgi.wsgi_getter) + token = context.attach(ctx) + span_kind = SpanKind.SERVER + span = tracer.start_span( span_name, - kind=trace.SpanKind.SERVER, + ctx, + kind=span_kind, start_time=start_time, ) @@ -107,11 +119,12 @@ def _before_traversal(event): for key, value in attributes.items(): span.set_attribute(key, value) - activation = trace.use_span(span, end_on_exit=True) + activation = use_span(span, end_on_exit=True) activation.__enter__() # pylint: disable=E1101 request_environ[_ENVIRON_ACTIVATION_KEY] = activation request_environ[_ENVIRON_SPAN_KEY] = span - request_environ[_ENVIRON_TOKEN] = token + if token: + request_environ[_ENVIRON_TOKEN] = token def trace_tween_factory(handler, registry): @@ -180,7 +193,8 @@ def trace_tween(request): else: activation.__exit__(None, None, None) - context.detach(request.environ.get(_ENVIRON_TOKEN)) + if request.environ.get(_ENVIRON_TOKEN, None) is not None: + context.detach(request.environ.get(_ENVIRON_TOKEN)) return response diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_automatic.py b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_automatic.py index b065e26064..37b2be4c76 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_automatic.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_automatic.py @@ -17,6 +17,7 @@ from opentelemetry.instrumentation.pyramid import PyramidInstrumentor from opentelemetry.test.test_base import TestBase from opentelemetry.test.wsgitestutil import WsgiTestBase +from opentelemetry.trace import SpanKind # pylint: disable=import-error from .pyramid_base_test import InstrumentationTest @@ -77,3 +78,34 @@ def test_tween_list(self): self.assertEqual([b"Hello: 123"], list(resp.response)) span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) + + +class TestWrappedWithOtherFramework( + InstrumentationTest, TestBase, WsgiTestBase +): + def setUp(self): + super().setUp() + PyramidInstrumentor().instrument() + self.config = Configurator() + self._common_initialization(self.config) + + def tearDown(self) -> None: + super().tearDown() + with self.disable_logging(): + PyramidInstrumentor().uninstrument() + + def test_with_existing_span(self): + tracer_provider, _ = self.create_tracer_provider() + tracer = tracer_provider.get_tracer(__name__) + + with tracer.start_as_current_span( + "test", kind=SpanKind.SERVER + ) as parent_span: + resp = self.client.get("/hello/123") + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(SpanKind.INTERNAL, span_list[0].kind) + self.assertEqual( + parent_span.get_span_context().span_id, + span_list[0].parent.span_id, + ) From 39ee6aa5728076098c3c3a013ff5132d4dc76435 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Wed, 19 Jan 2022 17:37:04 +0530 Subject: [PATCH 2/6] modifying the changelog file to add entry for PR #869 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64405acd89..b0683890a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.8.0-0.27b0...HEAD) +- `opentelemetry-instrumentation-pyramid` Pyramid: Conditionally create SERVER spans + ([#869](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/869)) ### Added From c1ce2d65345bc8f158fc2e3257d5b1c7691a4d9f Mon Sep 17 00:00:00 2001 From: Sanket Mehta Date: Wed, 19 Jan 2022 18:47:55 +0530 Subject: [PATCH 3/6] removing redundent get statement --- .../src/opentelemetry/instrumentation/pyramid/callbacks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index 1645c6feef..2364702719 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -193,8 +193,9 @@ def trace_tween(request): else: activation.__exit__(None, None, None) - if request.environ.get(_ENVIRON_TOKEN, None) is not None: - context.detach(request.environ.get(_ENVIRON_TOKEN)) + env_token = request.environ.get(_ENVIRON_TOKEN, None) + if env_token is not None: + context.detach(env_token) return response From a1079603b504200dbdb651f339567bebfa4ba56b Mon Sep 17 00:00:00 2001 From: Ashutosh Goel <39601429+ashu658@users.noreply.github.com> Date: Fri, 21 Jan 2022 21:54:01 +0530 Subject: [PATCH 4/6] Conditionally create server spans for falcon (#867) * Making span as internal for falcon in presence of a span in current context * Updating changelog * Fixing lint and generate build failures * Resolving comments: Converting snippet to re-usable function * Fixing build failures * Resolving comments: Creating wrapper for start span to make internal/server span * Rerun docker tests * Resolving comments: Refactoring --- CHANGELOG.md | 3 +- .../instrumentation/falcon/__init__.py | 18 +++++---- .../tests/test_falcon.py | 16 ++++++++ .../opentelemetry/instrumentation/utils.py | 39 +++++++++++++++++++ 4 files changed, 68 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0683890a0..769365cdaa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#817](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/817)) - `opentelemetry-instrumentation-kafka-python` added kafka-python module instrumentation. ([#814](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/814)) - +- `opentelemetry-instrumentation-falcon` Falcon: Conditionally create SERVER spans + ([#867](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/867)) ### Fixed - `opentelemetry-instrumentation-django` Django: Conditionally create SERVER spans diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index b079d9a656..4c19f9a1d7 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -107,10 +107,10 @@ def response_hook(span, req, resp): get_global_response_propagator, ) from opentelemetry.instrumentation.utils import ( + _start_internal_or_server_span, extract_attributes_from_object, http_status_to_status_code, ) -from opentelemetry.propagate import extract from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.status import Status from opentelemetry.util._time import _time_ns @@ -195,12 +195,14 @@ def __call__(self, env, start_response): start_time = _time_ns() - token = context.attach(extract(env, getter=otel_wsgi.wsgi_getter)) - span = self._tracer.start_span( - otel_wsgi.get_default_span_name(env), - kind=trace.SpanKind.SERVER, + span, token = _start_internal_or_server_span( + tracer=self._tracer, + span_name=otel_wsgi.get_default_span_name(env), start_time=start_time, + context_carrier=env, + context_getter=otel_wsgi.wsgi_getter, ) + if span.is_recording(): attributes = otel_wsgi.collect_request_attributes(env) for key, value in attributes.items(): @@ -216,7 +218,8 @@ def _start_response(status, response_headers, *args, **kwargs): status, response_headers, *args, **kwargs ) activation.__exit__(None, None, None) - context.detach(token) + if token is not None: + context.detach(token) return response try: @@ -227,7 +230,8 @@ def _start_response(status, response_headers, *args, **kwargs): exc, getattr(exc, "__traceback__", None), ) - context.detach(token) + if token is not None: + context.detach(token) raise diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index f971ec68a0..c178a696ca 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -16,6 +16,7 @@ from falcon import testing +from opentelemetry import trace from opentelemetry.instrumentation.falcon import FalconInstrumentor from opentelemetry.instrumentation.propagators import ( TraceResponsePropagator, @@ -264,3 +265,18 @@ def test_hooks(self): self.assertEqual( span.attributes["request_hook_attr"], "value from hook" ) + + +class TestFalconInstrumentationWrappedWithOtherFramework(TestFalconBase): + def test_mark_span_internal_in_presence_of_span_from_other_framework(self): + tracer = trace.get_tracer(__name__) + with tracer.start_as_current_span( + "test", kind=trace.SpanKind.SERVER + ) as parent_span: + self.client().simulate_request(method="GET", path="/hello") + span = self.memory_exporter.get_finished_spans()[0] + assert span.status.is_ok + self.assertEqual(trace.SpanKind.INTERNAL, span.kind) + self.assertEqual( + span.parent.span_id, parent_span.get_span_context().span_id + ) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index f5a470d707..100170bf7b 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -16,9 +16,12 @@ from wrapt import ObjectProxy +from opentelemetry import context, trace + # pylint: disable=unused-import # pylint: disable=E0611 from opentelemetry.context import _SUPPRESS_INSTRUMENTATION_KEY # noqa: F401 +from opentelemetry.propagate import extract from opentelemetry.trace import StatusCode @@ -67,3 +70,39 @@ def unwrap(obj, attr: str): func = getattr(obj, attr, None) if func and isinstance(func, ObjectProxy) and hasattr(func, "__wrapped__"): setattr(obj, attr, func.__wrapped__) + + +def _start_internal_or_server_span( + tracer, span_name, start_time, context_carrier, context_getter +): + """Returns internal or server span along with the token which can be used by caller to reset context + + + Args: + tracer : tracer in use by given instrumentation library + name (string): name of the span + start_time : start time of the span + context_carrier : object which contains values that are + used to construct a Context. This object + must be paired with an appropriate getter + which understands how to extract a value from it. + context_getter : an object which contains a get function that can retrieve zero + or more values from the carrier and a keys function that can get all the keys + from carrier. + """ + + token = ctx = span_kind = None + if trace.get_current_span() is trace.INVALID_SPAN: + ctx = extract(context_carrier, getter=context_getter) + token = context.attach(ctx) + span_kind = trace.SpanKind.SERVER + else: + ctx = context.get_current() + span_kind = trace.SpanKind.INTERNAL + span = tracer.start_span( + name=span_name, + context=ctx, + kind=span_kind, + start_time=start_time, + ) + return span, token From 7880add9c6f01061bf9988ebf3dc9eff68eef672 Mon Sep 17 00:00:00 2001 From: Dan Date: Fri, 21 Jan 2022 18:40:32 +0000 Subject: [PATCH 5/6] Fix Django 1.9 issue preventing use of MIDDLEWARE_CLASSES (#870) * Update CHANGELOG.md * Fix Django 1.9 issue preventing use of MIDDLEWARE_CLASSES Co-authored-by: Srikanth Chekuri --- CHANGELOG.md | 1 + .../src/opentelemetry/instrumentation/django/__init__.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 769365cdaa..17bea92345 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-asgi` ASGI: Conditionally create SERVER spans ([#843](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/843)) +- `opentelemetry-instrumentation-django` Django: fix issue preventing detection of MIDDLEWARE_CLASSES ## [1.8.0-0.27b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.8.0-0.27b0) - 2021-12-17 diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py index 46ee710640..d5e1f07279 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -105,7 +105,7 @@ def _get_django_middleware_setting() -> str: # In Django versions 1.x, setting MIDDLEWARE_CLASSES can be used as a legacy # alternative to MIDDLEWARE. This is the case when `settings.MIDDLEWARE` has # its default value (`None`). - if not DJANGO_2_0 and getattr(settings, "MIDDLEWARE", []) is None: + if not DJANGO_2_0 and getattr(settings, "MIDDLEWARE", None) is None: return "MIDDLEWARE_CLASSES" return "MIDDLEWARE" From ae092389660a807bcec9601e823d6ed35181bdc6 Mon Sep 17 00:00:00 2001 From: sanket Mehta Date: Sun, 23 Jan 2022 01:17:17 +0530 Subject: [PATCH 6/6] changing the import trace statement to resolve issue with unit test cases --- .../instrumentation/pyramid/callbacks.py | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index 2364702719..788c930e0d 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -20,20 +20,13 @@ from pyramid.tweens import EXCVIEW import opentelemetry.instrumentation.wsgi as otel_wsgi -from opentelemetry import context +from opentelemetry import context, trace from opentelemetry.instrumentation.propagators import ( get_global_response_propagator, ) from opentelemetry.instrumentation.pyramid.version import __version__ from opentelemetry.propagate import extract from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.trace import ( - INVALID_SPAN, - SpanKind, - get_current_span, - get_tracer, - use_span, -) from opentelemetry.util._time import _time_ns from opentelemetry.util.http import get_excluded_urls @@ -90,19 +83,18 @@ def _before_traversal(event): start_time = request_environ.get(_ENVIRON_STARTTIME_KEY) token = ctx = None - span_kind = SpanKind.INTERNAL - tracer = get_tracer(__name__, __version__) + span_kind = trace.SpanKind.INTERNAL + tracer = trace.get_tracer(__name__, __version__) if request.matched_route: span_name = request.matched_route.pattern else: span_name = otel_wsgi.get_default_span_name(request_environ) - if get_current_span() is INVALID_SPAN: + if trace.get_current_span() is trace.INVALID_SPAN: ctx = extract(request_environ, getter=otel_wsgi.wsgi_getter) token = context.attach(ctx) - span_kind = SpanKind.SERVER - + span_kind = trace.SpanKind.SERVER span = tracer.start_span( span_name, ctx, @@ -119,7 +111,7 @@ def _before_traversal(event): for key, value in attributes.items(): span.set_attribute(key, value) - activation = use_span(span, end_on_exit=True) + activation = trace.use_span(span, end_on_exit=True) activation.__enter__() # pylint: disable=E1101 request_environ[_ENVIRON_ACTIVATION_KEY] = activation request_environ[_ENVIRON_SPAN_KEY] = span