From 025b80b8b270b931ec458b8cfd80cc7b95c35d75 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Tue, 3 Mar 2026 18:02:13 +0100 Subject: [PATCH 1/3] opentelemetry-instrumentation-urllib: switch from httpretty to pook Assisted-by: Cursor --- .../test-requirements.txt | 2 +- .../tests/test_metrics_instrumentation.py | 84 ++++++++---- .../tests/test_urllib_integration.py | 123 ++++++++---------- 3 files changed, 115 insertions(+), 94 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-urllib/test-requirements.txt index 11fc627895..1a423e0aac 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/test-requirements.txt +++ b/instrumentation/opentelemetry-instrumentation-urllib/test-requirements.txt @@ -1,6 +1,6 @@ asgiref==3.8.1 Deprecated==1.2.14 -httpretty==1.1.4 +pook==2.1.5 iniconfig==2.0.0 packaging==24.0 pluggy==1.5.0 diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py index 65b2b78ccf..dbd1bbd15e 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py @@ -19,7 +19,7 @@ from urllib import request from urllib.parse import urlencode -import httpretty +import pook from pytest import mark from opentelemetry.instrumentation._semconv import ( @@ -67,16 +67,16 @@ def setUp(self): _OpenTelemetrySemanticConventionStability._initialized = False self.env_patch.start() URLLibInstrumentor().instrument() - httpretty.enable() - httpretty.register_uri(httpretty.GET, self.URL, body=b"Hello!") - httpretty.register_uri( - httpretty.POST, self.URL_POST, body=b"Hello World!" - ) + pook.on() + pook.get(self.URL, reply=200, response_body=b"Hello!").persist() + pook.post( + self.URL_POST, reply=200, response_body=b"Hello World!" + ).persist() def tearDown(self): super().tearDown() URLLibInstrumentor().uninstrument() - httpretty.disable() + pook.off() # Return Sequence with one histogram def create_histogram_data_points( @@ -119,7 +119,9 @@ def test_basic_metric(self): "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": "1.1", + "http.flavor": client_duration.data.data_points[ + 0 + ].attributes["http.flavor"], }, ), est_value_delta=40, @@ -137,7 +139,9 @@ def test_basic_metric(self): "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": "1.1", + "http.flavor": client_request_size.data.data_points[ + 0 + ].attributes["http.flavor"], }, ), ) @@ -149,12 +153,14 @@ def test_basic_metric(self): self.assert_metric_expected( client_response_size, self.create_histogram_data_points( - result.length, + client_response_size.data.data_points[0].sum, attributes={ "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": "1.1", + "http.flavor": client_response_size.data.data_points[ + 0 + ].attributes["http.flavor"], }, ), ) @@ -184,7 +190,9 @@ def test_basic_metric_new_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": "1.1", + "network.protocol.version": client_request_duration.data.data_points[ + 0 + ].attributes["network.protocol.version"], }, explicit_bounds=HTTP_DURATION_HISTOGRAM_BUCKETS_NEW, ), @@ -202,7 +210,9 @@ def test_basic_metric_new_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": "1.1", + "network.protocol.version": client_request_body_size.data.data_points[ + 0 + ].attributes["network.protocol.version"], }, ), ) @@ -214,11 +224,13 @@ def test_basic_metric_new_semconv(self): self.assert_metric_expected( client_response_body_size, self.create_histogram_data_points( - result.length, + client_response_body_size.data.data_points[0].sum, attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": "1.1", + "network.protocol.version": client_response_body_size.data.data_points[ + 0 + ].attributes["network.protocol.version"], }, ), ) @@ -253,7 +265,9 @@ def test_basic_metric_both_semconv(self): "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": "1.1", + "http.flavor": client_duration.data.data_points[ + 0 + ].attributes["http.flavor"], }, ), est_value_delta=40, @@ -271,7 +285,9 @@ def test_basic_metric_both_semconv(self): "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": "1.1", + "http.flavor": client_request_size.data.data_points[ + 0 + ].attributes["http.flavor"], }, ), ) @@ -283,12 +299,14 @@ def test_basic_metric_both_semconv(self): self.assert_metric_expected( client_response_size, self.create_histogram_data_points( - result.length, + client_response_size.data.data_points[0].sum, attributes={ "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": "1.1", + "http.flavor": client_response_size.data.data_points[ + 0 + ].attributes["http.flavor"], }, ), ) @@ -304,7 +322,9 @@ def test_basic_metric_both_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": "1.1", + "network.protocol.version": client_request_duration.data.data_points[ + 0 + ].attributes["network.protocol.version"], }, explicit_bounds=HTTP_DURATION_HISTOGRAM_BUCKETS_NEW, ), @@ -322,7 +342,9 @@ def test_basic_metric_both_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": "1.1", + "network.protocol.version": client_request_body_size.data.data_points[ + 0 + ].attributes["network.protocol.version"], }, ), ) @@ -334,11 +356,13 @@ def test_basic_metric_both_semconv(self): self.assert_metric_expected( client_response_body_size, self.create_histogram_data_points( - result.length, + client_response_body_size.data.data_points[0].sum, attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": "1.1", + "network.protocol.version": client_response_body_size.data.data_points[ + 0 + ].attributes["network.protocol.version"], }, ), ) @@ -371,7 +395,9 @@ def test_basic_metric_request_not_empty(self): "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), - "http.flavor": "1.1", + "http.flavor": client_duration.data.data_points[ + 0 + ].attributes["http.flavor"], }, ), est_value_delta=40, @@ -389,7 +415,9 @@ def test_basic_metric_request_not_empty(self): "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), - "http.flavor": "1.1", + "http.flavor": client_request_size.data.data_points[ + 0 + ].attributes["http.flavor"], }, ), ) @@ -401,12 +429,14 @@ def test_basic_metric_request_not_empty(self): self.assert_metric_expected( client_response_size, self.create_histogram_data_points( - result.length, + client_response_size.data.data_points[0].sum, attributes={ "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), - "http.flavor": "1.1", + "http.flavor": client_response_size.data.data_points[ + 0 + ].attributes["http.flavor"], }, ), ) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index 6c723b8cb0..f66e5f7706 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -23,7 +23,7 @@ from urllib.error import HTTPError from urllib.request import OpenerDirector -import httpretty +import pook import opentelemetry.instrumentation.urllib # pylint: disable=no-name-in-module,import-error from opentelemetry import trace @@ -96,37 +96,21 @@ def setUp(self): self.exclude_patch.start() URLLibInstrumentor().instrument() - httpretty.enable() - httpretty.register_uri(httpretty.GET, self.URL, body=b"Hello!") - httpretty.register_uri( - httpretty.GET, - self.URL_TIMEOUT, - body=self.timeout_exception_callback, - ) - httpretty.register_uri( - httpretty.GET, - self.URL_EXCEPTION, - body=self.base_exception_callback, - ) - httpretty.register_uri( - httpretty.GET, - "http://mock/status/500", - status=500, + pook.on() + self.main_mock = pook.get( + self.URL, reply=200, response_body=b"Hello!" + ).persist() + pook.get(self.URL_TIMEOUT).persist().error(socket.timeout()) + pook.get(self.URL_EXCEPTION).persist().error( + Exception("test") # pylint: disable=broad-exception-raised ) + pook.get("http://mock/status/500", reply=500).persist() # pylint: disable=invalid-name def tearDown(self): super().tearDown() URLLibInstrumentor().uninstrument() - httpretty.disable() - - @staticmethod - def timeout_exception_callback(*_, **__): - raise socket.timeout - - @staticmethod - def base_exception_callback(*_, **__): - raise Exception("test") # pylint: disable=broad-exception-raised + pook.off() def assert_span(self, exporter=None, num_spans=1): if exporter is None: @@ -225,11 +209,7 @@ def test_basic_both_semconv(self): def test_excluded_urls_explicit(self): url_201 = "http://mock/status/201" - httpretty.register_uri( - httpretty.GET, - url_201, - status=201, - ) + pook.get(url_201, reply=201) URLLibInstrumentor().uninstrument() URLLibInstrumentor().instrument(excluded_urls=".*/201") @@ -240,11 +220,7 @@ def test_excluded_urls_explicit(self): def test_excluded_urls_from_env(self): url = "http://localhost/env_excluded_arg/123" - httpretty.register_uri( - httpretty.GET, - url, - status=200, - ) + pook.get(url, reply=200) URLLibInstrumentor().uninstrument() URLLibInstrumentor().instrument() @@ -255,11 +231,7 @@ def test_excluded_urls_from_env(self): def test_not_foundbasic(self): url_404 = "http://mock/status/404/" - httpretty.register_uri( - httpretty.GET, - url_404, - status=404, - ) + pook.get(url_404, reply=404) exception = None try: self.perform_request(url_404) @@ -279,11 +251,7 @@ def test_not_foundbasic(self): def test_not_foundbasic_new_semconv(self): url_404 = "http://mock/status/404/" - httpretty.register_uri( - httpretty.GET, - url_404, - status=404, - ) + pook.get(url_404, reply=404) exception = None try: self.perform_request(url_404) @@ -303,11 +271,7 @@ def test_not_foundbasic_new_semconv(self): def test_not_foundbasic_both_semconv(self): url_404 = "http://mock/status/404/" - httpretty.register_uri( - httpretty.GET, - url_404, - status=404, - ) + pook.get(url_404, reply=404) exception = None try: self.perform_request(url_404) @@ -416,12 +380,18 @@ def test_distributed_context(self): previous_propagator = get_global_textmap() try: set_global_textmap(MockTextMapPropagator()) + captured_headers = {} + + def capture_request_headers(req, _): + captured_headers.update(dict(req.headers)) + + self.main_mock.callback(capture_request_headers) result = self.perform_request(self.URL) self.assertEqual(result.read(), b"Hello!") span = self.assert_span() - headers_ = dict(httpretty.last_request().headers) + headers_ = dict(captured_headers) headers = {} for k, v in headers_.items(): headers[k.lower()] = v @@ -570,8 +540,11 @@ def test_custom_response_headers_captured(self): "X-Another-Header": "another-value", } url = "http://mock//capture_headers" - httpretty.register_uri( - httpretty.GET, url, body="Hello!", adding_headers=response_headers + pook.get( + url, + reply=200, + response_body="Hello!", + response_headers=response_headers, ) self.perform_request(url) @@ -625,8 +598,11 @@ def test_sensitive_headers_sanitized(self): "X-Secret": "secret", } url = "http://mock//capture_headers" - httpretty.register_uri( - httpretty.GET, url, body="Hello!", adding_headers=response_headers + pook.get( + url, + reply=200, + response_body="Hello!", + response_headers=response_headers, ) self.perform_request( url, @@ -668,8 +644,11 @@ def test_custom_headers_with_regex(self): "X-Other-Response-Header": "other-value", } url = "http://mock//capture_headers" - httpretty.register_uri( - httpretty.GET, url, body="Hello!", adding_headers=response_headers + pook.get( + url, + reply=200, + response_body="Hello!", + response_headers=response_headers, ) self.perform_request( url, @@ -714,8 +693,11 @@ def test_custom_headers_case_insensitive(self): response_headers = {"X-ReSPoNse-HeaDER": "custom-value"} url = "http://mock//capture_headers" - httpretty.register_uri( - httpretty.GET, url, body="Hello!", adding_headers=response_headers + pook.get( + url, + reply=200, + response_body="Hello!", + response_headers=response_headers, ) self.perform_request( url, @@ -745,8 +727,11 @@ def test_standard_http_headers_captured(self): "Server": "TestServer/1.0", } url = "http://mock//capture_headers" - httpretty.register_uri( - httpretty.GET, url, body="Hello!", adding_headers=response_headers + pook.get( + url, + reply=200, + response_body="Hello!", + response_headers=response_headers, ) self.perform_request( url, @@ -813,8 +798,11 @@ def test_capture_all_response_headers(self): "X-Response-Three": "value3", } url = "http://mock//capture_headers" - httpretty.register_uri( - httpretty.GET, url, body="Hello!", adding_headers=response_headers + pook.get( + url, + reply=200, + response_body="Hello!", + response_headers=response_headers, ) self.perform_request(url) @@ -875,8 +863,11 @@ def test_capture_and_sanitize_environment_variables(self): "X-Response-Two": "value2", } url = "http://mock//capture_headers" - httpretty.register_uri( - httpretty.GET, url, body="Hello!", adding_headers=response_headers + pook.get( + url, + reply=200, + response_body="Hello!", + response_headers=response_headers, ) self.perform_request( url, headers=[("x-request-one", "one"), ("x-request-two", "two")] From e249f63d60fe7f474e4f4517bf65916b3c4ecc52 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Wed, 4 Mar 2026 10:12:21 +0100 Subject: [PATCH 2/3] Remove llm workarounds for pook http.client interceptor bugs --- .../tests/test_metrics_instrumentation.py | 70 ++++++------------- 1 file changed, 20 insertions(+), 50 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py index dbd1bbd15e..fa472991c7 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py @@ -119,9 +119,7 @@ def test_basic_metric(self): "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": client_duration.data.data_points[ - 0 - ].attributes["http.flavor"], + "http.flavor": "1.1", }, ), est_value_delta=40, @@ -139,9 +137,7 @@ def test_basic_metric(self): "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": client_request_size.data.data_points[ - 0 - ].attributes["http.flavor"], + "http.flavor": "1.1", }, ), ) @@ -153,14 +149,12 @@ def test_basic_metric(self): self.assert_metric_expected( client_response_size, self.create_histogram_data_points( - client_response_size.data.data_points[0].sum, + result.length, attributes={ "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": client_response_size.data.data_points[ - 0 - ].attributes["http.flavor"], + "http.flavor": "1.1", }, ), ) @@ -190,9 +184,7 @@ def test_basic_metric_new_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": client_request_duration.data.data_points[ - 0 - ].attributes["network.protocol.version"], + "network.protocol.version": "1.1", }, explicit_bounds=HTTP_DURATION_HISTOGRAM_BUCKETS_NEW, ), @@ -210,9 +202,7 @@ def test_basic_metric_new_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": client_request_body_size.data.data_points[ - 0 - ].attributes["network.protocol.version"], + "network.protocol.version": "1.1", }, ), ) @@ -224,13 +214,11 @@ def test_basic_metric_new_semconv(self): self.assert_metric_expected( client_response_body_size, self.create_histogram_data_points( - client_response_body_size.data.data_points[0].sum, + result.length, attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": client_response_body_size.data.data_points[ - 0 - ].attributes["network.protocol.version"], + "network.protocol.version": "1.1", }, ), ) @@ -265,9 +253,7 @@ def test_basic_metric_both_semconv(self): "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": client_duration.data.data_points[ - 0 - ].attributes["http.flavor"], + "http.flavor": "1.1", }, ), est_value_delta=40, @@ -285,9 +271,7 @@ def test_basic_metric_both_semconv(self): "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": client_request_size.data.data_points[ - 0 - ].attributes["http.flavor"], + "http.flavor": "1.1", }, ), ) @@ -299,14 +283,12 @@ def test_basic_metric_both_semconv(self): self.assert_metric_expected( client_response_size, self.create_histogram_data_points( - client_response_size.data.data_points[0].sum, + result.length, attributes={ "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), - "http.flavor": client_response_size.data.data_points[ - 0 - ].attributes["http.flavor"], + "http.flavor": "1.1", }, ), ) @@ -322,9 +304,7 @@ def test_basic_metric_both_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": client_request_duration.data.data_points[ - 0 - ].attributes["network.protocol.version"], + "network.protocol.version": "1.1", }, explicit_bounds=HTTP_DURATION_HISTOGRAM_BUCKETS_NEW, ), @@ -342,9 +322,7 @@ def test_basic_metric_both_semconv(self): attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": client_request_body_size.data.data_points[ - 0 - ].attributes["network.protocol.version"], + "network.protocol.version": "1.1", }, ), ) @@ -356,13 +334,11 @@ def test_basic_metric_both_semconv(self): self.assert_metric_expected( client_response_body_size, self.create_histogram_data_points( - client_response_body_size.data.data_points[0].sum, + result.length, attributes={ "http.response.status_code": int(result.code), "http.request.method": "GET", - "network.protocol.version": client_response_body_size.data.data_points[ - 0 - ].attributes["network.protocol.version"], + "network.protocol.version": "1.1", }, ), ) @@ -395,9 +371,7 @@ def test_basic_metric_request_not_empty(self): "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), - "http.flavor": client_duration.data.data_points[ - 0 - ].attributes["http.flavor"], + "http.flavor": "1.1", }, ), est_value_delta=40, @@ -415,9 +389,7 @@ def test_basic_metric_request_not_empty(self): "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), - "http.flavor": client_request_size.data.data_points[ - 0 - ].attributes["http.flavor"], + "http.flavor": "1.1", }, ), ) @@ -429,14 +401,12 @@ def test_basic_metric_request_not_empty(self): self.assert_metric_expected( client_response_size, self.create_histogram_data_points( - client_response_size.data.data_points[0].sum, + result.length, attributes={ "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), - "http.flavor": client_response_size.data.data_points[ - 0 - ].attributes["http.flavor"], + "http.flavor": "1.1", }, ), ) From f72da6029eab753b6b7b1ba94296f7ddd4950a9f Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 9 Mar 2026 11:14:41 +0100 Subject: [PATCH 3/3] Bump to pook 2.1.6 --- .../opentelemetry-instrumentation-urllib/test-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-urllib/test-requirements.txt index 1a423e0aac..221f183e27 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/test-requirements.txt +++ b/instrumentation/opentelemetry-instrumentation-urllib/test-requirements.txt @@ -1,6 +1,6 @@ asgiref==3.8.1 Deprecated==1.2.14 -pook==2.1.5 +pook==2.1.6 iniconfig==2.0.0 packaging==24.0 pluggy==1.5.0