diff --git a/CHANGELOG.md b/CHANGELOG.md index 712aedb938..1b09c75064 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#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)) +- `opentelemetry-instrumentation-urllib` Implement new semantic convention opt-in migration + ([#2736](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2736)) ### Breaking changes @@ -71,6 +73,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682)) - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `django` middleware ([#2714](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2714)) +- Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `urllib` instrumentation + ([#2736](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2736)) - `opentelemetry-instrumentation-httpx`, `opentelemetry-instrumentation-aiohttp-client`, `opentelemetry-instrumentation-requests` Populate `{method}` as `HTTP` on `_OTHER` methods ([#2726](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2726)) diff --git a/instrumentation/README.md b/instrumentation/README.md index 555e0bcd2a..20438e0a12 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -45,6 +45,6 @@ | [opentelemetry-instrumentation-threading](./opentelemetry-instrumentation-threading) | threading | No | experimental | [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-urllib](./opentelemetry-instrumentation-urllib) | urllib | Yes | migration | [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-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py index befc022b35..d9072ba727 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -85,25 +85,49 @@ def response_hook(span, request_obj, response) Request, ) +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_method, + _set_http_network_protocol_version, + _set_http_url, + _set_status, +) from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.urllib.package import _instruments from opentelemetry.instrumentation.urllib.version import __version__ from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, is_http_instrumentation_enabled, suppress_http_instrumentation, ) from opentelemetry.metrics import Histogram, get_meter from opentelemetry.propagate import inject +from opentelemetry.semconv._incubating.metrics.http_metrics import ( + HTTP_CLIENT_REQUEST_BODY_SIZE, + HTTP_CLIENT_RESPONSE_BODY_SIZE, + create_http_client_request_body_size, + create_http_client_response_body_size, +) +from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from opentelemetry.semconv.metrics import MetricInstruments +from opentelemetry.semconv.metrics.http_metrics import ( + HTTP_CLIENT_REQUEST_DURATION, +) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span, SpanKind, get_tracer -from opentelemetry.trace.status import Status from opentelemetry.util.http import ( ExcludeList, get_excluded_urls, parse_excluded_urls, remove_url_credentials, + sanitize_method, ) _excluded_urls_from_env = get_excluded_urls("URLLIB") @@ -133,12 +157,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") meter_provider = kwargs.get("meter_provider") @@ -146,10 +176,10 @@ def _instrument(self, **kwargs): __name__, __version__, meter_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=schema_url, ) - histograms = _create_client_histograms(meter) + histograms = _create_client_histograms(meter, sem_conv_opt_in_mode) _instrument( tracer, @@ -161,6 +191,7 @@ 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): @@ -173,12 +204,14 @@ def uninstrument_opener( _uninstrument_from(opener, restore_as_bound_func=True) +# pylint: disable=too-many-statements def _instrument( tracer, histograms: Dict[str, Histogram], request_hook: _RequestHookT = None, response_hook: _ResponseHookT = None, excluded_urls: ExcludeList = None, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): """Enables tracing of all requests calls that go through :code:`urllib.Client._make_request`""" @@ -214,14 +247,22 @@ def _instrumented_open_call( method = request.get_method().upper() - span_name = method.strip() + span_name = _get_span_name(method) url = remove_url_credentials(url) - labels = { - SpanAttributes.HTTP_METHOD: method, - SpanAttributes.HTTP_URL: url, - } + data = getattr(request, "data", None) + request_size = 0 if data is None else len(data) + + labels = {} + + _set_http_method( + labels, + method, + sanitize_method(method), + sem_conv_opt_in_mode, + ) + _set_http_url(labels, url, sem_conv_opt_in_mode) with tracer.start_as_current_span( span_name, kind=SpanKind.CLIENT, attributes=labels @@ -241,24 +282,50 @@ def _instrumented_open_call( exception = exc result = getattr(exc, "file", None) finally: - elapsed_time = round((default_timer() - start_time) * 1000) - + duration_s = default_timer() - start_time + response_size = 0 if result is not None: + response_size = int(result.headers.get("Content-Length", 0)) code_ = result.getcode() - labels[SpanAttributes.HTTP_STATUS_CODE] = str(code_) - - if span.is_recording() and code_ is not None: - span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, code_) - span.set_status(Status(http_status_to_status_code(code_))) + # set http status code based on semconv + if code_: + _set_status_code_attribute( + span, code_, labels, sem_conv_opt_in_mode + ) ver_ = str(getattr(result, "version", "")) if ver_: - labels[SpanAttributes.HTTP_FLAVOR] = ( - f"{ver_[:1]}.{ver_[:-1]}" + _set_http_network_protocol_version( + labels, f"{ver_[:1]}.{ver_[:-1]}", sem_conv_opt_in_mode ) + if exception is not None and _report_new(sem_conv_opt_in_mode): + span.set_attribute(ERROR_TYPE, type(exception).__qualname__) + labels[ERROR_TYPE] = type(exception).__qualname__ + + duration_attrs_old = _filter_semconv_duration_attrs( + labels, + _client_duration_attrs_old, + _client_duration_attrs_new, + sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT, + ) + duration_attrs_new = _filter_semconv_duration_attrs( + labels, + _client_duration_attrs_old, + _client_duration_attrs_new, + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP, + ) + + duration_attrs_old[SpanAttributes.HTTP_URL] = url + _record_histograms( - histograms, labels, request, result, elapsed_time + histograms, + duration_attrs_old, + duration_attrs_new, + request_size, + response_size, + duration_s, + sem_conv_opt_in_mode, ) if callable(response_hook): @@ -296,43 +363,108 @@ def _uninstrument_from(instr_root, restore_as_bound_func=False): setattr(instr_root, instr_func_name, original) -def _create_client_histograms(meter) -> Dict[str, Histogram]: - histograms = { - MetricInstruments.HTTP_CLIENT_DURATION: meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_DURATION, - unit="ms", - description="Measures the duration of outbound HTTP requests.", - ), - MetricInstruments.HTTP_CLIENT_REQUEST_SIZE: meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_REQUEST_SIZE, - unit="By", - description="Measures the size of HTTP request messages.", - ), - MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE: meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE, - unit="By", - description="Measures the size of HTTP response messages.", - ), - } +def _get_span_name(method: str) -> str: + method = sanitize_method(method.strip()) + if method == "_OTHER": + method = "HTTP" + return method + + +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_client_histograms( + meter, sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT +) -> Dict[str, Histogram]: + histograms = {} + if _report_old(sem_conv_opt_in_mode): + histograms[MetricInstruments.HTTP_CLIENT_DURATION] = ( + meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_DURATION, + unit="ms", + description="Measures the duration of the outbound HTTP request", + ) + ) + histograms[MetricInstruments.HTTP_CLIENT_REQUEST_SIZE] = ( + meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_REQUEST_SIZE, + unit="By", + description="Measures the size of HTTP request messages.", + ) + ) + histograms[MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE] = ( + meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE, + unit="By", + description="Measures the size of HTTP response messages.", + ) + ) + if _report_new(sem_conv_opt_in_mode): + histograms[HTTP_CLIENT_REQUEST_DURATION] = meter.create_histogram( + name=HTTP_CLIENT_REQUEST_DURATION, + unit="s", + description="Duration of HTTP client requests.", + ) + histograms[HTTP_CLIENT_REQUEST_BODY_SIZE] = ( + create_http_client_request_body_size(meter) + ) + histograms[HTTP_CLIENT_RESPONSE_BODY_SIZE] = ( + create_http_client_response_body_size(meter) + ) return histograms def _record_histograms( - histograms, metric_attributes, request, response, elapsed_time + histograms: Dict[str, Histogram], + metric_attributes_old: dict, + metric_attributes_new: dict, + request_size: int, + response_size: int, + duration_s: float, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): - histograms[MetricInstruments.HTTP_CLIENT_DURATION].record( - elapsed_time, attributes=metric_attributes - ) - - data = getattr(request, "data", None) - request_size = 0 if data is None else len(data) - histograms[MetricInstruments.HTTP_CLIENT_REQUEST_SIZE].record( - request_size, attributes=metric_attributes - ) - - if response is not None: - response_size = int(response.headers.get("Content-Length", 0)) + if _report_old(sem_conv_opt_in_mode): + duration = max(round(duration_s * 1000), 0) + histograms[MetricInstruments.HTTP_CLIENT_DURATION].record( + duration, attributes=metric_attributes_old + ) + histograms[MetricInstruments.HTTP_CLIENT_REQUEST_SIZE].record( + request_size, attributes=metric_attributes_old + ) histograms[MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE].record( - response_size, attributes=metric_attributes + response_size, attributes=metric_attributes_old + ) + if _report_new(sem_conv_opt_in_mode): + histograms[HTTP_CLIENT_REQUEST_DURATION].record( + duration_s, attributes=metric_attributes_new + ) + histograms[HTTP_CLIENT_REQUEST_BODY_SIZE].record( + request_size, attributes=metric_attributes_new + ) + histograms[HTTP_CLIENT_RESPONSE_BODY_SIZE].record( + response_size, attributes=metric_attributes_new ) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/package.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/package.py index 942f175da1..1bb8350a06 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/package.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/package.py @@ -16,3 +16,5 @@ _instruments = tuple() _supports_metrics = True + +_semconv_status = "migration" diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py index f79749dfd8..7a9bfd38f1 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py @@ -15,16 +15,28 @@ from platform import python_implementation from timeit import default_timer +from unittest.mock import patch from urllib import request from urllib.parse import urlencode import httpretty from pytest import mark +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.urllib import ( # pylint: disable=no-name-in-module,import-error URLLibInstrumentor, ) +from opentelemetry.semconv._incubating.metrics.http_metrics import ( + HTTP_CLIENT_REQUEST_BODY_SIZE, + HTTP_CLIENT_RESPONSE_BODY_SIZE, +) from opentelemetry.semconv.metrics import MetricInstruments +from opentelemetry.semconv.metrics.http_metrics import ( + HTTP_CLIENT_REQUEST_DURATION, +) from opentelemetry.test.test_base import TestBase @@ -34,6 +46,23 @@ class TestUrllibMetricsInstrumentation(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 = patch.dict( + "os.environ", + { + OTEL_SEMCONV_STABILITY_OPT_IN: sem_conv_mode, + }, + ) + _OpenTelemetrySemanticConventionStability._initialized = False + self.env_patch.start() URLLibInstrumentor().instrument() httpretty.enable() httpretty.register_uri(httpretty.GET, self.URL, body=b"Hello!") @@ -71,18 +100,19 @@ def test_basic_metric(self): self.assertEqual( client_duration.name, MetricInstruments.HTTP_CLIENT_DURATION ) + self.assert_metric_expected( client_duration, self.create_histogram_data_points( client_duration_estimated, attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), "http.flavor": "1.1", }, ), - est_value_delta=200, + est_value_delta=40, ) self.assertEqual( @@ -94,7 +124,7 @@ def test_basic_metric(self): self.create_histogram_data_points( 0, attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), "http.flavor": "1.1", @@ -111,7 +141,7 @@ def test_basic_metric(self): self.create_histogram_data_points( result.length, attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), "http.flavor": "1.1", @@ -119,6 +149,188 @@ def test_basic_metric(self): ), ) + def test_basic_metric_new_semconv(self): + start_time = default_timer() + with request.urlopen(self.URL) as result: + duration_s = default_timer() - start_time + + metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 3) + + ( + client_request_body_size, + client_request_duration, + client_response_body_size, + ) = metrics[:3] + + self.assertEqual( + client_request_duration.name, HTTP_CLIENT_REQUEST_DURATION + ) + + self.assert_metric_expected( + client_request_duration, + self.create_histogram_data_points( + duration_s, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + est_value_delta=40, + ) + + self.assertEqual( + client_request_body_size.name, + HTTP_CLIENT_REQUEST_BODY_SIZE, + ) + self.assert_metric_expected( + client_request_body_size, + self.create_histogram_data_points( + 0, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + ) + + self.assertEqual( + client_response_body_size.name, + HTTP_CLIENT_RESPONSE_BODY_SIZE, + ) + self.assert_metric_expected( + client_response_body_size, + self.create_histogram_data_points( + result.length, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + ) + + def test_basic_metric_both_semconv(self): + start_time = default_timer() + with request.urlopen(self.URL) as result: + duration_s = default_timer() - start_time + duration = max(round(duration_s * 1000), 0) + + metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 6) + + ( + client_duration, + client_request_body_size, + client_request_duration, + client_request_size, + client_response_body_size, + client_response_size, + ) = metrics[:6] + + self.assertEqual( + client_duration.name, MetricInstruments.HTTP_CLIENT_DURATION + ) + + self.assert_metric_expected( + client_duration, + self.create_histogram_data_points( + duration, + attributes={ + "http.status_code": int(result.code), + "http.method": "GET", + "http.url": str(result.url), + "http.flavor": "1.1", + }, + ), + est_value_delta=40, + ) + + self.assertEqual( + client_request_size.name, + MetricInstruments.HTTP_CLIENT_REQUEST_SIZE, + ) + self.assert_metric_expected( + client_request_size, + self.create_histogram_data_points( + 0, + attributes={ + "http.status_code": int(result.code), + "http.method": "GET", + "http.url": str(result.url), + "http.flavor": "1.1", + }, + ), + ) + + self.assertEqual( + client_response_size.name, + MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE, + ) + self.assert_metric_expected( + client_response_size, + self.create_histogram_data_points( + result.length, + attributes={ + "http.status_code": int(result.code), + "http.method": "GET", + "http.url": str(result.url), + "http.flavor": "1.1", + }, + ), + ) + + self.assertEqual( + client_request_duration.name, HTTP_CLIENT_REQUEST_DURATION + ) + + self.assert_metric_expected( + client_request_duration, + self.create_histogram_data_points( + duration_s, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + est_value_delta=40, + ) + + self.assertEqual( + client_request_body_size.name, + HTTP_CLIENT_REQUEST_BODY_SIZE, + ) + self.assert_metric_expected( + client_request_body_size, + self.create_histogram_data_points( + 0, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + ) + + self.assertEqual( + client_response_body_size.name, + HTTP_CLIENT_RESPONSE_BODY_SIZE, + ) + self.assert_metric_expected( + client_response_body_size, + self.create_histogram_data_points( + result.length, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + ) + def test_basic_metric_request_not_empty(self): data = {"header1": "value1", "header2": "value2"} data_encoded = urlencode(data).encode() @@ -144,13 +356,13 @@ def test_basic_metric_request_not_empty(self): self.create_histogram_data_points( client_duration_estimated, attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), "http.flavor": "1.1", }, ), - est_value_delta=200, + est_value_delta=40, ) self.assertEqual( @@ -162,7 +374,7 @@ def test_basic_metric_request_not_empty(self): self.create_histogram_data_points( len(data_encoded), attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), "http.flavor": "1.1", @@ -179,7 +391,7 @@ def test_basic_metric_request_not_empty(self): self.create_histogram_data_points( result.length, attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), "http.flavor": "1.1", diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index f73f0d1b97..8ac0284939 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -25,6 +25,10 @@ import opentelemetry.instrumentation.urllib # pylint: disable=no-name-in-module,import-error from opentelemetry import trace +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.urllib import ( # pylint: disable=no-name-in-module,import-error URLLibInstrumentor, ) @@ -34,6 +38,12 @@ ) from opentelemetry.propagate import get_global_textmap, set_global_textmap from opentelemetry.sdk import resources +from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE +from opentelemetry.semconv.attributes.http_attributes import ( + HTTP_REQUEST_METHOD, + 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 @@ -43,7 +53,7 @@ # pylint: disable=too-many-public-methods -class RequestsIntegrationTestBase(abc.ABC): +class URLLibIntegrationTestBase(abc.ABC): # pylint: disable=no-member URL = "http://mock/status/200" @@ -54,12 +64,23 @@ class RequestsIntegrationTestBase(abc.ABC): 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_URLLIB_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg" + "OTEL_PYTHON_URLLIB_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( @@ -141,6 +162,57 @@ def test_basic(self): span, opentelemetry.instrumentation.urllib ) + def test_basic_new_semconv(self): + result = self.perform_request(self.URL) + + self.assertEqual(result.read(), b"Hello!") + span = self.assert_span() + + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "GET") + + self.assertEqual( + span.attributes, + { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: self.URL, + HTTP_RESPONSE_STATUS_CODE: 200, + }, + ) + + self.assertIs(span.status.status_code, trace.StatusCode.UNSET) + + self.assertEqualSpanInstrumentationInfo( + span, opentelemetry.instrumentation.urllib + ) + + def test_basic_both_semconv(self): + result = self.perform_request(self.URL) + + self.assertEqual(result.read(), b"Hello!") + span = self.assert_span() + + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "GET") + + self.assertEqual( + span.attributes, + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: self.URL, + SpanAttributes.HTTP_STATUS_CODE: 200, + HTTP_REQUEST_METHOD: "GET", + URL_FULL: self.URL, + HTTP_RESPONSE_STATUS_CODE: 200, + }, + ) + + self.assertIs(span.status.status_code, trace.StatusCode.UNSET) + + self.assertEqualSpanInstrumentationInfo( + span, opentelemetry.instrumentation.urllib + ) + def test_excluded_urls_explicit(self): url_201 = "http://mock/status/201" httpretty.register_uri( @@ -197,6 +269,57 @@ 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, + ) + exception = None + try: + self.perform_request(url_404) + except Exception as err: # pylint: disable=broad-except + exception = err + code = exception.code + self.assertEqual(code, 404) + + span = self.assert_span() + + self.assertEqual(span.attributes.get(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, + ) + exception = None + try: + self.perform_request(url_404) + except Exception as err: # pylint: disable=broad-except + exception = err + code = exception.code + self.assertEqual(code, 404) + + span = self.assert_span() + + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_STATUS_CODE), 404 + ) + self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 404) + + self.assertIs( + span.status.status_code, + trace.StatusCode.ERROR, + ) + @staticmethod def mock_get_code(*args, **kwargs): return None @@ -339,6 +462,41 @@ def test_requests_exception_with_response(self, *_, **__): ) self.assertEqual(span.status.status_code, StatusCode.ERROR) + def test_requests_exception_with_response_new_semconv(self, *_, **__): + with self.assertRaises(HTTPError): + self.perform_request("http://mock/status/500") + + span = self.assert_span() + self.assertEqual( + dict(span.attributes), + { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: "http://mock/status/500", + HTTP_RESPONSE_STATUS_CODE: 500, + ERROR_TYPE: "HTTPError", + }, + ) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + + def test_requests_exception_with_response_both_semconv(self, *_, **__): + with self.assertRaises(HTTPError): + self.perform_request("http://mock/status/500") + + span = self.assert_span() + self.assertEqual( + dict(span.attributes), + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: "http://mock/status/500", + SpanAttributes.HTTP_STATUS_CODE: 500, + HTTP_REQUEST_METHOD: "GET", + URL_FULL: "http://mock/status/500", + HTTP_RESPONSE_STATUS_CODE: 500, + ERROR_TYPE: "HTTPError", + }, + ) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + def test_requests_basic_exception(self, *_, **__): with self.assertRaises(Exception): self.perform_request(self.URL_EXCEPTION) @@ -393,7 +551,7 @@ def test_no_op_tracer_provider(self): self.assert_span(num_spans=0) -class TestRequestsIntegration(RequestsIntegrationTestBase, TestBase): +class TestURLLibIntegration(URLLibIntegrationTestBase, TestBase): @staticmethod def perform_request(url: str, opener: OpenerDirector = None): if not opener: