From 7789448b7f7e5bd8a7ce84c6d43ad74084bab384 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Fri, 1 Aug 2025 08:04:03 -0700 Subject: [PATCH 1/8] Fix for FASTAPI unable to record AppService URL --- CHANGELOG.md | 3 +- .../instrumentation/asgi/__init__.py | 15 +- .../tests/test_asgi_middleware.py | 14 +- .../tests/test_fastapi_instrumentation.py | 256 ++++++++++++++++++ 4 files changed, 282 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e74a94985..4d650a6e7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Version 1.36.0/0.57b0 (2025-07-29) ### Fixed - +- `opentelemetry-instrumentation-asgi` Fixed issue where FastAPI reports IP instead of URL. + ([]()) - `opentelemetry-instrumentation`: Fix dependency conflict detection when instrumented packages are not installed by moving check back to before instrumentors are loaded. Add "instruments-any" feature for instrumentations that target multiple packages. ([#3610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3610)) - infra(ci): Fix git pull failures in core contrib test diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 7e72dbf11f..d596b6c3fb 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -445,7 +445,20 @@ def get_host_port_url_tuple(scope): """Returns (host, port, full_url) tuple.""" server = scope.get("server") or ["0.0.0.0", 80] port = server[1] + + host_header = asgi_getter.get(scope, "host") + if host_header: + host_value = host_header[0] + # Ensure host_value is a string, not bytes + if isinstance(host_value, bytes): + host_value = host_value.decode("utf-8") + + url_host = host_value + (":" + str(port) if str(port) != "80" else "") + + else: + url_host = server[0] + (":" + str(port) if str(port) != "80" else "") server_host = server[0] + (":" + str(port) if str(port) != "80" else "") + # using the scope path is enough, see: # - https://asgi.readthedocs.io/en/latest/specs/www.html#http-connection-scope (see: root_path and path) # - https://asgi.readthedocs.io/en/latest/specs/www.html#wsgi-compatibility (see: PATH_INFO) @@ -453,7 +466,7 @@ def get_host_port_url_tuple(scope): # -> that means that the path should contain the root_path already, so prefixing it again is not necessary # - https://wsgi.readthedocs.io/en/latest/definitions.html#envvar-PATH_INFO full_path = scope.get("path", "") - http_url = scope.get("scheme", "http") + "://" + server_host + full_path + http_url = scope.get("scheme", "http") + "://" + url_host + full_path return server_host, port, http_url diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index b8791cf730..8aa4a14c68 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -763,7 +763,10 @@ async def test_host_header(self): def update_expected_server(expected): expected[3]["attributes"].update( - {SpanAttributes.HTTP_SERVER_NAME: hostname.decode("utf8")} + { + SpanAttributes.HTTP_SERVER_NAME: hostname.decode("utf8"), + SpanAttributes.HTTP_URL: f"http://{hostname.decode('utf8')}/", + } ) return expected @@ -780,7 +783,10 @@ async def test_host_header_both_semconv(self): def update_expected_server(expected): expected[3]["attributes"].update( - {SpanAttributes.HTTP_SERVER_NAME: hostname.decode("utf8")} + { + SpanAttributes.HTTP_SERVER_NAME: hostname.decode("utf8"), + SpanAttributes.HTTP_URL: f"http://{hostname.decode('utf8')}/", + } ) return expected @@ -1677,7 +1683,7 @@ def test_request_attributes(self): SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_HOST: "127.0.0.1", SpanAttributes.HTTP_TARGET: "/", - SpanAttributes.HTTP_URL: "http://127.0.0.1/?foo=bar", + SpanAttributes.HTTP_URL: "http://test/?foo=bar", SpanAttributes.NET_HOST_PORT: 80, SpanAttributes.HTTP_SCHEME: "http", SpanAttributes.HTTP_SERVER_NAME: "test", @@ -1730,7 +1736,7 @@ def test_request_attributes_both_semconv(self): SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_HOST: "127.0.0.1", SpanAttributes.HTTP_TARGET: "/", - SpanAttributes.HTTP_URL: "http://127.0.0.1/?foo=bar", + SpanAttributes.HTTP_URL: "http://test/?foo=bar", SpanAttributes.NET_HOST_PORT: 80, SpanAttributes.HTTP_SCHEME: "http", SpanAttributes.HTTP_SERVER_NAME: "test", diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 523c165f85..230dd79b65 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1877,3 +1877,259 @@ def test_custom_header_not_present_in_non_recording_span(self): self.assertEqual(200, resp.status_code) span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 0) + + +class TestFastAPIHostHeaderURL(TestBaseManualFastAPI): + """Test suite for Host header URL functionality in FastAPI instrumentation.""" + + def test_host_header_url_construction(self): + """Test that URLs use Host header value instead of server IP when available.""" + # Test with a custom Host header - should use the domain name + resp = self._client.get( + "/foobar?param=value", + headers={"host": "api.mycompany.com"} + ) + self.assertEqual(200, resp.status_code) + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + + # Find the server span (the main span, not internal middleware spans) + server_span = None + for span in spans: + if HTTP_URL in span.attributes: + server_span = span + break + + self.assertIsNotNone(server_span, "Server span with HTTP_URL not found") + + # Verify the URL uses the Host header domain instead of testserver + expected_url = "https://api.mycompany.com:443/foobar?param=value" + actual_url = server_span.attributes[HTTP_URL] + self.assertEqual(expected_url, actual_url) + + # Also verify that the server name attribute is set correctly + self.assertEqual("api.mycompany.com", server_span.attributes.get("http.server_name")) + + def test_host_header_with_port_url_construction(self): + """Test Host header URL construction when host includes port.""" + resp = self._client.get( + "/user/123", + headers={"host": "staging.myapp.com:8443"} + ) + self.assertEqual(200, resp.status_code) + + spans = self.memory_exporter.get_finished_spans() + server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + self.assertIsNotNone(server_span) + + # Should use the host header value with non-standard port included + expected_url = "https://staging.myapp.com:8443:443/user/123" + actual_url = server_span.attributes[HTTP_URL] + self.assertEqual(expected_url, actual_url) + + def test_no_host_header_fallback_behavior(self): + """Test fallback to server name when no Host header is present.""" + # Make request without custom Host header - should use testserver (default TestClient base) + resp = self._client.get("/foobar") + self.assertEqual(200, resp.status_code) + + spans = self.memory_exporter.get_finished_spans() + server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + self.assertIsNotNone(server_span) + + # Should fallback to testserver (TestClient default, standard port stripped) + expected_url = "https://testserver:443/foobar" + actual_url = server_span.attributes[HTTP_URL] + self.assertEqual(expected_url, actual_url) + + def test_production_scenario_host_header(self): + """Test a realistic production scenario with Host header.""" + # Simulate a production request with public domain in Host header + resp = self._client.get( + "/foobar?limit=10&offset=20", + headers={ + "host": "prod-api.example.com", + "user-agent": "ProductionClient/1.0" + } + ) + self.assertEqual(200, resp.status_code) # Valid route should return 200 + + spans = self.memory_exporter.get_finished_spans() + server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + self.assertIsNotNone(server_span) + + # URL should use the production domain from Host header (AS-IS, no default port) + expected_url = "https://prod-api.example.com:443/foobar?limit=10&offset=20" + actual_url = server_span.attributes[HTTP_URL] + self.assertEqual(expected_url, actual_url) + + # Verify other attributes are still correct + self.assertEqual("GET", server_span.attributes[HTTP_METHOD]) + self.assertEqual("/foobar", server_span.attributes[HTTP_TARGET]) + self.assertEqual("prod-api.example.com", server_span.attributes.get("http.server_name")) + + def test_host_header_with_special_characters(self): + """Test Host header handling with special characters and edge cases.""" + test_cases = [ + ("api-v2.test-domain.com", "https://api-v2.test-domain.com:443/foobar"), + ("localhost", "https://localhost:443/foobar"), + ("192.168.1.100", "https://192.168.1.100:443/foobar"), # IP address as host + ("test.domain.co.uk", "https://test.domain.co.uk:443/foobar"), # Multiple dots + ] + + for host_value, expected_url in test_cases: + with self.subTest(host=host_value): + # Clear previous spans + self.memory_exporter.clear() + + resp = self._client.get( + "/foobar", + headers={"host": host_value} + ) + self.assertEqual(200, resp.status_code) + + spans = self.memory_exporter.get_finished_spans() + server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + self.assertIsNotNone(server_span) + + actual_url = server_span.attributes[HTTP_URL] + self.assertEqual(expected_url, actual_url) + + def test_host_header_bytes_handling(self): + """Test that Host header values are properly decoded from bytes.""" + # This test verifies the fix for bytes vs string handling in our implementation + resp = self._client.get( + "/foobar", + headers={"host": "bytes-test.example.com"} + ) + self.assertEqual(200, resp.status_code) + + spans = self.memory_exporter.get_finished_spans() + server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + self.assertIsNotNone(server_span) + + # Should properly decode and use the host header + expected_url = "https://bytes-test.example.com:443/foobar" + actual_url = server_span.attributes[HTTP_URL] + self.assertEqual(expected_url, actual_url) + + def test_host_header_maintains_span_attributes(self): + """Test that using Host header doesn't break other span attributes.""" + resp = self._client.get( + "/user/testuser?debug=true", + headers={ + "host": "api.testapp.com", + "user-agent": "TestClient/1.0" + } + ) + self.assertEqual(200, resp.status_code) + + spans = self.memory_exporter.get_finished_spans() + server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + self.assertIsNotNone(server_span) + + # Verify URL uses Host header + self.assertEqual("https://api.testapp.com:443/user/testuser?debug=true", + server_span.attributes[HTTP_URL]) + + # Verify all other attributes are still present and correct + self.assertEqual("GET", server_span.attributes[HTTP_METHOD]) + self.assertEqual("/user/testuser", server_span.attributes[HTTP_TARGET]) + self.assertEqual("https", server_span.attributes[HTTP_SCHEME]) + self.assertEqual("api.testapp.com", server_span.attributes.get("http.server_name")) + self.assertEqual(200, server_span.attributes[HTTP_STATUS_CODE]) + + # Check that route attribute is still set correctly + if HTTP_ROUTE in server_span.attributes: + self.assertEqual("/user/{username}", server_span.attributes[HTTP_ROUTE]) + + +class TestFastAPIHostHeaderURLNewSemconv(TestFastAPIHostHeaderURL): + """Test Host header URL functionality with new semantic conventions.""" + + def test_host_header_url_new_semconv(self): + """Test Host header URL construction with new semantic conventions. + + Note: With new semantic conventions, the URL is split into components + (url.scheme, server.address, url.path, etc.) rather than a single http.url. + Host header support may work differently with new semantic conventions. + """ + resp = self._client.get( + "/foobar?test=new_semconv", + headers={"host": "newapi.example.com"} + ) + self.assertEqual(200, resp.status_code) + + spans = self.memory_exporter.get_finished_spans() + # With new semantic conventions, look for the main HTTP span with route information + server_span = next((span for span in spans if "http.route" in span.attributes), None) + self.assertIsNotNone(server_span) + + # Verify we have the new semantic convention attributes + self.assertIn("url.scheme", server_span.attributes) + self.assertIn("server.address", server_span.attributes) + self.assertIn("url.path", server_span.attributes) + self.assertEqual("https", server_span.attributes.get("url.scheme")) + self.assertEqual("/foobar", server_span.attributes.get("url.path")) + + # Current behavior: Host header may not affect server.address in new semantic conventions + # This test documents the current behavior rather than enforcing Host header usage + server_address = server_span.attributes.get("server.address", "") + self.assertIsNotNone(server_address) # Should have some value + + +class TestFastAPIHostHeaderURLBothSemconv(TestFastAPIHostHeaderURL): + """Test Host header URL functionality with both old and new semantic conventions.""" + + def test_host_header_url_both_semconv(self): + """Test Host header URL construction with both semantic conventions enabled.""" + resp = self._client.get( + "/foobar?test=both_semconv", + headers={"host": "dual.example.com"} + ) + self.assertEqual(200, resp.status_code) + + spans = self.memory_exporter.get_finished_spans() + server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + self.assertIsNotNone(server_span) + + # Should use Host header for URL construction regardless of semantic convention mode + expected_url = "https://dual.example.com:443/foobar?test=both_semconv" + actual_url = server_span.attributes[HTTP_URL] + self.assertEqual(expected_url, actual_url) + + def test_fastapi_unhandled_exception(self): + """Override inherited test - use the both_semconv version instead.""" + self.skipTest("Use test_fastapi_unhandled_exception_both_semconv instead") + + def test_fastapi_unhandled_exception_both_semconv(self): + """If the application has an unhandled error the instrumentation should capture that a 500 response is returned.""" + try: + resp = self._client.get("/error") + assert ( + resp.status_code == 500 + ), resp.content # pragma: no cover, for debugging this test if an exception is _not_ raised + except UnhandledException: + pass + else: + self.fail("Expected UnhandledException") + + spans = self.memory_exporter.get_finished_spans() + # With both semantic conventions enabled, we expect 3 spans: + # 1. Server span (main HTTP span) + # 2. ASGI receive span + # 3. ASGI send span (for error response) + self.assertEqual(len(spans), 3) + + # Find the server span (it should have HTTP attributes) + server_spans = [span for span in spans if hasattr(span, 'attributes') and span.attributes and HTTP_URL in span.attributes] + + self.assertEqual(len(server_spans), 1, "Expected exactly one server span with HTTP_URL") + server_span = server_spans[0] + + # Ensure server_span is not None + assert server_span is not None + + self.assertEqual(server_span.name, "GET /error") + From 92e858239ae164ea9cb1ae23d6d794f6b5b264b6 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Fri, 1 Aug 2025 15:37:51 -0700 Subject: [PATCH 2/8] Fixed tests and pylint errors --- .../instrumentation/asgi/__init__.py | 4 +- .../tests/test_middleware_asgi.py | 16 ++-- .../tests/test_fastapi_instrumentation.py | 96 +++++++++---------- 3 files changed, 57 insertions(+), 59 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index d596b6c3fb..c191041706 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -453,8 +453,8 @@ def get_host_port_url_tuple(scope): if isinstance(host_value, bytes): host_value = host_value.decode("utf-8") - url_host = host_value + (":" + str(port) if str(port) != "80" else "") - + url_host = host_value + else: url_host = server[0] + (":" + str(port) if str(port) != "80" else "") server_host = server[0] + (":" + str(port) if str(port) != "80" else "") diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py index d06c9c635c..a527f404b2 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py @@ -174,7 +174,7 @@ async def test_templated_route_get(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") self.assertEqual( span.attributes[SpanAttributes.HTTP_URL], - "http://127.0.0.1/route/2020/template/", + "http://testserver/route/2020/template/", ) self.assertEqual( span.attributes[SpanAttributes.HTTP_ROUTE], @@ -219,7 +219,7 @@ async def test_templated_route_get_both_semconv(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") self.assertEqual( span.attributes[SpanAttributes.HTTP_URL], - "http://127.0.0.1/route/2020/template/", + "http://testserver/route/2020/template/", ) self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) @@ -248,7 +248,7 @@ async def test_traced_get(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") self.assertEqual( span.attributes[SpanAttributes.HTTP_URL], - "http://127.0.0.1/traced/", + "http://testserver/traced/", ) self.assertEqual( span.attributes[SpanAttributes.HTTP_ROUTE], "^traced/" @@ -289,7 +289,7 @@ async def test_traced_get_both_semconv(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") self.assertEqual( span.attributes[SpanAttributes.HTTP_URL], - "http://127.0.0.1/traced/", + "http://testserver/traced/", ) self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) @@ -328,7 +328,7 @@ async def test_traced_post(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "POST") self.assertEqual( span.attributes[SpanAttributes.HTTP_URL], - "http://127.0.0.1/traced/", + "http://testserver/traced/", ) self.assertEqual( span.attributes[SpanAttributes.HTTP_ROUTE], "^traced/" @@ -369,7 +369,7 @@ async def test_traced_post_both_semconv(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "POST") self.assertEqual( span.attributes[SpanAttributes.HTTP_URL], - "http://127.0.0.1/traced/", + "http://testserver/traced/", ) self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) @@ -396,7 +396,7 @@ async def test_error(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") self.assertEqual( span.attributes[SpanAttributes.HTTP_URL], - "http://127.0.0.1/error/", + "http://testserver/error/", ) self.assertEqual(span.attributes[SpanAttributes.HTTP_ROUTE], "^error/") self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") @@ -450,7 +450,7 @@ async def test_error_both_semconv(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") self.assertEqual( span.attributes[SpanAttributes.HTTP_URL], - "http://127.0.0.1/error/", + "http://testserver/error/", ) self.assertEqual(span.attributes[SpanAttributes.HTTP_ROUTE], "^error/") self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 230dd79b65..e06d0f219b 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -300,7 +300,7 @@ def test_sub_app_fastapi_call(self): for span in spans_with_http_attributes: self.assertEqual("/sub/home", span.attributes[HTTP_TARGET]) self.assertEqual( - "https://testserver:443/sub/home", + "https://testserver/sub/home", span.attributes[HTTP_URL], ) @@ -1244,7 +1244,7 @@ def test_sub_app_fastapi_call(self): for span in spans_with_http_attributes: self.assertEqual("/sub/home", span.attributes[HTTP_TARGET]) self.assertEqual( - "https://testserver:443/sub/home", + "https://testserver/sub/home", span.attributes[HTTP_URL], ) @@ -1332,7 +1332,7 @@ def test_sub_app_fastapi_call(self): for span in spans_with_http_attributes: self.assertEqual("/sub/home", span.attributes[HTTP_TARGET]) self.assertEqual( - "https://testserver:443/sub/home", + "https://testserver/sub/home", span.attributes[HTTP_URL], ) @@ -1881,7 +1881,7 @@ def test_custom_header_not_present_in_non_recording_span(self): class TestFastAPIHostHeaderURL(TestBaseManualFastAPI): """Test suite for Host header URL functionality in FastAPI instrumentation.""" - + def test_host_header_url_construction(self): """Test that URLs use Host header value instead of server IP when available.""" # Test with a custom Host header - should use the domain name @@ -1890,24 +1890,24 @@ def test_host_header_url_construction(self): headers={"host": "api.mycompany.com"} ) self.assertEqual(200, resp.status_code) - + spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) - + # Find the server span (the main span, not internal middleware spans) server_span = None for span in spans: if HTTP_URL in span.attributes: server_span = span break - + self.assertIsNotNone(server_span, "Server span with HTTP_URL not found") - + # Verify the URL uses the Host header domain instead of testserver - expected_url = "https://api.mycompany.com:443/foobar?param=value" + expected_url = "https://api.mycompany.com/foobar?param=value" actual_url = server_span.attributes[HTTP_URL] self.assertEqual(expected_url, actual_url) - + # Also verify that the server name attribute is set correctly self.assertEqual("api.mycompany.com", server_span.attributes.get("http.server_name")) @@ -1918,13 +1918,13 @@ def test_host_header_with_port_url_construction(self): headers={"host": "staging.myapp.com:8443"} ) self.assertEqual(200, resp.status_code) - + spans = self.memory_exporter.get_finished_spans() server_span = next((span for span in spans if HTTP_URL in span.attributes), None) self.assertIsNotNone(server_span) - + # Should use the host header value with non-standard port included - expected_url = "https://staging.myapp.com:8443:443/user/123" + expected_url = "https://staging.myapp.com:8443/user/123" actual_url = server_span.attributes[HTTP_URL] self.assertEqual(expected_url, actual_url) @@ -1933,13 +1933,13 @@ def test_no_host_header_fallback_behavior(self): # Make request without custom Host header - should use testserver (default TestClient base) resp = self._client.get("/foobar") self.assertEqual(200, resp.status_code) - + spans = self.memory_exporter.get_finished_spans() server_span = next((span for span in spans if HTTP_URL in span.attributes), None) self.assertIsNotNone(server_span) - + # Should fallback to testserver (TestClient default, standard port stripped) - expected_url = "https://testserver:443/foobar" + expected_url = "https://testserver/foobar" actual_url = server_span.attributes[HTTP_URL] self.assertEqual(expected_url, actual_url) @@ -1954,16 +1954,16 @@ def test_production_scenario_host_header(self): } ) self.assertEqual(200, resp.status_code) # Valid route should return 200 - + spans = self.memory_exporter.get_finished_spans() server_span = next((span for span in spans if HTTP_URL in span.attributes), None) self.assertIsNotNone(server_span) - + # URL should use the production domain from Host header (AS-IS, no default port) - expected_url = "https://prod-api.example.com:443/foobar?limit=10&offset=20" + expected_url = "https://prod-api.example.com/foobar?limit=10&offset=20" actual_url = server_span.attributes[HTTP_URL] self.assertEqual(expected_url, actual_url) - + # Verify other attributes are still correct self.assertEqual("GET", server_span.attributes[HTTP_METHOD]) self.assertEqual("/foobar", server_span.attributes[HTTP_TARGET]) @@ -1972,12 +1972,12 @@ def test_production_scenario_host_header(self): def test_host_header_with_special_characters(self): """Test Host header handling with special characters and edge cases.""" test_cases = [ - ("api-v2.test-domain.com", "https://api-v2.test-domain.com:443/foobar"), - ("localhost", "https://localhost:443/foobar"), - ("192.168.1.100", "https://192.168.1.100:443/foobar"), # IP address as host - ("test.domain.co.uk", "https://test.domain.co.uk:443/foobar"), # Multiple dots + ("api-v2.test-domain.com", "https://api-v2.test-domain.com/foobar"), + ("localhost", "https://localhost/foobar"), + ("192.168.1.100", "https://192.168.1.100/foobar"), # IP address as host + ("test.domain.co.uk", "https://test.domain.co.uk/foobar"), # Multiple dots ] - + for host_value, expected_url in test_cases: with self.subTest(host=host_value): # Clear previous spans @@ -1988,11 +1988,11 @@ def test_host_header_with_special_characters(self): headers={"host": host_value} ) self.assertEqual(200, resp.status_code) - + spans = self.memory_exporter.get_finished_spans() server_span = next((span for span in spans if HTTP_URL in span.attributes), None) self.assertIsNotNone(server_span) - + actual_url = server_span.attributes[HTTP_URL] self.assertEqual(expected_url, actual_url) @@ -2004,13 +2004,13 @@ def test_host_header_bytes_handling(self): headers={"host": "bytes-test.example.com"} ) self.assertEqual(200, resp.status_code) - + spans = self.memory_exporter.get_finished_spans() server_span = next((span for span in spans if HTTP_URL in span.attributes), None) self.assertIsNotNone(server_span) - + # Should properly decode and use the host header - expected_url = "https://bytes-test.example.com:443/foobar" + expected_url = "https://bytes-test.example.com/foobar" actual_url = server_span.attributes[HTTP_URL] self.assertEqual(expected_url, actual_url) @@ -2024,22 +2024,22 @@ def test_host_header_maintains_span_attributes(self): } ) self.assertEqual(200, resp.status_code) - + spans = self.memory_exporter.get_finished_spans() server_span = next((span for span in spans if HTTP_URL in span.attributes), None) self.assertIsNotNone(server_span) - + # Verify URL uses Host header - self.assertEqual("https://api.testapp.com:443/user/testuser?debug=true", + self.assertEqual("https://api.testapp.com/user/testuser?debug=true", server_span.attributes[HTTP_URL]) - + # Verify all other attributes are still present and correct self.assertEqual("GET", server_span.attributes[HTTP_METHOD]) self.assertEqual("/user/testuser", server_span.attributes[HTTP_TARGET]) self.assertEqual("https", server_span.attributes[HTTP_SCHEME]) self.assertEqual("api.testapp.com", server_span.attributes.get("http.server_name")) self.assertEqual(200, server_span.attributes[HTTP_STATUS_CODE]) - + # Check that route attribute is still set correctly if HTTP_ROUTE in server_span.attributes: self.assertEqual("/user/{username}", server_span.attributes[HTTP_ROUTE]) @@ -2047,10 +2047,10 @@ def test_host_header_maintains_span_attributes(self): class TestFastAPIHostHeaderURLNewSemconv(TestFastAPIHostHeaderURL): """Test Host header URL functionality with new semantic conventions.""" - + def test_host_header_url_new_semconv(self): """Test Host header URL construction with new semantic conventions. - + Note: With new semantic conventions, the URL is split into components (url.scheme, server.address, url.path, etc.) rather than a single http.url. Host header support may work differently with new semantic conventions. @@ -2060,19 +2060,19 @@ def test_host_header_url_new_semconv(self): headers={"host": "newapi.example.com"} ) self.assertEqual(200, resp.status_code) - + spans = self.memory_exporter.get_finished_spans() # With new semantic conventions, look for the main HTTP span with route information server_span = next((span for span in spans if "http.route" in span.attributes), None) self.assertIsNotNone(server_span) - + # Verify we have the new semantic convention attributes self.assertIn("url.scheme", server_span.attributes) self.assertIn("server.address", server_span.attributes) self.assertIn("url.path", server_span.attributes) self.assertEqual("https", server_span.attributes.get("url.scheme")) self.assertEqual("/foobar", server_span.attributes.get("url.path")) - + # Current behavior: Host header may not affect server.address in new semantic conventions # This test documents the current behavior rather than enforcing Host header usage server_address = server_span.attributes.get("server.address", "") @@ -2081,7 +2081,6 @@ def test_host_header_url_new_semconv(self): class TestFastAPIHostHeaderURLBothSemconv(TestFastAPIHostHeaderURL): """Test Host header URL functionality with both old and new semantic conventions.""" - def test_host_header_url_both_semconv(self): """Test Host header URL construction with both semantic conventions enabled.""" resp = self._client.get( @@ -2089,13 +2088,13 @@ def test_host_header_url_both_semconv(self): headers={"host": "dual.example.com"} ) self.assertEqual(200, resp.status_code) - + spans = self.memory_exporter.get_finished_spans() server_span = next((span for span in spans if HTTP_URL in span.attributes), None) self.assertIsNotNone(server_span) - + # Should use Host header for URL construction regardless of semantic convention mode - expected_url = "https://dual.example.com:443/foobar?test=both_semconv" + expected_url = "https://dual.example.com/foobar?test=both_semconv" actual_url = server_span.attributes[HTTP_URL] self.assertEqual(expected_url, actual_url) @@ -2121,15 +2120,14 @@ def test_fastapi_unhandled_exception_both_semconv(self): # 2. ASGI receive span # 3. ASGI send span (for error response) self.assertEqual(len(spans), 3) - + # Find the server span (it should have HTTP attributes) server_spans = [span for span in spans if hasattr(span, 'attributes') and span.attributes and HTTP_URL in span.attributes] - + self.assertEqual(len(server_spans), 1, "Expected exactly one server span with HTTP_URL") server_span = server_spans[0] - + # Ensure server_span is not None assert server_span is not None - - self.assertEqual(server_span.name, "GET /error") + self.assertEqual(server_span.name, "GET /error") From ba8535836dab8ecdf00f4a1104afebd546415023 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Fri, 1 Aug 2025 15:52:37 -0700 Subject: [PATCH 3/8] Fixed ruff format --- .../tests/test_fastapi_instrumentation.py | 130 ++++++++++++------ 1 file changed, 89 insertions(+), 41 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index e06d0f219b..0e95cf774a 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1886,8 +1886,7 @@ def test_host_header_url_construction(self): """Test that URLs use Host header value instead of server IP when available.""" # Test with a custom Host header - should use the domain name resp = self._client.get( - "/foobar?param=value", - headers={"host": "api.mycompany.com"} + "/foobar?param=value", headers={"host": "api.mycompany.com"} ) self.assertEqual(200, resp.status_code) @@ -1901,7 +1900,9 @@ def test_host_header_url_construction(self): server_span = span break - self.assertIsNotNone(server_span, "Server span with HTTP_URL not found") + self.assertIsNotNone( + server_span, "Server span with HTTP_URL not found" + ) # Verify the URL uses the Host header domain instead of testserver expected_url = "https://api.mycompany.com/foobar?param=value" @@ -1909,18 +1910,21 @@ def test_host_header_url_construction(self): self.assertEqual(expected_url, actual_url) # Also verify that the server name attribute is set correctly - self.assertEqual("api.mycompany.com", server_span.attributes.get("http.server_name")) + self.assertEqual( + "api.mycompany.com", server_span.attributes.get("http.server_name") + ) def test_host_header_with_port_url_construction(self): """Test Host header URL construction when host includes port.""" resp = self._client.get( - "/user/123", - headers={"host": "staging.myapp.com:8443"} + "/user/123", headers={"host": "staging.myapp.com:8443"} ) self.assertEqual(200, resp.status_code) spans = self.memory_exporter.get_finished_spans() - server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + server_span = next( + (span for span in spans if HTTP_URL in span.attributes), None + ) self.assertIsNotNone(server_span) # Should use the host header value with non-standard port included @@ -1935,7 +1939,9 @@ def test_no_host_header_fallback_behavior(self): self.assertEqual(200, resp.status_code) spans = self.memory_exporter.get_finished_spans() - server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + server_span = next( + (span for span in spans if HTTP_URL in span.attributes), None + ) self.assertIsNotNone(server_span) # Should fallback to testserver (TestClient default, standard port stripped) @@ -1947,16 +1953,20 @@ def test_production_scenario_host_header(self): """Test a realistic production scenario with Host header.""" # Simulate a production request with public domain in Host header resp = self._client.get( - "/foobar?limit=10&offset=20", + "/foobar?limit=10&offset=20", headers={ "host": "prod-api.example.com", - "user-agent": "ProductionClient/1.0" - } + "user-agent": "ProductionClient/1.0", + }, ) - self.assertEqual(200, resp.status_code) # Valid route should return 200 + self.assertEqual( + 200, resp.status_code + ) # Valid route should return 200 spans = self.memory_exporter.get_finished_spans() - server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + server_span = next( + (span for span in spans if HTTP_URL in span.attributes), None + ) self.assertIsNotNone(server_span) # URL should use the production domain from Host header (AS-IS, no default port) @@ -1967,30 +1977,44 @@ def test_production_scenario_host_header(self): # Verify other attributes are still correct self.assertEqual("GET", server_span.attributes[HTTP_METHOD]) self.assertEqual("/foobar", server_span.attributes[HTTP_TARGET]) - self.assertEqual("prod-api.example.com", server_span.attributes.get("http.server_name")) + self.assertEqual( + "prod-api.example.com", + server_span.attributes.get("http.server_name"), + ) def test_host_header_with_special_characters(self): """Test Host header handling with special characters and edge cases.""" test_cases = [ - ("api-v2.test-domain.com", "https://api-v2.test-domain.com/foobar"), + ( + "api-v2.test-domain.com", + "https://api-v2.test-domain.com/foobar", + ), ("localhost", "https://localhost/foobar"), - ("192.168.1.100", "https://192.168.1.100/foobar"), # IP address as host - ("test.domain.co.uk", "https://test.domain.co.uk/foobar"), # Multiple dots + ( + "192.168.1.100", + "https://192.168.1.100/foobar", + ), # IP address as host + ( + "test.domain.co.uk", + "https://test.domain.co.uk/foobar", + ), # Multiple dots ] for host_value, expected_url in test_cases: with self.subTest(host=host_value): # Clear previous spans self.memory_exporter.clear() - + resp = self._client.get( - "/foobar", - headers={"host": host_value} + "/foobar", headers={"host": host_value} ) self.assertEqual(200, resp.status_code) spans = self.memory_exporter.get_finished_spans() - server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + server_span = next( + (span for span in spans if HTTP_URL in span.attributes), + None, + ) self.assertIsNotNone(server_span) actual_url = server_span.attributes[HTTP_URL] @@ -2000,13 +2024,14 @@ def test_host_header_bytes_handling(self): """Test that Host header values are properly decoded from bytes.""" # This test verifies the fix for bytes vs string handling in our implementation resp = self._client.get( - "/foobar", - headers={"host": "bytes-test.example.com"} + "/foobar", headers={"host": "bytes-test.example.com"} ) self.assertEqual(200, resp.status_code) spans = self.memory_exporter.get_finished_spans() - server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + server_span = next( + (span for span in spans if HTTP_URL in span.attributes), None + ) self.assertIsNotNone(server_span) # Should properly decode and use the host header @@ -2020,29 +2045,37 @@ def test_host_header_maintains_span_attributes(self): "/user/testuser?debug=true", headers={ "host": "api.testapp.com", - "user-agent": "TestClient/1.0" - } + "user-agent": "TestClient/1.0", + }, ) self.assertEqual(200, resp.status_code) spans = self.memory_exporter.get_finished_spans() - server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + server_span = next( + (span for span in spans if HTTP_URL in span.attributes), None + ) self.assertIsNotNone(server_span) # Verify URL uses Host header - self.assertEqual("https://api.testapp.com/user/testuser?debug=true", - server_span.attributes[HTTP_URL]) + self.assertEqual( + "https://api.testapp.com/user/testuser?debug=true", + server_span.attributes[HTTP_URL], + ) # Verify all other attributes are still present and correct self.assertEqual("GET", server_span.attributes[HTTP_METHOD]) self.assertEqual("/user/testuser", server_span.attributes[HTTP_TARGET]) self.assertEqual("https", server_span.attributes[HTTP_SCHEME]) - self.assertEqual("api.testapp.com", server_span.attributes.get("http.server_name")) + self.assertEqual( + "api.testapp.com", server_span.attributes.get("http.server_name") + ) self.assertEqual(200, server_span.attributes[HTTP_STATUS_CODE]) # Check that route attribute is still set correctly if HTTP_ROUTE in server_span.attributes: - self.assertEqual("/user/{username}", server_span.attributes[HTTP_ROUTE]) + self.assertEqual( + "/user/{username}", server_span.attributes[HTTP_ROUTE] + ) class TestFastAPIHostHeaderURLNewSemconv(TestFastAPIHostHeaderURL): @@ -2056,14 +2089,15 @@ def test_host_header_url_new_semconv(self): Host header support may work differently with new semantic conventions. """ resp = self._client.get( - "/foobar?test=new_semconv", - headers={"host": "newapi.example.com"} + "/foobar?test=new_semconv", headers={"host": "newapi.example.com"} ) self.assertEqual(200, resp.status_code) spans = self.memory_exporter.get_finished_spans() # With new semantic conventions, look for the main HTTP span with route information - server_span = next((span for span in spans if "http.route" in span.attributes), None) + server_span = next( + (span for span in spans if "http.route" in span.attributes), None + ) self.assertIsNotNone(server_span) # Verify we have the new semantic convention attributes @@ -2081,16 +2115,18 @@ def test_host_header_url_new_semconv(self): class TestFastAPIHostHeaderURLBothSemconv(TestFastAPIHostHeaderURL): """Test Host header URL functionality with both old and new semantic conventions.""" + def test_host_header_url_both_semconv(self): """Test Host header URL construction with both semantic conventions enabled.""" resp = self._client.get( - "/foobar?test=both_semconv", - headers={"host": "dual.example.com"} + "/foobar?test=both_semconv", headers={"host": "dual.example.com"} ) self.assertEqual(200, resp.status_code) spans = self.memory_exporter.get_finished_spans() - server_span = next((span for span in spans if HTTP_URL in span.attributes), None) + server_span = next( + (span for span in spans if HTTP_URL in span.attributes), None + ) self.assertIsNotNone(server_span) # Should use Host header for URL construction regardless of semantic convention mode @@ -2100,7 +2136,9 @@ def test_host_header_url_both_semconv(self): def test_fastapi_unhandled_exception(self): """Override inherited test - use the both_semconv version instead.""" - self.skipTest("Use test_fastapi_unhandled_exception_both_semconv instead") + self.skipTest( + "Use test_fastapi_unhandled_exception_both_semconv instead" + ) def test_fastapi_unhandled_exception_both_semconv(self): """If the application has an unhandled error the instrumentation should capture that a 500 response is returned.""" @@ -2117,14 +2155,24 @@ def test_fastapi_unhandled_exception_both_semconv(self): spans = self.memory_exporter.get_finished_spans() # With both semantic conventions enabled, we expect 3 spans: # 1. Server span (main HTTP span) - # 2. ASGI receive span + # 2. ASGI receive span # 3. ASGI send span (for error response) self.assertEqual(len(spans), 3) # Find the server span (it should have HTTP attributes) - server_spans = [span for span in spans if hasattr(span, 'attributes') and span.attributes and HTTP_URL in span.attributes] + server_spans = [ + span + for span in spans + if hasattr(span, "attributes") + and span.attributes + and HTTP_URL in span.attributes + ] - self.assertEqual(len(server_spans), 1, "Expected exactly one server span with HTTP_URL") + self.assertEqual( + len(server_spans), + 1, + "Expected exactly one server span with HTTP_URL", + ) server_span = server_spans[0] # Ensure server_span is not None From 81f521a9a886a2dab379dc9a0b5cea3527cf6d27 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Fri, 1 Aug 2025 16:16:58 -0700 Subject: [PATCH 4/8] Updated CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84a97e7004..663ab0c1c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - `opentelemetry-instrumentation-asgi` Fixed issue where FastAPI reports IP instead of URL. - ([]()) + ([#3670](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3670)) - `opentelemetry-instrumentation`: Fix dependency conflict detection when instrumented packages are not installed by moving check back to before instrumentors are loaded. Add "instruments-any" feature for instrumentations that target multiple packages. ([#3610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3610)) - infra(ci): Fix git pull failures in core contrib test From bd5f31f8cb766c406f585e7bb0b6ac9d182450aa Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Mon, 4 Aug 2025 07:58:20 -0700 Subject: [PATCH 5/8] Updated CHANGELOG --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 663ab0c1c6..a6ec5b7a76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,12 +10,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 > Use [this search for a list of all CHANGELOG.md files in this repo](https://github.com/search?q=repo%3Aopen-telemetry%2Fopentelemetry-python-contrib+path%3A**%2FCHANGELOG.md&type=code). ## Unreleased +- `opentelemetry-instrumentation-asgi` Fixed issue where FastAPI reports IP instead of URL. + ([#3670](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3670)) ## Version 1.36.0/0.57b0 (2025-07-29) ### Fixed -- `opentelemetry-instrumentation-asgi` Fixed issue where FastAPI reports IP instead of URL. - ([#3670](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3670)) - `opentelemetry-instrumentation`: Fix dependency conflict detection when instrumented packages are not installed by moving check back to before instrumentors are loaded. Add "instruments-any" feature for instrumentations that target multiple packages. ([#3610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3610)) - infra(ci): Fix git pull failures in core contrib test From ba906fefabaa7f417d782b0c976a005048c63734 Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Thu, 21 Aug 2025 09:01:03 -0700 Subject: [PATCH 6/8] Addressed feedback --- .../instrumentation/asgi/__init__.py | 2 +- .../tests/test_fastapi_instrumentation.py | 40 +++++-------------- 2 files changed, 11 insertions(+), 31 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index c191041706..6309fcf363 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -451,7 +451,7 @@ def get_host_port_url_tuple(scope): host_value = host_header[0] # Ensure host_value is a string, not bytes if isinstance(host_value, bytes): - host_value = host_value.decode("utf-8") + host_value = _decode_header_item(host_value) url_host = host_value diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 0e95cf774a..437666aa9d 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1896,7 +1896,7 @@ def test_host_header_url_construction(self): # Find the server span (the main span, not internal middleware spans) server_span = None for span in spans: - if HTTP_URL in span.attributes: + if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes: server_span = span break @@ -1923,7 +1923,7 @@ def test_host_header_with_port_url_construction(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if HTTP_URL in span.attributes), None + (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), None ) self.assertIsNotNone(server_span) @@ -1940,7 +1940,7 @@ def test_no_host_header_fallback_behavior(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if HTTP_URL in span.attributes), None + (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), None ) self.assertIsNotNone(server_span) @@ -1965,7 +1965,7 @@ def test_production_scenario_host_header(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if HTTP_URL in span.attributes), None + (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), None ) self.assertIsNotNone(server_span) @@ -2012,33 +2012,13 @@ def test_host_header_with_special_characters(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if HTTP_URL in span.attributes), + (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), None, ) self.assertIsNotNone(server_span) - actual_url = server_span.attributes[HTTP_URL] self.assertEqual(expected_url, actual_url) - def test_host_header_bytes_handling(self): - """Test that Host header values are properly decoded from bytes.""" - # This test verifies the fix for bytes vs string handling in our implementation - resp = self._client.get( - "/foobar", headers={"host": "bytes-test.example.com"} - ) - self.assertEqual(200, resp.status_code) - - spans = self.memory_exporter.get_finished_spans() - server_span = next( - (span for span in spans if HTTP_URL in span.attributes), None - ) - self.assertIsNotNone(server_span) - - # Should properly decode and use the host header - expected_url = "https://bytes-test.example.com/foobar" - actual_url = server_span.attributes[HTTP_URL] - self.assertEqual(expected_url, actual_url) - def test_host_header_maintains_span_attributes(self): """Test that using Host header doesn't break other span attributes.""" resp = self._client.get( @@ -2052,7 +2032,7 @@ def test_host_header_maintains_span_attributes(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if HTTP_URL in span.attributes), None + (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), None ) self.assertIsNotNone(server_span) @@ -2096,7 +2076,7 @@ def test_host_header_url_new_semconv(self): spans = self.memory_exporter.get_finished_spans() # With new semantic conventions, look for the main HTTP span with route information server_span = next( - (span for span in spans if "http.route" in span.attributes), None + (span for span in spans if span.kind == trace.SpanKind.SERVER and "http.route" in span.attributes), None ) self.assertIsNotNone(server_span) @@ -2110,7 +2090,7 @@ def test_host_header_url_new_semconv(self): # Current behavior: Host header may not affect server.address in new semantic conventions # This test documents the current behavior rather than enforcing Host header usage server_address = server_span.attributes.get("server.address", "") - self.assertIsNotNone(server_address) # Should have some value + self.assertIsNotNone(server_address, "testserver") # Should have some value class TestFastAPIHostHeaderURLBothSemconv(TestFastAPIHostHeaderURL): @@ -2125,7 +2105,7 @@ def test_host_header_url_both_semconv(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if HTTP_URL in span.attributes), None + (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), None ) self.assertIsNotNone(server_span) @@ -2163,7 +2143,7 @@ def test_fastapi_unhandled_exception_both_semconv(self): server_spans = [ span for span in spans - if hasattr(span, "attributes") + if span.kind == trace.SpanKind.SERVER and hasattr(span, "attributes") and span.attributes and HTTP_URL in span.attributes ] From b7f628714b031b88992786c9f19c00f4cb31e13b Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Thu, 21 Aug 2025 13:08:42 -0700 Subject: [PATCH 7/8] Checking CI runs --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 6309fcf363..fb645ae084 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -451,7 +451,7 @@ def get_host_port_url_tuple(scope): host_value = host_header[0] # Ensure host_value is a string, not bytes if isinstance(host_value, bytes): - host_value = _decode_header_item(host_value) + host_value = _decode_header_item(host_value) # use exisiting function url_host = host_value From 10a24a75592deae109b577657017686df386482f Mon Sep 17 00:00:00 2001 From: Radhika Gupta Date: Thu, 21 Aug 2025 13:32:39 -0700 Subject: [PATCH 8/8] Fix ruff and spellcheck errors --- .../instrumentation/asgi/__init__.py | 4 +- .../tests/test_fastapi_instrumentation.py | 67 ++++++++++++++++--- 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index fb645ae084..39feb42e4e 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -451,7 +451,9 @@ def get_host_port_url_tuple(scope): host_value = host_header[0] # Ensure host_value is a string, not bytes if isinstance(host_value, bytes): - host_value = _decode_header_item(host_value) # use exisiting function + host_value = _decode_header_item( + host_value + ) # use existing function url_host = host_value diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 437666aa9d..36fc3d0250 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1896,7 +1896,10 @@ def test_host_header_url_construction(self): # Find the server span (the main span, not internal middleware spans) server_span = None for span in spans: - if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes: + if ( + span.kind == trace.SpanKind.SERVER + and HTTP_URL in span.attributes + ): server_span = span break @@ -1923,7 +1926,13 @@ def test_host_header_with_port_url_construction(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), None + ( + span + for span in spans + if span.kind == trace.SpanKind.SERVER + and HTTP_URL in span.attributes + ), + None, ) self.assertIsNotNone(server_span) @@ -1940,7 +1949,13 @@ def test_no_host_header_fallback_behavior(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), None + ( + span + for span in spans + if span.kind == trace.SpanKind.SERVER + and HTTP_URL in span.attributes + ), + None, ) self.assertIsNotNone(server_span) @@ -1965,7 +1980,13 @@ def test_production_scenario_host_header(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), None + ( + span + for span in spans + if span.kind == trace.SpanKind.SERVER + and HTTP_URL in span.attributes + ), + None, ) self.assertIsNotNone(server_span) @@ -2012,7 +2033,12 @@ def test_host_header_with_special_characters(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), + ( + span + for span in spans + if span.kind == trace.SpanKind.SERVER + and HTTP_URL in span.attributes + ), None, ) self.assertIsNotNone(server_span) @@ -2032,7 +2058,13 @@ def test_host_header_maintains_span_attributes(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), None + ( + span + for span in spans + if span.kind == trace.SpanKind.SERVER + and HTTP_URL in span.attributes + ), + None, ) self.assertIsNotNone(server_span) @@ -2076,7 +2108,13 @@ def test_host_header_url_new_semconv(self): spans = self.memory_exporter.get_finished_spans() # With new semantic conventions, look for the main HTTP span with route information server_span = next( - (span for span in spans if span.kind == trace.SpanKind.SERVER and "http.route" in span.attributes), None + ( + span + for span in spans + if span.kind == trace.SpanKind.SERVER + and "http.route" in span.attributes + ), + None, ) self.assertIsNotNone(server_span) @@ -2090,7 +2128,9 @@ def test_host_header_url_new_semconv(self): # Current behavior: Host header may not affect server.address in new semantic conventions # This test documents the current behavior rather than enforcing Host header usage server_address = server_span.attributes.get("server.address", "") - self.assertIsNotNone(server_address, "testserver") # Should have some value + self.assertIsNotNone( + server_address, "testserver" + ) # Should have some value class TestFastAPIHostHeaderURLBothSemconv(TestFastAPIHostHeaderURL): @@ -2105,7 +2145,13 @@ def test_host_header_url_both_semconv(self): spans = self.memory_exporter.get_finished_spans() server_span = next( - (span for span in spans if span.kind == trace.SpanKind.SERVER and HTTP_URL in span.attributes), None + ( + span + for span in spans + if span.kind == trace.SpanKind.SERVER + and HTTP_URL in span.attributes + ), + None, ) self.assertIsNotNone(server_span) @@ -2143,7 +2189,8 @@ def test_fastapi_unhandled_exception_both_semconv(self): server_spans = [ span for span in spans - if span.kind == trace.SpanKind.SERVER and hasattr(span, "attributes") + if span.kind == trace.SpanKind.SERVER + and hasattr(span, "attributes") and span.attributes and HTTP_URL in span.attributes ]