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

Support aio-pika 9.1 #1835

Closed
phillipuniverse opened this issue Jun 4, 2023 · 11 comments
Closed

Support aio-pika 9.1 #1835

phillipuniverse opened this issue Jun 4, 2023 · 11 comments

Comments

@phillipuniverse
Copy link
Contributor

phillipuniverse commented Jun 4, 2023

Is your feature request related to a problem?

The aio-pika instrumentation is currently broken for publish and consume spans in aio-pika 9.1+. This is due to a refactoring of connection properties on a channel in the 9.1.0 release.

Here's the exception:

self = <opentelemetry.instrumentation.aio_pika.span_builder.SpanBuilder object at 0x10fe952d0>
channel = <aio_pika.robust_channel.RobustChannel object at 0x10fed1900>

    def set_channel(self, channel: AbstractChannel):
>       connection = channel.connection
E       AttributeError: 'RobustChannel' object has no attribute 'connection'. Did you mean: '_connection'?

Describe the solution you'd like

Otel working with aio-pika 9.1

Describe alternatives you've considered

N/A

Additional context

I asked for clarification on the aio-pika maintainer's opinion of the correct fix at mosquito/aio-pika#533 (comment)

@phillipuniverse
Copy link
Contributor Author

phillipuniverse commented Jun 4, 2023

In the meantime, here is a fix that will work to override the method.

First create this patch_spanbuilder_set_channel() function

from aio_pika.abc import AbstractChannel
from opentelemetry.semconv.trace import SpanAttributes


def patch_spanbuilder_set_channel() -> None:
    """
    The default SpanBuilder.set_channel does not work with aio_pika 9.1 and the refactored connection
    attribute
    """
    import opentelemetry.instrumentation.aio_pika.span_builder
    from opentelemetry.instrumentation.aio_pika.span_builder import SpanBuilder

    def set_channel(self: SpanBuilder, channel: AbstractChannel) -> None:
        if hasattr(channel, "_connection"):
            url = channel._connection.url
            self._attributes.update(
                {
                    SpanAttributes.NET_PEER_NAME: url.host,
                    SpanAttributes.NET_PEER_PORT: url.port,
                }
            )

    opentelemetry.instrumentation.aio_pika.span_builder.SpanBuilder.set_channel = set_channel  # type: ignore[misc]

Add an ENTRYPOINT in your project to pre_instrument. In Poetry it looks like this in pyproject.toml:

[tool.poetry.plugins."opentelemetry_pre_instrument"]
aio_pika = "package.path.to.patch_spanbuilder_set_channel"

@LockedThread
Copy link

Any ETA on this?

@ericstengard
Copy link

ericstengard commented Jan 24, 2024

I'm also interested in this being resolved. Would it be appropriate to bump the comment at mosquito/aio-pika#533 (comment) since it seems that we're close to a resolution but have yet to get a reply from the aio-pika maintainers.

@doctorpangloss
Copy link

In the meantime, here is a fix that will work to override the method.

First create this patch_spanbuilder_set_channel() function

from aio_pika.abc import AbstractChannel
from opentelemetry.semconv.trace import SpanAttributes


def patch_spanbuilder_set_channel() -> None:
    """
    The default SpanBuilder.set_channel does not work with aio_pika 9.1 and the refactored connection
    attribute
    """
    import opentelemetry.instrumentation.aio_pika.span_builder
    from opentelemetry.instrumentation.aio_pika.span_builder import SpanBuilder

    def set_channel(self: SpanBuilder, channel: AbstractChannel) -> None:
        if hasattr(channel, "_connection"):
            url = channel._connection.url
            self._attributes.update(
                {
                    SpanAttributes.NET_PEER_NAME: url.host,
                    SpanAttributes.NET_PEER_PORT: url.port,
                }
            )

    opentelemetry.instrumentation.aio_pika.span_builder.SpanBuilder.set_channel = set_channel  # type: ignore[misc]

Add an ENTRYPOINT in your project to pre_instrument. In Poetry it looks like this in pyproject.toml:

[tool.poetry.plugins."opentelemetry_pre_instrument"]
aio_pika = "package.path.to.patch_spanbuilder_set_channel"

This should be:

from aio_pika.abc import AbstractChannel
from opentelemetry.semconv.trace import SpanAttributes


def patch_spanbuilder_set_channel() -> None:
    """
    The default SpanBuilder.set_channel does not work with aio_pika 9.1 and the refactored connection
    attribute
    """
    import opentelemetry.instrumentation.aio_pika.span_builder
    from opentelemetry.instrumentation.aio_pika.span_builder import SpanBuilder

    def set_channel(self: SpanBuilder, channel: AbstractChannel) -> None:
        if hasattr(channel, "_connection"):
            url = channel._connection.url
            port = url.port or 5672
            self._attributes.update(
                {
                    SpanAttributes.NET_PEER_NAME: url.host,
                    SpanAttributes.NET_PEER_PORT: port,
                }
            )

    opentelemetry.instrumentation.aio_pika.span_builder.SpanBuilder.set_channel = set_channel  # type: ignore[misc]

thank you for your valuable fix

the pyproject.toml change can be omitted for users who directly invoke instrumentation:

patch_spanbuilder_set_channel()
AioPikaInstrumentor().instrument()

is also sufficient

@RedLine89
Copy link

While there is the PR, somehow it still seems to be an issue with opentelemetry-instrumentation-aio-pika = "^0.45b0" and aio-pika = "^9.4.1"

@phillipuniverse
Copy link
Contributor Author

phillipuniverse commented May 23, 2024

@RedLine89 the fix is not yet apart of a release but will be in 0.46b0. I’m unsure of the release schedule.

@emil-vdw
Copy link

emil-vdw commented Aug 1, 2024

Should this issue not be closed now that #2450 has been merged?

@LockedThread
Copy link

Should this issue not be closed now that #2450 has been merged?

Yes, it should be.

@LockedThread
Copy link

@phillipuniverse Can you close this?

@gchadid
Copy link

gchadid commented Sep 2, 2024

hi, with the autoinstrumentation using the image ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-python:0.46b0 the problem persists

cbeddd6e-63f5-4fd0-8bc2-da81e1770231

@emdneto
Copy link
Member

emdneto commented Sep 3, 2024

@gchadid could you please open a new issue for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants