diff --git a/CHANGELOG.md b/CHANGELOG.md index 9318994a41..f1193f685b 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 - `opentelemetry-instrumentation-openai-v2` Update doc for OpenAI Instrumentation to support OpenAI Compatible Platforms ([#3279](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3279)) -- `opentelemetry-instrumentation-system-metrics` Add `process` metrics and deprecated `process.runtime` prefixed ones +- `opentelemetry-instrumentation-system-metrics` Add `process` metrics and deprecated `process.runtime` prefixed ones ([#3250](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3250)) - `opentelemetry-instrumentation-botocore` Add support for GenAI user events and lazy initialize tracer ([#3258](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3258)) @@ -41,6 +41,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3247](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3247)) - `opentelemetry-instrumentation-asyncpg` Fix fallback for empty queries. ([#3253](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3253)) +- `opentelemetry-instrumentation-requests` always record span status code in duration metric + ([#3323](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3323)) ## Version 1.30.0/0.51b0 (2025-02-03) @@ -100,7 +102,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Breaking changes -- `opentelemetry-exporter-prometheus-remote-write` updated protobuf required version from 4.21 to 5.26 and regenerated protobufs +- `opentelemetry-exporter-prometheus-remote-write` updated protobuf required version from 4.21 to 5.26 and regenerated protobufs ([#3219](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3219)) - `opentelemetry-instrumentation-sqlalchemy` including sqlcomment in `db.statement` span attribute value is now opt-in ([#3112](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3112)) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 0764a29b12..1940e2f64b 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -101,15 +101,14 @@ def response_hook(span, request_obj, response): _set_http_network_protocol_version, _set_http_peer_port_client, _set_http_scheme, - _set_http_status_code, _set_http_url, + _set_status, _StabilityMode, ) from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.requests.package import _instruments from opentelemetry.instrumentation.requests.version import __version__ from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, is_http_instrumentation_enabled, suppress_http_instrumentation, ) @@ -126,7 +125,6 @@ def response_hook(span, request_obj, response): ) from opentelemetry.trace import SpanKind, Tracer, get_tracer from opentelemetry.trace.span import Span -from opentelemetry.trace.status import StatusCode from opentelemetry.util.http import ( ExcludeList, get_excluded_urls, @@ -142,6 +140,32 @@ def response_hook(span, request_obj, response): _ResponseHookT = Optional[Callable[[Span, PreparedRequest, Response], None]] +def _set_http_status_code_attribute( + span, + status_code, + metric_attributes=None, + sem_conv_opt_in_mode=_StabilityMode.DEFAULT, +): + status_code_str = str(status_code) + try: + status_code = int(status_code) + except ValueError: + status_code = -1 + if metric_attributes is None: + metric_attributes = {} + # When we have durations we should set metrics only once + # Also the decision to include status code on a histogram should + # not be dependent on tracing decisions. + _set_status( + span, + metric_attributes, + status_code, + status_code_str, + server_span=False, + sem_conv_opt_in_mode=sem_conv_opt_in_mode, + ) + + # pylint: disable=unused-argument # pylint: disable=R0915 def _instrument( @@ -269,25 +293,12 @@ def get_or_create_headers(): if isinstance(result, Response): span_attributes = {} - if span.is_recording(): - _set_http_status_code( - span_attributes, - result.status_code, - sem_conv_opt_in_mode, - ) - _set_http_status_code( - metric_labels, result.status_code, sem_conv_opt_in_mode - ) - status_code = http_status_to_status_code( - result.status_code - ) - span.set_status(status_code) - if ( - _report_new(sem_conv_opt_in_mode) - and status_code is StatusCode.ERROR - ): - span_attributes[ERROR_TYPE] = str(result.status_code) - metric_labels[ERROR_TYPE] = str(result.status_code) + _set_http_status_code_attribute( + span, + result.status_code, + metric_labels, + sem_conv_opt_in_mode, + ) if result.raw is not None: version = getattr(result.raw, "version", None) diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 366cd0c233..efe9f75f56 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -832,3 +832,34 @@ def test_basic_metric_both_semconv(self): dict(data_point.attributes), ) self.assertEqual(data_point.count, 1) + + def test_basic_metric_non_recording_span(self): + expected_attributes = { + SpanAttributes.HTTP_STATUS_CODE: 200, + SpanAttributes.HTTP_HOST: "examplehost", + SpanAttributes.NET_PEER_PORT: 8000, + SpanAttributes.NET_PEER_NAME: "examplehost", + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SCHEME: "http", + } + + with mock.patch("opentelemetry.trace.INVALID_SPAN") as mock_span: + RequestsInstrumentor().uninstrument() + RequestsInstrumentor().instrument( + tracer_provider=trace.NoOpTracerProvider() + ) + mock_span.is_recording.return_value = False + result = self.perform_request(self.URL) + self.assertEqual(result.text, "Hello!") + self.assertFalse(mock_span.is_recording()) + self.assertTrue(mock_span.is_recording.called) + self.assertFalse(mock_span.set_attribute.called) + self.assertFalse(mock_span.set_status.called) + metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 1) + duration_data_point = metrics[0].data.data_points[0] + self.assertDictEqual( + expected_attributes, dict(duration_data_point.attributes) + ) + self.assertEqual(duration_data_point.count, 1)