httpx: fix handling of async hooks

Don't default to sync hooks for async hooks, then check that they are
actually async.

Fixes open-telemetry#2734

Author: xrmx

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

@@ -192,6 +192,7 @@ async def async_response_hook(span, request, response):
 """
 import logging
 import typing
+from asyncio import iscoroutinefunction
 from types import TracebackType
 
 import httpx
@@ -731,15 +732,15 @@ def _instrument(self, **kwargs):
         self._original_async_client = httpx.AsyncClient
         request_hook = kwargs.get("request_hook")
         response_hook = kwargs.get("response_hook")
-        async_request_hook = kwargs.get("async_request_hook", request_hook)
-        async_response_hook = kwargs.get("async_response_hook", response_hook)
+        async_request_hook = kwargs.get("async_request_hook")
+        async_response_hook = kwargs.get("async_response_hook")
         if callable(request_hook):
             _InstrumentedClient._request_hook = request_hook
-        if callable(async_request_hook):
+        if callable(async_request_hook) and iscoroutinefunction(async_request_hook):
             _InstrumentedAsyncClient._request_hook = async_request_hook
         if callable(response_hook):
             _InstrumentedClient._response_hook = response_hook
-        if callable(async_response_hook):
+        if callable(async_response_hook) and iscoroutinefunction(async_response_hook):
             _InstrumentedAsyncClient._response_hook = async_response_hook
         tracer_provider = kwargs.get("tracer_provider")
         _InstrumentedClient._tracer_provider = tracer_provider

Diff for: instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py

@@ -780,9 +780,13 @@ def test_custom_tracer_provider(self):
             HTTPXClientInstrumentor().uninstrument()
 
         def test_response_hook(self):
+            response_hook_key = "async_response_hook" if asyncio.iscoroutinefunction(self.response_hook) else "response_hook"
+            response_hook_kwargs = {
+               response_hook_key: self.response_hook
+            }
             HTTPXClientInstrumentor().instrument(
                 tracer_provider=self.tracer_provider,
-                response_hook=self.response_hook,
+                **response_hook_kwargs
             )
             client = self.create_client()
             result = self.perform_request(self.URL, client=client)
@@ -823,9 +827,13 @@ def test_response_hook_sync_async_kwargs(self):
             HTTPXClientInstrumentor().uninstrument()
 
         def test_request_hook(self):
+            request_hook_key = "async_request_hook" if asyncio.iscoroutinefunction(self.request_hook) else "request_hook"
+            request_hook_kwargs = {
+               request_hook_key: self.request_hook
+            }
             HTTPXClientInstrumentor().instrument(
                 tracer_provider=self.tracer_provider,
-                request_hook=self.request_hook,
+                **request_hook_kwargs,
             )
             client = self.create_client()
             result = self.perform_request(self.URL, client=client)
@@ -1214,3 +1222,36 @@ def test_basic_multiple(self):
         self.perform_request(self.URL, client=self.client)
         self.perform_request(self.URL, client=self.client2)
         self.assert_span(num_spans=2)
+
+    def test_async_response_hook_does_nothing_if_not_coroutine(self):
+        HTTPXClientInstrumentor().instrument(
+            tracer_provider=self.tracer_provider,
+            async_response_hook=_response_hook,
+        )
+        client = self.create_client()
+        result = self.perform_request(self.URL, client=client)
+
+        self.assertEqual(result.text, "Hello!")
+        span = self.assert_span()
+        self.assertEqual(
+            dict(span.attributes),
+            {
+                SpanAttributes.HTTP_METHOD: "GET",
+                SpanAttributes.HTTP_URL: self.URL,
+                SpanAttributes.HTTP_STATUS_CODE: 200,
+            },
+        )
+        HTTPXClientInstrumentor().uninstrument()
+
+    def test_async_request_hook_does_nothing_if_not_coroutine(self):
+        HTTPXClientInstrumentor().instrument(
+            tracer_provider=self.tracer_provider,
+            async_request_hook=_request_hook,
+        )
+        client = self.create_client()
+        result = self.perform_request(self.URL, client=client)
+
+        self.assertEqual(result.text, "Hello!")
+        span = self.assert_span()
+        self.assertEqual(span.name, "GET")
+        HTTPXClientInstrumentor().uninstrument()