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 29e2cae8bd..0019cc310a 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -70,6 +70,7 @@ http_status_to_status_code, ) from opentelemetry.instrumentation._semconv import ( + _filter_duration_attrs, _report_old, _report_new, _set_http_hostname, @@ -80,6 +81,9 @@ _set_http_scheme, _set_http_url, _set_http_status_code, + _SPAN_ATTRIBUTES_ERROR_TYPE, + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS, + _SPAN_ATTRIBUTES_NETWORK_PEER_PORT, _OpenTelemetrySemanticConventionStability, _OpenTelemetryStabilityMode, _OpenTelemetryStabilitySignalType, @@ -105,11 +109,6 @@ _RequestHookT = Optional[Callable[[Span, PreparedRequest], None]] _ResponseHookT = Optional[Callable[[Span, PreparedRequest], None]] -# TODO: will come through semconv package once updated -_SPAN_ATTRIBUTES_ERROR_TYPE = "error.type" -_SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS ="network.peer.address" -_SPAN_ATTRIBUTES_NETWORK_PEER_PORT = "network.peer.port" - # pylint: disable=unused-argument # pylint: disable=R0915 @@ -177,11 +176,13 @@ def get_or_create_headers(): _set_http_net_peer_name(metric_labels, parsed_url.hostname, sem_conv_opt_in_mode) if _report_new(sem_conv_opt_in_mode): _set_http_hostname(span_attributes, parsed_url.hostname, sem_conv_opt_in_mode) + # Use semconv library when available span_attributes[_SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS] = parsed_url.hostname if parsed_url.port: _set_http_port(metric_labels, parsed_url.port, sem_conv_opt_in_mode) if _report_new(sem_conv_opt_in_mode): _set_http_port(span_attributes, parsed_url.port, sem_conv_opt_in_mode) + # Use semconv library when available span_attributes[_SPAN_ATTRIBUTES_NETWORK_PEER_PORT] = parsed_url.port except ValueError: pass @@ -229,20 +230,23 @@ def get_or_create_headers(): _set_http_network_protocol_version(metric_labels, version_text, sem_conv_opt_in_mode) if _report_new(sem_conv_opt_in_mode): _set_http_network_protocol_version(span_attributes, version_text, sem_conv_opt_in_mode) - if exception is not None: - if _report_new(sem_conv_opt_in_mode): - span_attributes[_SPAN_ATTRIBUTES_ERROR_TYPE] = type(exception).__name__ - metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = type(exception).__name__ for k, v in span_attributes.items(): span.set_attribute(k, v) if callable(response_hook): response_hook(span, request, result) - if _report_old(sem_conv_opt_in_mode) and duration_histogram_old is not None: - duration_histogram_old.record(max(round(elapsed_time * 1000), 0), attributes=metric_labels) - # duration_histogram_new.record(elapsed_time, attributes=metric_labels) - + if exception is not None and _report_new(sem_conv_opt_in_mode): + span.set_attribute(_SPAN_ATTRIBUTES_ERROR_TYPE, type(exception).__name__) + metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = type(exception).__name__ + + if duration_histogram_old is not None: + duration_attrs_old = _filter_duration_attrs(metric_labels, _OpenTelemetryStabilityMode.DEFAULT) + duration_histogram_old.record(max(round(elapsed_time * 1000), 0), attributes=duration_attrs_old) + if duration_histogram_new is not None: + duration_attrs_new = _filter_duration_attrs(metric_labels, _OpenTelemetryStabilityMode.HTTP) + duration_histogram_new.record(elapsed_time, attributes=duration_attrs_new) + if exception is not None: raise exception.with_traceback(exception.__traceback__) diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 12178bc987..c845f73d19 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -28,6 +28,8 @@ from opentelemetry.instrumentation.requests import RequestsInstrumentor from opentelemetry.instrumentation.utils import _SUPPRESS_INSTRUMENTATION_KEY from opentelemetry.instrumentation._semconv import ( + _SPAN_ATTRIBUTES_ERROR_TYPE, + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS, _OTEL_SEMCONV_STABILITY_OPT_IN_KEY, _OpenTelemetrySemanticConventionStability, ) @@ -68,7 +70,7 @@ class RequestsIntegrationTestBase(abc.ABC): # pylint: disable=too-many-public-methods URL = "http://mock/status/200" - HOST = "mock/status" + HOST = "mock" # pylint: disable=invalid-name def setUp(self): @@ -150,33 +152,62 @@ def test_basic(self): span, opentelemetry.instrumentation.requests ) + def test_basic_new_semconv(self): + result = self.perform_request(self.URL) + self.assertEqual(result.text, "Hello!") + span = self.assert_span() - # def test_basic_new_semconv(self): - # print("here2") - # result = self.perform_request(self.URL) - # self.assertEqual(result.text, "Hello!") - # span = self.assert_span() + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "GET") - # self.assertIs(span.kind, trace.SpanKind.CLIENT) - # self.assertEqual(span.name, "GET") + self.assertEqual( + span.attributes, + { + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.URL_FULL: self.URL, + SpanAttributes.SERVER_ADDRESS: self.HOST, + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS: "mock", + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + }, + ) + + self.assertIs(span.status.status_code, trace.StatusCode.UNSET) + + self.assertEqualSpanInstrumentationScope( + span, opentelemetry.instrumentation.requests + ) + + def test_basic_both_semconv(self): + result = self.perform_request(self.URL) + self.assertEqual(result.text, "Hello!") + span = self.assert_span() - # self.assertEqual( - # span.attributes, - # { - # SpanAttributes.HTTP_REQUEST_METHOD: "GET", - # SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL: "GET", - # SpanAttributes.URL_FULL: self.URL, - # SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, - # SpanAttributes.SERVER_ADDRESS: self.HOST, - # }, - # ) + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "GET") - # self.assertIs(span.status.status_code, trace.StatusCode.UNSET) + self.assertEqual( + span.attributes, + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.HTTP_URL: self.URL, + SpanAttributes.URL_FULL: self.URL, + SpanAttributes.HTTP_HOST: self.HOST, + SpanAttributes.SERVER_ADDRESS: self.HOST, + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS: "mock", + SpanAttributes.HTTP_STATUS_CODE: 200, + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + }, + ) - # self.assertEqualSpanInstrumentationInfo( - # span, opentelemetry.instrumentation.requests - # ) + self.assertIs(span.status.status_code, trace.StatusCode.UNSET) + self.assertEqualSpanInstrumentationScope( + span, opentelemetry.instrumentation.requests + ) def test_hooks(self): def request_hook(span, request_obj): @@ -259,6 +290,51 @@ def test_not_foundbasic(self): trace.StatusCode.ERROR, ) + def test_not_foundbasic_new_semconv(self): + url_404 = "http://mock/status/404" + httpretty.register_uri( + httpretty.GET, + url_404, + status=404, + ) + result = self.perform_request(url_404) + self.assertEqual(result.status_code, 404) + + span = self.assert_span() + + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_RESPONSE_STATUS_CODE), 404 + ) + + self.assertIs( + span.status.status_code, + trace.StatusCode.ERROR, + ) + + def test_not_foundbasic_both_semconv(self): + url_404 = "http://mock/status/404" + httpretty.register_uri( + httpretty.GET, + url_404, + status=404, + ) + result = self.perform_request(url_404) + self.assertEqual(result.status_code, 404) + + span = self.assert_span() + + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_STATUS_CODE), 404 + ) + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_RESPONSE_STATUS_CODE), 404 + ) + + self.assertIs( + span.status.status_code, + trace.StatusCode.ERROR, + ) + def test_uninstrument(self): RequestsInstrumentor().uninstrument() result = self.perform_request(self.URL) @@ -413,6 +489,27 @@ def test_requests_exception_without_response(self, *_, **__): ) self.assertEqual(span.status.status_code, StatusCode.ERROR) + @mock.patch( + "requests.adapters.HTTPAdapter.send", + side_effect=requests.RequestException, + ) + def test_requests_exception_new_semconv(self, *_, **__): + with self.assertRaises(requests.RequestException): + self.perform_request(self.URL) + + span = self.assert_span() + self.assertEqual( + span.attributes, + { + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.URL_FULL: self.URL, + SpanAttributes.SERVER_ADDRESS: self.HOST, + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS: "mock", + _SPAN_ATTRIBUTES_ERROR_TYPE: "RequestException", + }, + ) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + mocked_response = requests.Response() mocked_response.status_code = 500 mocked_response.reason = "Internal Server Error" @@ -534,6 +631,22 @@ class TestRequestsIntergrationMetric(TestBase): def setUp(self): super().setUp() + test_name = "" + if hasattr(self, "_testMethodName"): + test_name = self._testMethodName + sem_conv_mode = "default" + if "new_semconv" in test_name: + sem_conv_mode = "http" + elif "both_semconv" in test_name: + sem_conv_mode = "http/dup" + self.env_patch = mock.patch.dict( + "os.environ", + { + _OTEL_SEMCONV_STABILITY_OPT_IN_KEY: sem_conv_mode, + }, + ) + self.env_patch.start() + _OpenTelemetrySemanticConventionStability._initialized = False RequestsInstrumentor().instrument(meter_provider=self.meter_provider) httpretty.enable() @@ -541,6 +654,7 @@ def setUp(self): def tearDown(self): super().tearDown() + self.env_patch.stop() RequestsInstrumentor().uninstrument() httpretty.disable() @@ -552,22 +666,90 @@ def test_basic_metric_success(self): self.perform_request(self.URL) expected_attributes = { - "http.status_code": 200, - "http.host": "examplehost", - "net.peer.port": 8000, - "net.peer.name": "examplehost", - "http.method": "GET", - "http.flavor": "1.1", - "http.scheme": "http", + 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", } for ( resource_metrics ) in self.memory_metrics_reader.get_metrics_data().resource_metrics: for scope_metrics in resource_metrics.scope_metrics: + self.assertEqual(len(scope_metrics.metrics), 1) for metric in scope_metrics.metrics: + self.assertEqual(metric.unit, "ms") + self.assertEqual(metric.description, "measures the duration of the outbound HTTP request") for data_point in metric.data.data_points: self.assertDictEqual( expected_attributes, dict(data_point.attributes) ) self.assertEqual(data_point.count, 1) + + def test_basic_metric_new_semconv(self): + self.perform_request(self.URL) + + expected_attributes = { + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.SERVER_ADDRESS: "examplehost", + SpanAttributes.SERVER_PORT: 8000, + SpanAttributes.SERVER_ADDRESS: "examplehost", + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + } + + for ( + resource_metrics + ) in self.memory_metrics_reader.get_metrics_data().resource_metrics: + for scope_metrics in resource_metrics.scope_metrics: + self.assertEqual(len(scope_metrics.metrics), 1) + for metric in scope_metrics.metrics: + self.assertEqual(metric.unit, "s") + self.assertEqual(metric.description, "Duration of HTTP client requests.") + for data_point in metric.data.data_points: + self.assertDictEqual( + expected_attributes, dict(data_point.attributes) + ) + self.assertEqual(data_point.count, 1) + + def test_basic_metric_both_semconv(self): + self.perform_request(self.URL) + + expected_attributes_old = { + 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", + } + + expected_attributes_new = { + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.SERVER_ADDRESS: "examplehost", + SpanAttributes.SERVER_PORT: 8000, + SpanAttributes.SERVER_ADDRESS: "examplehost", + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + } + + for ( + resource_metrics + ) in self.memory_metrics_reader.get_metrics_data().resource_metrics: + for scope_metrics in resource_metrics.scope_metrics: + self.assertEqual(len(scope_metrics.metrics), 2) + for metric in scope_metrics.metrics: + for data_point in metric.data.data_points: + if metric.unit == "ms": + self.assertDictEqual( + expected_attributes_old, dict(data_point.attributes) + ) + else: + self.assertDictEqual( + expected_attributes_new, dict(data_point.attributes) + ) + self.assertEqual(data_point.count, 1) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 526284a955..39dd1ec726 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -18,6 +18,40 @@ from opentelemetry.semconv.trace import SpanAttributes +# TODO: will come through semconv package once updated +_SPAN_ATTRIBUTES_ERROR_TYPE = "error.type" +_SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS ="network.peer.address" +_SPAN_ATTRIBUTES_NETWORK_PEER_PORT = "network.peer.port" + +_client_duration_attrs_old = [ + SpanAttributes.HTTP_STATUS_CODE, + SpanAttributes.HTTP_HOST, + SpanAttributes.NET_PEER_PORT, + SpanAttributes.NET_PEER_NAME, + SpanAttributes.HTTP_METHOD, + SpanAttributes.HTTP_FLAVOR, + SpanAttributes.HTTP_SCHEME, +] + +_client_duration_attrs_new = [ + _SPAN_ATTRIBUTES_ERROR_TYPE, + SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL, + SpanAttributes.HTTP_REQUEST_METHOD, + SpanAttributes.HTTP_RESPONSE_STATUS_CODE, + SpanAttributes.NET_PROTOCOL_VERSION, + SpanAttributes.SERVER_ADDRESS, + SpanAttributes.SERVER_PORT, + SpanAttributes.URL_SCHEME, +] + +def _filter_duration_attrs(attrs, sem_conv_opt_in_mode): + filtered_attrs = {} + allowed_attributes = _client_duration_attrs_new if sem_conv_opt_in_mode == _OpenTelemetryStabilityMode.HTTP else _client_duration_attrs_old + for key, val in attrs.items(): + if key in allowed_attributes: + filtered_attrs[key] = val + return filtered_attrs + def set_string_attribute(dict, key, value): if value: @@ -37,7 +71,7 @@ def _set_http_method(result, original, normalized, sem_conv_opt_in_mode): normalized = normalized.strip() # See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#common-attributes # Method is case sensitive. "http.request.method_original" should not be sanitized or automatically capitalized. - if original != normalized and sem_conv_opt_in_mode != _OpenTelemetryStabilityMode.DEFAULT: + if original != normalized and _report_new(): set_string_attribute(result, SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL, original) if _report_old(sem_conv_opt_in_mode): @@ -84,9 +118,9 @@ def _set_http_port(result, port, sem_conv_opt_in_mode): def _set_http_status_code(result, code, sem_conv_opt_in_mode): if _report_old(sem_conv_opt_in_mode): - set_string_attribute(result, SpanAttributes.HTTP_STATUS_CODE, code) + set_int_attribute(result, SpanAttributes.HTTP_STATUS_CODE, code) if _report_new(sem_conv_opt_in_mode): - set_string_attribute(result, SpanAttributes.HTTP_RESPONSE_STATUS_CODE, code) + set_int_attribute(result, SpanAttributes.HTTP_RESPONSE_STATUS_CODE, code) def _set_http_network_protocol_version(result, version, sem_conv_opt_in_mode):