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

add b3multi propagator #1647

Merged
merged 2 commits into from
Nov 18, 2020
Merged

add b3multi propagator #1647

merged 2 commits into from
Nov 18, 2020

Conversation

malafeev
Copy link
Contributor

@malafeev malafeev commented Nov 16, 2020

from: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/context/api-propagators.md#b3-requirements:

MUST provide configuration to change the default injection format to B3 multi-header

Signed-off-by: Sergei Malafeev <sergei@malafeev.org>
@malafeev malafeev changed the title revert support for b3_single and b3 (multi) propagator add back support for b3_single and b3 (multi) propagator Nov 16, 2020
@malafeev malafeev changed the title add back support for b3_single and b3 (multi) propagator restore support for b3_single and b3 (multi) propagator Nov 16, 2020
Signed-off-by: Sergei Malafeev <sergei@malafeev.org>
@malafeev malafeev changed the title restore support for b3_single and b3 (multi) propagator add b3_multi propagator Nov 17, 2020
@malafeev malafeev changed the title add b3_multi propagator add b3multi propagator Nov 17, 2020
@trask
Copy link
Member

trask commented Nov 17, 2020

hey @malafeev, can you submit a PR to add b3multi to the spec?

Known values for OTEL_PROPAGATORS are: "tracecontext", "baggage", "b3", "jaeger".

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sdk-environment-variables.md#general-sdk-configuration

@malafeev
Copy link
Contributor Author

malafeev commented Nov 17, 2020

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

thanks @malafeev!

@iNikem
Copy link
Contributor

iNikem commented Nov 17, 2020

@trask do you want to wait for Spec PR to be merged? Or can we merge this right now?

@trask
Copy link
Member

trask commented Nov 17, 2020

let's give the spec PR 24-hours to see if any concerns are raised, if nothing surfaces then I'm good merging speculatively

@@ -157,7 +157,7 @@ outgoing requests using all the configured propagator formats.

| System property | Environment variable | Description |
|------------------|----------------------|-----------------------------------------------------------------------------------------------------------------|
| otel.propagators | OTEL_PROPAGATORS | Default is `tracecontext` (W3C). Other supported values are `b3`, `b3single`, `jaeger`, `ottracer`, and `xray`. |
| otel.propagators | OTEL_PROPAGATORS | Default is `tracecontext` (W3C). Other supported values are `b3`, `b3multi`, `jaeger`, `ottracer`, and `xray`. |
Copy link
Contributor

Choose a reason for hiding this comment

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

The behavior of the b3 propagators is a bit confusing so maybe needs some additional docs

`b3` and `b3multi` both extract from the same set of headers, x-b3 multi format and b3 single format. But `b3` injects as b3 single, and `b3multi` injects as x-b3 multi. If you set both `b3` and `b3multi` you can accept and inject all types of b3 headers.

@iNikem iNikem merged commit da9502c into open-telemetry:master Nov 18, 2020
iNikem pushed a commit to iNikem/opentelemetry-java-instrumentation that referenced this pull request Nov 18, 2020
* revert support for b3_single and b3 (multi) propagator

Signed-off-by: Sergei Malafeev <sergei@malafeev.org>

* add b3multi, default for b3 is single

Signed-off-by: Sergei Malafeev <sergei@malafeev.org>
(cherry picked from commit da9502c)
iNikem added a commit that referenced this pull request Nov 18, 2020
* Update publish.gradle (#1611)

Do not automatically publish to Bintray after upload

(cherry picked from commit 7f012c5)

* add b3multi propagator (#1647)

* revert support for b3_single and b3 (multi) propagator

Signed-off-by: Sergei Malafeev <sergei@malafeev.org>

* add b3multi, default for b3 is single

Signed-off-by: Sergei Malafeev <sergei@malafeev.org>
(cherry picked from commit da9502c)

Co-authored-by: Sergei Malafeev <sergei@malafeev.org>
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.

None yet

4 participants