-
Notifications
You must be signed in to change notification settings - Fork 140
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
Feature Proposal: W3C Context propagation integration via OpenTelemetry #43
Comments
I think it's fair to start with standardising a header name or two for this purpose. But I would like to know how that would look like across all the protocols we currently support (AMQP0.9.1, AMQP1.0, MQTT, STOMP and Stream). |
Related to #22 |
with this addition #96 publishing interface becomes really nice - we don't have to use carriers/propagator concept for publishing and just pass our context. |
Is the idea to extract the In this case, I think there should be an opt-in/opt-out mechanism. |
I don't think we need something special here. IIRC by default the propagator is no-op. |
I'm not following. If the propagator is a no-op, how are we going to achieve context propagation, as per the spec? |
by configuring OT SDK to use "proper" propagator :-) For example: https://github.com/open-telemetry/opentelemetry-go/blob/e6839571d26cfc6458050da275f79618fbfd4d9d/propagation.go#L30. It's up to our users to set configure. We going to use only API part. |
Sure, I'm not suggesting we use the SDK and configure a specific implementation. My question is whether we are simply going to mimic KNative behaviour (link from OP). In other words, what KNative does is:
As a first step, is our intention to do the same in this client? Are we going to introduce the headers suggested in this spec? |
main...ik-tracecontext-bump simplest way of doing context injection into message. On consume/get side the call to otel.GetTextMapPropagator().Extract(ctx, msg) will be needed. Key names, for headers, their format is up to propagator. |
I understand that the scope of this issue is simply to propagate the context, and any values set by the user, via message headers. We implement the TextMapPropagator in the publishing message, so our struct knows how to propagate. Is that right? Automatic instrumentation, like go-pg/pg is out of the scope of this issue. Correct? |
Hello, any updates here? |
@maxim-badarau-m10 this issue is still open, so no, it has not been implemented. There is no reason to ask if there are updates. If this is a critical feature, please contribute to this library or indicate that you have a RabbitMQ support agreement. |
This PR extracts opentelemetry utility functions from my private project and adds them to this project without calling them. It resolves rabbitmq#43 I'd like a broader discussion about whether these should be automatically called by the library where possible, or if they should simply be provided to clients to use if they so wish. I did my best to follow OpenTelemetry semantic conventions as described here https://opentelemetry.io/docs/specs/semconv/messaging/messaging-spans/, but they are at times ambiguous for rabbitmq-- e.g. is the destination for a message the Queue or the Consumer Tag the message was delivered to. Given the channel based approaches of this library, it is impossible for the library to know the full execution of a consumer. Unless autoack=false, we cannot actually know when to end the span associated with a delivery, so at least in the consumer case, it's probably best to allow the client to manage spans for themselves. We *can* manage spans on the producer side, and at the very least extract span identifiers to include on published headers automatically, and provide utilities for pulling them back out again. My intention with putting this PR up is to move the conversation forward. Because the PR *only* provides private methods (if I left members public please call them out), it can be safely merged while these questions are worked out.
I notice increasing demand/expectations from user for built-in integration with Distributed tracing.
One such example is Knative where Distributed tracing was integrated from the start.
Some time ago two major parties - OpenCensus and OpenTracing merged to OpenTelemetry and now Tracing API considered mature - https://github.com/open-telemetry/opentelemetry-go.
As a first step towards full Distributed tracing support I propose to integrate Tracing context propagation.
Feedback is much appreciated!
The text was updated successfully, but these errors were encountered: