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

[chore][headerssetter] Update docs to clarify how to use "from_context" #31283

Merged
merged 3 commits into from
Feb 19, 2024

Conversation

ptodev
Copy link
Contributor

@ptodev ptodev commented Feb 15, 2024

This clarifies potential confusion about how to access metadata using the from_context configuration argument.

A few months ago I submitted #27465 to remove an obsolete warning, but then a user asked about why their from_context isn't working properly with a batch processor. I agree that it's not obvious, so I hope this PR clarifies it enough.

I did not try running the update example, so I hope it works :)

cc @jpkrohling

@ptodev ptodev requested a review from a team February 15, 2024 18:21
ptodev referenced this pull request Feb 15, 2024
The docs for the `headers_setter` extension state that the
`from_context` config option does not work if the batch processor is
used. I believe this comment is out of date, because it seems that the
[PR](open-telemetry/opentelemetry-collector#7578)
linked to the
[issue](open-telemetry/opentelemetry-collector#4544)
has been merged. Note that I haven't tested that this actually works
with the header setter extension. Please let me know if you think
testing is necessary.

Co-authored-by: Juraci Paixão Kröhling <[email protected]>
@evan-bradley evan-bradley added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Feb 15, 2024
@jpkrohling jpkrohling changed the title [headerssetter] Update docs to clarify how to use "from_context" [chore][headerssetter] Update docs to clarify how to use "from_context" Feb 19, 2024
Copy link

codecov bot commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6bb95a6) 82.04% compared to head (5ef164c) 82.08%.
Report is 17 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #31283      +/-   ##
==========================================
+ Coverage   82.04%   82.08%   +0.04%     
==========================================
  Files        1855     1862       +7     
  Lines      171689   172236     +547     
==========================================
+ Hits       140856   141380     +524     
- Misses      26665    26677      +12     
- Partials     4168     4179      +11     

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

@jpkrohling jpkrohling merged commit 090a18f into open-telemetry:main Feb 19, 2024
283 of 285 checks passed
@github-actions github-actions bot added this to the next release milestone Feb 19, 2024
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
…n-telemetry#31283)

This clarifies potential confusion about how to access metadata using
the `from_context` configuration argument.

A few months ago I submitted open-telemetry#27465 to remove an obsolete warning, but
then a user
[asked](open-telemetry@d50e094#commitcomment-131957576)
about why their `from_context` isn't working properly with a batch
processor. I agree that it's not obvious, so I hope this PR clarifies it
enough.

I did not try running the update example, so I hope it works :) 

cc @jpkrohling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension/headerssetter Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants