From 92da527977390dc55807ab2547f31f8788255e9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Tue, 23 Jul 2024 14:09:11 -0300 Subject: [PATCH] HTTP semantic convention stability migration for urllib3 (#2715) --- CHANGELOG.md | 6 + instrumentation/README.md | 2 +- .../instrumentation/urllib3/__init__.py | 313 ++++++++++++++---- .../instrumentation/urllib3/package.py | 2 + .../tests/test_urllib3_integration.py | 230 ++++++++++++- .../tests/test_urllib3_metrics.py | 305 +++++++++++++++-- 6 files changed, 748 insertions(+), 110 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7caff5e940..bc400865a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2673](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2673)) - `opentelemetry-instrumentation-django` Add `http.target` to Django duration metric attributes ([#2624](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2624)) +- `opentelemetry-instrumentation-urllib3` Implement new semantic convention opt-in migration + ([#2715](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2715)) - `opentelemetry-instrumentation-django` Implement new semantic convention opt-in with stable http semantic conventions ([#2714](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2714)) @@ -48,6 +50,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2580](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2580)) - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `asgi` middleware ([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610)) +- Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `fastapi` middleware + ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682)) +- `opentelemetry-instrumentation-urllib3` Populate `{method}` as `HTTP` on `_OTHER` methods for span name + ([#2715](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2715)) - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `fastapi` instrumentation ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682)) - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `django` middleware diff --git a/instrumentation/README.md b/instrumentation/README.md index 900ee6ea74..555e0bcd2a 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -46,5 +46,5 @@ | [opentelemetry-instrumentation-tornado](./opentelemetry-instrumentation-tornado) | tornado >= 5.1.1 | Yes | experimental | [opentelemetry-instrumentation-tortoiseorm](./opentelemetry-instrumentation-tortoiseorm) | tortoise-orm >= 0.17.0 | No | experimental | [opentelemetry-instrumentation-urllib](./opentelemetry-instrumentation-urllib) | urllib | Yes | experimental -| [opentelemetry-instrumentation-urllib3](./opentelemetry-instrumentation-urllib3) | urllib3 >= 1.0.0, < 3.0.0 | Yes | experimental +| [opentelemetry-instrumentation-urllib3](./opentelemetry-instrumentation-urllib3) | urllib3 >= 1.0.0, < 3.0.0 | Yes | migration | [opentelemetry-instrumentation-wsgi](./opentelemetry-instrumentation-wsgi) | wsgi | Yes | migration \ No newline at end of file diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py index add5db8f19..a05084725d 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py @@ -87,25 +87,49 @@ def response_hook(span, request, response): import urllib3.connectionpool import wrapt +from opentelemetry.instrumentation._semconv import ( + _client_duration_attrs_new, + _client_duration_attrs_old, + _filter_semconv_duration_attrs, + _get_schema_url, + _HTTPStabilityMode, + _OpenTelemetrySemanticConventionStability, + _OpenTelemetryStabilitySignalType, + _report_new, + _report_old, + _set_http_host, + _set_http_method, + _set_http_net_peer_name_client, + _set_http_network_protocol_version, + _set_http_peer_port_client, + _set_http_scheme, + _set_http_url, + _set_status, +) from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.urllib3.package import _instruments from opentelemetry.instrumentation.urllib3.version import __version__ from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, is_http_instrumentation_enabled, suppress_http_instrumentation, unwrap, ) from opentelemetry.metrics import Histogram, get_meter from opentelemetry.propagate import inject +from opentelemetry.semconv._incubating.metrics.http_metrics import ( + create_http_client_request_body_size, + create_http_client_response_body_size, +) from opentelemetry.semconv.metrics import MetricInstruments -from opentelemetry.semconv.trace import SpanAttributes +from opentelemetry.semconv.metrics.http_metrics import ( + HTTP_CLIENT_REQUEST_DURATION, +) from opentelemetry.trace import Span, SpanKind, Tracer, get_tracer -from opentelemetry.trace.status import Status from opentelemetry.util.http import ( ExcludeList, get_excluded_urls, parse_excluded_urls, + sanitize_method, ) from opentelemetry.util.http.httplib import set_ip_on_next_http_connection @@ -158,12 +182,18 @@ def _instrument(self, **kwargs): ``excluded_urls``: A string containing a comma-delimited list of regexes used to exclude URLs from tracking """ + # initialize semantic conventions opt-in if needed + _OpenTelemetrySemanticConventionStability._initialize() + sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( + _OpenTelemetryStabilitySignalType.HTTP, + ) + schema_url = _get_schema_url(sem_conv_opt_in_mode) tracer_provider = kwargs.get("tracer_provider") tracer = get_tracer( __name__, __version__, tracer_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=schema_url, ) excluded_urls = kwargs.get("excluded_urls") @@ -173,30 +203,57 @@ def _instrument(self, **kwargs): __name__, __version__, meter_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", - ) - - duration_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_DURATION, - unit="ms", - description="Measures the duration of outbound HTTP requests.", - ) - request_size_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_REQUEST_SIZE, - unit="By", - description="Measures the size of HTTP request messages.", - ) - response_size_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE, - unit="By", - description="Measures the size of HTTP response messages.", + schema_url=schema_url, ) + duration_histogram_old = None + request_size_histogram_old = None + response_size_histogram_old = None + if _report_old(sem_conv_opt_in_mode): + # http.client.duration histogram + duration_histogram_old = meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_DURATION, + unit="ms", + description="Measures the duration of the outbound HTTP request", + ) + # http.client.request.size histogram + request_size_histogram_old = meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_REQUEST_SIZE, + unit="By", + description="Measures the size of HTTP request messages.", + ) + # http.client.response.size histogram + response_size_histogram_old = meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE, + unit="By", + description="Measures the size of HTTP response messages.", + ) + duration_histogram_new = None + request_size_histogram_new = None + response_size_histogram_new = None + if _report_new(sem_conv_opt_in_mode): + # http.client.request.duration histogram + duration_histogram_new = meter.create_histogram( + name=HTTP_CLIENT_REQUEST_DURATION, + unit="s", + description="Duration of HTTP client requests.", + ) + # http.client.request.body.size histogram + request_size_histogram_new = create_http_client_request_body_size( + meter + ) + # http.client.response.body.size histogram + response_size_histogram_new = ( + create_http_client_response_body_size(meter) + ) _instrument( tracer, - duration_histogram, - request_size_histogram, - response_size_histogram, + duration_histogram_old, + duration_histogram_new, + request_size_histogram_old, + request_size_histogram_new, + response_size_histogram_old, + response_size_histogram_new, request_hook=kwargs.get("request_hook"), response_hook=kwargs.get("response_hook"), url_filter=kwargs.get("url_filter"), @@ -205,21 +262,33 @@ def _instrument(self, **kwargs): if excluded_urls is None else parse_excluded_urls(excluded_urls) ), + sem_conv_opt_in_mode=sem_conv_opt_in_mode, ) def _uninstrument(self, **kwargs): _uninstrument() +def _get_span_name(method: str) -> str: + method = sanitize_method(method.strip()) + if method == "_OTHER": + method = "HTTP" + return method + + def _instrument( tracer: Tracer, - duration_histogram: Histogram, - request_size_histogram: Histogram, - response_size_histogram: Histogram, + duration_histogram_old: Histogram, + duration_histogram_new: Histogram, + request_size_histogram_old: Histogram, + request_size_histogram_new: Histogram, + response_size_histogram_old: Histogram, + response_size_histogram_new: Histogram, request_hook: _RequestHookT = None, response_hook: _ResponseHookT = None, url_filter: _UrlFilterT = None, excluded_urls: ExcludeList = None, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): def instrumented_urlopen(wrapped, instance, args, kwargs): if not is_http_instrumentation_enabled(): @@ -233,11 +302,16 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): headers = _prepare_headers(kwargs) body = _get_url_open_arg("body", args, kwargs) - span_name = method.strip() - span_attributes = { - SpanAttributes.HTTP_METHOD: method, - SpanAttributes.HTTP_URL: url, - } + span_name = _get_span_name(method) + span_attributes = {} + + _set_http_method( + span_attributes, + method, + sanitize_method(method), + sem_conv_opt_in_mode, + ) + _set_http_url(span_attributes, url, sem_conv_opt_in_mode) with tracer.start_as_current_span( span_name, kind=SpanKind.CLIENT, attributes=span_attributes @@ -245,32 +319,43 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): if callable(request_hook): request_hook(span, instance, headers, body) inject(headers) - + # TODO: add error handling to also set exception `error.type` in new semconv with suppress_http_instrumentation(): start_time = default_timer() response = wrapped(*args, **kwargs) - elapsed_time = round((default_timer() - start_time) * 1000) + duration_s = default_timer() - start_time + # set http status code based on semconv + metric_attributes = {} + _set_status_code_attribute( + span, response.status, metric_attributes, sem_conv_opt_in_mode + ) - _apply_response(span, response) if callable(response_hook): response_hook(span, instance, response) request_size = _get_body_size(body) response_size = int(response.headers.get("Content-Length", 0)) - metric_attributes = _create_metric_attributes( - instance, response, method + _set_metric_attributes( + metric_attributes, + instance, + response, + method, + sem_conv_opt_in_mode, ) - duration_histogram.record( - elapsed_time, attributes=metric_attributes - ) - if request_size is not None: - request_size_histogram.record( - request_size, attributes=metric_attributes - ) - response_size_histogram.record( - response_size, attributes=metric_attributes + _record_metrics( + metric_attributes, + duration_histogram_old, + duration_histogram_new, + request_size_histogram_old, + request_size_histogram_new, + response_size_histogram_old, + response_size_histogram_new, + duration_s, + request_size, + response_size, + sem_conv_opt_in_mode, ) return response @@ -342,35 +427,133 @@ def _prepare_headers(urlopen_kwargs: typing.Dict) -> typing.Dict: return headers -def _apply_response(span: Span, response: urllib3.response.HTTPResponse): - if not span.is_recording(): - return - - span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, response.status) - span.set_status(Status(http_status_to_status_code(response.status))) +def _set_status_code_attribute( + span: Span, + status_code: int, + metric_attributes: dict = None, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, +) -> None: + + status_code_str = str(status_code) + try: + status_code = int(status_code) + except ValueError: + status_code = -1 + + if metric_attributes is None: + metric_attributes = {} + + _set_status( + span, + metric_attributes, + status_code, + status_code_str, + server_span=False, + sem_conv_opt_in_mode=sem_conv_opt_in_mode, + ) -def _create_metric_attributes( +def _set_metric_attributes( + metric_attributes: dict, instance: urllib3.connectionpool.HTTPConnectionPool, response: urllib3.response.HTTPResponse, method: str, -) -> dict: - metric_attributes = { - SpanAttributes.HTTP_METHOD: method, - SpanAttributes.HTTP_HOST: instance.host, - SpanAttributes.HTTP_SCHEME: instance.scheme, - SpanAttributes.HTTP_STATUS_CODE: response.status, - SpanAttributes.NET_PEER_NAME: instance.host, - SpanAttributes.NET_PEER_PORT: instance.port, - } + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, +) -> None: + + _set_http_host(metric_attributes, instance.host, sem_conv_opt_in_mode) + _set_http_scheme(metric_attributes, instance.scheme, sem_conv_opt_in_mode) + _set_http_method( + metric_attributes, + method, + sanitize_method(method), + sem_conv_opt_in_mode, + ) + _set_http_net_peer_name_client( + metric_attributes, instance.host, sem_conv_opt_in_mode + ) + _set_http_peer_port_client( + metric_attributes, instance.port, sem_conv_opt_in_mode + ) version = getattr(response, "version") if version: - metric_attributes[SpanAttributes.HTTP_FLAVOR] = ( - "1.1" if version == 11 else "1.0" + http_version = "1.1" if version == 11 else "1.0" + _set_http_network_protocol_version( + metric_attributes, http_version, sem_conv_opt_in_mode ) - return metric_attributes + +def _filter_attributes_semconv( + metric_attributes, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, +): + duration_attrs_old = None + duration_attrs_new = None + if _report_old(sem_conv_opt_in_mode): + duration_attrs_old = _filter_semconv_duration_attrs( + metric_attributes, + _client_duration_attrs_old, + _client_duration_attrs_new, + _HTTPStabilityMode.DEFAULT, + ) + if _report_new(sem_conv_opt_in_mode): + duration_attrs_new = _filter_semconv_duration_attrs( + metric_attributes, + _client_duration_attrs_old, + _client_duration_attrs_new, + _HTTPStabilityMode.HTTP, + ) + + return (duration_attrs_old, duration_attrs_new) + + +def _record_metrics( + metric_attributes: dict, + duration_histogram_old: Histogram, + duration_histogram_new: Histogram, + request_size_histogram_old: Histogram, + request_size_histogram_new: Histogram, + response_size_histogram_old: Histogram, + response_size_histogram_new: Histogram, + duration_s: float, + request_size: typing.Optional[int], + response_size: int, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, +): + attrs_old, attrs_new = _filter_attributes_semconv( + metric_attributes, sem_conv_opt_in_mode + ) + if duration_histogram_old: + # Default behavior is to record the duration in milliseconds + duration_histogram_old.record( + max(round(duration_s * 1000), 0), + attributes=attrs_old, + ) + + if duration_histogram_new: + # New semconv record the duration in seconds + duration_histogram_new.record( + duration_s, + attributes=attrs_new, + ) + + if request_size is not None: + if request_size_histogram_old: + request_size_histogram_old.record( + request_size, attributes=attrs_old + ) + + if request_size_histogram_new: + request_size_histogram_new.record( + request_size, attributes=attrs_new + ) + + if response_size_histogram_old: + response_size_histogram_old.record(response_size, attributes=attrs_old) + + if response_size_histogram_new: + response_size_histogram_new.record(response_size, attributes=attrs_new) def _uninstrument(): diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/package.py b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/package.py index 9d52db0a1f..568120c44d 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/package.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/package.py @@ -16,3 +16,5 @@ _instruments = ("urllib3 >= 1.0.0, < 3.0.0",) _supports_metrics = True + +_semconv_status = "migration" diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index 23124ea590..7dc899691f 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -16,16 +16,29 @@ from unittest import mock import httpretty +import httpretty.core +import httpretty.http import urllib3 import urllib3.exceptions from opentelemetry import trace +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _HTTPStabilityMode, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.urllib3 import URLLib3Instrumentor from opentelemetry.instrumentation.utils import ( suppress_http_instrumentation, suppress_instrumentation, ) from opentelemetry.propagate import get_global_textmap, set_global_textmap +from opentelemetry.semconv.attributes.http_attributes import ( + HTTP_REQUEST_METHOD, + HTTP_REQUEST_METHOD_ORIGINAL, + HTTP_RESPONSE_STATUS_CODE, +) +from opentelemetry.semconv.attributes.url_attributes import URL_FULL from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.mock_textmap import MockTextMapPropagator from opentelemetry.test.test_base import TestBase @@ -41,12 +54,23 @@ class TestURLLib3Instrumentor(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_PYTHON_URLLIB3_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg" + "OTEL_PYTHON_URLLIB3_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg", + OTEL_SEMCONV_STABILITY_OPT_IN: sem_conv_mode, }, ) + _OpenTelemetrySemanticConventionStability._initialized = False self.env_patch.start() self.exclude_patch = mock.patch( @@ -64,6 +88,7 @@ def setUp(self): def tearDown(self): super().tearDown() + self.env_patch.stop() URLLib3Instrumentor().uninstrument() httpretty.disable() @@ -81,46 +106,103 @@ def assert_span(self, exporter=None, num_spans=1): return span_list def assert_success_span( - self, response: urllib3.response.HTTPResponse, url: str + self, + response: urllib3.response.HTTPResponse, + url: str, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): self.assertEqual(b"Hello!", response.data) span = self.assert_span() self.assertIs(trace.SpanKind.CLIENT, span.kind) self.assertEqual("GET", span.name) - - attributes = { + self.assertEqual( + span.status.status_code, trace.status.StatusCode.UNSET + ) + attr_old = { SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_URL: url, SpanAttributes.HTTP_STATUS_CODE: 200, } - self.assertEqual(attributes, span.attributes) - def assert_exception_span(self, url: str): - span = self.assert_span() + attr_new = { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: url, + HTTP_RESPONSE_STATUS_CODE: 200, + } attributes = { + _HTTPStabilityMode.DEFAULT: attr_old, + _HTTPStabilityMode.HTTP: attr_new, + _HTTPStabilityMode.HTTP_DUP: {**attr_new, **attr_old}, + } + self.assertEqual(span.attributes, attributes.get(sem_conv_opt_in_mode)) + + def assert_exception_span( + self, + url: str, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, + ): + span = self.assert_span() + + attr_old = { SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_URL: url, } - self.assertEqual(attributes, span.attributes) + + attr_new = { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: url, + # TODO: Add `error.type` attribute when supported + } + + attributes = { + _HTTPStabilityMode.DEFAULT: attr_old, + _HTTPStabilityMode.HTTP: attr_new, + _HTTPStabilityMode.HTTP_DUP: {**attr_new, **attr_old}, + } + + self.assertEqual(span.attributes, attributes.get(sem_conv_opt_in_mode)) self.assertEqual( trace.status.StatusCode.ERROR, span.status.status_code ) @staticmethod def perform_request( - url: str, headers: typing.Mapping = None, retries: urllib3.Retry = None + url: str, + headers: typing.Mapping = None, + retries: urllib3.Retry = None, + method: str = "GET", ) -> urllib3.response.HTTPResponse: if retries is None: retries = urllib3.Retry.from_int(0) pool = urllib3.PoolManager() - return pool.request("GET", url, headers=headers, retries=retries) + return pool.request(method, url, headers=headers, retries=retries) def test_basic_http_success(self): response = self.perform_request(self.HTTP_URL) - self.assert_success_span(response, self.HTTP_URL) + self.assert_success_span( + response, + self.HTTP_URL, + sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT, + ) + + def test_basic_http_success_new_semconv(self): + response = self.perform_request(self.HTTP_URL) + self.assert_success_span( + response, + self.HTTP_URL, + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP, + ) + + def test_basic_http_success_both_semconv(self): + response = self.perform_request(self.HTTP_URL) + self.assert_success_span( + response, + self.HTTP_URL, + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP_DUP, + ) def test_basic_http_success_using_connection_pool(self): pool = urllib3.HTTPConnectionPool("mock") @@ -145,10 +227,32 @@ def test_schema_url(self): self.assertEqual(b"Hello!", response.data) span = self.assert_span() self.assertEqual( - span.instrumentation_info.schema_url, + span.instrumentation_scope.schema_url, "https://opentelemetry.io/schemas/1.11.0", ) + def test_schema_url_new_semconv(self): + pool = urllib3.HTTPSConnectionPool("mock") + response = pool.request("GET", "/status/200") + + self.assertEqual(b"Hello!", response.data) + span = self.assert_span() + self.assertEqual( + span.instrumentation_scope.schema_url, + "https://opentelemetry.io/schemas/v1.21.0", + ) + + def test_schema_url_both_semconv(self): + pool = urllib3.HTTPSConnectionPool("mock") + response = pool.request("GET", "/status/200") + + self.assertEqual(b"Hello!", response.data) + span = self.assert_span() + self.assertEqual( + span.instrumentation_scope.schema_url, + "https://opentelemetry.io/schemas/v1.21.0", + ) + def test_basic_not_found(self): url_404 = "http://mock/status/404" httpretty.register_uri(httpretty.GET, url_404, status=404) @@ -162,6 +266,80 @@ def test_basic_not_found(self): ) self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) + def test_basic_not_found_new_semconv(self): + url_404 = "http://mock/status/404" + httpretty.register_uri(httpretty.GET, url_404, status=404) + + response = self.perform_request(url_404) + self.assertEqual(404, response.status) + + span = self.assert_span() + self.assertEqual(404, span.attributes.get(HTTP_RESPONSE_STATUS_CODE)) + self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) + + def test_basic_not_found_both_semconv(self): + url_404 = "http://mock/status/404" + httpretty.register_uri(httpretty.GET, url_404, status=404) + + response = self.perform_request(url_404) + self.assertEqual(404, response.status) + + span = self.assert_span() + self.assertEqual(404, span.attributes.get(HTTP_RESPONSE_STATUS_CODE)) + self.assertEqual( + 404, span.attributes.get(SpanAttributes.HTTP_STATUS_CODE) + ) + self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) + + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_nonstandard_http_method(self): + httpretty.register_uri( + "NONSTANDARD", self.HTTP_URL, body="Hello!", status=405 + ) + self.perform_request(self.HTTP_URL, method="NONSTANDARD") + span = self.assert_span() + self.assertEqual("HTTP", span.name) + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_METHOD), "_OTHER" + ) + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_STATUS_CODE), 405 + ) + + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_nonstandard_http_method_new_semconv(self): + httpretty.register_uri( + "NONSTANDARD", self.HTTP_URL, body="Hello!", status=405 + ) + self.perform_request(self.HTTP_URL, method="NONSTANDARD") + span = self.assert_span() + self.assertEqual("HTTP", span.name) + self.assertEqual(span.attributes.get(HTTP_REQUEST_METHOD), "_OTHER") + self.assertEqual( + span.attributes.get(HTTP_REQUEST_METHOD_ORIGINAL), "NONSTANDARD" + ) + self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 405) + + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_nonstandard_http_method_both_semconv(self): + httpretty.register_uri( + "NONSTANDARD", self.HTTP_URL, body="Hello!", status=405 + ) + self.perform_request(self.HTTP_URL, method="NONSTANDARD") + span = self.assert_span() + self.assertEqual("HTTP", span.name) + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_METHOD), "_OTHER" + ) + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_STATUS_CODE), 405 + ) + self.assertEqual(span.attributes.get(HTTP_REQUEST_METHOD), "_OTHER") + self.assertEqual( + span.attributes.get(HTTP_REQUEST_METHOD_ORIGINAL), "NONSTANDARD" + ) + self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 405) + def test_basic_http_non_default_port(self): url = "http://mock:666/status/200" httpretty.register_uri(httpretty.GET, url, body="Hello!") @@ -287,6 +465,34 @@ def test_request_exception(self, _): self.assert_exception_span(self.HTTP_URL) + @mock.patch( + "urllib3.connectionpool.HTTPConnectionPool._make_request", + side_effect=urllib3.exceptions.ConnectTimeoutError, + ) + def test_request_exception_new_semconv(self, _): + with self.assertRaises(urllib3.exceptions.ConnectTimeoutError): + self.perform_request( + self.HTTP_URL, retries=urllib3.Retry(connect=False) + ) + + self.assert_exception_span( + self.HTTP_URL, sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP + ) + + @mock.patch( + "urllib3.connectionpool.HTTPConnectionPool._make_request", + side_effect=urllib3.exceptions.ConnectTimeoutError, + ) + def test_request_exception_both_semconv(self, _): + with self.assertRaises(urllib3.exceptions.ConnectTimeoutError): + self.perform_request( + self.HTTP_URL, retries=urllib3.Retry(connect=False) + ) + + self.assert_exception_span( + self.HTTP_URL, sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP_DUP + ) + @mock.patch( "urllib3.connectionpool.HTTPConnectionPool._make_request", side_effect=urllib3.exceptions.ProtocolError, diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py index 787b920d7c..c6e9011351 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py @@ -14,13 +14,30 @@ import io from timeit import default_timer +from unittest import mock import httpretty import urllib3 import urllib3.exceptions from urllib3 import encode_multipart_formdata +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.urllib3 import URLLib3Instrumentor +from opentelemetry.semconv.attributes.http_attributes import ( + HTTP_REQUEST_METHOD, + HTTP_RESPONSE_STATUS_CODE, +) +from opentelemetry.semconv.attributes.network_attributes import ( + NETWORK_PROTOCOL_VERSION, +) +from opentelemetry.semconv.attributes.server_attributes import ( + SERVER_ADDRESS, + SERVER_PORT, +) +from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.httptest import HttpTestBase from opentelemetry.test.test_base import TestBase @@ -30,6 +47,24 @@ class TestURLLib3InstrumentorMetric(HttpTestBase, 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: sem_conv_mode, + }, + ) + _OpenTelemetrySemanticConventionStability._initialized = False + self.env_patch.start() URLLib3Instrumentor().instrument() httpretty.enable(allow_net_connect=False) httpretty.register_uri(httpretty.GET, self.HTTP_URL, body="Hello!") @@ -38,6 +73,7 @@ def setUp(self): def tearDown(self): super().tearDown() + self.env_patch.stop() self.pool.clear() URLLib3Instrumentor().uninstrument() @@ -47,7 +83,149 @@ def tearDown(self): def test_basic_metrics(self): start_time = default_timer() response = self.pool.request("GET", self.HTTP_URL) - client_duration_estimated = (default_timer() - start_time) * 1000 + duration_ms = max(round((default_timer() - start_time) * 1000), 0) + metrics = self.get_sorted_metrics() + + ( + client_duration, + client_request_size, + client_response_size, + ) = metrics + + attrs_old = { + SpanAttributes.HTTP_STATUS_CODE: 200, + SpanAttributes.HTTP_HOST: "mock", + SpanAttributes.NET_PEER_PORT: 80, + SpanAttributes.NET_PEER_NAME: "mock", + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SCHEME: "http", + } + + self.assertEqual(client_duration.name, "http.client.duration") + self.assert_metric_expected( + client_duration, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=duration_ms, + max_data_point=duration_ms, + min_data_point=duration_ms, + attributes=attrs_old, + ) + ], + est_value_delta=40, + ) + + self.assertEqual(client_request_size.name, "http.client.request.size") + self.assert_metric_expected( + client_request_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=0, + max_data_point=0, + min_data_point=0, + attributes=attrs_old, + ) + ], + ) + + expected_size = len(response.data) + self.assertEqual( + client_response_size.name, "http.client.response.size" + ) + self.assert_metric_expected( + client_response_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=expected_size, + max_data_point=expected_size, + min_data_point=expected_size, + attributes=attrs_old, + ) + ], + ) + + def test_basic_metrics_new_semconv(self): + start_time = default_timer() + response = self.pool.request("GET", self.HTTP_URL) + duration_s = max(default_timer() - start_time, 0) + + metrics = self.get_sorted_metrics() + ( + client_request_size, + client_duration, + client_response_size, + ) = metrics + + attrs_new = { + NETWORK_PROTOCOL_VERSION: "1.1", + SERVER_ADDRESS: "mock", + SERVER_PORT: 80, + HTTP_REQUEST_METHOD: "GET", + HTTP_RESPONSE_STATUS_CODE: 200, + # TODO: add URL_SCHEME to tests when supported in the implementation + } + + self.assertEqual(client_duration.name, "http.client.request.duration") + self.assert_metric_expected( + client_duration, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=duration_s, + max_data_point=duration_s, + min_data_point=duration_s, + attributes=attrs_new, + ) + ], + est_value_delta=40 / 1000, + ) + + self.assertEqual( + client_request_size.name, "http.client.request.body.size" + ) + self.assert_metric_expected( + client_request_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=0, + max_data_point=0, + min_data_point=0, + attributes=attrs_new, + ) + ], + ) + + expected_size = len(response.data) + self.assertEqual( + client_response_size.name, "http.client.response.body.size" + ) + self.assert_metric_expected( + client_response_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=expected_size, + max_data_point=expected_size, + min_data_point=expected_size, + attributes=attrs_new, + ) + ], + ) + + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_basic_metrics_nonstandard_http_method(self): + httpretty.register_uri( + "NONSTANDARD", self.HTTP_URL, body="", status=405 + ) + + start_time = default_timer() + response = self.pool.request("NONSTANDARD", self.HTTP_URL) + duration_ms = max(round((default_timer() - start_time) * 1000), 0) metrics = self.get_sorted_metrics() @@ -57,27 +235,29 @@ def test_basic_metrics(self): client_response_size, ) = metrics + attrs_old = { + SpanAttributes.HTTP_STATUS_CODE: 405, + SpanAttributes.HTTP_HOST: "mock", + SpanAttributes.NET_PEER_PORT: 80, + SpanAttributes.NET_PEER_NAME: "mock", + SpanAttributes.HTTP_METHOD: "_OTHER", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SCHEME: "http", + } + self.assertEqual(client_duration.name, "http.client.duration") self.assert_metric_expected( client_duration, [ self.create_histogram_data_point( count=1, - sum_data_point=client_duration_estimated, - max_data_point=client_duration_estimated, - min_data_point=client_duration_estimated, - attributes={ - "http.flavor": "1.1", - "http.host": "mock", - "http.method": "GET", - "http.scheme": "http", - "http.status_code": 200, - "net.peer.name": "mock", - "net.peer.port": 80, - }, + sum_data_point=duration_ms, + max_data_point=duration_ms, + min_data_point=duration_ms, + attributes=attrs_old, ) ], - est_value_delta=200, + est_value_delta=40, ) self.assertEqual(client_request_size.name, "http.client.request.size") @@ -89,15 +269,7 @@ def test_basic_metrics(self): sum_data_point=0, max_data_point=0, min_data_point=0, - attributes={ - "http.flavor": "1.1", - "http.host": "mock", - "http.method": "GET", - "http.scheme": "http", - "http.status_code": 200, - "net.peer.name": "mock", - "net.peer.port": 80, - }, + attributes=attrs_old, ) ], ) @@ -114,15 +286,82 @@ def test_basic_metrics(self): sum_data_point=expected_size, max_data_point=expected_size, min_data_point=expected_size, - attributes={ - "http.flavor": "1.1", - "http.host": "mock", - "http.method": "GET", - "http.scheme": "http", - "http.status_code": 200, - "net.peer.name": "mock", - "net.peer.port": 80, - }, + attributes=attrs_old, + ) + ], + ) + + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_basic_metrics_nonstandard_http_method_new_semconv(self): + httpretty.register_uri( + "NONSTANDARD", self.HTTP_URL, body="", status=405 + ) + start_time = default_timer() + response = self.pool.request("NONSTANDARD", self.HTTP_URL) + duration_s = max(default_timer() - start_time, 0) + + metrics = self.get_sorted_metrics() + + ( + client_request_size, + client_duration, + client_response_size, + ) = metrics + + attrs_new = { + NETWORK_PROTOCOL_VERSION: "1.1", + SERVER_ADDRESS: "mock", + SERVER_PORT: 80, + HTTP_REQUEST_METHOD: "_OTHER", + HTTP_RESPONSE_STATUS_CODE: 405, + "error.type": "405", + # TODO: add URL_SCHEME to tests when supported in the implementation + } + + self.assertEqual(client_duration.name, "http.client.request.duration") + self.assert_metric_expected( + client_duration, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=duration_s, + max_data_point=duration_s, + min_data_point=duration_s, + attributes=attrs_new, + ) + ], + est_value_delta=40 / 1000, + ) + + self.assertEqual( + client_request_size.name, "http.client.request.body.size" + ) + self.assert_metric_expected( + client_request_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=0, + max_data_point=0, + min_data_point=0, + attributes=attrs_new, + ) + ], + ) + + expected_size = len(response.data) + self.assertEqual( + client_response_size.name, "http.client.response.body.size" + ) + self.assert_metric_expected( + client_response_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=expected_size, + max_data_point=expected_size, + min_data_point=expected_size, + attributes=attrs_new, ) ], ) @@ -274,3 +513,5 @@ def test_metric_uninstrument(self): for metric in metrics: for point in list(metric.data.data_points): self.assertEqual(point.count, 1) + # instrument again to avoid warning message on tearDown + URLLib3Instrumentor().instrument()