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

Allow values containing '=' in OTEL_RESOURCE_ATTRIBUTES #2120

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wperron
Copy link
Contributor

@wperron wperron commented Sep 16, 2024

Values passed in to OTEL_RESOURCE_ATTRIBUTES containing an equal sign "=" are currently ignored by the Resource constructor, but should be accepted as it is part of the W3C Baggage octet
range
.

Fixes #2110

Changes

Updates the EnvResourceDetector to allow resource attributes values containing an equal sign ("=").

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@wperron wperron requested a review from a team September 16, 2024 13:57
@wperron wperron force-pushed the fix-otel-resource-attributes-env-var branch from 2d61328 to 0b5d2b5 Compare September 16, 2024 13:58
Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.3%. Comparing base (7ab5e0f) to head (1e469c4).

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #2120   +/-   ##
=====================================
  Coverage   78.3%   78.3%           
=====================================
  Files        121     121           
  Lines      20815   20817    +2     
=====================================
+ Hits       16308   16312    +4     
+ Misses      4507    4505    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Values passed in to `OTEL_RESOURCE_ATTRIBUTES` containing an equal sign
`"="` are currently ignored by the Resource constructor, but should be
accepted as it is part of the [W3C Baggage octet
range](https://www.w3.org/TR/baggage/#header-content).

Fixes open-telemetry#2110
@wperron wperron force-pushed the fix-otel-resource-attributes-env-var branch from 0b5d2b5 to 1e469c4 Compare September 16, 2024 14:03
Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Could you resolve the conflicts so we can merge?
Apologies for the delay in getting to review this.

@wperron
Copy link
Contributor Author

wperron commented Nov 24, 2024 via email

Copy link
Contributor

@hdost hdost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the change!

@@ -104,7 +104,7 @@ mod tests {
[
(
"OTEL_RESOURCE_ATTRIBUTES",
Some("key=value, k = v , a= x, a=z"),
Some("key=value, k = v , a= x, a=z,equal=value=,empty=,some=other=val"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this with an example using properties. The primary reason for allowing the additional = is in support of this feature.

https://www.w3.org/TR/baggage/#property

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.

[Bug]: OTEL_RESOURCE_ATTRIBUTES doesn't support some characters in the accepted range
3 participants