From 1a44fb7acac70715d0651a53beb5f1913d38b836 Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Tue, 18 Jan 2022 16:00:45 +0530 Subject: [PATCH 1/8] Making span as internal for falcon in presence of a span in current context --- .../instrumentation/falcon/__init__.py | 21 +++++++++++++++---- .../tests/test_falcon.py | 16 +++++++++++++- 2 files changed, 32 insertions(+), 5 deletions(-) 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..6989a8b42c 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -195,12 +195,23 @@ def __call__(self, env, start_response): start_time = _time_ns() - token = context.attach(extract(env, getter=otel_wsgi.wsgi_getter)) + token = ctx = span_kind = None + + if trace.get_current_span() is trace.INVALID_SPAN: + ctx = extract(env, 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 = self._tracer.start_span( otel_wsgi.get_default_span_name(env), - kind=trace.SpanKind.SERVER, + context=ctx, + kind=span_kind, start_time=start_time, ) + if span.is_recording(): attributes = otel_wsgi.collect_request_attributes(env) for key, value in attributes.items(): @@ -216,7 +227,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 +239,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..9362be21c6 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -27,7 +27,7 @@ from opentelemetry.test.test_base import TestBase from opentelemetry.test.wsgitestutil import WsgiTestBase from opentelemetry.trace import StatusCode - +from opentelemetry import trace from .app import make_app @@ -264,3 +264,17 @@ 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) + + \ No newline at end of file From c12c54c1861e434fd04868ec982307978758aebc Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Tue, 18 Jan 2022 16:21:50 +0530 Subject: [PATCH 2/8] Updating changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64405acd89..90b79f1ebe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,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 From 721b15a92171b0251a284347b406b817364738ff Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Tue, 18 Jan 2022 16:39:41 +0530 Subject: [PATCH 3/8] Fixing lint and generate build failures --- .../opentelemetry/instrumentation/falcon/__init__.py | 2 +- .../tests/test_falcon.py | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) 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 6989a8b42c..31a4ebbc85 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -204,7 +204,7 @@ def __call__(self, env, start_response): else: ctx = context.get_current() span_kind = trace.SpanKind.INTERNAL - + span = self._tracer.start_span( otel_wsgi.get_default_span_name(env), context=ctx, diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index 9362be21c6..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, @@ -27,7 +28,7 @@ from opentelemetry.test.test_base import TestBase from opentelemetry.test.wsgitestutil import WsgiTestBase from opentelemetry.trace import StatusCode -from opentelemetry import trace + from .app import make_app @@ -265,6 +266,7 @@ def test_hooks(self): 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__) @@ -275,6 +277,6 @@ def test_mark_span_internal_in_presence_of_span_from_other_framework(self): 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) - - \ No newline at end of file + self.assertEqual( + span.parent.span_id, parent_span.get_span_context().span_id + ) From 9c7847f29158a327516149f3c71270abdb1f5f3c Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Tue, 18 Jan 2022 22:06:02 +0530 Subject: [PATCH 4/8] Resolving comments: Converting snippet to re-usable function --- .../instrumentation/falcon/__init__.py | 13 ++------- .../opentelemetry/instrumentation/utils.py | 28 +++++++++++++++++++ 2 files changed, 31 insertions(+), 10 deletions(-) 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 31a4ebbc85..26ae1fd95f 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -109,6 +109,7 @@ def response_hook(span, req, resp): from opentelemetry.instrumentation.utils import ( extract_attributes_from_object, http_status_to_status_code, + get_token_context_span_kind ) from opentelemetry.propagate import extract from opentelemetry.semconv.trace import SpanAttributes @@ -195,16 +196,8 @@ def __call__(self, env, start_response): start_time = _time_ns() - token = ctx = span_kind = None - - if trace.get_current_span() is trace.INVALID_SPAN: - ctx = extract(env, getter=otel_wsgi.wsgi_getter) - token = context.attach(ctx) - span_kind = trace.SpanKind.SERVER - else: - ctx = context.get_current() - span_kind = trace.SpanKind.INTERNAL - + token, ctx, span_kind = get_token_context_span_kind(env, getter=otel_wsgi.wsgi_getter) + span = self._tracer.start_span( otel_wsgi.get_default_span_name(env), context=ctx, diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index f5a470d707..b069c5220b 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -14,12 +14,14 @@ from typing import Dict, Sequence +from opentelemetry import context, trace from wrapt import ObjectProxy # pylint: disable=unused-import # pylint: disable=E0611 from opentelemetry.context import _SUPPRESS_INSTRUMENTATION_KEY # noqa: F401 from opentelemetry.trace import StatusCode +from opentelemetry.propagate import extract def extract_attributes_from_object( @@ -67,3 +69,29 @@ 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 get_token_context_span_kind(env, getter): + """Based on presence of active span, extracts context and initializes token and span_kind + + Args: + env : 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. + 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(env, getter=getter) + token = context.attach(ctx) + span_kind = trace.SpanKind.SERVER + else: + ctx = context.get_current() + span_kind = trace.SpanKind.INTERNAL + + return token, ctx, span_kind \ No newline at end of file From cae0057429880d0d25eebad7b7523b10edc13a67 Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Tue, 18 Jan 2022 23:45:03 +0530 Subject: [PATCH 5/8] Fixing build failures --- .../opentelemetry/instrumentation/falcon/__init__.py | 9 +++++---- .../src/opentelemetry/instrumentation/utils.py | 11 ++++++----- 2 files changed, 11 insertions(+), 9 deletions(-) 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 26ae1fd95f..8fe2b7f85a 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -108,10 +108,9 @@ def response_hook(span, req, resp): ) from opentelemetry.instrumentation.utils import ( extract_attributes_from_object, + get_token_context_span_kind, http_status_to_status_code, - get_token_context_span_kind ) -from opentelemetry.propagate import extract from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.status import Status from opentelemetry.util._time import _time_ns @@ -196,8 +195,10 @@ def __call__(self, env, start_response): start_time = _time_ns() - token, ctx, span_kind = get_token_context_span_kind(env, getter=otel_wsgi.wsgi_getter) - + token, ctx, span_kind = get_token_context_span_kind( + env, getter=otel_wsgi.wsgi_getter + ) + span = self._tracer.start_span( otel_wsgi.get_default_span_name(env), context=ctx, diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index b069c5220b..e723200c41 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -14,14 +14,15 @@ from typing import Dict, Sequence -from opentelemetry import context, trace 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.trace import StatusCode from opentelemetry.propagate import extract +from opentelemetry.trace import StatusCode def extract_attributes_from_object( @@ -72,7 +73,7 @@ def unwrap(obj, attr: str): def get_token_context_span_kind(env, getter): - """Based on presence of active span, extracts context and initializes token and span_kind + """Based on presence of active span, extracts context and initializes token and span_kind Args: env : object which contains values that are @@ -81,7 +82,7 @@ def get_token_context_span_kind(env, getter): which understands how to extract a value from it. 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. + from carrier. """ token = ctx = span_kind = None @@ -94,4 +95,4 @@ def get_token_context_span_kind(env, getter): ctx = context.get_current() span_kind = trace.SpanKind.INTERNAL - return token, ctx, span_kind \ No newline at end of file + return token, ctx, span_kind From b9b8efb2af000969ad58b540ad6f2107e8e6bfa5 Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Thu, 20 Jan 2022 19:09:04 +0530 Subject: [PATCH 6/8] Resolving comments: Creating wrapper for start span to make internal/server span --- .../instrumentation/falcon/__init__.py | 15 +++++------- .../opentelemetry/instrumentation/utils.py | 23 +++++++++++++++---- 2 files changed, 24 insertions(+), 14 deletions(-) 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 8fe2b7f85a..6ae458c13a 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -108,8 +108,8 @@ def response_hook(span, req, resp): ) from opentelemetry.instrumentation.utils import ( extract_attributes_from_object, - get_token_context_span_kind, http_status_to_status_code, + start_internal_or_server_span, ) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.status import Status @@ -195,15 +195,12 @@ def __call__(self, env, start_response): start_time = _time_ns() - token, ctx, span_kind = get_token_context_span_kind( - env, getter=otel_wsgi.wsgi_getter - ) - - span = self._tracer.start_span( - otel_wsgi.get_default_span_name(env), - context=ctx, - kind=span_kind, + span, token = start_internal_or_server_span( + tracer=self._tracer, + span_name=otel_wsgi.get_default_span_name(env), start_time=start_time, + env=env, + context_getter=otel_wsgi.wsgi_getter, ) if span.is_recording(): diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index e723200c41..2b5d0fea35 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -72,15 +72,21 @@ def unwrap(obj, attr: str): setattr(obj, attr, func.__wrapped__) -def get_token_context_span_kind(env, getter): - """Based on presence of active span, extracts context and initializes token and span_kind +def start_internal_or_server_span( + tracer, span_name, start_time, env, 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 env : 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. - getter : an object which contains a get function that can retrieve zero + 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. """ @@ -88,11 +94,18 @@ def get_token_context_span_kind(env, getter): token = ctx = span_kind = None if trace.get_current_span() is trace.INVALID_SPAN: - ctx = extract(env, getter=getter) + ctx = extract(env, getter=context_getter) token = context.attach(ctx) span_kind = trace.SpanKind.SERVER else: ctx = context.get_current() span_kind = trace.SpanKind.INTERNAL - return token, ctx, span_kind + span = tracer.start_span( + name=span_name, + context=ctx, + kind=span_kind, + start_time=start_time, + ) + + return span, token From 85659a6e42ec8e2f5cd13340c91343c330156757 Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Thu, 20 Jan 2022 19:33:54 +0530 Subject: [PATCH 7/8] Rerun docker tests --- .../src/opentelemetry/instrumentation/utils.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 2b5d0fea35..773b1c2de0 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -92,7 +92,6 @@ def start_internal_or_server_span( """ token = ctx = span_kind = None - if trace.get_current_span() is trace.INVALID_SPAN: ctx = extract(env, getter=context_getter) token = context.attach(ctx) @@ -100,12 +99,10 @@ def start_internal_or_server_span( 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 95b76bf53c202ec43b62c60e3b81d45dd5d4cdd0 Mon Sep 17 00:00:00 2001 From: Ashutosh Goel Date: Thu, 20 Jan 2022 22:22:56 +0530 Subject: [PATCH 8/8] Resolving comments: Refactoring --- .../src/opentelemetry/instrumentation/falcon/__init__.py | 6 +++--- .../src/opentelemetry/instrumentation/utils.py | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) 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 6ae458c13a..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,9 +107,9 @@ 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, - start_internal_or_server_span, ) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.status import Status @@ -195,11 +195,11 @@ def __call__(self, env, start_response): start_time = _time_ns() - span, token = start_internal_or_server_span( + span, token = _start_internal_or_server_span( tracer=self._tracer, span_name=otel_wsgi.get_default_span_name(env), start_time=start_time, - env=env, + context_carrier=env, context_getter=otel_wsgi.wsgi_getter, ) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 773b1c2de0..100170bf7b 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -72,8 +72,8 @@ def unwrap(obj, attr: str): setattr(obj, attr, func.__wrapped__) -def start_internal_or_server_span( - tracer, span_name, start_time, env, context_getter +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 @@ -82,7 +82,7 @@ def start_internal_or_server_span( tracer : tracer in use by given instrumentation library name (string): name of the span start_time : start time of the span - env : object which contains values that are + 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. @@ -93,7 +93,7 @@ def start_internal_or_server_span( token = ctx = span_kind = None if trace.get_current_span() is trace.INVALID_SPAN: - ctx = extract(env, getter=context_getter) + ctx = extract(context_carrier, getter=context_getter) token = context.attach(ctx) span_kind = trace.SpanKind.SERVER else: