search

open-telemetry / opentelemetry-python-contrib

OpenTelemetry instrumentation for Python modules
https://opentelemetry.io
Apache License 2.0
697 stars 579 forks source link

`asgiref` 3.8 compatibility? #2776

Closed drupol closed 2 weeks ago

drupol commented 1 month ago

Describe your environment

OS: Nix/NixOS Python version: 3.12 Package version: 0.47b0

What happened?

When trying to package opentelemetry-instrumentation-asgi, it's failing

Build log ``` Executing pytestCheckPhase ============================= test session starts ============================== platform linux -- Python 3.12.4, pytest-8.2.2, pluggy-1.5.0 -- /nix/store/l014xp1qxdl6gim3zc0jv3mpxhbp346s-python3-3.12.4/bin/python3.12 cachedir: .pytest_cache rootdir: /build/source configfile: pytest.ini collecting ... ^Mcollected 80 items tests/test_asgi_custom_headers.py::TestCustomHeadersEnv::test_http_custom_request_headers_in_span_attributes FAILED [ 1%] tests/test_asgi_custom_headers.py::TestCustomHeadersEnv::test_http_custom_request_headers_not_in_span_attributes FAILED [ 2%] tests/test_asgi_custom_headers.py::TestCustomHeadersEnv::test_http_custom_response_headers_in_span_attributes -------------------------------- live log call --------------------------------- WARNING opentelemetry.sdk.metrics._internal:__init__.py:204 An instrument with name http.server.duration, type Histogram, unit ms and description Duration of HTTP server requests. has been created already. WARNING opentelemetry.sdk.metrics._internal:__init__.py:204 An instrument with name http.server.response.size, type Histogram, unit By and description measures the size of HTTP response messages (compressed). has been created already. WARNING opentelemetry.sdk.metrics._internal:__init__.py:204 An instrument with name http.server.request.size, type Histogram, unit By and description Measures the size of HTTP request messages (compressed). has been created already. WARNING opentelemetry.sdk.metrics._internal:__init__.py:129 An instrument with name http.server.active_requests, type UpDownCounter, unit {request} and description Number of active HTTP server requests. has been created already. FAILED [ 3%] tests/test_asgi_custom_headers.py::TestCustomHeadersEnv::test_http_custom_response_headers_not_in_span_attributes -------------------------------- live log call --------------------------------- WARNING opentelemetry.sdk.metrics._internal:__init__.py:204 An instrument with name http.server.duration, type Histogram, unit ms and description Duration of HTTP server requests. has been created already. WARNING opentelemetry.sdk.metrics._internal:__init__.py:204 An instrument with name http.server.response.size, type Histogram, unit By and description measures the size of HTTP response messages (compressed). has been created already. WARNING opentelemetry.sdk.metrics._internal:__init__.py:204 An instrument with name http.server.request.size, type Histogram, unit By and description Measures the size of HTTP request messages (compressed). has been created already. WARNING opentelemetry.sdk.metrics._internal:__init__.py:129 An instrument with name http.server.active_requests, type UpDownCounter, unit {request} and description Number of active HTTP server requests. has been created already. Exception ignored in: Traceback (most recent call last): File "/nix/store/g1wfp82pn8w93ijzirndj9a9k7wnirh0-python3.12-asgiref-3.8.1/lib/python3.12/site-packages/asgiref/testing.py", line 58, in __del__ self.stop(exceptions=False) File "/nix/store/g1wfp82pn8w93ijzirndj9a9k7wnirh0-python3.12-asgiref-3.8.1/lib/python3.12/site-packages/asgiref/testing.py", line 49, in stop if not self.future.done(): ^^^^^^^^^^^ AttributeError: 'ApplicationCommunicator' object has no attribute 'future' Exception ignored in: Traceback (most recent call last): File "/nix/store/g1wfp82pn8w93ijzirndj9a9k7wnirh0-python3.12-asgiref-3.8.1/lib/python3.12/site-packages/asgiref/testing.py", line 58, in __del__ self.stop(exceptions=False) File "/nix/store/g1wfp82pn8w93ijzirndj9a9k7wnirh0-python3.12-asgiref-3.8.1/lib/python3.12/site-packages/asgiref/testing.py", line 49, in stop if not self.future.done(): ^^^^^^^^^^^ AttributeError: 'ApplicationCommunicator' object has no attribute 'future' Exception ignored in: Traceback (most recent call last): File "/nix/store/g1wfp82pn8w93ijzirndj9a9k7wnirh0-python3.12-asgiref-3.8.1/lib/python3.12/site-packages/asgiref/testing.py", line 58, in __del__ self.stop(exceptions=False) File "/nix/store/g1wfp82pn8w93ijzirndj9a9k7wnirh0-python3.12-asgiref-3.8.1/lib/python3.12/site-packages/asgiref/testing.py", line 49, in stop if not self.future.done(): ^^^^^^^^^^^ AttributeError: 'ApplicationCommunicator' object has no attribute 'future' FAILED [ 5%] tests/test_asgi_custom_headers.py::TestCustomHeadersEnv::test_http_repeat_request_headers_in_span_attributes FAILED [ 6%] tests/test_asgi_custom_headers.py::TestCustomHeadersEnv::test_http_repeat_response_headers_in_span_attributes ```

Steps to Reproduce

With Nix:

nix build github:NixOS/nixpkgs/master#python3Packages.opentelemetry-instrumentation-asgi -L --show-trace

Without Nix:

With Python ^3.12 and asgiref ^3.8, run the tests in opentelemetry-instrumentation-asgi.

Expected Result

Tests should pass

Actual Result

Tests are failing

Additional context

No response

Would you like to implement a fix?

None

xrmx commented 1 month ago

