feat(library-config)!: caller-supplied threadlocal schema and extra process-context attributes#2162
Open
szegedi wants to merge 1 commit into
Open
feat(library-config)!: caller-supplied threadlocal schema and extra process-context attributes#2162szegedi wants to merge 1 commit into
szegedi wants to merge 1 commit into
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 79d90e9 | Docs | Datadog PR Page | Give us feedback! |
Contributor
📚 Documentation Check Results📦
|
Contributor
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. |
Contributor
🔒 Cargo Deny Results📦
|
szegedi
force-pushed
the
szegedi/threadlocal-schema-and-extras
branch
from
c22f97e to
9543321
Compare
szegedi
changed the title
szegedi
force-pushed
the
szegedi/threadlocal-schema-and-extras
branch
from
9543321 to
ce85ea7
Compare
szegedi
changed the title
…ributes The OTel process-context block emitted by TracerMetadata::to_otel_process_ctx used to hardcode 'threadlocal.schema_version' to 'tlsdesc_v1_dev' and offered no way to add further threadlocal.* attributes. Language-specific writers (e.g. a Node.js writer that publishes its own V8 layout constants under a different schema name) couldn't fully drive the process context through this path. Add two new fields on TracerMetadata, gated on the otel-thread-ctx feature: - threadlocal_schema_version: Option<String> — explicit override of the schema attribute. Falls back to 'tlsdesc_v1_dev' when None and threadlocal_attribute_keys is Some, preserving backward-compatible behavior for existing callers. - threadlocal_extra_attributes: Vec<(String, ProcessContextAttrValue)> — additional KeyValues emitted in the process context. The value type is a small enum (String or Int) covering the cases needed for writer layout / runtime metadata. Emission of each piece is now independent: the schema attribute, the key map, and each extra are emitted iff their source field is set.
szegedi
force-pushed
the
szegedi/threadlocal-schema-and-extras
branch
from
ce85ea7 to
79d90e9
Compare
Contributor
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
|
![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: 79d90e9755
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -132,7 +171,9 @@ impl TracerMetadata { | |||
| if let Some(threadlocal_attribute_keys) = threadlocal_attribute_keys.as_ref() { | |||
There was a problem hiding this comment.
When a caller sets threadlocal_schema_version or threadlocal_extra_attributes but has no additional attribute keys to publish, this guard skips the whole block and silently drops the supplied schema/layout attributes. That prevents the new caller-supplied process-context fields from working unless callers also know to set threadlocal_attribute_keys to Some(vec![]), even when they do not need a key map; gate the schema, key map, and extras independently so provided extras are actually published.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TracerMetadata::to_otel_process_ctxused to hardcodethreadlocal.schema_version = "tlsdesc_v1_dev"and offered no way to publish additionalthreadlocal.*attributes. A language-specific writer (e.g. a Node.js writer with its own V8 layout constants under a different schema) couldn't fully drive the process context through this path.This PR adds two new fields to
TracerMetadata, gated on the existingotel-thread-ctxfeature:threadlocal_schema_version: Option<String>— explicit override of the schema attribute. Falls back to"tlsdesc_v1_dev"whenNone, preserving the behavior current callers (e.g.libdatadog-nodejs) rely on.threadlocal_extra_attributes: Vec<(String, ProcessContextAttrValue)>— additional KeyValues emitted in the process context, e.g. the V8threadlocal.wrapped_object_offset/threadlocal.tagged_sizelayout constants the Node.js reader needs. The value type is a small enum (String/Int) covering what writers actually publish today.The whole
threadlocal.*block (schema, key map, and extras) continues to be gated behindthreadlocal_attribute_keys.is_some()— that field remains the single switch for whether the threadlocal section is published at all. The new fields are no-ops when it'sNone.Backward compatibility
Existing callers that only set
threadlocal_attribute_keyscontinue to getthreadlocal.schema_version = "tlsdesc_v1_dev"automatically. Both new fields default to empty /None.Adding public fields to
TracerMetadatais a major-semver event (exhaustive struct literals / destructuring at call sites would no longer compile) — hence the!in the PR title.Test plan
cargo test -p libdd-library-config --features otel-thread-ctxlibdatadog-nodejs'sprocess_discoverycratedd-trace-jspass schema"nodejs_v1_dev"+ V8 layout constants through, dropping its own ad-hoc handling