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

Baggage propagator does not handle multiple headers #6154

Open
samuong opened this issue Jan 11, 2025 · 1 comment
Open

Baggage propagator does not handle multiple headers #6154

samuong opened this issue Jan 11, 2025 · 1 comment
Labels
bug Something isn't working

Comments

@samuong
Copy link

samuong commented Jan 11, 2025

Description

According to section 3 of the W3C Baggage spec:

Multiple baggage headers are allowed. Values can be combined in a single header according to RFC 7230.

Section 3.4 gives this example:

Context might be split into multiple headers:

baggage: userId=alice
baggage: serverNode=DF%2028,isProduction=false

But the baggage propagator's Extract() method ends up using https://pkg.go.dev/net/http#Header.Get rather than https://pkg.go.dev/net/http#Header.Values, so it is only able to get a single header value. For the example above (which I've reproduced in the test below) it only sees the key-value pair for userId=alice, and not the other two.

I'm not sure if there are other, better, ways to do this but in my test I am using a HeaderCarrier to adapt http.Header to a TextMapCarrier. This only has a Get() method in the interface, no Values():

// TextMapCarrier is the storage medium used by a TextMapPropagator.
type TextMapCarrier interface {
// DO NOT CHANGE: any modification will not be backwards compatible and
// must never be done outside of a new major release.
// Get returns the value associated with the passed key.
Get(key string) string
// DO NOT CHANGE: any modification will not be backwards compatible and
// must never be done outside of a new major release.
// Set stores the key-value pair.
Set(key string, value string)
// DO NOT CHANGE: any modification will not be backwards compatible and
// must never be done outside of a new major release.
// Keys lists the keys stored in this carrier.
Keys() []string
// DO NOT CHANGE: any modification will not be backwards compatible and
// must never be done outside of a new major release.
}

If it's important to avoid changing the TextMapCarrier interface, then perhaps the HeaderCarrier.Get() method could join together multiple header values into a single comma-separated string?

Environment

  • OS: Fedora 41
  • Architecture: amd64
  • Go Version: 1.23.4
  • opentelemetry-go version: v1.33.0

Steps To Reproduce

  1. Run https://goplay.tools/snippet/1DWw9PC4tHr
  2. See error

Expected behavior

I expected all the tests to pass, and for the values of userId, serverNode and isProduction to be alice, DF%2028 and false, respectively.

@samuong samuong added the bug Something isn't working label Jan 11, 2025
@pellared
Copy link
Member

Related prototype: #5973

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants