Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asgiref 3.8 compatibility? #2776

Closed
drupol opened this issue Aug 4, 2024 · 4 comments · Fixed by #2778
Closed

asgiref 3.8 compatibility? #2776

drupol opened this issue Aug 4, 2024 · 4 comments · Fixed by #2778
Labels
bug Something isn't working

Comments

@drupol
Copy link

drupol commented Aug 4, 2024

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: <function ApplicationCommunicator.__del__ at 0x7ffff5c59760>
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: <function ApplicationCommunicator.__del__ at 0x7ffff5c59760>
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: <function ApplicationCommunicator.__del__ at 0x7ffff5c59760>
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
Copy link
Contributor

xrmx commented Aug 5, 2024

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

@xrmx
Copy link
Contributor

xrmx commented Aug 5, 2024

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
Copy link
Author

drupol commented Aug 5, 2024

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
Copy link
Contributor

xrmx commented Aug 5, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants