Unexpected Baggage Value Modification Due to Potential Skipped Escaping.

[Cirilla-zmh]

Hello everyone,

In our application system, we use baggage to carry various requests-related tags. We deploy a sidecar outside of the application process to intercept outgoing requests and parse these tags, allowing us to dispatch requests based on the tag values. For this purpose, we integrate the opentelemetry-go with W3C Baggage Propagator in our applications to handle the extraction and injection of baggage.

This system has been functioning smoothly until we introduced a baggage value that contains the = character (encoded as %3D). While this value is properly decoded when requests enter the application, it's not being re-encoded back to %3D when requests are sent downstream with the baggage.

For example, the request sent to the application is:

GET /api/test HTTP/1.1
Host: localhost:8080
baggage: gray_key=version%3Dv2
traceparent: 00-f123456789abcdef0123456789abcdef-f123456789abcdef-01

But the request sent by the application is:

GET /api/down HTTP/1.1
Host: localhost:8080
baggage: gray_key=version=v2
traceparent: 00-f123456789abcdef0123456789abcdef-f1239abc45678def-01

Upon a detailed examination of the code, we believe the issue stems from a few lines that prevent the propagator from encoding the = character back to %3D during the injection process.

opentelemetry-go/baggage/baggage.go

Lines 1012 to 1018 in 6731dc7

	func shouldEscape(c byte) bool {
	if c == '%' {
	// The percent character must be encoded so that percent-encoding can work.
	return true
	}
	return !validateValueChar(int32(c))
	}

Previously, we also integrated the opentelemetry-java implementation, which has worked exceptionally well for Java applications.

https://github.com/open-telemetry/opentelemetry-java/blame/fa826fdef07167b22089b96b957749e9f41e72e2/api/all/src/main/java/io/opentelemetry/api/internal/PercentEscaper.java#L72-L78

Although we can modify our sidecar as a workaround, we believe that the OpenTelemetry SDK should ensure the accurate transmission of baggage, rather than making any passive changes during the application's processing. This is to ensure that what is retrieved matches what is set, as stated in the OpenTelemetry spec——Language API MUST accept any valid UTF-8 string as baggage value in Set and return the same value from Get.

Thank you for your attention to this issue!

[pellared]

OTel Go implementation is compliant with the baggage specification. See https://www.w3.org/TR/baggage/#value

The percent code point (U+0025) MUST be percent-encoded

Actually, it looks like there is a bug in OTel Java. It should probably encode the baggage as gray_key=version%253Dv2.

Notice that https://pkg.go.dev/go.opentelemetry.io/otel/baggage#NewMemberRaw accepts any valid UTF-8 string.

You are mixing the "propagation" layer (which has to be encoded and decoded) with the OpenTelemetry API layer.

[Cirilla-zmh]

@pellared Thanks for your reply!

Actually, it looks like there is a bug in OTel Java. It should probably encode the baggage as gray_key=version%253Dv2.

I believe there may be some misunderstanding here, so let me try to elaborate:

• Our application is working with OTel Go and is using the W3C Propagator implementation.
• When a request enters this application, the Propagator parses the baggage containing gray_key=version%3Dv2 into gray_key=version=v2, which is stored in memory as a baggage.Member with the value of version=v2.
• When this application makes downstream requests, the Propagator does not convert version=v2 back to version%3Dv2, which results in our sidecar receiving a baggage value that is different from what was originally set.

You are mixing the "propagation" layer (which has to be encoded and decoded) with the OpenTelemetry API layer.

I understand that the "propagation" layer extracts baggage from the carrier as Baggage instances and subsequently serializes those instances to inject them back into the carrier. Throughout this process, there will be decoding and encoding phases, which will invoke certain functions from the API layer.

opentelemetry-go/propagation/baggage.go

Line 24 in f710cec

bStr := baggage.FromContext(ctx).String()

opentelemetry-go/propagation/baggage.go

Line 37 in f710cec

bag, err := baggage.Parse(bStr)

I believe this is the theme of this issue: the content of the baggage is not consistent before decoding and after encoding, even though we haven’t modified it using any APIs during the process.

[pellared]

gray_key=version%3Dv2 and gray_key=version=v2 values are equivalent. The Go implementation avoids percent encoding when it is not necessary. Nothing requires to pass the exact same header value during propagation.