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

Use SingleHeader format for unspecified B3 inject encoding #614

Merged
merged 3 commits into from
Mar 18, 2021

Conversation

gunturaf
Copy link
Contributor

Which problem is this PR solving?

This PR changes default behavior when B3 inject encoding is unspecified from multi-header to single-header, per #613.

Short description of the changes

  • Use single-header format when B3 inject encoding is unspecified
  • Modify test data for cases involving unspecified B3 inject encoding

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 13, 2021

CLA Signed

The committers are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Mar 13, 2021

Codecov Report

Merging #614 (d2ddc0f) into main (7a8e6f5) will increase coverage by 0.0%.
The diff coverage is 100.0%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #614   +/-   ##
=====================================
  Coverage   78.0%   78.1%           
=====================================
  Files         57      57           
  Lines       2664    2664           
=====================================
+ Hits        2080    2082    +2     
+ Misses       452     451    -1     
+ Partials     132     131    -1     
Impacted Files Coverage Δ
propagators/b3/b3_propagator.go 98.6% <100.0%> (ø)
...n/github.com/Shopify/sarama/otelsarama/producer.go 94.8% <0.0%> (+2.0%) ⬆️

propagators/b3/b3_data_test.go Show resolved Hide resolved
propagators/b3/b3_data_test.go Show resolved Hide resolved
propagators/b3/b3_data_test.go Show resolved Hide resolved
propagators/b3/b3_data_test.go Show resolved Hide resolved
propagators/b3/b3_data_test.go Show resolved Hide resolved
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@gunturaf gunturaf requested a review from MrAlias March 17, 2021 02:05
@Aneurysm9 Aneurysm9 merged commit ef90356 into open-telemetry:main Mar 18, 2021
@MrAlias MrAlias linked an issue Mar 18, 2021 that may be closed by this pull request
MrAlias added a commit that referenced this pull request Mar 22, 2021
Missing documentation change from #614.
plantfansam referenced this pull request in plantfansam/opentelemetry-go-contrib Mar 18, 2022
* Update OTLP SpanData transform

The ParentSpanId needs to be empty for root spans according to the OTLP
[docs](https://github.com/open-telemetry/opentelemetry-proto/blob/6c2a86ed2f74ca46f979f22c77f0aad449fb9629/gen/go/trace/v1/trace.pb.go#L284-L286).
This updates the SpanData transform function to not add the ParentSpanID
if it is not a valid span ID (which includes if it is the nil span ID
used for an unset ID).

Additionally, this adds a test to prevent regression.

* Simplify test to just check parent span ID transform

Co-authored-by: Rahul Patel <rahulpa@google.com>
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.

Default to SingleHeader format when unspecified B3 encoding
3 participants