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

Baggage not propagated for consumers who use ruby_kafka instrumentation #549

Closed
Asafb26 opened this issue Jan 12, 2021 · 3 comments
Closed

Comments

@Asafb26
Copy link
Contributor

Asafb26 commented Jan 12, 2021

currently, there is a different logic between:

parent_context = OpenTelemetry.propagation.text.extract({traceparent: ..., baggage: my_baggage})
tracer.in_span("test", with_parent: parent_context, attributes: attributes, kind: :consumer) do
  puts OpenTelemetry.baggage.values
end

#prints nil

and

parent_context = OpenTelemetry.propagation.text.extract({traceparent: ..., baggage: my_baggage})
OpenTelemetry::Context.with_current(parent_context) do
  tracer.in_span("test", attributes: attributes, kind: :consumer) do
    puts OpenTelemetry.baggage.values
  end
end

#prints the content of my_baggage

The first keeps the current context active while use the parent span_context data, while the second replaces the current context, yet use the parent span_context data (from the replaced context).

The issue starts with the fact that baggage is not a part of span_context, so for proper propagation of baggage data - we must use the second approach.

@fbogsany
Copy link
Contributor

This difference seems really unfortunate. Are we correctly implementing the spec here in tracer.in_span(..., with_parent: ...)? If not, we should fix that. If we are, I wonder if this difference in behaviour should be raised as a spec issue?

@Asafb26
Copy link
Contributor Author

Asafb26 commented Jan 12, 2021

AFAIK most instrumentation uses the second approach, so in my perspective - if it's not an issue, I can align the implementation of ruby_kafka with the rest.
regarding the specs - the baggage is not a part of the span_context, which I found it bit weird, but yet confirms that we are aligned with the specs

@fbogsany
Copy link
Contributor

Fixed in #550

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

No branches or pull requests

2 participants