search

open-telemetry / opentelemetry-python-contrib

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

Test failures in opentelemetry-instrumentation-aiohttp-client with latest yarl, aiohttp #2917

Open musicinmybrain opened 1 month ago

musicinmybrain commented 1 month ago

Describe your environment

OS: Fedora 42/Rawhide (development version) Python version: 3.13.0 (or 3.12.6) Package version: 1.27.0 (or current main, e4ece57a8136310ed379c04fa77c6b6f8d009bbc)

What happened?

I know that you currently have yarl pinned to yarl==1.9.4 and aiohttp pinned to aiohttp==3.10.2 for testing, but in Fedora we use the system versions of all dependencies.

This is a follow-up to https://github.com/open-telemetry/opentelemetry-python-contrib/issues/2916, which I closed after I discovered that the original regression from yarl 1.13.0 was fixed by updating aiohttp to 3.10.7 or later.

Steps to Reproduce

To reproduce this in a virtualenv, I ran the tests manually, which is a bit messy, but the below steps work. I had to unpin zipp to fix a dependency conflict. I started with a “control” run:

$ gh repo clone open-telemetry/opentelemetry-python
$ gh repo clone open-telemetry/opentelemetry-python-contrib
$ cd opentelemetry-python-contrib
$ python3.12 -m venv _e
$ . _e/bin/activate
(_e) $ pip install ../opentelemetry-python/opentelemetry-api/
(_e) $ pip install ../opentelemetry-python/opentelemetry-semantic-conventions/
(_e) $ pip install ./opentelemetry-instrumentation/
(_e) $ pip install ./util/opentelemetry-util-http/
(_e) $ pip install ../opentelemetry-python/opentelemetry-sdk/
(_e) $ pip install ../opentelemetry-python/tests/opentelemetry-test-utils/
(_e) $ cd ./instrumentation/opentelemetry-instrumentation-aiohttp-client/
(_e) $ pip install -e .
(_e) $ pip install -r <(sed -r -e '/^-e /d' -e 's/(zipp)==/\1>=/' test-requirements.txt)
(_e) $ python -m pytest

(All tests passed for both packages.)

Now, updating yarl and aiohttp:

(_e) $ pip install --upgrade yarl aiohttp
[…]
Successfully installed aiohttp-3.10.10 propcache-0.2.0 yarl-1.15.5
(_e) $ python -m pytest

Expected Result

= 33 passed, 4 warnings in 5.51s =

Actual Result

============================================================================================= test session starts ==============================================================================================
platform linux -- Python 3.12.6, pytest-7.4.4, pluggy-1.5.0 -- /home/ben/src/forks/opentelemetry-python-contrib/_e/bin/python
cachedir: .pytest_cache
rootdir: /home/ben/src/forks/opentelemetry-python-contrib
configfile: pytest.ini
plugins: asyncio-0.23.5, aiohttp-1.0.5
asyncio: mode=Mode.STRICT
collected 33 items                                                                                                                                                                                             

tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_basic_exception
------------------------------------------------------------------------------------------------ live log call -------------------------------------------------------------------------------------------------
ERROR    aiohttp.server:web_protocol.py:448 Missing return statement on request handler
Traceback (most recent call last):
  File "/home/ben/src/forks/opentelemetry-python-contrib/_e/lib64/python3.12/site-packages/aiohttp/web_protocol.py", line 653, in finish_response
    prepare_meth = resp.prepare
                   ^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'prepare'
FAILED                                                                                                                                                                                                   [  3%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_basic_exception_both_semconv
------------------------------------------------------------------------------------------------ live log call -------------------------------------------------------------------------------------------------
ERROR    aiohttp.server:web_protocol.py:448 Missing return statement on request handler
Traceback (most recent call last):
  File "/home/ben/src/forks/opentelemetry-python-contrib/_e/lib64/python3.12/site-packages/aiohttp/web_protocol.py", line 653, in finish_response
    prepare_meth = resp.prepare
                   ^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'prepare'
FAILED                                                                                                                                                                                                   [  6%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_basic_exception_new_semconv
------------------------------------------------------------------------------------------------ live log call -------------------------------------------------------------------------------------------------
ERROR    aiohttp.server:web_protocol.py:448 Missing return statement on request handler
Traceback (most recent call last):
  File "/home/ben/src/forks/opentelemetry-python-contrib/_e/lib64/python3.12/site-packages/aiohttp/web_protocol.py", line 653, in finish_response
    prepare_meth = resp.prepare
                   ^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'prepare'
FAILED                                                                                                                                                                                                   [  9%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_connection_errors FAILED                                                                                                          [ 12%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_credential_removal PASSED                                                                                                         [ 15%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_hooks PASSED                                                                                                                      [ 18%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_nonstandard_http_method PASSED                                                                                                    [ 21%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_nonstandard_http_method_new_semconv PASSED                                                                                        [ 24%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_not_recording PASSED                                                                                                              [ 27%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_schema_url PASSED                                                                                                                 [ 30%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_schema_url_both_semconv PASSED                                                                                                    [ 33%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_schema_url_new_semconv PASSED                                                                                                     [ 36%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_status_codes PASSED                                                                                                               [ 39%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_status_codes_both_semconv PASSED                                                                                                  [ 42%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_status_codes_new_semconv PASSED                                                                                                   [ 45%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_timeout PASSED                                                                                                                    [ 48%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_too_many_redirects PASSED                                                                                                         [ 51%]
tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_url_filter_option PASSED                                                                                                          [ 54%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_every_request_by_new_session_creates_one_span PASSED                                                                       [ 57%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_hooks PASSED                                                                                                               [ 60%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_instrument PASSED                                                                                                          [ 63%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_instrument_both_semconv PASSED                                                                                             [ 66%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_instrument_new_semconv PASSED                                                                                              [ 69%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_instrument_with_custom_trace_config PASSED                                                                                 [ 72%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_instrument_with_existing_trace_config PASSED                                                                               [ 75%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_no_op_tracer_provider PASSED                                                                                               [ 78%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_suppress_instrumentation PASSED                                                                                            [ 81%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_suppress_instrumentation_after_creation PASSED                                                                             [ 84%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_suppress_instrumentation_with_server_exception PASSED                                                                      [ 87%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_uninstrument PASSED                                                                                                        [ 90%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_uninstrument_session PASSED                                                                                                [ 93%]
tests/test_aiohttp_client_integration.py::TestAioHttpClientInstrumentor::test_url_filter PASSED                                                                                                          [ 96%]
tests/test_aiohttp_client_integration.py::TestLoadingAioHttpInstrumentor::test_loading_instrumentor PASSED                                                                                               [100%]

=================================================================================================== FAILURES ===================================================================================================
_________________________________________________________________________________ TestAioHttpIntegration.test_basic_exception __________________________________________________________________________________

self = <tests.test_aiohttp_client_integration.TestAioHttpIntegration testMethod=test_basic_exception>

    def test_basic_exception(self):
        async def request_handler(request):
            assert "traceparent" in request.headers

        host, port = self._http_request(
            trace_config=aiohttp_client.create_trace_config(),
            url="/test",
            request_handler=request_handler,
        )
        span = self.memory_exporter.get_finished_spans()[0]
>       self.assertEqual(len(span.events), 1)
E       AssertionError: 0 != 1

tests/test_aiohttp_client_integration.py:381: AssertionError
---------------------------------------------------------------------------------------------- Captured log call -----------------------------------------------------------------------------------------------
ERROR    aiohttp.server:web_protocol.py:448 Missing return statement on request handler
Traceback (most recent call last):
  File "/home/ben/src/forks/opentelemetry-python-contrib/_e/lib64/python3.12/site-packages/aiohttp/web_protocol.py", line 653, in finish_response
    prepare_meth = resp.prepare
                   ^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'prepare'
___________________________________________________________________________ TestAioHttpIntegration.test_basic_exception_both_semconv ___________________________________________________________________________

self = <tests.test_aiohttp_client_integration.TestAioHttpIntegration testMethod=test_basic_exception_both_semconv>

    def test_basic_exception_both_semconv(self):
        async def request_handler(request):
            assert "traceparent" in request.headers

        host, port = self._http_request(
            trace_config=aiohttp_client.create_trace_config(
                sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP_DUP
            ),
            url="/test",
            request_handler=request_handler,
        )
        span = self.memory_exporter.get_finished_spans()[0]
>       self.assertEqual(len(span.events), 1)
E       AssertionError: 0 != 1

tests/test_aiohttp_client_integration.py:436: AssertionError
---------------------------------------------------------------------------------------------- Captured log call -----------------------------------------------------------------------------------------------
ERROR    aiohttp.server:web_protocol.py:448 Missing return statement on request handler
Traceback (most recent call last):
  File "/home/ben/src/forks/opentelemetry-python-contrib/_e/lib64/python3.12/site-packages/aiohttp/web_protocol.py", line 653, in finish_response
    prepare_meth = resp.prepare
                   ^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'prepare'
___________________________________________________________________________ TestAioHttpIntegration.test_basic_exception_new_semconv ____________________________________________________________________________

self = <tests.test_aiohttp_client_integration.TestAioHttpIntegration testMethod=test_basic_exception_new_semconv>

    def test_basic_exception_new_semconv(self):
        async def request_handler(request):
            assert "traceparent" in request.headers

        host, port = self._http_request(
            trace_config=aiohttp_client.create_trace_config(
                sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP
            ),
            url="/test",
            request_handler=request_handler,
        )
        span = self.memory_exporter.get_finished_spans()[0]
>       self.assertEqual(len(span.events), 1)
E       AssertionError: 0 != 1

tests/test_aiohttp_client_integration.py:408: AssertionError
---------------------------------------------------------------------------------------------- Captured log call -----------------------------------------------------------------------------------------------
ERROR    aiohttp.server:web_protocol.py:448 Missing return statement on request handler
Traceback (most recent call last):
  File "/home/ben/src/forks/opentelemetry-python-contrib/_e/lib64/python3.12/site-packages/aiohttp/web_protocol.py", line 653, in finish_response
    prepare_meth = resp.prepare
                   ^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'prepare'
________________________________________________________________________________ TestAioHttpIntegration.test_connection_errors _________________________________________________________________________________

self = <tests.test_aiohttp_client_integration.TestAioHttpIntegration testMethod=test_connection_errors>

    def test_connection_errors(self):
        trace_configs = [aiohttp_client.create_trace_config()]

        for url, expected_status in (
            ("http://this-is-unknown.local/", StatusCode.ERROR),
            ("http://127.0.0.1:1/", StatusCode.ERROR),
        ):
            with self.subTest(expected_status=expected_status):

                async def do_request(url):
                    async with aiohttp.ClientSession(
                        trace_configs=trace_configs,
                    ) as session:
                        async with session.get(url):
                            pass

                loop = asyncio.get_event_loop()
                with self.assertRaises(aiohttp.ClientConnectorError):
                    loop.run_until_complete(do_request(url))

>           self.assert_spans(
                [
                    (
                        "GET",
                        (expected_status, "ClientConnectorError"),
                        {
                            SpanAttributes.HTTP_METHOD: "GET",
                            SpanAttributes.HTTP_URL: url,
                        },
                    )
                ]
            )

tests/test_aiohttp_client_integration.py:357:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_aiohttp_client_integration.py:91: in assert_spans
    self.assertEqual(
E   AssertionError: Lists differ: [('GE[39 chars]ectorDNSError'), {'http.method': 'GET', 'http.[35 chars]/'})] != [('GE[39 chars]ectorError'), {'http.method': 'GET', 'http.url[32 chars]/'})]
E   
E   First differing element 0:
E   ('GET[38 chars]ectorDNSError'), {'http.method': 'GET', 'http.[34 chars]l/'})
E   ('GET[38 chars]ectorError'), {'http.method': 'GET', 'http.url[31 chars]l/'})
E   
E     [('GET',
E   -   (<StatusCode.ERROR: 2>, 'ClientConnectorDNSError'),
E   ?                                           ---
E   
E   +   (<StatusCode.ERROR: 2>, 'ClientConnectorError'),
E       {'http.method': 'GET', 'http.url': 'http://this-is-unknown.local/'})]
=============================================================================================== warnings summary ===============================================================================================
instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_basic_exception
  /home/ben/src/forks/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py:69: DeprecationWarning: There is no current event loop
    loop = asyncio.get_event_loop()

instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_schema_url
  /home/ben/src/forks/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py:208: DeprecationWarning: Call to deprecated function (or staticmethod) instrumentation_info. (You should use instrumentation_scope) -- Deprecated since version 1.11.1.
    span.instrumentation_info.schema_url,

instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_schema_url_both_semconv
  /home/ben/src/forks/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py:242: DeprecationWarning: Call to deprecated function (or staticmethod) instrumentation_info. (You should use instrumentation_scope) -- Deprecated since version 1.11.1.
    span.instrumentation_info.schema_url,

instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py::TestAioHttpIntegration::test_schema_url_new_semconv
  /home/ben/src/forks/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py:225: DeprecationWarning: Call to deprecated function (or staticmethod) instrumentation_info. (You should use instrumentation_scope) -- Deprecated since version 1.11.1.
    span.instrumentation_info.schema_url,

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=================================================================================== 4 failed, 29 passed, 4 warnings in 5.47s ===================================================================================

Additional context

I was able to pin a similar issue in aioflo, https://github.com/bachya/aioflo/issues/67, down to aiohttp 3.10.6, so I suspect that may be where the relevant behavior change occurred for this package as well. See https://github.com/aio-libs/aiohttp/blob/v3.10.6/CHANGES.rst#3106-2024-09-24.

The tests for opentelemetry-instrumentation-aiohttp-server succeed with the latest yarl and aiohttp.

Would you like to implement a fix?

None

xrmx commented 1 month ago

@musicinmybrain thanks for reporting, fyi if you install tox you can run tox -e py312-test-instrumentation-aiohttp-client instead of installing each dependency manually