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

Update SDK spec to the latest W3C Baggage spec version #2670

Merged
merged 4 commits into from Aug 8, 2022

Conversation

lgfa29
Copy link
Contributor

@lgfa29 lgfa29 commented Jul 16, 2022

Fixes #2639

Changes

Update the version of the W3C Baggage spec used by the Resource SDK for the OTEL_RESOURCE_ATTRIBUTES environment variable so it matches the version used by the baggage propagation API.

The key difference is the requirement to percent-encode characters outside the baggage-octate range, and so this is now highlighted in the SDK spec.

Full diff between the latest version and the version used before:
https://github.com/w3c/baggage/compare/fdc7a5c4f4a31ba2a36717541055e551c2b032e4..main#diff-f65e951e116caf027fff48025b09977ad3539456579743d5c8018b9313484229

@lgfa29 lgfa29 requested review from a team as code owners July 16, 2022 18:30
@lgfa29
Copy link
Contributor Author

lgfa29 commented Jul 16, 2022

I wasn't sure if this could be considered a significant change, and so I didn't update the CHANGELOG.md nor spec-compliance-matrix.md. But maybe the OTEL_RESOURCE_ATTRIBUTES row in the Environment Variables section should be updated since they don't percent-decode values?

@jsuereth
Copy link
Contributor

A few comments on my approval:

  • I think you should update CHANGELOG.md. For spec compliance matrix
  • I think you should ping maintainers to see if they need to update their compliance around parsing this attribute.
  • I do not think existing usage of OTEL_RESOURCE_ATTRIBUTES will be affected at all by this specification change, even though it's more restrictive. The way I read the differences is that previously behavior was pretty unclear for these extended characters.

@lgfa29
Copy link
Contributor Author

lgfa29 commented Jul 31, 2022

Thanks @jsuereth!

I think you should update CHANGELOG.md. For spec compliance matrix

I pushed a new commit with a CHANGELOG entry. I wasn't sure where to add it, so I placed it under the Resource section, but let me know if it should go somewhere else.

I think you should ping maintainers to see if they need to update their compliance around parsing this attribute.

I'm not sure who the maintainers are, so I will open an issue on each SDK repo and link back to this PR to help maintainers keep track of the changes necessary for their own project.

@github-actions
Copy link

github-actions bot commented Aug 8, 2022

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatibility between the W3C Baggage spec version used in the Resource spec and the Baggage spec
6 participants