fix(opentelemetry-resources): Update the Env Var Parsing Logic to Match Spec#6261
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6261 +/- ##
==========================================
+ Coverage 95.48% 95.66% +0.18%
==========================================
Files 363 362 -1
Lines 11563 11731 +168
Branches 2669 2733 +64
==========================================
+ Hits 11041 11223 +182
+ Misses 522 508 -14
🚀 New features to boost your workflow:
|
|
The spec on parsing
Regarding this PR
Some other thoughts(I hesitate to add this because it might derail the PR, though that isn't my intent.) I would like EnvDetector comments/docs to be clearer about how it is basically ignoring parts of the spec there. That is already the case with the support for quoted values. Unless I'm missing something in the baggage spec. It would probably be good to engage with the spec -- though I don't want to slam a blocker on this PR. I find the spec weird here. Perhaps the reference to (somewhat) follow the Baggage rules is historical. If many/most SDK implementations aren't following the spec, perhaps the spec needs to update. Some notes on the weirdness of the JS implementation:
It does treat semicolon as special for baggage attributes. Which means that |
|
Update is based on open-telemetry/opentelemetry-specification#4856 |
trentm
left a comment
There was a problem hiding this comment.
Looking good. Two Qs: breaking change in a stable package, and the quoting weirdness.
trentm
left a comment
There was a problem hiding this comment.
LGTM, with a suggested update for the changelog.
Co-authored-by: Trent Mick <trentm@gmail.com>
Which problem is this PR solving?
This pull request updates the parsing logic for the
OTEL_RESOURCE_ATTRIBUTESenvironment variable in the OpenTelemetry JS resources package to match recent specification changes. The main change is stricter handling: any parsing error now causes the entire environment variable to be discarded. The implementation and test suite have been updated to reflect these rules, while retaining backward compatibility for certain legacy behaviors.Resource attribute parsing and validation:
EnvDetectorto discard the entireOTEL_RESOURCE_ATTRIBUTESvalue on any parsing error, as required by the spec. This includes errors such as invalid percent-encoding, missing key/value separators, or values exceeding 255 characters. [1] [2]Test suite improvements:
EnvDetector.test.tsto cover new spec-compliant behaviors, including error cases (invalid percent-encoding, unencoded=in values, missing separators), edge cases (spaces in keys/values, percent-encoded delimiters), and backward compatibility for quoted values. [1] [2] [3] [4] [5]Logging and error handling:
Documentation:
Type of change
How Has This Been Tested?
Added unit tests to ensure the behavior of the new env var parsing logic.
Checklist: