Repository navigation
Introduce TagStore for eager tag post-processors (dense store - part 3) #13001
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
Changes from all commits
eabce40
19b2e88
6e65a31
d72b784
2271e4f
fa9b441
7894411
8e57e87
3c91e7f
ae4ec9a
f1b60e4
54ed705
e0b022c
884c508
590f6e7
38dc4b3
4540183
7739065
08b0645
f5fa140
a0f2786
0c73aff
933a688
36bbbd2
b9dbfa9
3eed220
710f8e9
cdfc195
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| package datadog.trace.core.tagprocessor; | ||
|
|
||
| import datadog.trace.api.TagStore; | ||
| import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks; | ||
| import datadog.trace.core.DDSpanContext; | ||
| import java.util.Objects; | ||
| import javax.annotation.Nonnull; | ||
|
|
||
| /** Runs eager post-processors in order. */ | ||
| public final class EagerPostProcessorChain extends EagerTagsPostProcessor { | ||
| private final EagerTagsPostProcessor[] chain; | ||
|
|
||
| public EagerPostProcessorChain(@Nonnull final EagerTagsPostProcessor... processors) { | ||
| chain = Objects.requireNonNull(processors); | ||
| } | ||
|
|
||
| @Override | ||
| public void processTags( | ||
| TagStore unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) { | ||
| for (final EagerTagsPostProcessor processor : chain) { | ||
| processor.processTags(unsafeTags, spanContext, spanLinks); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package datadog.trace.core.tagprocessor; | ||
|
|
||
| import datadog.trace.api.TagStore; | ||
| import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks; | ||
| import datadog.trace.core.DDSpanContext; | ||
|
|
||
| /** | ||
| * A tag post-processor run on the application thread as a span finishes. It works on the span's | ||
| * tags as stored, through a {@link TagStore}, rather than a {@code TagMap} the span would have to | ||
| * build for it. The span passes itself as that store, which allocates nothing. | ||
| */ | ||
| public abstract class EagerTagsPostProcessor { | ||
| /** | ||
| * Processes the span's tags. | ||
| * | ||
| * @param unsafeTags the span's tags as stored, to read and write; the caller holds the span's tag | ||
| * lock | ||
| * @param spanContext the span, for its other properties | ||
| * @param spanLinks the span's links | ||
| */ | ||
| public abstract void processTags( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this PR, DDSpanContext becomes TagStore, but I think we still want to keep them conceptually separate. I made that change because TagStore is direct to the underlying store without locking or extra interpretation -- just like Map & TagMap before. The main reason that DDSpanComponent implements TagStore directly is to avoid allocation. Ideally, I think EA (escape analysis)-friendly wrapper could have been used, but the EagerPostProcessorChain dispatch loop isn't EA-friendly. |
||
| TagStore unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,17 @@ | ||
| package datadog.trace.core.tagprocessor; | ||
|
|
||
| import static datadog.trace.bootstrap.instrumentation.api.Tags.VERSION; | ||
| import static datadog.trace.api.KnownTags.DD_BASE_SERVICE_ID; | ||
| import static datadog.trace.api.KnownTags.VERSION_ID; | ||
|
|
||
| import datadog.trace.api.DDTags; | ||
| import datadog.trace.api.TagMap; | ||
| import datadog.trace.api.TagStore; | ||
| import datadog.trace.bootstrap.instrumentation.api.AppendableSpanLinks; | ||
| import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; | ||
| import datadog.trace.core.DDSpanContext; | ||
| import javax.annotation.Nonnull; | ||
| import javax.annotation.Nullable; | ||
|
|
||
| public final class InternalTagsAdder extends TagsPostProcessor { | ||
| public final class InternalTagsAdder extends EagerTagsPostProcessor { | ||
| private final UTF8BytesString ddService; | ||
|
|
||
| // Prebuilt once to avoid per-span Entry allocation. | ||
|
|
@@ -19,30 +20,30 @@ public final class InternalTagsAdder extends TagsPostProcessor { | |
|
|
||
| public InternalTagsAdder(@Nonnull final String ddService, @Nullable final String version) { | ||
| this.ddService = UTF8BytesString.create(ddService); | ||
| this.baseServiceEntry = TagMap.Entry.create(DDTags.BASE_SERVICE, this.ddService); | ||
| this.baseServiceEntry = TagMap.Entry.create(DD_BASE_SERVICE_ID, this.ddService); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once dense store is on by default, we can drop the TagMap.Entry creation in the post processors. |
||
| this.versionEntry = | ||
| version != null && !version.isEmpty() | ||
| ? TagMap.Entry.create(VERSION, UTF8BytesString.create(version)) | ||
| ? TagMap.Entry.create(VERSION_ID, UTF8BytesString.create(version)) | ||
| : null; | ||
| } | ||
|
|
||
| @Override | ||
| public void processTags( | ||
| TagMap unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) { | ||
| TagStore unsafeTags, DDSpanContext spanContext, AppendableSpanLinks spanLinks) { | ||
| if (spanContext == null) { | ||
| return; | ||
| } | ||
|
|
||
| if (!ddService.toString().equalsIgnoreCase(spanContext.getServiceName())) { | ||
| if (baseServiceEntry != null) { | ||
| // service name != DD_SERVICE | ||
| unsafeTags.set(baseServiceEntry); | ||
| unsafeTags.store(baseServiceEntry); | ||
| } | ||
| } else { | ||
| // as per config consistency, the version tag is added across tracers only if | ||
| // the service name is DD_SERVICE and version tag is not manually set | ||
| if (versionEntry != null && !unsafeTags.containsKey(VERSION)) { | ||
| unsafeTags.set(versionEntry); | ||
| if (versionEntry != null && unsafeTags.load(VERSION_ID) == null) { | ||
| unsafeTags.store(versionEntry); | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❔ question: Does it make sense to forbid null but allow empty array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It mirrors the existing
PostProcessorChainon master. An empty chain is a valid no-op (no processors enabled), while a null array is a programming error. In practice it's never empty:TagsPostProcessorFactoryalways addsPeerServiceCalculator.Reply written by Claude (Claude Code) on behalf of dougqh.