Repository navigation
Add the tag registry and map OpenTelemetry tag names through it (otlp - tag registry - phase 1) - #12354
Conversation
dougqh
added
comp: core
|
@codex review |
![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: a98850bf24
ℹ️ 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".
🟡 Java Benchmark SLOs — Performance SLO warning (near threshold)
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
6d5eb31 to
d712502
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d712502ee8
ℹ️ 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".
80011c9 to
6439499
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e1491718c
ℹ️ 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".
9e14917 to
8d60761
Compare
dougqh
changed the title
dougqh
changed the title
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for 46f23b3: What to do next?
DetailsSince those jobs are not marked as being allowed to fail, the pipeline will most likely fail. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for 2e2f367: What to do next?
DetailsSince those jobs are not marked as being allowed to fail, the pipeline will most likely fail. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
The merge request has been interrupted because the build 4368097690923153236 took longer than expected. The current limit for the base branch 'master' is 120 minutes. Possible reasons:
|
…-otel # Conflicts: # .github/CODEOWNERS
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
devflow unqueued this merge request: It did not become mergeable within the expected time |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for 06b54a5: What to do next?
DetailsSince those jobs are not marked as being allowed to fail, the pipeline will most likely fail. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
7 participants
What Does This Do
Extracts the tag registry — generated tag ids plus name resolution — from the dense-store stack, so it can land on its own, and uses it to emit OTLP attributes under their OpenTelemetry names.
A tag id is identity only: a globally unique serial plus the trace/span level bit. It says what a tag is, never how it is stored or set. Bits
[47-32]are documented and held vacant for the co-occurrence slot the dense tag store assigns, which lands with that store.On top of that identity, each tag gets a per-namespace name:
keyOfis many→one — a Datadog name or an OpenTelemetry name both resolve to the one id.datadogNameOf/openTelemetryNameOftake the name back out per namespace;openTelemetryTagOfis the one place the pass-through policy lives (declared rename, else the Datadog name).OtlpTraceProtoandOtlpTraceJsonemit known tags — including interceptedMetadatafields such ashttp.status_code, andservice.name— under their OpenTelemetry names whentrace.otel.semantics.enabledis on, and under Datadog names otherwise.Aliases are one tag, on write as well as on export
TagMapcanonicalizes a known tag's OpenTelemetry spelling to its Datadog name atEntryconstruction, so settinghttp.request.methodandhttp.methodon one span yields one entry, not two attributes with the same OTLP key. Lookup (getTag,contains, removal) canonicalizes the same way.TagInterceptortreats both spellings alike for every intercepted tag that has a rename (service,http.method,http.status_code,http.url,db.statement), so behavior no longer depends on the spelling, or on whether the attribute was set on the builder or after start.db.statement/db.query.textis consumed intoresource.nameunder either spelling, per the policy noted intag-conventions.yaml.DDSpanContext.getTag()recognizes thehttp.response.status_codealias of the intercepted status.Conventions rules the generator enforces
tag-conventions.yamlis the single, language-agnostic source. Beyond parsing, the generator rejects:{ ref: <dd-name>, required: <level> }, which may override only the requirement level.http.urlmoved to the sharedhttpparent.span-kind-neutral: true. Canonicalization ignores span kind, so a rename is only right if the OpenTelemetry attribute means that tag on every span kind it appears on. Renames in a shared scope need no flag.dd-name, a ref to an undeclared tag, and colliding generated constant names.Renames deliberately left out
Each of these maps differently by span kind, which a flat rename cannot express, so it passes through under its Datadog name for now:
http.hostname→server.address— on client spans OTel'sserver.addressis the remote server.network.client.ip→network.peer.address— OTel'snetwork.peer.addressis the remote end on every span kind.http.query.string→url.query—url.fullalready carries the query, andQueryObfuscatorwould append it twice.A follow-on will model this directly (relative vs absolute names, resolved by span direction) rather than as renames.
Build
The generator is a
build-logicplugin (dd-trace-java.tag-registry-generator).KnownTags.javaand its reports are generated underinternal-api/build/generated/tag-registryand wired into themainsource set, socompileJavaalways compiles against the current YAML — nothing generated is committed. Tag ids are not a stable contract, so renumbering when tags are added is expected; the YAML is what gets reviewed.Why
Entry.tagId()is not memoizedTagMap$Entryis the tracer's largest allocation source, one per tag per span on the app thread. A memo field widens every entry and adds aputfieldper construction, for a value only OTLP serialization reads — on the background thread, once per entry. Resolving on demand is cheaper. (The constructor does now pay oneStringIndexlookup to canonicalize the name; that is the price of aliases being one tag.)Why intercept modelling is not in the registry
The registry originally mirrored
TagInterceptor.needsInterceptas an id bit. It had already drifted from the switch that is the actual authority, and nothing consumed it. Interception is aTagInterceptorproperty; the classification returns with the work that consumes it (an id→handler dispatch that retires the switch), and adding it then is purely additive.Why there is no resolver registration
KnownTagCodec(bit layout, naming policy) and generatedKnownTags(the name↔id tables) are two halves of one class. They were first joined at runtime —KnownTagsregistered its resolver — which left a window where an early read latched an empty fallback permanently, with no error. SinceKnownTagsis generated into the same module and package,KnownTagCodecnamesKnownTags.RESOLVERdirectly: reading a name is what initializes the registry, so the window is gone, along withinit(), theCoreTracercoupling and the fallback codec. A nested holder remains so the codec finishes initializing beforeKnownTags(which calls back into it) starts.Motivation
PR #12230 maps OpenTelemetry tag names via the registry, and another team is waiting on it. It was based on #12047, which sits on the dense store (#12045) and the colored-slot encoding (#12046), so the OTel work was blocked behind the whole storage stack.
TagRegistry.build()already separated serial assignment and name validation from graph coloring; this PR is the identity-and-names half, targeting master directly.Additional Notes
ci_visibility'stest.*); the gap is reported inresolved-tags.txtrather than silently dropped.peermixin modelspeer.*as client-only, though server spans also carrypeer.ipv4/6/peer.port(for the client's address). Harmless today — none of those tags has an OpenTelemetry name — and addressed by the follow-on.@TableTestforOpenTelemetry14ConventionsTestis left to the OTel/OTLP owners.Contributor Checklist
./gradlew spotlessApplybuild-logic:tag-registry:test(generator unit + TestKit) green:internal-api:test(KnownTags*,TagMap*),:internal-api:spotlessCheckgreen:dd-trace-core:test(*otlp*,*taginterceptor*) green:dd-java-agent:instrumentation:opentelemetry:opentelemetry-1.4:test(OpenTelemetry14ConventionsTest) greenJira ticket
N/A
🤖 Generated with Claude Code