Repository navigation
Write global tags once on the OTLP trace resource - #13065
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
Conversation
🟢 Java Benchmark SLOs — All performance SLOs passed
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. |
dougqh
left a comment
There was a problem hiding this comment.
TagMap changes look good to me
Claude had a couple suggestions about other areas that you may want to address
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5de38534f8
ℹ️ 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".
There was a problem hiding this comment.
With OTel semantics enabled, DD_TAGS=db.system:postgresql loses its db.system attribute and is exported as db.type instead. Consumers relying on the configured OTLP attribute name will no longer find it.
🤖 Bits Code Review · Commit 5de3853
mcculls
left a comment
There was a problem hiding this comment.
Overall looking good - just the comments about canonical tags need checking over.
Also I agree with Doug that it would be nice to avoid prematurely flushing the payload if possible. That's best done as a follow-up PR though - basically you'd move that check into the JSON/Proto collectors and when the global tags differ you'd complete the current resourceSpans and start a new one.
|
Make sure to merge in latest origin/master to pick up the OTel semantic refactoring from @zacharycmontoya (it should merge cleanly) Also could you coordinate with @anna-git since this overlaps slightly with #12985, basically agree which one will get merged first as the other will need to resolve any conflicts. |
Build the OTLP trace resource from each trace's config snapshot (DD_TAGS or its remote config override) instead of static Config, and stop repeating those global tags on every span. Spans keep their own tags plus the span defaults they still carry. The payload is flushed when a trace's global tags differ from the payload's, so each payload has one accurate resource. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A trace whose config snapshot can't be read (not a DDSpan) now resolves to the startup global tags instead of null, so its payload's resource keeps Config's global tags as a floor and a trace with different tags can't join that payload and lose its own. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The trace's global tags come from its config snapshot as a TagMap, which stores known tags under their Datadog name, so the resource emitted that name even under OTel semantics (db.system came out as db.type). Name them like span attributes instead: the OpenTelemetry name under OTel semantics, else the Datadog name. Other exports keep Config's keys as configured. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
4df682d to
eb350ad
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
d19c3c7
into
master
What Does This Do
Writes the trace's global tags (
DD_TAGS, or their remote configtracing_tagsoverride) once, as OTLP resource attributes, instead of also repeating them on every exported span.ConfigSnapshot.globalTags(via the newDDSpanContext.getGlobalTags()) instead of staticConfig, so it follows remote config tag changes. Each collector caches the encoded resource and re-encodes only when the global-tags map changes.OtlpSpanTagswrites a span's direct tags (newTagMap.forEachLocal) plus the span defaults (Metadata.getSpanDefaultTags():DD_TRACE_SPAN_TAGS,env, dsm/djm) the span still carries and doesn't set itself. Global tags inherited through the read-through parent are left to the resource. A span default the span removed, or that interception consumed into a span field (e.g.resource.name), is not written.OtlpPayloadDispatcherflushes the current payload before adding a trace whose global tags differ from the payload's (OtlpTraceCollector.sharesResource), so a remote config change mid-batch can't put traces under the wrong resource.DDSpan) resolves to the startup global tags, so its resource keepsConfig's global tags as a floor and a trace with different tags still gets its own resource.Motivation
Global tags were exported twice: on the resource (built once from static
Config) and on every span via the span's read-through parent. The static resource also went stale when remote config changedtracing_tags.This supersedes #12813, rebuilt on #12937 / #12960 as suggested in review: no
parent()/readsThroughToParentAPI and no value comparison againstConfig; the resource comes from the snapshot's global tags and spans write their direct tags plus span defaults.Additional Notes
isTraceOtelSemanticsEnabled.Config.isTraceOtelSemanticsEnabled()(DD_TRACE_OTEL_SEMANTICS_ENABLED) decides which tag names the tracer exports over OTLP, for span attributes and now for the global tags on the resource alike: the OpenTelemetry name when enabled (e.g.db.system,http.request.method), the Datadog name otherwise (db.type,http.method). The resource'sdatadog.sdk.semanticsattribute (otel/datadog) records the same choice. Global tags from the snapshot are aTagMap, which stores known tags under their Datadog name, so a key configured in the other spelling is normalized and two spellings of one tag collapse to one key. Metrics / logs resources keepConfig's keys as configured.TraceSegmentstamp-then-equalscheck onceTagMapgainsequals/hashCode.DD_TAGS(follow-up).OtlpSpanTagsTest(resource vs span attributes, span overrides, removed and consumed span defaults,sharesResourceacross tag maps) andTagMapReadThroughTest.forEachLocalSkipsParentEntries; existing OTLP trace/resource/dispatcher andTagMap*tests pass.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue🤖 Generated with Claude Code