propagation: extract of multiple header values #5973

jamesmoessis · 2024-11-14T02:42:52Z

Add ValuesGetter interface, an optional TextMapCarrier feature that adds Values(string) []string.
HeaderCarrier implements ValuesGetter.
Change Baggage to use the Values() method if it's implemented.

Adds tests extracting requests with multiple 'baggage' headers set.
Does not introduce any breaking changes or alter any existing tests.

Spec issue: open-telemetry/opentelemetry-specification#433
Corresponding Java prototype: open-telemetry/opentelemetry-java#6852

codecov · 2024-11-14T02:44:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.0%. Comparing base (0385f83) to head (1307ad4).
Report is 1 commits behind head on main.

Additional details and impacted files

@@     Coverage Diff      @@
##   main   #5973   +/-   ##
============================
============================

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

propagation/propagation.go

propagation/baggage.go

pellared

I think your prototype is good. For an actual PR we would need to improve the docs and call out the MultiTextMapCarrier in more places.

PS. Not feeling well today so treat my review with low confidence (like an pre-approval). I will do my best to look at it again once I recover and have a clearer mind.

pellared · 2024-11-18T07:27:24Z

@open-telemetry/go-maintainers, PTAL at this prototype

propagation/baggage.go

propagation/propagation.go

…ey (#4295) Fixes #433 Discussed in 11/5/24 Spec SIG, and prototyped in Java and Go. ## Changes Adds `GetAll` method to Getter, allowing for the retrieval of multiple keys for the same value. For example, an HTTP request may have multiple `baggage` headers. As written in the changes, the implementation of `GetAll` is strictly optional. This is for two reasons: 1. Backwards compatibility with existing types/interfaces 2. Carriers do not necessarily support returning multiple values for a single key ## Links to Prototypes This includes implementations of `GetAll()` in Java and Go, as well as using the method in the W3C Baggage Propagators (multiple baggage headers are allowed, [spec reference](https://www.w3.org/TR/baggage/#baggage-http-header-format)). - Java: open-telemetry/opentelemetry-java#6852 - Go: open-telemetry/opentelemetry-go#5973 * [x] Links to the prototypes (when adding or changing features) * [x] [`CHANGELOG.md`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/CHANGELOG.md) file updated for non-trivial changes --------- Co-authored-by: Trask Stalnaker <[email protected]> Co-authored-by: jack-berg <[email protected]> Co-authored-by: Robert Pająk <[email protected]>

…, extending TextMapCarrier. Gives example extracting requests with multiple 'baggage' headers set.

…mpty bVals

jamesmoessis · 2025-04-28T05:23:14Z

hello @pellared @MrAlias I have addressed all outstanding issues and marked this as ready for review. Please take a look when you can!

pellared

👍

propagation/baggage.go

propagation/propagation.go

propagation/baggage.go

propagation/propagation.go

jamesmoessis · 2025-05-08T05:18:01Z

Seems there's a build failure relating to links checking, don't think it's got anything to do with this PR

jamesmoessis · 2025-05-08T05:18:52Z

@MrAlias @pellared - all conversations are resolved, should be ready

pellared

Nice 👍

Mostly nit comments.

My biggest concern is the lack of unit test covering extractSingleBaggage.

propagation/propagation.go

CHANGELOG.md

propagation/baggage.go

Co-authored-by: Robert Pająk <[email protected]>

…rier does not implement ValuesGetter so increases test coverage on single-valued path

jamesmoessis · 2025-05-09T00:00:16Z

Thankyou again @pellared 👍 everything is addressed

propagation/baggage_test.go

pellared · 2025-05-09T07:56:13Z

@open-telemetry/go-approvers, PTAL

MrAlias · 2025-05-13T18:34:41Z

@XSAM @dmathieu PTAL.

propagation/propagation.go

Co-authored-by: Robert Pająk <[email protected]>

dmathieu · 2025-05-14T07:46:12Z

Could you fix the CI?

jamesmoessis · 2025-05-15T01:21:51Z

Thanks @dmathieu I've fixed the linting issue. Last failure seems to be 429 from the link checking - unrelated to this pr

pellared · 2025-05-15T07:44:42Z

@jamesmoessis, thanks a lot for your contribution 🏅

# Overview Closes #6786 ### Added - Add exponential histogram support in `go.opentelemetry.io/otel/exporters/prometheus`. (#6421) - The `go.opentelemetry.io/otel/semconv/v1.31.0` package. The package contains semantic conventions from the `v1.31.0` version of the OpenTelemetry Semantic Conventions. See the [migration documentation](./semconv/v1.31.0/MIGRATION.md) for information on how to upgrade from `go.opentelemetry.io/otel/semconv/v1.30.0`. (#6479) - Add `Recording`, `Scope`, and `Record` types in `go.opentelemetry.io/otel/log/logtest`. (#6507) - Add `WithHTTPClient` option to configure the `http.Client` used by `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#6751) - Add `WithHTTPClient` option to configure the `http.Client` used by `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#6752) - Add `WithHTTPClient` option to configure the `http.Client` used by `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#6688) - Add `ValuesGetter` in `go.opentelemetry.io/otel/propagation`, a `TextMapCarrier` that supports retrieving multiple values for a single key. (#5973) - Add `Values` method to `HeaderCarrier` to implement the new `ValuesGetter` interface in `go.opentelemetry.io/otel/propagation`. (#5973) - Update `Baggage` in `go.opentelemetry.io/otel/propagation` to retrieve multiple values for a key when the carrier implements `ValuesGetter`. (#5973) - Add `AssertEqual` function in `go.opentelemetry.io/otel/log/logtest`. (#6662) - The `go.opentelemetry.io/otel/semconv/v1.32.0` package. The package contains semantic conventions from the `v1.32.0` version of the OpenTelemetry Semantic Conventions. See the [migration documentation](./semconv/v1.32.0/MIGRATION.md) for information on how to upgrade from `go.opentelemetry.io/otel/semconv/v1.31.0`(#6782) - Add `Transform` option in `go.opentelemetry.io/otel/log/logtest`. (#6794) - Add `Desc` option in `go.opentelemetry.io/otel/log/logtest`. (#6796) ### Removed - Drop support for [Go 1.22]. (#6381, #6418) - Remove `Resource` field from `EnabledParameters` in `go.opentelemetry.io/otel/sdk/log`. (#6494) - Remove `RecordFactory` type from `go.opentelemetry.io/otel/log/logtest`. (#6492) - Remove `ScopeRecords`, `EmittedRecord`, and `RecordFactory` types from `go.opentelemetry.io/otel/log/logtest`. (#6507) - Remove `AssertRecordEqual` function in `go.opentelemetry.io/otel/log/logtest`, use `AssertEqual` instead. (#6662) ### Changed - ⚠️ Update `github.com/prometheus/client_golang` to `v1.21.1`, which changes the `NameValidationScheme` to `UTF8Validation`. This allows metrics names to keep original delimiters (e.g. `.`), rather than replacing with underscores. This can be reverted by setting `github.com/prometheus/common/model.NameValidationScheme` to `LegacyValidation` in `github.com/prometheus/common/model`. (#6433) - Initialize map with `len(keys)` in `NewAllowKeysFilter` and `NewDenyKeysFilter` to avoid unnecessary allocations in `go.opentelemetry.io/otel/attribute`. (#6455) - `go.opentelemetry.io/otel/log/logtest` is now a separate Go module. (#6465) - `go.opentelemetry.io/otel/sdk/log/logtest` is now a separate Go module. (#6466) - `Recorder` in `go.opentelemetry.io/otel/log/logtest` no longer separately stores records emitted by loggers with the same instrumentation scope. (#6507) - Improve performance of `BatchProcessor` in `go.opentelemetry.io/otel/sdk/log` by not exporting when exporter cannot accept more. (#6569, #6641) ### Deprecated - Deprecate support for `model.LegacyValidation` for `go.opentelemetry.io/otel/exporters/prometheus`. (#6449) ### Fixes - Stop percent encoding header environment variables in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc` and `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#6392) - Ensure the `noopSpan.tracerProvider` method is not inlined in `go.opentelemetry.io/otel/trace` so the `go.opentelemetry.io/auto` instrumentation can instrument non-recording spans. (#6456) - Use a `sync.Pool` instead of allocating `metricdata.ResourceMetrics` in `go.opentelemetry.io/otel/exporters/prometheus`. (#6472) --------- Co-authored-by: Tyler Yahn <[email protected]> Co-authored-by: Robert Pająk <[email protected]>

jamesmoessis changed the title ~~Prototypes proposed spec changes to allow multiple header values~~ Prototypes proposed spec changes to allow extraction of multiple header values Nov 14, 2024

pellared reviewed Nov 14, 2024

View reviewed changes

propagation/propagation.go Outdated Show resolved Hide resolved

propagation/propagation.go Outdated Show resolved Hide resolved

jack-berg reviewed Nov 15, 2024

View reviewed changes

propagation/baggage.go Show resolved Hide resolved

pellared reviewed Nov 15, 2024

View reviewed changes

jamesmoessis mentioned this pull request Nov 18, 2024

Propagation - Extract handles multiple values on carrier using same key open-telemetry/opentelemetry-specification#4295

Merged

2 tasks

pellared reviewed Nov 18, 2024

View reviewed changes

propagation/baggage.go Outdated Show resolved Hide resolved

dmathieu reviewed Nov 26, 2024

View reviewed changes

propagation/baggage.go Outdated Show resolved Hide resolved

MrAlias reviewed Nov 26, 2024

View reviewed changes

propagation/propagation.go Outdated Show resolved Hide resolved

MrAlias reviewed Nov 26, 2024

View reviewed changes

propagation/propagation.go Outdated Show resolved Hide resolved

pellared mentioned this pull request Jan 11, 2025

Baggage propagator does not handle multiple headers #6154

Closed

jamesmoessis mentioned this pull request Apr 2, 2025

Stabilize GetAll on the TextMap Extract in Propagators API open-telemetry/opentelemetry-specification#4471

Closed

MrAlias mentioned this pull request Apr 11, 2025

Add GetAll to TextMapPropagator Extraction #6652

Closed

jamesmoessis added 5 commits April 28, 2025 14:53

Prototypes propagation with multiple values. Adds MultiTextMapCarrier…

4b08c01

…, extending TextMapCarrier. Gives example extracting requests with multiple 'baggage' headers set.

implement composability suggestion with regards to interface design

4cffc3c

rename GetAll to Values as suggested

5970fd4

ensure backwards compatibility message is in all places it should be

a63c63a

optimise when bVals is empty as suggested -- also add test case for e…

9e4e348

…mpty bVals

jamesmoessis force-pushed the multi-baggage-propagation-prototype branch from 530b431 to 9e4e348 Compare April 28, 2025 05:15

jamesmoessis marked this pull request as ready for review April 28, 2025 05:15

jamesmoessis requested review from XSAM and dashpole as code owners April 28, 2025 05:15

jamesmoessis changed the title ~~Prototypes proposed spec changes to allow extraction of multiple header values~~ Implement spec changes to allow extraction of multiple header values Apr 28, 2025

add changelog entry

c72ade6

appease linter

9de8b16

jamesmoessis force-pushed the multi-baggage-propagation-prototype branch from be8a45a to 9de8b16 Compare April 28, 2025 05:36

pellared reviewed Apr 28, 2025

View reviewed changes

pellared changed the title ~~Implement spec changes to allow extraction of multiple header values~~ propagation: extract of multiple header values May 8, 2025

pellared reviewed May 8, 2025

View reviewed changes

propagation/propagation.go Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

propagation/baggage.go Outdated Show resolved Hide resolved

propagation/baggage.go Show resolved Hide resolved

jamesmoessis and others added 4 commits May 9, 2025 09:36

Update propagation/baggage.go

47204d0

Co-authored-by: Robert Pająk <[email protected]>

Update CHANGELOG.md

a239650

Co-authored-by: Robert Pająk <[email protected]>

add compile time checks for implementing interfaces

41191a9

update test to additionally test baggage with map carrier, as map car…

9698552

…rier does not implement ValuesGetter so increases test coverage on single-valued path

pellared approved these changes May 9, 2025

View reviewed changes

propagation/baggage_test.go Outdated Show resolved Hide resolved

propagation/baggage_test.go Show resolved Hide resolved

pellared requested review from MrAlias and dmathieu May 9, 2025 07:56

Merge branch 'main' into multi-baggage-propagation-prototype

f7254e2

MrAlias reviewed May 13, 2025

View reviewed changes

propagation/propagation.go Show resolved Hide resolved

jamesmoessis and others added 3 commits May 14, 2025 12:02

Update propagation/baggage_test.go

d116229

Co-authored-by: Robert Pająk <[email protected]>

Update propagation/baggage_test.go

0d124fe

Co-authored-by: Robert Pająk <[email protected]>

Merge branch 'main' into multi-baggage-propagation-prototype

f9c5a64

fix applied suggestion

91a4ffc

Merge branch 'main' into multi-baggage-propagation-prototype

1307ad4

dmathieu approved these changes May 15, 2025

View reviewed changes

dmathieu merged commit f410084 into open-telemetry:main May 15, 2025
30 checks passed

jamesmoessis deleted the multi-baggage-propagation-prototype branch May 15, 2025 23:38

dmathieu mentioned this pull request May 19, 2025

Release v1.36.0/v0.58.0/v0.12.0 #6793

Merged

pellared added this to the v1.36.0 milestone May 21, 2025

propagation: extract of multiple header values #5973

propagation: extract of multiple header values #5973

Uh oh!

Conversation

jamesmoessis commented Nov 14, 2024 • edited by pellared Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pellared left a comment

Choose a reason for hiding this comment

Uh oh!

pellared commented Nov 18, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jamesmoessis commented Apr 28, 2025

Uh oh!

pellared left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jamesmoessis commented May 8, 2025

Uh oh!

jamesmoessis commented May 8, 2025

Uh oh!

pellared left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jamesmoessis commented May 9, 2025

Uh oh!

Uh oh!

Uh oh!

pellared commented May 9, 2025

Uh oh!

MrAlias commented May 13, 2025

Uh oh!

Uh oh!

dmathieu commented May 14, 2025

Uh oh!

jamesmoessis commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

pellared commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jamesmoessis commented Nov 14, 2024 •

edited by pellared

Loading

codecov bot commented Nov 14, 2024 •

edited

Loading

jamesmoessis commented May 15, 2025 •

edited

Loading

pellared commented May 15, 2025 •

edited

Loading