Thanks for reporting. This looks like a bug in the AsgiTestBase class in core though.

xrmx commented 1 month ago

This is not specific to python 3.12.

Not sure it's the right approach but

diff --git a/instrumentation/opentelemetry-instrumentation-asgi/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-asgi/test-requirements.txt
index ebe439d1..18a11500 100644
--- a/instrumentation/opentelemetry-instrumentation-asgi/test-requirements.txt
+++ b/instrumentation/opentelemetry-instrumentation-asgi/test-requirements.txt
@@ -1,4 +1,4 @@
-asgiref==3.7.2
+asgiref==3.8.1
 Deprecated==1.2.14
 importlib-metadata==6.11.0
 iniconfig==2.0.0
diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_custom_headers.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_custom_headers.py
index 5394d62f..49af8d42 100644
--- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_custom_headers.py
+++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_custom_headers.py
@@ -89,8 +89,8 @@ async def websocket_app_with_custom_headers(scope, receive, send):
         if message.get("type") == "websocket.disconnect":
             break

-
-class TestCustomHeaders(AsgiTestBase, TestBase):
+from unittest import IsolatedAsyncioTestCase
+class TestCustomHeaders(AsgiTestBase, IsolatedAsyncioTestCase):
     constructor_params = {}
     __test__ = False

@@ -108,7 +108,7 @@ class TestCustomHeaders(AsgiTestBase, TestBase):
             **self.constructor_params,
         )

-    def test_http_custom_request_headers_in_span_attributes(self):
+    async def test_http_custom_request_headers_in_span_attributes(self):
         self.scope["headers"].extend(
             [
                 (b"custom-test-header-1", b"test-header-value-1"),
@@ -139,7 +139,7 @@ class TestCustomHeaders(AsgiTestBase, TestBase):
             if span.kind == SpanKind.SERVER:
                 self.assertSpanHasAttributes(span, expected)

-    def test_http_repeat_request_headers_in_span_attributes(self):
+    async def test_http_repeat_request_headers_in_span_attributes(self):
         self.scope["headers"].extend(
             [
                 (b"custom-test-header-1", b"test-header-value-1"),
@@ -147,8 +147,8 @@ class TestCustomHeaders(AsgiTestBase, TestBase):
             ]
         )
         self.seed_app(self.app)
-        self.send_default_request()
-        self.get_all_output()
+        await self.send_default_request()
+        await self.get_all_output()
         span_list = self.exporter.get_finished_spans()
         expected = {
             "http.request.header.custom_test_header_1": (
@@ -159,7 +159,7 @@ class TestCustomHeaders(AsgiTestBase, TestBase):
         span = next(span for span in span_list if span.kind == SpanKind.SERVER)
         self.assertSpanHasAttributes(span, expected)

-    def test_http_custom_request_headers_not_in_span_attributes(self):
+    async def test_http_custom_request_headers_not_in_span_attributes(self):
         self.scope["headers"].extend(
             [
                 (b"custom-test-header-1", b"test-header-value-1"),

and

diff --git a/tests/opentelemetry-test-utils/src/opentelemetry/test/asgitestutil.py b/tests/opentelemetry-test-utils/src/opentelemetry/test/asgitestutil.py
index 05be4e02..3366a91d 100644
--- a/tests/opentelemetry-test-utils/src/opentelemetry/test/asgitestutil.py
+++ b/tests/opentelemetry-test-utils/src/opentelemetry/test/asgitestutil.py
@@ -52,25 +52,21 @@ class AsgiTestBase(TestBase):
     def seed_app(self, app):
         self.communicator = ApplicationCommunicator(app, self.scope)

-    def send_input(self, message):
-        asyncio.get_event_loop().run_until_complete(
-            self.communicator.send_input(message)
-        )
+    async def send_input(self, message):
+        await self.communicator.send_input(message)

-    def send_default_request(self):
-        self.send_input({"type": "http.request", "body": b""})
+    async def send_default_request(self):
+        await self.send_input({"type": "http.request", "body": b""})

-    def get_output(self):
-        output = asyncio.get_event_loop().run_until_complete(
-            self.communicator.receive_output(0)
-        )
+    async def get_output(self):
+        output = await self.communicator.receive_output(0)
         return output

-    def get_all_output(self):
+    async def get_all_output(self):
         outputs = []
         while True:
             try:
-                outputs.append(self.get_output())
+                outputs.append(await self.get_output())
             except asyncio.TimeoutError:
                 break
         return outputs

makes tox -e py310-test-instrumentation-asgi -- -k test_http_repeat_request_headers_in_span_attributes pass

I guess that at least AsgiTestBase should inherit from IsolatedAsyncioTestCase

drupol commented 1 month ago

Before:

python3.12-opentelemetry-instrumentation-asgi> tests/test_asgi_custom_headers.py::TestCustomHeadersEnv::test_http_custom_request_headers_in_span_attributes FAILED [  1%]
python3.12-opentelemetry-instrumentation-asgi> tests/test_asgi_custom_headers.py::TestCustomHeadersEnv::test_http_custom_request_headers_not_in_span_attributes FAILED [  2%]

After applying the two patches:

python3.12-opentelemetry-instrumentation-asgi> tests/test_asgi_custom_headers.py::TestCustomHeadersEnv::test_http_custom_request_headers_in_span_attributes PASSED [  1%]
python3.12-opentelemetry-instrumentation-asgi> tests/test_asgi_custom_headers.py::TestCustomHeadersEnv::test_http_custom_request_headers_not_in_span_attributes PASSED [  2%]

Then the rest of the tests were broken.

xrmx commented 1 month ago

Good news is that the updated code works for both asgiref 3.7.2 and 3.8.1