From 0f7ae6b2ba791908fa5d62d0de188a5bf89147a5 Mon Sep 17 00:00:00 2001 From: Itay Gibel Date: Wed, 1 Sep 2021 10:40:14 +0300 Subject: [PATCH 1/7] Urllib3: extend request hook with request body and headers --- .../instrumentation/urllib3/__init__.py | 30 +++++++++++++++++-- .../tests/test_urllib3_integration.py | 27 ++++++++++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py index 49d8a85b6b..078e295c34 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py @@ -92,6 +92,18 @@ def response_hook(span, request, response): _RequestHookT = typing.Optional[ typing.Callable[[Span, urllib3.connectionpool.HTTPConnectionPool], None] ] +_ExtendedRequestHookT = typing.Optional[ + typing.Callable[ + [ + Span, + urllib3.connectionpool.HTTPConnectionPool, + # Request headers dict + typing.Dict, + # Request Body + str, + ], + None] +] _ResponseHookT = typing.Optional[ typing.Callable[ [ @@ -139,7 +151,7 @@ def _uninstrument(self, **kwargs): def _instrument( tracer, - request_hook: _RequestHookT = None, + request_hook: typing.Union[_RequestHookT, _ExtendedRequestHookT] = None, response_hook: _ResponseHookT = None, url_filter: _UrlFilterT = None, ): @@ -150,6 +162,7 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): method = _get_url_open_arg("method", args, kwargs).upper() url = _get_url(instance, args, kwargs, url_filter) headers = _prepare_headers(kwargs) + body = _get_url_open_arg("body", args, kwargs) span_name = "HTTP {}".format(method.strip()) span_attributes = { @@ -161,7 +174,7 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): span_name, kind=SpanKind.CLIENT, attributes=span_attributes ) as span: if callable(request_hook): - request_hook(span, instance) + _call_request_hook(request_hook, span, instance, headers, body) inject(headers) with _suppress_further_instrumentation(): @@ -179,6 +192,19 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): ) +def _call_request_hook(request_hook: typing.Union[_RequestHookT, _ExtendedRequestHookT], + span: Span, + connection_pool: urllib3.connectionpool.HTTPConnectionPool, + headers: typing.Dict, + body: str): + try: + # First assume request_hook is a function of type _ExtendedRequestHookT + request_hook(span, connection_pool, headers, body) + except TypeError: + # Fallback to call request_hook as a function of type _RequestHookT + request_hook(span, connection_pool) + + def _get_url_open_arg(name: str, args: typing.List, kwargs: typing.Mapping): arg_idx = _URL_OPEN_ARG_TO_INDEX_MAPPING.get(name) if arg_idx is not None: diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index 9e32162d34..c91420424f 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - +import json import typing from unittest import mock @@ -279,3 +279,28 @@ def response_hook(span, request, response): self.assertEqual(span.name, "name set from hook") self.assertIn("response_hook_attr", span.attributes) self.assertEqual(span.attributes["response_hook_attr"], "value") + + def test_extended_request_hook(self): + def extended_request_hook(span, request, headers, body): + span.set_attribute("request_hook_headers", json.dumps(headers)) + span.set_attribute("request_hook_body", body) + + URLLib3Instrumentor().uninstrument() + URLLib3Instrumentor().instrument( + request_hook=extended_request_hook, + ) + + headers = {"header1": "value1", "header2": "value2"} + body = "param1=1¶m2=2" + + pool = urllib3.HTTPConnectionPool("httpbin.org") + response = pool.request("GET", "/status/200", body=body, headers=headers) + + self.assertEqual(b"Hello!", response.data) + + span = self.assert_span() + + self.assertIn("request_hook_headers", span.attributes) + self.assertEqual(span.attributes["request_hook_headers"], json.dumps(headers)) + self.assertIn("request_hook_body", span.attributes) + self.assertEqual(span.attributes["request_hook_body"], body) From b63152737397ff0c81ebe240396636f538f808b7 Mon Sep 17 00:00:00 2001 From: Itay Gibel Date: Thu, 2 Sep 2021 08:12:20 +0300 Subject: [PATCH 2/7] Change GET to POST in test_extended_request_hook --- .../tests/test_urllib3_integration.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index c91420424f..4504364b1f 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -45,6 +45,8 @@ def setUp(self): httpretty.enable(allow_net_connect=False) httpretty.register_uri(httpretty.GET, self.HTTP_URL, body="Hello!") httpretty.register_uri(httpretty.GET, self.HTTPS_URL, body="Hello!") + httpretty.register_uri(httpretty.POST, self.HTTP_URL, body="Hello!") + def tearDown(self): super().tearDown() @@ -294,7 +296,7 @@ def extended_request_hook(span, request, headers, body): body = "param1=1¶m2=2" pool = urllib3.HTTPConnectionPool("httpbin.org") - response = pool.request("GET", "/status/200", body=body, headers=headers) + response = pool.request("POST", "/status/200", body=body, headers=headers) self.assertEqual(b"Hello!", response.data) From 2dc0f1feb9e1981f93e96c103f27d216d7dd411f Mon Sep 17 00:00:00 2001 From: Itay Gibel Date: Thu, 2 Sep 2021 13:44:00 +0300 Subject: [PATCH 3/7] added changelog entry --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffe410a6a1..37f20d423f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.5.0-0.24b0...HEAD) +### Changed + +- `opentelemetry-instrumentation-urllib3` Added `_ExtendedRequestHookT` as an optional type of `request_hook`. + this type extends the existing `_RequestHookT` with two more fields - the request body and the request headers + ## [1.5.0-0.24b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.5.0-0.24b0) - 2021-08-26 ### Added From 42995da43587709603f6f6d63279f4ff153fcb5b Mon Sep 17 00:00:00 2001 From: Itay Gibel Date: Thu, 2 Sep 2021 14:03:44 +0300 Subject: [PATCH 4/7] update ExtendedReqeustHookT --- .../src/opentelemetry/instrumentation/urllib3/__init__.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py index 078e295c34..f8ad2c57a7 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py @@ -97,12 +97,11 @@ def response_hook(span, request, response): [ Span, urllib3.connectionpool.HTTPConnectionPool, - # Request headers dict typing.Dict, - # Request Body - str, + typing.Optional[str], ], - None] + None, + ] ] _ResponseHookT = typing.Optional[ typing.Callable[ From 76f1f23e6100b9a1361324f1116fbb93af7ddd47 Mon Sep 17 00:00:00 2001 From: Itay Gibel Date: Thu, 2 Sep 2021 14:07:31 +0300 Subject: [PATCH 5/7] adding up to date generated code --- .../instrumentation/urllib3/__init__.py | 12 +++++++----- .../tests/test_urllib3_integration.py | 13 +++++++------ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py index f8ad2c57a7..a177618358 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py @@ -191,11 +191,13 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): ) -def _call_request_hook(request_hook: typing.Union[_RequestHookT, _ExtendedRequestHookT], - span: Span, - connection_pool: urllib3.connectionpool.HTTPConnectionPool, - headers: typing.Dict, - body: str): +def _call_request_hook( + request_hook: typing.Union[_RequestHookT, _ExtendedRequestHookT], + span: Span, + connection_pool: urllib3.connectionpool.HTTPConnectionPool, + headers: typing.Dict, + body: str, +): try: # First assume request_hook is a function of type _ExtendedRequestHookT request_hook(span, connection_pool, headers, body) diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index 4504364b1f..27bdd7c7a2 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -47,7 +47,6 @@ def setUp(self): httpretty.register_uri(httpretty.GET, self.HTTPS_URL, body="Hello!") httpretty.register_uri(httpretty.POST, self.HTTP_URL, body="Hello!") - def tearDown(self): super().tearDown() URLLib3Instrumentor().uninstrument() @@ -288,21 +287,23 @@ def extended_request_hook(span, request, headers, body): span.set_attribute("request_hook_body", body) URLLib3Instrumentor().uninstrument() - URLLib3Instrumentor().instrument( - request_hook=extended_request_hook, - ) + URLLib3Instrumentor().instrument(request_hook=extended_request_hook,) headers = {"header1": "value1", "header2": "value2"} body = "param1=1¶m2=2" pool = urllib3.HTTPConnectionPool("httpbin.org") - response = pool.request("POST", "/status/200", body=body, headers=headers) + response = pool.request( + "POST", "/status/200", body=body, headers=headers + ) self.assertEqual(b"Hello!", response.data) span = self.assert_span() self.assertIn("request_hook_headers", span.attributes) - self.assertEqual(span.attributes["request_hook_headers"], json.dumps(headers)) + self.assertEqual( + span.attributes["request_hook_headers"], json.dumps(headers) + ) self.assertIn("request_hook_body", span.attributes) self.assertEqual(span.attributes["request_hook_body"], body) From 2b3dc62465deeec17da33e222968a878ee2c505d Mon Sep 17 00:00:00 2001 From: Itay Gibel Date: Sun, 5 Sep 2021 10:54:31 +0300 Subject: [PATCH 6/7] replace _RequestHookT with _ExtendedRequestHookT --- .../instrumentation/urllib3/__init__.py | 22 ++----------------- .../tests/test_urllib3_integration.py | 8 +++---- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py index a177618358..240ab29477 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py @@ -90,9 +90,6 @@ def response_hook(span, request, response): _UrlFilterT = typing.Optional[typing.Callable[[str], str]] _RequestHookT = typing.Optional[ - typing.Callable[[Span, urllib3.connectionpool.HTTPConnectionPool], None] -] -_ExtendedRequestHookT = typing.Optional[ typing.Callable[ [ Span, @@ -150,7 +147,7 @@ def _uninstrument(self, **kwargs): def _instrument( tracer, - request_hook: typing.Union[_RequestHookT, _ExtendedRequestHookT] = None, + request_hook: _RequestHookT = None, response_hook: _ResponseHookT = None, url_filter: _UrlFilterT = None, ): @@ -173,7 +170,7 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): span_name, kind=SpanKind.CLIENT, attributes=span_attributes ) as span: if callable(request_hook): - _call_request_hook(request_hook, span, instance, headers, body) + request_hook(span, instance, headers, body) inject(headers) with _suppress_further_instrumentation(): @@ -191,21 +188,6 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): ) -def _call_request_hook( - request_hook: typing.Union[_RequestHookT, _ExtendedRequestHookT], - span: Span, - connection_pool: urllib3.connectionpool.HTTPConnectionPool, - headers: typing.Dict, - body: str, -): - try: - # First assume request_hook is a function of type _ExtendedRequestHookT - request_hook(span, connection_pool, headers, body) - except TypeError: - # Fallback to call request_hook as a function of type _RequestHookT - request_hook(span, connection_pool) - - def _get_url_open_arg(name: str, args: typing.List, kwargs: typing.Mapping): arg_idx = _URL_OPEN_ARG_TO_INDEX_MAPPING.get(name) if arg_idx is not None: diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index 27bdd7c7a2..d2893d037b 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -262,7 +262,7 @@ def test_credential_removal(self): self.assert_success_span(response, self.HTTP_URL) def test_hooks(self): - def request_hook(span, request): + def request_hook(span, request, body, headers): span.update_name("name set from hook") def response_hook(span, request, response): @@ -281,13 +281,13 @@ def response_hook(span, request, response): self.assertIn("response_hook_attr", span.attributes) self.assertEqual(span.attributes["response_hook_attr"], "value") - def test_extended_request_hook(self): - def extended_request_hook(span, request, headers, body): + def test_request_hook_params(self): + def request_hook(span, request, headers, body): span.set_attribute("request_hook_headers", json.dumps(headers)) span.set_attribute("request_hook_body", body) URLLib3Instrumentor().uninstrument() - URLLib3Instrumentor().instrument(request_hook=extended_request_hook,) + URLLib3Instrumentor().instrument(request_hook=request_hook,) headers = {"header1": "value1", "header2": "value2"} body = "param1=1¶m2=2" From 7bce6e898c3e76fa6f915b4eb9e14f68f6eec4f5 Mon Sep 17 00:00:00 2001 From: Itay Gibel Date: Thu, 9 Sep 2021 12:11:06 +0300 Subject: [PATCH 7/7] updated Changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37f20d423f..1fe1f5acb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- `opentelemetry-instrumentation-urllib3` Added `_ExtendedRequestHookT` as an optional type of `request_hook`. - this type extends the existing `_RequestHookT` with two more fields - the request body and the request headers +- `opentelemetry-instrumentation-urllib3` Updated `_RequestHookT` with two additional fields - the request body and the request headers +([#660](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/660)) ## [1.5.0-0.24b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.5.0-0.24b0) - 2021-08-26