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

fix: ActiveJob Propagate baggage information properly when performing #1214

Merged

Conversation

yoheyk
Copy link
Contributor

@yoheyk yoheyk commented Oct 24, 2024

After upgrading ActiveJob instrumentation to 0.7.2, we have noticed that baggage information is not propagating properly via ActiveJob.

Looks like the deletion of this line is causing the issue (We are using the child propagation style):
https://github.com/open-telemetry/opentelemetry-ruby-contrib/pull/1018/files#diff-bc60a254cc5ccc8843a524d0eb4c8[…]218817496946fa815a22a39a010b5ddL39

Passing the extracted context as the parent for the consumer context seems to resolve this issue.

@@ -225,6 +210,21 @@
_(process_span.links[0].span_context.trace_id).must_equal(publish_span.trace_id)
_(process_span.links[0].span_context.span_id).must_equal(publish_span.span_id)
end

it 'propagates baggage' do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test case needs to be run with an async queue adapter

@arielvalentin
Copy link
Collaborator

@yoheyk perhaps the better thing to do here is revert #1018

@yoheyk
Copy link
Contributor Author

yoheyk commented Oct 24, 2024

we need to do more than just reverting #1018
it makes the baggage propagation possible for the child propagation style but not for link

@arielvalentin arielvalentin merged commit 5b1c09d into open-telemetry:main Oct 24, 2024
57 checks passed
@github-actions github-actions bot mentioned this pull request Oct 24, 2024
@yoheyk yoheyk deleted the active-job-fix-baggage-propagation branch October 25, 2024 05:13
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

Successfully merging this pull request may close these issues.

2 participants