diff --git a/CHANGELOG.md b/CHANGELOG.md index 39236a33f3..8c31e01235 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2385](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2385)) - `opentelemetry-instrumentation-asyncio` Fixes async generator coroutines not being awaited ([#2792](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2792)) +- `opentelemetry-instrumentation-tornado` Handle http client exception and record exception info into span + ([#2563](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2563)) ## Version 1.26.0/0.47b0 (2024-07-23) diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py index 090f87a88b..fa0e53bf95 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/client.py @@ -21,7 +21,7 @@ from opentelemetry.instrumentation.utils import http_status_to_status_code from opentelemetry.propagate import inject from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.trace.status import Status +from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util.http import remove_url_credentials @@ -105,37 +105,53 @@ def _finish_tracing_callback( request_size_histogram, response_size_histogram, ): + response = None status_code = None + status = None description = None - exc = future.exception() - - response = future.result() - if span.is_recording() and exc: + exc = future.exception() + if exc: + description = f"{type(exc).__qualname__}: {exc}" if isinstance(exc, HTTPError): + response = exc.response status_code = exc.code - description = f"{type(exc).__name__}: {exc}" + status = Status( + status_code=http_status_to_status_code(status_code), + description=description, + ) + else: + status = Status( + status_code=StatusCode.ERROR, + description=description, + ) + span.record_exception(exc) else: + response = future.result() status_code = response.code + status = Status( + status_code=http_status_to_status_code(status_code), + description=description, + ) if status_code is not None: span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) - span.set_status( - Status( - status_code=http_status_to_status_code(status_code), - description=description, - ) - ) + span.set_status(status) - metric_attributes = _create_metric_attributes(response) - request_size = int(response.request.headers.get("Content-Length", 0)) - response_size = int(response.headers.get("Content-Length", 0)) + if response is not None: + metric_attributes = _create_metric_attributes(response) + request_size = int(response.request.headers.get("Content-Length", 0)) + response_size = int(response.headers.get("Content-Length", 0)) - duration_histogram.record( - response.request_time, attributes=metric_attributes - ) - request_size_histogram.record(request_size, attributes=metric_attributes) - response_size_histogram.record(response_size, attributes=metric_attributes) + duration_histogram.record( + response.request_time, attributes=metric_attributes + ) + request_size_histogram.record( + request_size, attributes=metric_attributes + ) + response_size_histogram.record( + response_size, attributes=metric_attributes + ) if response_hook: response_hook(span, future) diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py index 01cdddceed..daf2ddd846 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py @@ -16,6 +16,7 @@ from unittest.mock import Mock, patch from http_server_mock import HttpServerMock +from tornado.httpclient import HTTPClientError from tornado.testing import AsyncHTTPTestCase from opentelemetry import trace @@ -32,7 +33,7 @@ from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.test_base import TestBase from opentelemetry.test.wsgitestutil import WsgiTestBase -from opentelemetry.trace import SpanKind +from opentelemetry.trace import SpanKind, StatusCode from opentelemetry.util.http import ( OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, @@ -493,7 +494,6 @@ def test_response_headers(self): self.assertEqual(len(spans), 3) self.assertTraceResponseHeaderMatchesSpan(response.headers, spans[1]) - self.memory_exporter.clear() set_global_response_propagator(orig) def test_credential_removal(self): @@ -602,6 +602,49 @@ def client_response_hook(span, response): self.memory_exporter.clear() +class TestTornadoHTTPClientInstrumentation(TornadoTest, WsgiTestBase): + def test_http_client_success_response(self): + response = self.fetch("/") + self.assertEqual(response.code, 201) + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + manual, server, client = self.sorted_spans(spans) + self.assertEqual(manual.name, "manual") + self.assertEqual(server.name, "GET /") + self.assertEqual(client.name, "GET") + self.assertEqual(client.status.status_code, StatusCode.UNSET) + self.memory_exporter.clear() + + def test_http_client_failed_response(self): + # when an exception isn't thrown + response = self.fetch("/some-404") + self.assertEqual(response.code, 404) + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + server, client = self.sorted_spans(spans) + self.assertEqual(server.name, "GET /some-404") + self.assertEqual(client.name, "GET") + self.assertEqual(client.status.status_code, StatusCode.ERROR) + self.memory_exporter.clear() + + # when an exception is thrown + try: + response = self.fetch("/some-404", raise_error=True) + self.assertEqual(response.code, 404) + except HTTPClientError: + pass # expected exception - continue + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + server, client = self.sorted_spans(spans) + self.assertEqual(server.name, "GET /some-404") + self.assertEqual(client.name, "GET") + self.assertEqual(client.status.status_code, StatusCode.ERROR) + self.memory_exporter.clear() + + class TestTornadoUninstrument(TornadoTest): def test_uninstrument(self): response = self.fetch("/")