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

Wire W3C propagator to HTTP & gRPC propagation #2458

Merged
merged 1 commit into from
Dec 5, 2022
Merged

Wire W3C propagator to HTTP & gRPC propagation #2458

merged 1 commit into from
Dec 5, 2022

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Dec 2, 2022

This PR adds support for the newly introduced W3C Trace Context propagator.

This means that it is now possible to use the propagation style tracecontext, introduced in #2454, as a valid propagation style.

@marcotc marcotc requested a review from a team December 2, 2022 23:31
@github-actions github-actions bot added integrations Involves tracing integrations tracing labels Dec 2, 2022
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

This seems reasonable, although I haven't been following the overall change closely.

I grepped the codebase, and noticed that we usually also have the PROPAGATION_STYLE_... constants in settings.rb (+ settings_spec.rb) + all propagation styles in spec/datadog/tracing/distributed/propagation_spec.rb.

Do check if it makes sense to change those as well.

Otherwise, it LGTM 👍

@marcotc
Copy link
Member Author

marcotc commented Dec 5, 2022

@ivoanjo thanks for bringing out these two points.
The PR as it stands is correct settings.rb & settings_spec.rb would only change if W3C propagation because a default value, which is not the case yet.

And even though propagation_spec.rb currently has all other propagation styles, the testing there is only relevant for Datadog vs non-Datadog propagation styles, so there's no reason to introduce W3C there despite B3 being that file.

@marcotc marcotc merged commit ed95d17 into master Dec 5, 2022
@marcotc marcotc deleted the w3c-wire branch December 5, 2022 23:36
@github-actions github-actions bot added this to the 1.8.0 milestone Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants