fix(otel-thread-ctx): put the threadlocal attributes at the right place in the context#2167
fix(otel-thread-ctx): put the threadlocal attributes at the right place in the context#2167yannham wants to merge 1 commit into
Conversation
📚 Documentation Check Results📦
|
yannham
force-pushed
the
yannham/fix-context-process-setup
branch
from
d266efc to
591a444
Compare
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats by Crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. |
yannham
changed the title
|
🔒 Cargo Deny Results📦
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d266efc1e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[cfg(feature = "otel-thread-ctx")] | ||
| if let Some(threadlocal_attribute_keys) = threadlocal_attribute_keys.as_ref() { | ||
| attributes.push(key_value( | ||
| extra_attributes.push(key_value( |
There was a problem hiding this comment.
When otel-thread-ctx is enabled, the existing threadlocal_attrs_present_with_correct_values test still calls find_attr, which only searches ctx.resource.attributes; after this line writes threadlocal.schema_version (and the key map below) into ProcessContext.extra_attributes, the test panics with threadlocal.schema_version should be present. This breaks the crate's all-features test path unless the test/lookup is updated to read extra_attributes.
Useful? React with 👍 / 👎.
| ]; | ||
|
|
||
|
|
||
| let mut extra_attributes = vec![key_value_opt("datadog.process_tags", process_tags)]; |
There was a problem hiding this comment.
When libdd-library-config is checked without otel-thread-ctx, the only mutation of extra_attributes is cfg-stripped, so this binding becomes an unused_mut warning; with the repository's -D warnings policy, cargo clippy -p libdd-library-config --no-default-features -- -D warnings fails with -D unused-mut. Add the same conditional allow pattern here or construct the vector mutably only under the feature.
Useful? React with 👍 / 👎.
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
1 participant
What does this PR do?
This PR stores the
threadlocal*attributes in theattributesfield of the process context, instead of theresourcefield.Additionally, fix an unrelated issue (remove undue
#[cfg(..)]gate) that was introduced in the otel thread feature-gating, preventing normal tracer metadata from being published in the context when theotel-thread-ctxfeature is active.Motivation
After experimenting more with OTel thread context sharing, I found out the way we set up the process context is incorrect. The
threadlocal*attributes should go in theattributes/extra_attributespart of the process context, not theresourcepart. Tested that it now works with the context reader locally.Additional Notes
N/A
How to test the change?
I'm using this version in a prototype integration in dd-trace-rs, using the context reader. This is how I uncovered the issue initially. Now I'm seeing context properly read with this fix.