From 27e4c2ce34156bd0dd967cafb79c91aa1e85f498 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 3 Apr 2024 15:20:57 -0700 Subject: [PATCH 01/18] Update .pylintrc --- .pylintrc | 1 + 1 file changed, 1 insertion(+) diff --git a/.pylintrc b/.pylintrc index 5ea4385ea0..114dadef75 100644 --- a/.pylintrc +++ b/.pylintrc @@ -81,6 +81,7 @@ disable=missing-docstring, missing-module-docstring, # temp-pylint-upgrade import-error, # needed as a workaround as reported here: https://github.com/open-telemetry/opentelemetry-python-contrib/issues/290 cyclic-import, + not-context-manager # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option From f477c4c80067cfe72a563f767ba4f7cde57c7044 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 24 Jul 2024 15:40:47 -0700 Subject: [PATCH 02/18] urllib --- CHANGELOG.md | 4 + .../instrumentation/urllib/__init__.py | 204 ++++++++++++++---- .../instrumentation/urllib/package.py | 2 + .../tests/test_metrics_instrumentation.py | 13 +- 4 files changed, 173 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc400865a2..0ac09a16bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,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 + ([#2715](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2715)) ### Breaking changes @@ -58,6 +60,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 + ([#2714](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2714)) - `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/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py index befc022b35..c9b71bccfa 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,48 @@ 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_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 ( + create_http_client_request_body_size, + create_http_client_response_body_size, + HTTP_CLIENT_REQUEST_BODY_SIZE, + 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 +156,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,7 +175,7 @@ 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) @@ -161,6 +190,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): @@ -179,6 +209,7 @@ def _instrument( 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 +245,18 @@ 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 = {} - labels = { - SpanAttributes.HTTP_METHOD: method, - SpanAttributes.HTTP_URL: url, - } + _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 +276,42 @@ 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 if result is not None: 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_))) - - ver_ = str(getattr(result, "version", "")) - if ver_: - labels[SpanAttributes.HTTP_FLAVOR] = ( - f"{ver_[:1]}.{ver_[:-1]}" + # set http status code based on semconv + if code_: + _set_status_code_attribute( + span, code_, labels, sem_conv_opt_in_mode ) + if _report_old(sem_conv_opt_in_mode): + ver_ = str(getattr(result, "version", "")) + if ver_: + labels[SpanAttributes.HTTP_FLAVOR] = ( + f"{ver_[:1]}.{ver_[:-1]}" + ) + + 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 = _filter_semconv_duration_attrs( + labels, + _client_duration_attrs_old, + _client_duration_attrs_new + ) + if _report_old(sem_conv_opt_in_mode): + duration_attrs[SpanAttributes.HTTP_URL] = url + _record_histograms( - histograms, labels, request, result, elapsed_time + histograms, + duration_attrs, + request, + result, + duration_s, + sem_conv_opt_in_mode, ) if callable(response_hook): @@ -296,43 +349,106 @@ 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( +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 outbound HTTP requests.", - ), - MetricInstruments.HTTP_CLIENT_REQUEST_SIZE: meter.create_histogram( + 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.", - ), - MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE: meter.create_histogram( + ) + 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: Histogram, + metric_attributes: dict, + request, + response, + duration_s: float, + sem_conv_opt_in_mode = _HTTPStabilityMode.DEFAULT, ): - histograms[MetricInstruments.HTTP_CLIENT_DURATION].record( - elapsed_time, attributes=metric_attributes - ) - + duration = max(round(duration_s * 1000), 0) 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)) - histograms[MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE].record( - response_size, attributes=metric_attributes + if _report_old(sem_conv_opt_in_mode): + histograms[MetricInstruments.HTTP_CLIENT_DURATION].record( + duration, attributes=metric_attributes ) + 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)) + histograms[MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE].record( + response_size, attributes=metric_attributes + ) + if _report_new(sem_conv_opt_in_mode): + histograms[HTTP_CLIENT_REQUEST_DURATION].record( + duration_s, attributes=metric_attributes + ) + histograms[HTTP_CLIENT_REQUEST_BODY_SIZE].record( + request_size, attributes=metric_attributes + ) + if response is not None: + response_size = int(response.headers.get("Content-Length", 0)) + histograms[HTTP_CLIENT_RESPONSE_BODY_SIZE].record( + response_size, attributes=metric_attributes + ) 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..47571f5a49 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py @@ -71,12 +71,13 @@ 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", @@ -94,7 +95,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 +112,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", @@ -144,7 +145,7 @@ 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", @@ -162,7 +163,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 +180,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", From a7d8393c73a2301d911074349cf12df47086211f Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 25 Jul 2024 15:06:36 -0700 Subject: [PATCH 03/18] tests --- CHANGELOG.md | 4 +- instrumentation/README.md | 2 +- .../instrumentation/urllib/__init__.py | 70 +++--- .../tests/test_metrics_instrumentation.py | 205 ++++++++++++++++++ .../tests/test_urllib_integration.py | 168 +++++++++++++- 5 files changed, 413 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac09a16bd..edd70bad71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,7 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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 - ([#2715](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2715)) + ([#2736](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2736)) ### Breaking changes @@ -61,7 +61,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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 - ([#2714](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2714)) + ([#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 c9b71bccfa..d65c4dcdf7 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -178,7 +178,10 @@ def _instrument(self, **kwargs): schema_url=schema_url, ) - histograms = _create_client_histograms(meter) + histograms = _create_client_histograms( + meter, + sem_conv_opt_in_mode + ) _instrument( tracer, @@ -248,6 +251,10 @@ def _instrumented_open_call( span_name = _get_span_name(method) url = remove_url_credentials(url) + + data = getattr(request, "data", None) + request_size = 0 if data is None else len(data) + labels = {} _set_http_method( @@ -277,8 +284,9 @@ def _instrumented_open_call( result = getattr(exc, "file", None) finally: 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() # set http status code based on semconv if code_: @@ -297,19 +305,27 @@ def _instrumented_open_call( span.set_attribute(ERROR_TYPE, type(exception).__qualname__) labels[ERROR_TYPE] = type(exception).__qualname__ - duration_attrs = _filter_semconv_duration_attrs( + duration_attrs_old = _filter_semconv_duration_attrs( labels, _client_duration_attrs_old, - _client_duration_attrs_new + _client_duration_attrs_new, + sem_conv_opt_in_mode = _HTTPStabilityMode.DEFAULT, ) - if _report_old(sem_conv_opt_in_mode): - duration_attrs[SpanAttributes.HTTP_URL] = url + 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, - duration_attrs, - request, - result, + duration_attrs_old, + duration_attrs_new, + request_size, + response_size, duration_s, sem_conv_opt_in_mode, ) @@ -413,42 +429,36 @@ def _create_client_histograms(meter, sem_conv_opt_in_mode = _HTTPStabilityMode.D meter ) - return histograms def _record_histograms( histograms: Histogram, - metric_attributes: dict, - request, - response, + metric_attributes_old: dict, + metric_attributes_new: dict, + request_size: int, + response_size: int, duration_s: float, - sem_conv_opt_in_mode = _HTTPStabilityMode.DEFAULT, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): duration = max(round(duration_s * 1000), 0) - data = getattr(request, "data", None) - request_size = 0 if data is None else len(data) if _report_old(sem_conv_opt_in_mode): histograms[MetricInstruments.HTTP_CLIENT_DURATION].record( - duration, attributes=metric_attributes + duration, attributes=metric_attributes_old ) histograms[MetricInstruments.HTTP_CLIENT_REQUEST_SIZE].record( - request_size, attributes=metric_attributes + request_size, attributes=metric_attributes_old + ) + histograms[MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE].record( + response_size, attributes=metric_attributes_old ) - if response is not None: - response_size = int(response.headers.get("Content-Length", 0)) - histograms[MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE].record( - response_size, attributes=metric_attributes - ) if _report_new(sem_conv_opt_in_mode): histograms[HTTP_CLIENT_REQUEST_DURATION].record( - duration_s, attributes=metric_attributes + duration_s, attributes=metric_attributes_new ) histograms[HTTP_CLIENT_REQUEST_BODY_SIZE].record( - request_size, attributes=metric_attributes + request_size, attributes=metric_attributes_new + ) + histograms[HTTP_CLIENT_RESPONSE_BODY_SIZE].record( + response_size, attributes=metric_attributes_new ) - if response is not None: - response_size = int(response.headers.get("Content-Length", 0)) - histograms[HTTP_CLIENT_RESPONSE_BODY_SIZE].record( - response_size, attributes=metric_attributes - ) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py index 47571f5a49..265a753f9d 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!") @@ -120,6 +149,182 @@ 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", + }, + ), + est_value_delta=200, + ) + + 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", + }, + ), + ) + + 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", + }, + ), + ) + + 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=200, + ) + + 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", + }, + ), + est_value_delta=200, + ) + + 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", + }, + ), + ) + + 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", + }, + ), + ) + def test_basic_metric_request_not_empty(self): data = {"header1": "value1", "header2": "value2"} data_encoded = urlencode(data).encode() diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index f73f0d1b97..51add9b05b 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -23,6 +23,10 @@ import httpretty +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _OpenTelemetrySemanticConventionStability, +) import opentelemetry.instrumentation.urllib # pylint: disable=no-name-in-module,import-error from opentelemetry import trace from opentelemetry.instrumentation.urllib import ( # pylint: disable=no-name-in-module,import-error @@ -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,61 @@ 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 +466,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 +555,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: From 3a06428e0f727abaec01949df6143cf7ca19a08a Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 25 Jul 2024 17:24:29 -0700 Subject: [PATCH 04/18] lint --- .../instrumentation/urllib/__init__.py | 45 ++++++++++--------- .../tests/test_urllib_integration.py | 8 +--- 2 files changed, 27 insertions(+), 26 deletions(-) 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 d65c4dcdf7..853cdc1e04 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -178,10 +178,7 @@ def _instrument(self, **kwargs): schema_url=schema_url, ) - histograms = _create_client_histograms( - meter, - sem_conv_opt_in_mode - ) + histograms = _create_client_histograms(meter, sem_conv_opt_in_mode) _instrument( tracer, @@ -254,7 +251,7 @@ def _instrumented_open_call( data = getattr(request, "data", None) request_size = 0 if data is None else len(data) - + labels = {} _set_http_method( @@ -297,9 +294,9 @@ def _instrumented_open_call( if _report_old(sem_conv_opt_in_mode): ver_ = str(getattr(result, "version", "")) if ver_: - labels[SpanAttributes.HTTP_FLAVOR] = ( - f"{ver_[:1]}.{ver_[:-1]}" - ) + labels[ + SpanAttributes.HTTP_FLAVOR + ] = f"{ver_[:1]}.{ver_[:-1]}" if exception is not None and _report_new(sem_conv_opt_in_mode): span.set_attribute(ERROR_TYPE, type(exception).__qualname__) @@ -309,13 +306,13 @@ def _instrumented_open_call( labels, _client_duration_attrs_old, _client_duration_attrs_new, - sem_conv_opt_in_mode = _HTTPStabilityMode.DEFAULT, + 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, + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP, ) duration_attrs_old[SpanAttributes.HTTP_URL] = url @@ -398,20 +395,28 @@ def _set_status_code_attribute( ) -def _create_client_histograms(meter, sem_conv_opt_in_mode = _HTTPStabilityMode.DEFAULT) -> Dict[str, Histogram]: +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( + 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( + 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( + 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.", @@ -422,12 +427,12 @@ def _create_client_histograms(meter, sem_conv_opt_in_mode = _HTTPStabilityMode.D 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 - ) + 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 diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index 51add9b05b..1cc225d2e4 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -286,9 +286,7 @@ def test_not_foundbasic_new_semconv(self): span = self.assert_span() - self.assertEqual( - span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 404 - ) + self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 404) self.assertIs( span.status.status_code, @@ -315,9 +313,7 @@ def test_not_foundbasic_both_semconv(self): self.assertEqual( span.attributes.get(SpanAttributes.HTTP_STATUS_CODE), 404 ) - self.assertEqual( - span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 404 - ) + self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 404) self.assertIs( span.status.status_code, From ecaa7ed1f698fac1a08689f3cf704474d3b3820f Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 26 Jul 2024 09:34:24 -0700 Subject: [PATCH 05/18] lint --- .../instrumentation/urllib/__init__.py | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) 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 853cdc1e04..d31c8a4068 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -294,9 +294,9 @@ def _instrumented_open_call( if _report_old(sem_conv_opt_in_mode): ver_ = str(getattr(result, "version", "")) if ver_: - labels[ - SpanAttributes.HTTP_FLAVOR - ] = f"{ver_[:1]}.{ver_[:-1]}" + labels[SpanAttributes.HTTP_FLAVOR] = ( + f"{ver_[:1]}.{ver_[:-1]}" + ) if exception is not None and _report_new(sem_conv_opt_in_mode): span.set_attribute(ERROR_TYPE, type(exception).__qualname__) @@ -400,26 +400,26 @@ def _create_client_histograms( ) -> 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_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_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.", + 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( @@ -427,12 +427,12 @@ def _create_client_histograms( 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) + 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 From 21f7f803dc941a62d2c82760f89567df8b539ad3 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 26 Jul 2024 09:38:31 -0700 Subject: [PATCH 06/18] Update __init__.py --- .../src/opentelemetry/instrumentation/urllib/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 d31c8a4068..944c41eeef 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -428,10 +428,10 @@ def _create_client_histograms( description="Duration of HTTP client requests.", ) histograms[HTTP_CLIENT_REQUEST_BODY_SIZE] = ( - create_http_client_request_body_size(meter) + create_http_client_request_body_size(meter) ) histograms[HTTP_CLIENT_RESPONSE_BODY_SIZE] = ( - create_http_client_response_body_size(meter) + create_http_client_response_body_size(meter) ) return histograms From f13cc2bf0048875e2425d8e5da60520a606a6342 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 26 Jul 2024 11:01:19 -0700 Subject: [PATCH 07/18] lint --- .../src/opentelemetry/instrumentation/urllib/__init__.py | 4 ++-- .../tests/test_urllib_integration.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) 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 944c41eeef..17bbfaf7b1 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -109,10 +109,10 @@ def response_hook(span, request_obj, response) 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, 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 diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index 1cc225d2e4..8ac0284939 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -23,12 +23,12 @@ import httpretty +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, ) -import opentelemetry.instrumentation.urllib # pylint: disable=no-name-in-module,import-error -from opentelemetry import trace from opentelemetry.instrumentation.urllib import ( # pylint: disable=no-name-in-module,import-error URLLibInstrumentor, ) From 695569d4b6ad73b0886ed4008bb847c3f5f92589 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 26 Jul 2024 11:08:32 -0700 Subject: [PATCH 08/18] pylint --- .../src/opentelemetry/instrumentation/urllib/__init__.py | 1 + 1 file changed, 1 insertion(+) 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 17bbfaf7b1..1fb9b0edfa 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -203,6 +203,7 @@ def uninstrument_opener( _uninstrument_from(opener, restore_as_bound_func=True) +# pylint: disable=too-many-statements def _instrument( tracer, histograms: Dict[str, Histogram], From cdf3ed55ff30bc062d7fe8e0f6a6345014604d69 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 29 Jul 2024 11:23:39 -0700 Subject: [PATCH 09/18] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca2590683a..d7a992a73a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,7 +47,7 @@ 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 +- `opentelemetry-instrumentation-urllib` Implement new semantic convention opt-in migration ([#2736](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2736)) ### Breaking changes From a1ce13c8b897bab2df69f2fba19d3ddb39d3b906 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 29 Jul 2024 11:23:45 -0700 Subject: [PATCH 10/18] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7a992a73a..035b533431 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,7 +64,7 @@ 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 +- 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 From c91d6fb3e398ebef532d862c40e3b65a286e10b7 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 29 Jul 2024 11:29:14 -0700 Subject: [PATCH 11/18] Update instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> --- .../src/opentelemetry/instrumentation/urllib/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1fb9b0edfa..8a853c3428 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -447,8 +447,8 @@ def _record_histograms( duration_s: float, sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): - duration = max(round(duration_s * 1000), 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 ) From a7155016db647d00a4a922db0e579062141bdd0d Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 29 Jul 2024 11:29:46 -0700 Subject: [PATCH 12/18] Update instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> --- .../src/opentelemetry/instrumentation/urllib/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8a853c3428..4c6f84b7f5 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -439,7 +439,7 @@ def _create_client_histograms( def _record_histograms( - histograms: Histogram, + histograms: Dict[str, Histogram], metric_attributes_old: dict, metric_attributes_new: dict, request_size: int, From bb1cabb6878b1e9efd64edca093dd071f581ae01 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 29 Jul 2024 11:34:04 -0700 Subject: [PATCH 13/18] flavor --- .../instrumentation/urllib/__init__.py | 14 ++++++++------ .../tests/test_metrics_instrumentation.py | 10 +++++----- 2 files changed, 13 insertions(+), 11 deletions(-) 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 4c6f84b7f5..2aff1a11ca 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -95,6 +95,7 @@ def response_hook(span, request_obj, response) _OpenTelemetryStabilitySignalType, _report_new, _report_old, + _set_http_flavor_version, _set_http_method, _set_http_url, _set_status, @@ -292,12 +293,13 @@ def _instrumented_open_call( span, code_, labels, sem_conv_opt_in_mode ) - if _report_old(sem_conv_opt_in_mode): - ver_ = str(getattr(result, "version", "")) - if ver_: - labels[SpanAttributes.HTTP_FLAVOR] = ( - f"{ver_[:1]}.{ver_[:-1]}" - ) + ver_ = str(getattr(result, "version", "")) + if ver_: + _set_http_flavor_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__) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py index 265a753f9d..64698a2820 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py @@ -112,7 +112,7 @@ def test_basic_metric(self): "http.flavor": "1.1", }, ), - est_value_delta=200, + est_value_delta=40, ) self.assertEqual( @@ -176,7 +176,7 @@ def test_basic_metric_new_semconv(self): "http.request.method": "GET", }, ), - est_value_delta=200, + est_value_delta=40, ) self.assertEqual( @@ -242,7 +242,7 @@ def test_basic_metric_both_semconv(self): "http.flavor": "1.1", }, ), - est_value_delta=200, + est_value_delta=40, ) self.assertEqual( @@ -292,7 +292,7 @@ def test_basic_metric_both_semconv(self): "http.request.method": "GET", }, ), - est_value_delta=200, + est_value_delta=40, ) self.assertEqual( @@ -356,7 +356,7 @@ def test_basic_metric_request_not_empty(self): "http.flavor": "1.1", }, ), - est_value_delta=200, + est_value_delta=40, ) self.assertEqual( From b51f8b7e521c678eaf2a3ff2dcac5949d8b18ae3 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 29 Jul 2024 11:48:34 -0700 Subject: [PATCH 14/18] flavor --- .../tests/test_metrics_instrumentation.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py index 64698a2820..7a9bfd38f1 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py @@ -174,6 +174,7 @@ def test_basic_metric_new_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", + "network.protocol.version": "1.1", }, ), est_value_delta=40, @@ -190,6 +191,7 @@ def test_basic_metric_new_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", + "network.protocol.version": "1.1", }, ), ) @@ -205,6 +207,7 @@ def test_basic_metric_new_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", + "network.protocol.version": "1.1", }, ), ) @@ -290,6 +293,7 @@ def test_basic_metric_both_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", + "network.protocol.version": "1.1", }, ), est_value_delta=40, @@ -306,6 +310,7 @@ def test_basic_metric_both_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", + "network.protocol.version": "1.1", }, ), ) @@ -321,6 +326,7 @@ def test_basic_metric_both_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", + "network.protocol.version": "1.1", }, ), ) From 463b0fac3656cce19df34b66f15de7d56835e07b Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 29 Jul 2024 11:53:14 -0700 Subject: [PATCH 15/18] Update __init__.py --- .../src/opentelemetry/instrumentation/urllib/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 2aff1a11ca..079fe7dd91 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -296,9 +296,7 @@ def _instrumented_open_call( ver_ = str(getattr(result, "version", "")) if ver_: _set_http_flavor_version( - labels, - f"{ver_[:1]}.{ver_[:-1]}", - sem_conv_opt_in_mode + labels, f"{ver_[:1]}.{ver_[:-1]}", sem_conv_opt_in_mode ) if exception is not None and _report_new(sem_conv_opt_in_mode): From de389083867c9c82d2ca2fb7d42b4e81c9f5b72d Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 30 Jul 2024 09:24:11 -0700 Subject: [PATCH 16/18] Update instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> --- .../src/opentelemetry/instrumentation/urllib/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 079fe7dd91..c4ef567ecc 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -295,7 +295,7 @@ def _instrumented_open_call( ver_ = str(getattr(result, "version", "")) if ver_: - _set_http_flavor_version( + _set_http_network_protocol_version( labels, f"{ver_[:1]}.{ver_[:-1]}", sem_conv_opt_in_mode ) From 8c5a57890a3fa719b16b6b371397d888f9867992 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 30 Jul 2024 09:45:52 -0700 Subject: [PATCH 17/18] Update __init__.py --- .../src/opentelemetry/instrumentation/urllib/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c4ef567ecc..e7833404d3 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -95,9 +95,9 @@ def response_hook(span, request_obj, response) _OpenTelemetryStabilitySignalType, _report_new, _report_old, - _set_http_flavor_version, _set_http_method, _set_http_url, + _set_http_network_protocol_version, _set_status, ) from opentelemetry.instrumentation.instrumentor import BaseInstrumentor From 1924c5af4c5fe000f26db8a19e4af7639da6b204 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 30 Jul 2024 09:56:44 -0700 Subject: [PATCH 18/18] Update __init__.py --- .../src/opentelemetry/instrumentation/urllib/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e7833404d3..d9072ba727 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -96,8 +96,8 @@ def response_hook(span, request_obj, response) _report_new, _report_old, _set_http_method, - _set_http_url, _set_http_network_protocol_version, + _set_http_url, _set_status, ) from opentelemetry.instrumentation.instrumentor import BaseInstrumentor