feat(urllib3)!: refactor request hook parameters

Author: eigenein

Diff for: CHANGELOG.md

@@ -46,8 +46,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2580](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2580))
 - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `asgi` middleware
   ([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610))
-  - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `fastapi` middleware
-  ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682))
+- Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `fastapi` middleware
+([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682))
+- `urllib3`: the request hook has been refactored: the `headers` and `body` parameters have been replaced by a single `request_info: RequestInfo` parameter that now contains `method` and `url` as well ([#2711](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2711))
 
 ### Fixed
 - Handle `redis.exceptions.WatchError` as a non-error event in redis instrumentation

Diff for: instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py

@@ -47,14 +47,19 @@ def strip_query_params(url: str) -> str:
 
 .. code:: python
 
-    # `request` is an instance of urllib3.connectionpool.HTTPConnectionPool
-    def request_hook(span, request):
-        pass
-
-    # `request` is an instance of urllib3.connectionpool.HTTPConnectionPool
-    # `response` is an instance of urllib3.response.HTTPResponse
-    def response_hook(span, request, response):
-        pass
+    def request_hook(
+        span: Span,
+        pool: urllib3.connectionpool.HTTPConnectionPool,
+        request_info: RequestInfo,
+    ) -> Any:
+        ...
+
+    def response_hook(
+        span: Span,
+        pool: urllib3.connectionpool.HTTPConnectionPool,
+        response: urllib3.response.HTTPResponse,
+    ) -> Any:
+        ...
 
     URLLib3Instrumentor().instrument(
         request_hook=request_hook, response_hook=response_hook
@@ -81,6 +86,7 @@ def response_hook(span, request, response):
 import collections.abc
 import io
 import typing
+from dataclasses import dataclass
 from timeit import default_timer
 from typing import Collection
 
@@ -111,14 +117,29 @@ def response_hook(span, request, response):
 
 _excluded_urls_from_env = get_excluded_urls("URLLIB3")
 
+
+@dataclass
+class RequestInfo:
+    """Arguments that were passed to the ``urlopen()`` call."""
+
+    __slots__ = ("method", "url", "headers", "body")
+
+    # The type annotations here come from ``HTTPConnectionPool.urlopen()``.
+    method: str
+    url: str
+    headers: typing.Optional[typing.Mapping[str, str]]
+    body: typing.Union[
+        bytes, typing.IO[typing.Any], typing.Iterable[bytes], str, None
+    ]
+
+
 _UrlFilterT = typing.Optional[typing.Callable[[str], str]]
 _RequestHookT = typing.Optional[
     typing.Callable[
         [
             Span,
             urllib3.connectionpool.HTTPConnectionPool,
-            typing.Dict,
-            typing.Optional[str],
+            RequestInfo,
         ],
         None,
     ]
@@ -243,7 +264,13 @@ def instrumented_urlopen(wrapped, instance, args, kwargs):
             span_name, kind=SpanKind.CLIENT, attributes=span_attributes
         ) as span, set_ip_on_next_http_connection(span):
             if callable(request_hook):
-                request_hook(span, instance, headers, body)
+                request_info = RequestInfo(
+                    method=method,
+                    url=url,
+                    headers=headers,
+                    body=body,
+                )
+                request_hook(span, instance, request_info)
             inject(headers)
 
             with suppress_http_instrumentation():

Diff for: instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py

@@ -20,7 +20,10 @@
 import urllib3.exceptions
 
 from opentelemetry import trace
-from opentelemetry.instrumentation.urllib3 import URLLib3Instrumentor
+from opentelemetry.instrumentation.urllib3 import (
+    RequestInfo,
+    URLLib3Instrumentor,
+)
 from opentelemetry.instrumentation.utils import (
     suppress_http_instrumentation,
     suppress_instrumentation,
@@ -29,6 +32,7 @@
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.test.mock_textmap import MockTextMapPropagator
 from opentelemetry.test.test_base import TestBase
+from opentelemetry.trace import Span
 from opentelemetry.util.http import get_excluded_urls
 
 # pylint: disable=too-many-public-methods
@@ -315,10 +319,10 @@ def test_credential_removal(self):
         self.assert_success_span(response, self.HTTP_URL)
 
     def test_hooks(self):
-        def request_hook(span, request, body, headers):
+        def request_hook(span, pool, request_info):
             span.update_name("name set from hook")
 
-        def response_hook(span, request, response):
+        def response_hook(span, pool, response):
             span.set_attribute("response_hook_attr", "value")
 
         URLLib3Instrumentor().uninstrument()
@@ -335,11 +339,17 @@ def response_hook(span, request, response):
         self.assertEqual(span.attributes["response_hook_attr"], "value")
 
     def test_request_hook_params(self):
-        def request_hook(span, request, headers, body):
+        def request_hook(
+            span: Span,
+            _pool: urllib3.connectionpool.ConnectionPool,
+            request_info: RequestInfo,
+        ) -> None:
+            span.set_attribute("request_hook_method", request_info.method)
+            span.set_attribute("request_hook_url", request_info.url)
             span.set_attribute(
-                "request_hook_headers", json.dumps(dict(headers))
+                "request_hook_headers", json.dumps(dict(request_info.headers))
             )
-            span.set_attribute("request_hook_body", body)
+            span.set_attribute("request_hook_body", request_info.body)
 
         URLLib3Instrumentor().uninstrument()
         URLLib3Instrumentor().instrument(
@@ -358,6 +368,10 @@ def request_hook(span, request, headers, body):
 
         span = self.assert_span()
 
+        self.assertEqual(span.attributes["request_hook_method"], "POST")
+        self.assertEqual(
+            span.attributes["request_hook_url"], "http://mock/status/200"
+        )
         self.assertIn("request_hook_headers", span.attributes)
         self.assertEqual(
             span.attributes["request_hook_headers"], json.dumps(headers)
@@ -366,8 +380,12 @@ def request_hook(span, request, headers, body):
         self.assertEqual(span.attributes["request_hook_body"], body)
 
     def test_request_positional_body(self):
-        def request_hook(span, request, headers, body):
-            span.set_attribute("request_hook_body", body)
+        def request_hook(
+            span: Span,
+            _pool: urllib3.connectionpool.ConnectionPool,
+            request_info: RequestInfo,
+        ) -> None:
+            span.set_attribute("request_hook_body", request_info.body)
 
         URLLib3Instrumentor().uninstrument()
         URLLib3Instrumentor().instrument(