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

bridge/opentracing: propagated baggage item key is canonicalized #4799

Closed
scorpionknifes opened this issue Dec 27, 2023 · 0 comments · Fixed by #4776
Closed

bridge/opentracing: propagated baggage item key is canonicalized #4799

scorpionknifes opened this issue Dec 27, 2023 · 0 comments · Fixed by #4776
Assignees
Labels
area:baggage Part of OpenTelemetry baggage bug Something isn't working pkg:bridges Related to a bridge package
Milestone

Comments

@scorpionknifes
Copy link
Member

Description

When a baggage item key is added to a bridge span, they shouldn't be canonicalized by http.CanonicalHeaderKey

func (c *bridgeSpanContext) setBaggageItem(restrictedKey, value string) {
crk := http.CanonicalHeaderKey(restrictedKey)
m, err := baggage.NewMember(crk, value)
if err != nil {
return
}
c.bag, _ = c.bag.SetMember(m)
}
func (c *bridgeSpanContext) baggageItem(restrictedKey string) baggage.Member {
crk := http.CanonicalHeaderKey(restrictedKey)
return c.bag.Member(crk)
}

Baggage keys are case sensitive, they are not supposed to be mangled this way --
https://github.com/w3c/baggage/blob/ebda53c1d33031d95f1104f44840d3d4b875eb89/baggage/HTTP_HEADER_FORMAT_RATIONALE.md?plain=1#L55
Originally posted by @yurishkuro in #4776 (comment)

Additional Info
Seems like this was added in the initial opentracing bridge implementation in https://www.github.com/open-telemetry/opentelemetry-go/issues/98

opentelemetry-java does not canonicalize the keys: https://github.com/open-telemetry/opentelemetry-java/blob/227580ba0956fcd43429d684b7e0798c8723481a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/SpanShim.java#L166-L188

opentelemetry-dotnet also does not canonicalize the key: https://github.com/open-telemetry/opentelemetry-dotnet/blob/0889e8dc32badbb4c9be8fd24a974fb7e61fa779/src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs#L140-L144

There aren't any references to canonicalizing in spec: https://opentelemetry.io/docs/specs/otel/compatibility/opentracing/#set-baggage-item

Environment

  • OS: macOS
  • Architecture: arm
  • Go Version: 1.21
  • opentelemetry-go version: v1.21.0

Steps To Reproduce

Test from #4776
https://github.com/scorpionknifes/opentelemetry-go/blob/a112d3f67926fe27a2948a8b15380d5c5ccdcbe1/bridge/opentracing/bridge_test.go#L603-L614

Expected behavior

The baggage item key should be the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:baggage Part of OpenTelemetry baggage bug Something isn't working pkg:bridges Related to a bridge package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants