open-telemetry/opentelemetry-ruby

Decode values from `OTEL_RESOURCE_ATTRIBUTES`

lgfa29 opened this issue · 4 comments

While using the Go SDK I noticed an inconsistency in how OTEL_RESOURCE_ATTRIBUTES baggage values are encoded and decoded. This was due to a difference in the version of the W3C Baggage spec used for baggage propagation and the OTEL_RESOURCE_ATTRIBUTES decoding.

open-telemetry/opentelemetry-specification#2670 updates the W3C Baggage spec version used by the Resource spec to require percent-decoding values from OTEL_RESOURCE_ATTRIBUTES.

I haven't parsed the differences between the versions of the W3C spec in question... but if it turns out that the current decoding logic in the baggage text-map propagator is correct then this would be a relatively easy change for us to make (thank you CGI.unescape!).

However, we should wait until the spec change is accepted. Once it is, we can definitely address this. Thank you for the heads-up, I'll add this to our spec-compliance tracking project. ❤️ 🙇

I think the main change is the requirement to percent-decode values. There are some additional considerations about privacy and security but I think they are mostly targeted at end-users.

Looking at the code you linked it does seem like that's all that is needed. Here's the relevant Go update as reference: https://github.com/open-telemetry/opentelemetry-go/pull/2963/files#diff-b00b30ddb9242080b6237eb51f35a5120cb88036d6892571a8f430d8f3152e0dR94

One interesting thing I noticed is that the W3C spec mandates that the keys must only contain visible ASCII characters (that's why they don't need percent-decoding), but a had a quick glance at the Otel SDKs and none of them seem to enforce this.

Noting here that the spec update has merged to the spec, so we are green-lit for rubbing percents on things.