Skip to content

Commit

Permalink
Fix B3 propagator and add tests (#882)
Browse files Browse the repository at this point in the history
* Correct B3 propagators and add tests

* Break up external integration and internal unit tests

* Add changes to Changelog.

* Update Changelog with PR number

* Fix lint issues

* Update trace flags

Add a new "not sampled" mask to complement the existing "sampled" one.

Rename `FlagsUnused` to `FlagsUnset`.

Add documentation for each of the flags to help understand their
purpose.

* Update extractSingle to support unset sampling

* Update existing tests to appropriately use FlagsUnset

* Remove bogus debug flag test

The B3 specification states "Debug is encoded as `X-B3-Flags: 1`. Absent
or any other values can be ignored", so testing of other values should
not result in an error.

* B3 Extract now supports parsing both headers

Remove test cases that would fail if the fallback header format was
expected to not be used.

* Feedback

* Switch to bitmask inject encoding field

Add the B3Encoding and valid HTTP based values. Change the B3 propagator
to use these bitmask fields to specify the inject encoding it will
propagate.

* Add comments

* Migrate B3 integration tests to existing testtrace

* Update comment

* Benchmark invalid B3 injects as well

* Update trace flags

Add a FlagsDebug and FlagsDeferred to track the B3 trace state.

Add helper methods to the SpanContext to check the debug and deferred
bit of the trace flags.

Update SpanContext.IsSampled to return if the sampling decision is to
sample rather than if the sample bit is set. This means that if the
debug bit is also set it will return true.

* Revert SpanContext.IsSampled back

* Add comment to b3 test data generation

* Update Changelog

* Fix trace flag name in Changelog

* Fix Changelog formatting

* Update Changelog

* Remove valid check at start of B3 injectg

This check makes sample only headers not propagate.

* Update B3 inject integration tests

Use the passed SpanContext and check directly the span ID.

* Update B3 integration tests

Run update checked SpanID to match sent.

Add tests to validate sample only transmissions and debug flag support.

* Rename injectTest parentSc to sc

This is no longer the parent.

* Update GetAllKeys for B3

* Un-Export the B3 headers

The B3SingleHeader name will conflict with the upcoming change to prefix
the SingleHeader encoding with "B3". There are a few options to address
this conflict, but in the end we do not need to be exporting these
values. They are duplicates of the OpenZipkin package and users should
use those.

* Rename B3 encodings and move support method to B3Encoding

Include a `B3` prefix to scope the encoding names.

Move the related support method to the B3Encoding itself, instead of the
B3 propagator.

Add tests to provide a sanity check for encoding bitmasks.

* Update span_context_test tests

Update test name to better describe how unused bits have no affect on
the sampling decision. Include the inverse of this test as well: not
sampled but has unused bits.

* Use named const for Single Header decoding widths

* Update api/trace/b3_propagator.go

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
  • Loading branch information
MrAlias and Aneurysm9 committed Jul 7, 2020
1 parent 3475d55 commit c506e99
Show file tree
Hide file tree
Showing 11 changed files with 1,431 additions and 488 deletions.
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Added

- The `B3Encoding` type to represent the B3 encoding(s) the B3 propagator can inject.
A value for HTTP supported encodings (Multiple Header: `MultipleHeader`, Single Header: `SingleHeader`) are included. (#882)
- The `FlagsDeferred` trace flag to indicate if the trace sampling decision has been deferred. (#882)
- The `FlagsDebug` trace flag to indicate if the trace is a debug trace. (#882)
- Add `peer.service` semantic attribute. (#898)
- Add database-specific semantic attributes. (#899)
- Add semantic convention for `faas.coldstart` and `container.id`. (#909)
Expand All @@ -18,10 +22,28 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- Update `CONTRIBUTING.md` to ask for updates to `CHANGELOG.md` with each pull request. (#879)
- Use lowercase header names for B3 Multiple Headers. (#881)
- The B3 propagator `SingleHeader` field has been replaced with `InjectEncoding`.
This new field can be set to combinations of the `B3Encoding` bitmasks and will inject trace information in these encodings.
If no encoding is set, the propagator will default to `MultipleHeader` encoding. (#882)
- The B3 propagator now extracts from either HTTP encoding of B3 (Single Header or Multiple Header) based on what is contained in the header.
Preference is given to Single Header encoding with Multiple Header being the fallback if Single Header is not found or is invalid.
This behavior change is made to dynamically support all correctly encoded traces received instead of having to guess the expected encoding prior to receiving. (#882)

### Removed

- The `FlagsUnused` trace flag is removed.
The purpose of this flag was to act as the inverse of `FlagsSampled`, the inverse of `FlagsSampled` is used instead. (#882)
- The B3 header constants (`B3SingleHeader`, `B3DebugFlagHeader`, `B3TraceIDHeader`, `B3SpanIDHeader`, `B3SampledHeader`, `B3ParentSpanIDHeader`) are removed.
If B3 header keys are needed [the authoritative OpenZipkin package constants](https://pkg.go.dev/github.com/openzipkin/zipkin-go@v0.2.2/propagation/b3?tab=doc#pkg-constants) should be used instead. (#882)

### Fixed

- The B3 Single Header name is now correctly `b3` instead of the previous `X-B3`. (#881)
- The B3 propagator now correctly supports sampling only values (`b3: 0`, `b3: 1`, or `b3: d`) for a Single B3 Header. (#882)
- The B3 propagator now propagates the debug flag.
This removes the behavior of changing the debug flag into a set sampling bit.
Instead, this now follow the B3 specification and omits the `X-B3-Sampling` header. (#882)
- The B3 propagator now tracks "unset" sampling state (meaning "defer the decision") and does not set the `X-B3-Sampling` header when injecting. (#882)
- Ensure span status is not set to `Unknown` when no HTTP status code is provided as it is assumed to be `200 OK`. (#908)
- Ensure `httptrace.clientTracer` closes `http.headers` span. (#912)
- Prometheus exporter will not apply stale updates or forget inactive metrics. (#903)
Expand Down
Loading

0 comments on commit c506e99

Please sign in to comment.