From 6ca49a800bf377d386c5dbae22c53d2499ac20e1 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 10 Mar 2022 09:00:46 -0800 Subject: [PATCH 1/6] [instrumentation/wsgi] fix NonRecordingSpan bug There was a bug caused by accessing `.kind` on a NonRecordingSpan. Added a test to validate the fix. Fix #956 --- .../instrumentation/wsgi/__init__.py | 5 +++-- .../tests/test_wsgi_middleware.py | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index ad4d425b25..8531d02c56 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -116,6 +116,7 @@ def response_hook(span: Span, environ: WSGIEnvironment, status: str, response_he from opentelemetry.instrumentation.wsgi.version import __version__ from opentelemetry.propagators.textmap import Getter from opentelemetry.semconv.trace import SpanAttributes +from opentelemetry.trace import NonRecordingSpan from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util.http import ( OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, @@ -313,7 +314,7 @@ def _create_start_response(span, start_response, response_hook): @functools.wraps(start_response) def _start_response(status, response_headers, *args, **kwargs): add_response_attributes(span, status, response_headers) - if span.kind == trace.SpanKind.SERVER: + if not isinstance(span, NonRecordingSpan) and span.kind == trace.SpanKind.SERVER: add_custom_response_headers(span, response_headers) if response_hook: response_hook(status, response_headers) @@ -336,7 +337,7 @@ def __call__(self, environ, start_response): context_getter=wsgi_getter, attributes=collect_request_attributes(environ), ) - if span.kind == trace.SpanKind.SERVER: + if not isinstance(span, NonRecordingSpan) and span.kind == trace.SpanKind.SERVER: add_custom_request_headers(span, environ) if self.request_hook: diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index 35f8e0577a..945e6f823f 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -475,6 +475,27 @@ def iterate_response(self, response): except StopIteration: break + @mock.patch.dict( + "os.environ", + { + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST: "Custom-Test-Header-1,Custom-Test-Header-2,Custom-Test-Header-3", + }, + ) + def test_custom_request_headers_non_recording_span(self): + try: + tracer_provider = trace_api.NoOpTracerProvider() + self.environ.update( + { + "HTTP_CUSTOM_TEST_HEADER_1": "Test Value 2", + "HTTP_CUSTOM_TEST_HEADER_2": "TestValue2,TestValue3", + } + ) + app = otel_wsgi.OpenTelemetryMiddleware(simple_wsgi, tracer_provider=tracer_provider) + response = app(self.environ, self.start_response) + self.iterate_response(response) + except Exception as e: + self.fail(f"Exception raised with NonRecordingSpan {e}") + @mock.patch.dict( "os.environ", { From b5f16c8f8927a1bb19be1466ae2705f45d279744 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 10 Mar 2022 09:13:18 -0800 Subject: [PATCH 2/6] fix lint --- .../src/opentelemetry/instrumentation/wsgi/__init__.py | 10 ++++++++-- .../tests/test_wsgi_middleware.py | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 8531d02c56..5347dc0ed7 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -314,7 +314,10 @@ def _create_start_response(span, start_response, response_hook): @functools.wraps(start_response) def _start_response(status, response_headers, *args, **kwargs): add_response_attributes(span, status, response_headers) - if not isinstance(span, NonRecordingSpan) and span.kind == trace.SpanKind.SERVER: + if ( + not isinstance(span, NonRecordingSpan) + and span.kind == trace.SpanKind.SERVER + ): add_custom_response_headers(span, response_headers) if response_hook: response_hook(status, response_headers) @@ -337,7 +340,10 @@ def __call__(self, environ, start_response): context_getter=wsgi_getter, attributes=collect_request_attributes(environ), ) - if not isinstance(span, NonRecordingSpan) and span.kind == trace.SpanKind.SERVER: + if ( + not isinstance(span, NonRecordingSpan) + and span.kind == trace.SpanKind.SERVER + ): add_custom_request_headers(span, environ) if self.request_hook: diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index 945e6f823f..ea49c7f290 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -490,7 +490,9 @@ def test_custom_request_headers_non_recording_span(self): "HTTP_CUSTOM_TEST_HEADER_2": "TestValue2,TestValue3", } ) - app = otel_wsgi.OpenTelemetryMiddleware(simple_wsgi, tracer_provider=tracer_provider) + app = otel_wsgi.OpenTelemetryMiddleware( + simple_wsgi, tracer_provider=tracer_provider + ) response = app(self.environ, self.start_response) self.iterate_response(response) except Exception as e: From 49ed7273c0fdd86f6c2b5dfa707991948c76f784 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 10 Mar 2022 09:14:55 -0800 Subject: [PATCH 3/6] use is_recording --- .../src/opentelemetry/instrumentation/wsgi/__init__.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 5347dc0ed7..23e83bb5e0 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -314,10 +314,7 @@ def _create_start_response(span, start_response, response_hook): @functools.wraps(start_response) def _start_response(status, response_headers, *args, **kwargs): add_response_attributes(span, status, response_headers) - if ( - not isinstance(span, NonRecordingSpan) - and span.kind == trace.SpanKind.SERVER - ): + if span.is_recording() and span.kind == trace.SpanKind.SERVER: add_custom_response_headers(span, response_headers) if response_hook: response_hook(status, response_headers) @@ -340,10 +337,7 @@ def __call__(self, environ, start_response): context_getter=wsgi_getter, attributes=collect_request_attributes(environ), ) - if ( - not isinstance(span, NonRecordingSpan) - and span.kind == trace.SpanKind.SERVER - ): + if span.is_recording() and span.kind == trace.SpanKind.SERVER: add_custom_request_headers(span, environ) if self.request_hook: From 3aefd4dea5b8ed60789e9a886e50b3f22813838f Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 10 Mar 2022 09:21:13 -0800 Subject: [PATCH 4/6] fix lint --- .../src/opentelemetry/instrumentation/wsgi/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 23e83bb5e0..57a0c8a4c0 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -116,7 +116,6 @@ def response_hook(span: Span, environ: WSGIEnvironment, status: str, response_he from opentelemetry.instrumentation.wsgi.version import __version__ from opentelemetry.propagators.textmap import Getter from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.trace import NonRecordingSpan from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util.http import ( OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, From 8ef96f6e14af9473b260ce5bda82f242d5fe6cc3 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 10 Mar 2022 09:31:45 -0800 Subject: [PATCH 5/6] fix lint --- .../tests/test_wsgi_middleware.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index ea49c7f290..944495993e 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -495,8 +495,8 @@ def test_custom_request_headers_non_recording_span(self): ) response = app(self.environ, self.start_response) self.iterate_response(response) - except Exception as e: - self.fail(f"Exception raised with NonRecordingSpan {e}") + except Exception as exc: # pylint: disable=W0703 + self.fail(f"Exception raised with NonRecordingSpan {exc}") @mock.patch.dict( "os.environ", From e72a88c11c10df8b1a843db20fef3faf077f6d2b Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 10 Mar 2022 09:37:45 -0800 Subject: [PATCH 6/6] fix lint --- .../tests/test_wsgi_middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index 944495993e..3238930792 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -495,7 +495,7 @@ def test_custom_request_headers_non_recording_span(self): ) response = app(self.environ, self.start_response) self.iterate_response(response) - except Exception as exc: # pylint: disable=W0703 + except Exception as exc: # pylint: disable=W0703 self.fail(f"Exception raised with NonRecordingSpan {exc}") @mock.patch.dict(