diff --git a/.changelog/4723.fixed b/.changelog/4723.fixed new file mode 100644 index 0000000000..e6fb6b4ece --- /dev/null +++ b/.changelog/4723.fixed @@ -0,0 +1 @@ +`opentelemetry-instrumentation-django`: fix spans ending prematurely for StreamingHttpResponse \ No newline at end of file diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py index be8bd70b13..4191eca0e5 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py @@ -419,7 +419,17 @@ def process_response(self, request, response): self._active_request_counter.add(-1, active_requests_count_attrs) if activation and span: - if exception: + if response.streaming and not exception: + original_close = response.close + + def _end_span_on_close(*args, **kwargs): + try: + original_close(*args, **kwargs) + finally: + activation.__exit__(None, None, None) + + response.close = _end_span_on_close + elif exception: activation.__exit__( type(exception), exception, diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 3d00443a6b..afd21698ca 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -58,6 +58,7 @@ excluded_noarg2, response_with_custom_header, route_span_name, + streaming_view, traced, traced_template, ) @@ -1136,3 +1137,70 @@ def test_metrics_recorded_with_exception(self): for metric in sm.metrics ) self.assertTrue(histogram_found) + + +class TestMiddlewareStreamingResponse(WsgiTestBase): + """Test span end timing relative to StreamingHttpResponse consumption.""" + + @classmethod + def setUpClass(cls): + conf.settings.configure(ROOT_URLCONF=modules[__name__]) + super().setUpClass() + + def setUp(self): + super().setUp() + setup_test_environment() + _django_instrumentor.instrument() + + def tearDown(self): + super().tearDown() + teardown_test_environment() + _django_instrumentor.uninstrument() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + conf.settings = conf.LazySettings() + + def test_span_ends_before_streaming_content_consumed(self): + """Reproduces the bug: span ends before the stream is actually sent.""" + events = [] + + def view(request): + return streaming_view(request, events) + + route = re_path(r"^streaming/", view) + urlpatterns.append(route) + self.addCleanup(urlpatterns.remove, route) + + original_end = Span.end + + def patched_end(self, *args, **kwargs): + events.append("span_closed") + return original_end(self, *args, **kwargs) + + with patch.object(Span, "end", patched_end): + response = Client().get("/streaming/") + spans_before_consumption = ( + self.memory_exporter.get_finished_spans() + ) + # Now actually consume the stream, simulating a real WSGI server + list(response.streaming_content) + + self.assertEqual( + len(spans_before_consumption), + 0, + "Span should not be finished yet: stream not yet consumed", + ) + self.assertIn("span_closed", events) + self.assertIn("generator_started", events) + + # THIS is the assertion that proves the bug: + # the generator should start BEFORE the span closes. + self.assertLess( + events.index("generator_started"), + events.index("span_closed"), + "Span ended before streaming content was consumed: " + f"event order was {events}", + ) + diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/views.py b/instrumentation/opentelemetry-instrumentation-django/tests/views.py index d9aa62a5c1..8b864d5206 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/views.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/views.py @@ -1,7 +1,7 @@ # Copyright The OpenTelemetry Authors # SPDX-License-Identifier: Apache-2.0 -from django.http import HttpResponse +from django.http import HttpResponse, StreamingHttpResponse def traced(request): # pylint: disable=unused-argument @@ -79,3 +79,12 @@ async def async_with_custom_header(request): response.headers["custom-test-header-1"] = "test-header-value-1" response.headers["custom-test-header-2"] = "test-header-value-2" return response + +def streaming_view(request, events): # pylint: disable=unused-argument + def stream_generator(): + events.append("generator_started") + yield b"chunk1" + yield b"chunk2" + events.append("generator_finished") + + return StreamingHttpResponse(stream_generator())