From 4438fd78b0cb52b39443366d7d8b16437fc77b25 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Fri, 16 Feb 2024 11:05:17 +0000 Subject: [PATCH 1/9] add header parameters to FastAPIInstrumentor().instrument_app --- .../instrumentation/fastapi/__init__.py | 26 +- .../tests/test_fastapi_instrumentation.py | 463 ++++++++++++++++++ 2 files changed, 481 insertions(+), 8 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index bfb4f4a682..721597bf2a 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -76,7 +76,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A if span and span.is_recording(): span.set_attribute("custom_user_attribute_from_response_hook", "some-value") - FastAPIInstrumentor().instrument(server_request_hook=server_request_hook, client_request_hook=client_request_hook, client_response_hook=client_response_hook) + FastAPIInstrumentor().instrument_app(server_request_hook=server_request_hook, client_request_hook=client_request_hook, client_response_hook=client_response_hook) Capture HTTP request and response headers ***************************************** @@ -86,9 +86,10 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A Request headers *************** To capture HTTP request headers as span attributes, set the environment variable -``OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST`` to a comma delimited list of HTTP header names. +``OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST`` to a comma delimited list of HTTP header names, +or pass the ``http_capture_headers_server_request`` keyword argument to the ``instrument_app`` method. -For example, +For example using the environment variable, :: export OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST="content-type,custom_request_header" @@ -120,9 +121,10 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A Response headers **************** To capture HTTP response headers as span attributes, set the environment variable -``OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE`` to a comma delimited list of HTTP header names. +``OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE`` to a comma delimited list of HTTP header names, +or pass the ``http_capture_headers_server_response`` keyword argument to the ``instrument_app`` method. -For example, +For example using the environment variable, :: export OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE="content-type,custom_response_header" @@ -155,10 +157,12 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A ****************** In order to prevent storing sensitive data such as personally identifiable information (PII), session keys, passwords, etc, set the environment variable ``OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS`` -to a comma delimited list of HTTP header names to be sanitized. Regexes may be used, and all header names will be -matched in a case-insensitive manner. +to a comma delimited list of HTTP header names to be sanitized, or pass the ``http_capture_headers_sanitize_fields`` +keyword argument to the ``instrument_app`` method. -For example, +Regexes may be used, and all header names will be matched in a case-insensitive manner. + +For example using the environment variable, :: export OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS=".*session.*,set-cookie" @@ -227,6 +231,9 @@ def instrument_app( tracer_provider=None, meter_provider=None, excluded_urls=None, + http_capture_headers_server_request: list[str] | None = None, + http_capture_headers_server_response: list[str] | None = None, + http_capture_headers_sanitize_fields: list[str] | None = None, ): """Instrument an uninstrumented FastAPI application.""" if not hasattr(app, "_is_instrumented_by_opentelemetry"): @@ -265,6 +272,9 @@ def instrument_app( # Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation tracer=tracer, meter=meter, + http_capture_headers_server_request=http_capture_headers_server_request, + http_capture_headers_server_response=http_capture_headers_server_response, + http_capture_headers_sanitize_fields=http_capture_headers_sanitize_fields, ) app._is_instrumented_by_opentelemetry = True if app not in _InstrumentedFastAPI._instrumented_fastapi_apps: diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index d233331283..ba4f701f96 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1227,3 +1227,466 @@ def test_instrumentation(self): should_be_original = fastapi.FastAPI self.assertIs(original, should_be_original) + + +class TestWrappedApplication(TestBase): + def setUp(self): + super().setUp() + + self.app = fastapi.FastAPI() + + @self.app.get("/foobar") + async def _(): + return {"message": "hello world"} + + otel_fastapi.FastAPIInstrumentor().instrument_app(self.app) + self.client = TestClient(self.app) + self.tracer = self.tracer_provider.get_tracer(__name__) + + def tearDown(self) -> None: + super().tearDown() + with self.disable_logging(): + otel_fastapi.FastAPIInstrumentor().uninstrument_app(self.app) + + def test_mark_span_internal_in_presence_of_span_from_other_framework(self): + with self.tracer.start_as_current_span( + "test", kind=trace.SpanKind.SERVER + ) as parent_span: + resp = self.client.get("/foobar") + self.assertEqual(200, resp.status_code) + + span_list = self.memory_exporter.get_finished_spans() + for span in span_list: + print(str(span.__class__) + ": " + str(span.__dict__)) + + # there should be 4 spans - single SERVER "test" and three INTERNAL "FastAPI" + self.assertEqual(trace.SpanKind.INTERNAL, span_list[0].kind) + self.assertEqual(trace.SpanKind.INTERNAL, span_list[1].kind) + # main INTERNAL span - child of test + self.assertEqual(trace.SpanKind.INTERNAL, span_list[2].kind) + self.assertEqual( + parent_span.context.span_id, span_list[2].parent.span_id + ) + # SERVER "test" + self.assertEqual(trace.SpanKind.SERVER, span_list[3].kind) + self.assertEqual( + parent_span.context.span_id, span_list[3].context.span_id + ) + + +@patch.dict( + "os.environ", + { + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS: ".*my-secret.*", + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST: "Custom-Test-Header-1,Custom-Test-Header-2,Custom-Test-Header-3,Regex-Test-Header-.*,Regex-Invalid-Test-Header-.*,.*my-secret.*", + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE: "Custom-Test-Header-1,Custom-Test-Header-2,Custom-Test-Header-3,my-custom-regex-header-.*,invalid-regex-header-.*,.*my-secret.*", + }, +) +class TestHTTPAppWithCustomHeaders(TestBase): + def setUp(self): + super().setUp() + self.app = self._create_app() + otel_fastapi.FastAPIInstrumentor().instrument_app(self.app) + self.client = TestClient(self.app) + + def tearDown(self) -> None: + super().tearDown() + with self.disable_logging(): + otel_fastapi.FastAPIInstrumentor().uninstrument_app(self.app) + + @staticmethod + def _create_app(): + app = fastapi.FastAPI() + + @app.get("/foobar") + async def _(): + headers = { + "custom-test-header-1": "test-header-value-1", + "custom-test-header-2": "test-header-value-2", + "my-custom-regex-header-1": "my-custom-regex-value-1,my-custom-regex-value-2", + "My-Custom-Regex-Header-2": "my-custom-regex-value-3,my-custom-regex-value-4", + "My-Secret-Header": "My Secret Value", + } + content = {"message": "hello world"} + return JSONResponse(content=content, headers=headers) + + return app + + def test_http_custom_request_headers_in_span_attributes(self): + expected = { + "http.request.header.custom_test_header_1": ( + "test-header-value-1", + ), + "http.request.header.custom_test_header_2": ( + "test-header-value-2", + ), + "http.request.header.regex_test_header_1": ("Regex Test Value 1",), + "http.request.header.regex_test_header_2": ( + "RegexTestValue2,RegexTestValue3", + ), + "http.request.header.my_secret_header": ("[REDACTED]",), + } + resp = self.client.get( + "/foobar", + headers={ + "custom-test-header-1": "test-header-value-1", + "custom-test-header-2": "test-header-value-2", + "Regex-Test-Header-1": "Regex Test Value 1", + "regex-test-header-2": "RegexTestValue2,RegexTestValue3", + "My-Secret-Header": "My Secret Value", + }, + ) + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 3) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + + self.assertSpanHasAttributes(server_span, expected) + + def test_http_custom_request_headers_not_in_span_attributes(self): + not_expected = { + "http.request.header.custom_test_header_3": ( + "test-header-value-3", + ), + } + resp = self.client.get( + "/foobar", + headers={ + "custom-test-header-1": "test-header-value-1", + "custom-test-header-2": "test-header-value-2", + "Regex-Test-Header-1": "Regex Test Value 1", + "regex-test-header-2": "RegexTestValue2,RegexTestValue3", + "My-Secret-Header": "My Secret Value", + }, + ) + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 3) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + + for key, _ in not_expected.items(): + self.assertNotIn(key, server_span.attributes) + + def test_http_custom_response_headers_in_span_attributes(self): + expected = { + "http.response.header.custom_test_header_1": ( + "test-header-value-1", + ), + "http.response.header.custom_test_header_2": ( + "test-header-value-2", + ), + "http.response.header.my_custom_regex_header_1": ( + "my-custom-regex-value-1,my-custom-regex-value-2", + ), + "http.response.header.my_custom_regex_header_2": ( + "my-custom-regex-value-3,my-custom-regex-value-4", + ), + "http.response.header.my_secret_header": ("[REDACTED]",), + } + resp = self.client.get("/foobar") + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 3) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + self.assertSpanHasAttributes(server_span, expected) + + def test_http_custom_response_headers_not_in_span_attributes(self): + not_expected = { + "http.response.header.custom_test_header_3": ( + "test-header-value-3", + ), + } + resp = self.client.get("/foobar") + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 3) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + + for key, _ in not_expected.items(): + self.assertNotIn(key, server_span.attributes) + + +class TestHTTPAppWithCustomHeadersParameters(TestBase): + """Minimal tests here since the behavior of this logic is tested above and in the ASGI tests.""" + def setUp(self): + super().setUp() + self.app = self._create_app() + otel_fastapi.FastAPIInstrumentor().instrument_app( + self.app, + http_capture_headers_server_request=["a.*", "b.*"], + http_capture_headers_server_response=["c.*", "d.*"], + http_capture_headers_sanitize_fields=[".*secret.*"] + ) + self.client = TestClient(self.app) + + def tearDown(self) -> None: + super().tearDown() + with self.disable_logging(): + otel_fastapi.FastAPIInstrumentor().uninstrument_app(self.app) + + @staticmethod + def _create_app(): + app = fastapi.FastAPI() + + @app.get("/foobar") + async def _(): + headers = { + "carrot": "bar", + "date-secret": "yellow", + "egg": "ham", + } + content = {"message": "hello world"} + return JSONResponse(content=content, headers=headers) + + return app + + def test_http_custom_request_headers_in_span_attributes(self): + resp = self.client.get("/foobar", headers={ + "apple": "red", "banana-secret": "yellow", "fig": "green" + }) + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 3) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + + from pprint import pprint + pprint(server_span) + expected = { + # apple should be included because it starts with a + "http.request.header.apple": ("red",), + # same with banana because it starts with b, + # redacted because it contains "secret" + "http.request.header.banana_secret": ("[REDACTED]",), + } + self.assertSpanHasAttributes(server_span, expected) + self.assertNotIn("http.request.header.fig", server_span.attributes) + + def test_http_custom_response_headers_in_span_attributes(self): + resp = self.client.get("/foobar") + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 3) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + + expected = { + "http.response.header.carrot": ("bar",), + "http.response.header.date_secret": ("[REDACTED]",), + } + self.assertSpanHasAttributes(server_span, expected) + self.assertNotIn("http.response.header.egg", server_span.attributes) + + +@patch.dict( + "os.environ", + { + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS: ".*my-secret.*", + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST: "Custom-Test-Header-1,Custom-Test-Header-2,Custom-Test-Header-3,Regex-Test-Header-.*,Regex-Invalid-Test-Header-.*,.*my-secret.*", + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE: "Custom-Test-Header-1,Custom-Test-Header-2,Custom-Test-Header-3,my-custom-regex-header-.*,invalid-regex-header-.*,.*my-secret.*", + }, +) +class TestWebSocketAppWithCustomHeaders(TestBase): + def setUp(self): + super().setUp() + self.app = self._create_app() + otel_fastapi.FastAPIInstrumentor().instrument_app(self.app) + self.client = TestClient(self.app) + + def tearDown(self) -> None: + super().tearDown() + with self.disable_logging(): + otel_fastapi.FastAPIInstrumentor().uninstrument_app(self.app) + + @staticmethod + def _create_app(): + app = fastapi.FastAPI() + + @app.websocket("/foobar_web") + async def _(websocket: fastapi.WebSocket): + message = await websocket.receive() + if message.get("type") == "websocket.connect": + await websocket.send( + { + "type": "websocket.accept", + "headers": [ + (b"custom-test-header-1", b"test-header-value-1"), + (b"custom-test-header-2", b"test-header-value-2"), + (b"Regex-Test-Header-1", b"Regex Test Value 1"), + ( + b"regex-test-header-2", + b"RegexTestValue2,RegexTestValue3", + ), + (b"My-Secret-Header", b"My Secret Value"), + ], + } + ) + await websocket.send_json({"message": "hello world"}) + await websocket.close() + if message.get("type") == "websocket.disconnect": + pass + + return app + + def test_web_socket_custom_request_headers_in_span_attributes(self): + expected = { + "http.request.header.custom_test_header_1": ( + "test-header-value-1", + ), + "http.request.header.custom_test_header_2": ( + "test-header-value-2", + ), + } + + with self.client.websocket_connect( + "/foobar_web", + headers={ + "custom-test-header-1": "test-header-value-1", + "custom-test-header-2": "test-header-value-2", + }, + ) as websocket: + data = websocket.receive_json() + self.assertEqual(data, {"message": "hello world"}) + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 5) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + + self.assertSpanHasAttributes(server_span, expected) + + @patch.dict( + "os.environ", + { + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS: ".*my-secret.*", + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST: "Custom-Test-Header-1,Custom-Test-Header-2,Custom-Test-Header-3,Regex-Test-Header-.*,Regex-Invalid-Test-Header-.*,.*my-secret.*", + }, + ) + def test_web_socket_custom_request_headers_not_in_span_attributes(self): + not_expected = { + "http.request.header.custom_test_header_3": ( + "test-header-value-3", + ), + } + + with self.client.websocket_connect( + "/foobar_web", + headers={ + "custom-test-header-1": "test-header-value-1", + "custom-test-header-2": "test-header-value-2", + }, + ) as websocket: + data = websocket.receive_json() + self.assertEqual(data, {"message": "hello world"}) + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 5) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + + for key, _ in not_expected.items(): + self.assertNotIn(key, server_span.attributes) + + def test_web_socket_custom_response_headers_in_span_attributes(self): + expected = { + "http.response.header.custom_test_header_1": ( + "test-header-value-1", + ), + "http.response.header.custom_test_header_2": ( + "test-header-value-2", + ), + } + + with self.client.websocket_connect("/foobar_web") as websocket: + data = websocket.receive_json() + self.assertEqual(data, {"message": "hello world"}) + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 5) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + + self.assertSpanHasAttributes(server_span, expected) + + def test_web_socket_custom_response_headers_not_in_span_attributes(self): + not_expected = { + "http.response.header.custom_test_header_3": ( + "test-header-value-3", + ), + } + + with self.client.websocket_connect("/foobar_web") as websocket: + data = websocket.receive_json() + self.assertEqual(data, {"message": "hello world"}) + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 5) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + + for key, _ in not_expected.items(): + self.assertNotIn(key, server_span.attributes) + + +@patch.dict( + "os.environ", + { + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST: "Custom-Test-Header-1,Custom-Test-Header-2,Custom-Test-Header-3", + }, +) +class TestNonRecordingSpanWithCustomHeaders(TestBase): + def setUp(self): + super().setUp() + self.app = fastapi.FastAPI() + + @self.app.get("/foobar") + async def _(): + return {"message": "hello world"} + + reset_trace_globals() + tracer_provider = trace.NoOpTracerProvider() + trace.set_tracer_provider(tracer_provider=tracer_provider) + + self._instrumentor = otel_fastapi.FastAPIInstrumentor() + self._instrumentor.instrument_app(self.app) + self.client = TestClient(self.app) + + def tearDown(self) -> None: + super().tearDown() + with self.disable_logging(): + self._instrumentor.uninstrument_app(self.app) + + def test_custom_header_not_present_in_non_recording_span(self): + resp = self.client.get( + "/foobar", + headers={ + "custom-test-header-1": "test-header-value-1", + }, + ) + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 0) From dd54710b2ae75f6a248e90b6496309c4aea5bbd2 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Fri, 16 Feb 2024 11:13:01 +0000 Subject: [PATCH 2/9] add changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0984efa3de..fc572f0c31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -216,6 +216,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2367](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2367)) +### Added +- `opentelemetry-instrumentation-fastapi` Add support for configuring header extraction via runtime constructor parameters + ([#2241](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2241)) + ## Version 1.23.0/0.44b0 (2024-02-23) - Drop support for 3.7 From 5086fb9c00eb0776edaaa175d5b2834d602c6913 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Fri, 16 Feb 2024 11:15:22 +0000 Subject: [PATCH 3/9] move #2072 changelog to unreleased --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc572f0c31..f3d9b5e40a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -240,6 +240,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `opentelemetry-instrumentation-psycopg` Initial release for psycopg 3.x +- `opentelemetry-instrumentation-asgi` Add support for configuring ASGI middleware header extraction via runtime constructor parameters + ([#2026](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2026)) ## Version 1.22.0/0.43b0 (2023-12-14) @@ -279,8 +281,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1948](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1948)) - Added schema_url (`"https://opentelemetry.io/schemas/1.11.0"`) to all metrics and traces ([#1977](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1977)) -- Add support for configuring ASGI middleware header extraction via runtime constructor parameters - ([#2026](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2026)) ### Fixed From eb27e72d8b64958b19ad23a7697ef2f5c77e931f Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Mon, 19 Feb 2024 20:39:00 +0000 Subject: [PATCH 4/9] remove accidental pprint --- .../tests/test_fastapi_instrumentation.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index ba4f701f96..809d7280f0 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1464,8 +1464,6 @@ def test_http_custom_request_headers_in_span_attributes(self): span for span in span_list if span.kind == trace.SpanKind.SERVER ][0] - from pprint import pprint - pprint(server_span) expected = { # apple should be included because it starts with a "http.request.header.apple": ("red",), From f6f7fdcac27edb3998da564e2be53ee59953b465 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Tue, 20 Feb 2024 13:50:28 +0000 Subject: [PATCH 5/9] linting --- .../tests/test_fastapi_instrumentation.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 809d7280f0..7851e08dc6 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1420,6 +1420,7 @@ def test_http_custom_response_headers_not_in_span_attributes(self): class TestHTTPAppWithCustomHeadersParameters(TestBase): """Minimal tests here since the behavior of this logic is tested above and in the ASGI tests.""" + def setUp(self): super().setUp() self.app = self._create_app() @@ -1427,7 +1428,7 @@ def setUp(self): self.app, http_capture_headers_server_request=["a.*", "b.*"], http_capture_headers_server_response=["c.*", "d.*"], - http_capture_headers_sanitize_fields=[".*secret.*"] + http_capture_headers_sanitize_fields=[".*secret.*"], ) self.client = TestClient(self.app) @@ -1453,9 +1454,14 @@ async def _(): return app def test_http_custom_request_headers_in_span_attributes(self): - resp = self.client.get("/foobar", headers={ - "apple": "red", "banana-secret": "yellow", "fig": "green" - }) + resp = self.client.get( + "/foobar", + headers={ + "apple": "red", + "banana-secret": "yellow", + "fig": "green", + }, + ) self.assertEqual(200, resp.status_code) span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 3) From 070f6bf988fd546a2bfbb77b6e775f6dbd6a56a5 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Tue, 20 Feb 2024 13:53:03 +0000 Subject: [PATCH 6/9] future annotations for fastapi --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 721597bf2a..c7a21db12f 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -175,6 +175,9 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A API --- """ + +from __future__ import annotations + import logging from importlib.metadata import PackageNotFoundError, distribution from typing import Collection From 329bfe5e7333aa8a363c447a395e690e996a8052 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Mon, 18 Mar 2024 18:45:49 +0000 Subject: [PATCH 7/9] same logic for instrument() --- .../instrumentation/fastapi/__init__.py | 14 +++- .../tests/test_fastapi_instrumentation.py | 75 ++++++++++++++++--- 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index c7a21db12f..fdb035baa8 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -76,7 +76,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A if span and span.is_recording(): span.set_attribute("custom_user_attribute_from_response_hook", "some-value") - FastAPIInstrumentor().instrument_app(server_request_hook=server_request_hook, client_request_hook=client_request_hook, client_response_hook=client_response_hook) + FastAPIInstrumentor().instrument(server_request_hook=server_request_hook, client_request_hook=client_request_hook, client_response_hook=client_response_hook) Capture HTTP request and response headers ***************************************** @@ -327,6 +327,15 @@ def _instrument(self, **kwargs): _InstrumentedFastAPI._client_response_hook = kwargs.get( "client_response_hook" ) + _InstrumentedFastAPI._http_capture_headers_server_request = kwargs.get( + "http_capture_headers_server_request" + ) + _InstrumentedFastAPI._http_capture_headers_server_response = ( + kwargs.get("http_capture_headers_server_response") + ) + _InstrumentedFastAPI._http_capture_headers_sanitize_fields = ( + kwargs.get("http_capture_headers_sanitize_fields") + ) _excluded_urls = kwargs.get("excluded_urls") _InstrumentedFastAPI._excluded_urls = ( _excluded_urls_from_env @@ -381,6 +390,9 @@ def __init__(self, *args, **kwargs): # Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation tracer=tracer, meter=meter, + http_capture_headers_server_request=_InstrumentedFastAPI._http_capture_headers_server_request, + http_capture_headers_server_response=_InstrumentedFastAPI._http_capture_headers_server_response, + http_capture_headers_sanitize_fields=_InstrumentedFastAPI._http_capture_headers_sanitize_fields, ) self._is_instrumented_by_opentelemetry = True _InstrumentedFastAPI._instrumented_fastapi_apps.add(self) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 7851e08dc6..61a847690e 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1423,19 +1423,21 @@ class TestHTTPAppWithCustomHeadersParameters(TestBase): def setUp(self): super().setUp() - self.app = self._create_app() - otel_fastapi.FastAPIInstrumentor().instrument_app( - self.app, + self.instrumentor = otel_fastapi.FastAPIInstrumentor() + self.kwargs = dict( http_capture_headers_server_request=["a.*", "b.*"], http_capture_headers_server_response=["c.*", "d.*"], http_capture_headers_sanitize_fields=[".*secret.*"], ) - self.client = TestClient(self.app) + self.app = None def tearDown(self) -> None: super().tearDown() with self.disable_logging(): - otel_fastapi.FastAPIInstrumentor().uninstrument_app(self.app) + if self.app: + self.instrumentor.uninstrument_app(self.app) + else: + self.instrumentor.uninstrument() @staticmethod def _create_app(): @@ -1453,8 +1455,11 @@ async def _(): return app - def test_http_custom_request_headers_in_span_attributes(self): - resp = self.client.get( + def test_http_custom_request_headers_in_span_attributes_app(self): + self.app = self._create_app() + self.instrumentor.instrument_app(self.app, **self.kwargs) + + resp = TestClient(self.app).get( "/foobar", headers={ "apple": "red", @@ -1480,8 +1485,60 @@ def test_http_custom_request_headers_in_span_attributes(self): self.assertSpanHasAttributes(server_span, expected) self.assertNotIn("http.request.header.fig", server_span.attributes) - def test_http_custom_response_headers_in_span_attributes(self): - resp = self.client.get("/foobar") + def test_http_custom_request_headers_in_span_attributes_instr(self): + """As above, but use instrument(), not instrument_app().""" + self.instrumentor.instrument(**self.kwargs) + + resp = TestClient(self._create_app()).get( + "/foobar", + headers={ + "apple": "red", + "banana-secret": "yellow", + "fig": "green", + }, + ) + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 3) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + + expected = { + # apple should be included because it starts with a + "http.request.header.apple": ("red",), + # same with banana because it starts with b, + # redacted because it contains "secret" + "http.request.header.banana_secret": ("[REDACTED]",), + } + self.assertSpanHasAttributes(server_span, expected) + self.assertNotIn("http.request.header.fig", server_span.attributes) + + def test_http_custom_response_headers_in_span_attributes_app(self): + self.app = self._create_app() + self.instrumentor.instrument_app(self.app, **self.kwargs) + resp = TestClient(self.app).get("/foobar") + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 3) + + server_span = [ + span for span in span_list if span.kind == trace.SpanKind.SERVER + ][0] + + expected = { + "http.response.header.carrot": ("bar",), + "http.response.header.date_secret": ("[REDACTED]",), + } + self.assertSpanHasAttributes(server_span, expected) + self.assertNotIn("http.response.header.egg", server_span.attributes) + + def test_http_custom_response_headers_in_span_attributes_inst(self): + """As above, but use instrument(), not instrument_app().""" + self.instrumentor.instrument(**self.kwargs) + + resp = TestClient(self._create_app()).get("/foobar") self.assertEqual(200, resp.status_code) span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 3) From 813d73770699ea32f5996b01946c8df004ecf936 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Fri, 22 Mar 2024 17:44:42 -0600 Subject: [PATCH 8/9] Fix lint --- .../tests/test_fastapi_instrumentation.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 61a847690e..20d82448cb 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1424,11 +1424,11 @@ class TestHTTPAppWithCustomHeadersParameters(TestBase): def setUp(self): super().setUp() self.instrumentor = otel_fastapi.FastAPIInstrumentor() - self.kwargs = dict( - http_capture_headers_server_request=["a.*", "b.*"], - http_capture_headers_server_response=["c.*", "d.*"], - http_capture_headers_sanitize_fields=[".*secret.*"], - ) + self.kwargs = { + "http_capture_headers_server_request": ["a.*", "b.*"], + "http_capture_headers_server_response": ["c.*", "d.*"], + "http_capture_headers_sanitize_fields": [".*secret.*"], + } self.app = None def tearDown(self) -> None: From f4bcaa8321b200a20186c7e323124cbb266232e3 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 1 Aug 2024 16:01:40 -0600 Subject: [PATCH 9/9] Fix test case --- .../tests/test_fastapi_instrumentation.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 20d82448cb..03fdd6749d 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -20,9 +20,11 @@ import fastapi from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware +from fastapi.responses import JSONResponse from fastapi.testclient import TestClient import opentelemetry.instrumentation.fastapi as otel_fastapi +from opentelemetry import trace from opentelemetry.instrumentation._semconv import ( OTEL_SEMCONV_STABILITY_OPT_IN, _OpenTelemetrySemanticConventionStability, @@ -47,8 +49,14 @@ ) from opentelemetry.semconv.attributes.url_attributes import URL_SCHEME from opentelemetry.semconv.trace import SpanAttributes +from opentelemetry.test.globals_test import reset_trace_globals from opentelemetry.test.test_base import TestBase -from opentelemetry.util.http import get_excluded_urls +from opentelemetry.util.http import ( + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS, + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, + get_excluded_urls, +) _expected_metric_names_old = [ "http.server.active_requests",