amqp091-go
amqp091-go copied to clipboard
rabbitmq
Feature Proposal: W3C Context propagation integration via OpenTelemetry
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.
- I suggest to use headers as it is done for example here - https://github.com/knative-sandbox/eventing-rabbitmq/blob/main/cmd/ingress/main.go#L153 to carry context information.
- I suggest to have same header names across all officially supproted RabbitMQ clients.
- I argue that merely providing the header naming suggestion in the official docs will be a great step forward as it sets the ground for future development and solves compatibility issues immediately.
Feedback is much appreciated!
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/messaging.md
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).
Also see https://www.w3.org/TR/trace-context/ and https://w3c.github.io/trace-context-amqp/
with this addition https://github.com/rabbitmq/amqp091-go/pull/96 publishing interface becomes really nice - we don't have to use carriers/propagator concept for publishing and just pass our context.
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 tracestate and traceparent information from the context? And automagically add those values as message headers?
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:
- Obtain a span from the context (link) (it is a no-op span if context doesn't have a Span)
- Inject the Span attributes as message headers (see this and this)
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?
https://github.com/rabbitmq/amqp091-go/compare/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?
@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.
