From 0696fab910b59be8d4ed8cc1572a7e659e0ec033 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 23 Oct 2026 00:09:31 -0400 Subject: [PATCH] Have the span own its tag stores directly DDSpanContext holds its IdStore (also its tag lock) and its HashStore instead of one TagMap, and routes each write to the store that holds the tag: a name is resolved once and routed, and a write by id goes straight to the id store. The custom store is borrowed read-only from the read-through parent until the span's first custom write. The eager post-processors already read and write the span through SpanContents. The lazy post-processors and the serializer still take a TagMap: the span builds a view over its stores once, at serialization, and caches it (about 32 B per span until they move to SpanContents). TagMap.Internals, the core's trusted side door, is gone: the stores are the direct path. The constructors drop the unused tagsSize parameter. Allocation, 7-span web-request trace: +104 B (+1.3%), from the new DDSpanContext fields (8 B per span) and the first custom store. Co-Authored-By: Claude Opus 5.5 --- .../src/test/groovy/TypeConverterTest.groovy | 1 - .../src/test/groovy/TypeConverterTest.groovy | 1 - .../src/test/groovy/TypeConverterTest.groovy | 1 - .../writer/ddagent/TraceMapperBenchmark.java | 1 - .../datadog/trace/core/GetTagBenchmark.java | 116 +++++ .../datadog/trace/core/PendingTraceWrite.java | 2 - .../datadog/trace/core/TracerMapperMap.java | 1 - .../core/propagation/InjectorBenchmark.java | 1 - .../java/datadog/trace/core/CoreTracer.java | 66 ++- .../datadog/trace/core/DDSpanContext.java | 464 +++++++++++++----- .../writer/PayloadDispatcherImplTest.java | 1 - .../trace/core/DDCoreJavaSpecification.java | 1 - .../trace/core/DDSpanContextStoresTest.java | 297 +++++++++++ .../trace/core/DDSpanContextTagViewTest.java | 60 +++ .../trace/core/DDSpanSerializationTest.java | 17 +- .../java/datadog/trace/core/DDSpanTest.java | 1 - .../core/LongRunningTracesTrackerTest.java | 1 - .../trace/core/PendingTraceBufferTest.java | 2 - .../datadog/trace/core/PendingTraceTest.java | 1 - .../core/RootSpanTagsReadThroughTest.java | 90 ++++ .../propagation/AbstractHttpInjectorTest.java | 1 - .../taginterceptor/TagInterceptorTest.java | 3 +- .../forbiddenApiFilters/instrumentation.txt | 5 - .../main/java/datadog/trace/api/TagMap.java | 58 --- .../datadog/trace/api/TagMapSetByIdTest.java | 36 -- 25 files changed, 977 insertions(+), 251 deletions(-) create mode 100644 dd-trace-core/src/jmh/java/datadog/trace/core/GetTagBenchmark.java create mode 100644 dd-trace-core/src/test/java/datadog/trace/core/DDSpanContextStoresTest.java create mode 100644 dd-trace-core/src/test/java/datadog/trace/core/DDSpanContextTagViewTest.java create mode 100644 dd-trace-core/src/test/java/datadog/trace/core/RootSpanTagsReadThroughTest.java diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy index 60c0f43d9be..9dcf709d75f 100644 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy +++ b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy @@ -53,7 +53,6 @@ class TypeConverterTest extends InstrumentationSpecification { [:], false, "fakeType", - 0, trace, null, null, diff --git a/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/test/groovy/TypeConverterTest.groovy b/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/test/groovy/TypeConverterTest.groovy index 3a3b5c6efca..db29f8de153 100644 --- a/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/test/groovy/TypeConverterTest.groovy +++ b/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/test/groovy/TypeConverterTest.groovy @@ -65,7 +65,6 @@ class TypeConverterTest extends InstrumentationSpecification { [:], false, "fakeType", - 0, trace, null, null, diff --git a/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/test/groovy/TypeConverterTest.groovy b/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/test/groovy/TypeConverterTest.groovy index 9ecfd5221ef..1506f3eb983 100644 --- a/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/test/groovy/TypeConverterTest.groovy +++ b/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/test/groovy/TypeConverterTest.groovy @@ -65,7 +65,6 @@ class TypeConverterTest extends InstrumentationSpecification { [:], false, "fakeType", - 0, trace, null, null, diff --git a/dd-trace-core/src/jmh/java/datadog/trace/common/writer/ddagent/TraceMapperBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/common/writer/ddagent/TraceMapperBenchmark.java index 54192804b05..f7a6fec6e2c 100644 --- a/dd-trace-core/src/jmh/java/datadog/trace/common/writer/ddagent/TraceMapperBenchmark.java +++ b/dd-trace-core/src/jmh/java/datadog/trace/common/writer/ddagent/TraceMapperBenchmark.java @@ -116,7 +116,6 @@ public void init(Blackhole blackhole) { Collections.emptyMap(), false, UTF8BytesString.create("type"), - 0, traceCollector, null, null, diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/GetTagBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/GetTagBenchmark.java new file mode 100644 index 00000000000..57ce57b404f --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/GetTagBenchmark.java @@ -0,0 +1,116 @@ +package datadog.trace.core; + +import static java.util.concurrent.TimeUnit.NANOSECONDS; + +import datadog.trace.api.KnownTags; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Warmup; + +/** + * Measures reading tags back from a live span, by how the tag is named: a known tag's name, a + * custom name, an OpenTelemetry name ({@code server.address}, which the core treats as a custom + * name), and misses. {@code getTag} takes the span's tag lock; {@code unsafeGetTag} reads without + * it, as the tracer's internal readers do. + */ +@State(Scope.Thread) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(NANOSECONDS) +@Fork(value = 3) +public class GetTagBenchmark { + static final CoreTracer TRACER = CoreTracer.builder().build(); + + private DDSpan span; + + /** + * The known tags the span holds, read in turn by the {@code *_cycle} arms so a read's cost is + * averaged over the span's tags rather than decided by whether one particular tag collides in its + * bucket. A power-of-two length lets the index wrap with a mask. + */ + private static final String[] KNOWN_HELD = { + "span.kind", + KnownTags.HTTP_ROUTE_NAME, + KnownTags.PEER_HOSTNAME_NAME, + KnownTags.HTTP_STATUS_CODE_NAME + }; + + private int next; + + @Setup + public void setup() { + span = (DDSpan) TRACER.startSpan("benchmark", "get.tag"); + span.setTag("span.kind", "client"); + span.setTag(KnownTags.HTTP_ROUTE_NAME, "/users/{id}"); + span.setTag(KnownTags.PEER_HOSTNAME_NAME, "db.example"); + span.setTag(KnownTags.HTTP_STATUS_CODE_NAME, 200); + span.setTag("my.custom.tag", "custom"); + span.setTag("my.other.tag", 42); + } + + @TearDown + public void tearDown() { + span.finish(); + } + + @Benchmark + public Object knownTag_getTag() { + return span.getTag(KnownTags.HTTP_ROUTE_NAME); + } + + @Benchmark + public Object customTag_getTag() { + return span.getTag("my.custom.tag"); + } + + @Benchmark + public Object aliasName_getTag() { + return span.getTag("server.address"); + } + + @Benchmark + public Object missingTag_getTag() { + return span.getTag("not.there"); + } + + /** A known tag the span doesn't hold: a clear slot bit can answer without a probe. */ + @Benchmark + public Object missingKnownTag_getTag() { + return span.getTag(KnownTags.HTTP_METHOD_NAME); + } + + @Benchmark + public Object missingKnownTag_unsafeGetTag() { + return span.unsafeGetTag(KnownTags.HTTP_METHOD_NAME); + } + + @Benchmark + public Object knownTag_unsafeGetTag() { + return span.unsafeGetTag(KnownTags.HTTP_ROUTE_NAME); + } + + /** Every known tag the span holds, in turn; see {@link #KNOWN_HELD}. */ + @Benchmark + public Object knownTags_cycle_unsafeGetTag() { + return span.unsafeGetTag(KNOWN_HELD[next++ & (KNOWN_HELD.length - 1)]); + } + + @Benchmark + public Object knownTags_cycle_getTag() { + return span.getTag(KNOWN_HELD[next++ & (KNOWN_HELD.length - 1)]); + } + + @Benchmark + public Object customTag_unsafeGetTag() { + return span.unsafeGetTag("my.custom.tag"); + } +} diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/PendingTraceWrite.java b/dd-trace-core/src/jmh/java/datadog/trace/core/PendingTraceWrite.java index b0a90c7bffb..dda438dfc16 100644 --- a/dd-trace-core/src/jmh/java/datadog/trace/core/PendingTraceWrite.java +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/PendingTraceWrite.java @@ -55,7 +55,6 @@ public void init(TraceCounters counters, Blackhole blackhole) { Collections.emptyMap(), false, "type", - 0, traceCollector, null, null, @@ -80,7 +79,6 @@ public void init(TraceCounters counters, Blackhole blackhole) { Collections.emptyMap(), false, "type", - 0, traceCollector, null, null, diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/TracerMapperMap.java b/dd-trace-core/src/jmh/java/datadog/trace/core/TracerMapperMap.java index 4075274cca2..0e9f4c57ac2 100644 --- a/dd-trace-core/src/jmh/java/datadog/trace/core/TracerMapperMap.java +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/TracerMapperMap.java @@ -135,7 +135,6 @@ private DDSpan createSpanWithOrigin(int iter, final String origin) { Collections.emptyMap(), false, "type", - 0, traceCollector, null, null, diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/propagation/InjectorBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/propagation/InjectorBenchmark.java index 1c070167377..b06ce7b3558 100644 --- a/dd-trace-core/src/jmh/java/datadog/trace/core/propagation/InjectorBenchmark.java +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/propagation/InjectorBenchmark.java @@ -122,7 +122,6 @@ public void setUp(Blackhole blackhole) { Collections.emptyMap(), false, "type", - 0, tracer.createTraceCollector(traceId), null, null, diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 2f10cceba4b..dda914cc6d7 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -207,10 +207,43 @@ private static final class LocalRootSpanTags { final TagMap tags; final boolean needsIntercept; + /** The last {@link #traceLevelParent}, with the merged tracer tags it was built over. */ + private volatile TraceLevelTags traceLevel; + LocalRootSpanTags(final TagMap tags, final boolean needsIntercept) { this.tags = tags; this.needsIntercept = needsIntercept; } + + /** + * All of a root span's trace-level tags in one frozen map -- these tags over {@code + * mergedTracerTags} -- for the root span to read through instead of copying them in. The tag + * registry colors trace-level tags separately, so they reuse span-level slot bits; keeping them + * out of the span's own store keeps its slot bits exact. Built once per config snapshot; a racy + * rebuild is harmless, since it builds the same map. + */ + TagMap traceLevelParent(final TagMap mergedTracerTags) { + TraceLevelTags cached = this.traceLevel; + if (cached != null && cached.mergedTracerTags == mergedTracerTags) { + return cached.parent; + } + TagMap parent = TagMap.create(); + parent.putAll(mergedTracerTags); + parent.putAll(this.tags); + parent.freeze(); + this.traceLevel = new TraceLevelTags(mergedTracerTags, parent); + return parent; + } + } + + private static final class TraceLevelTags { + final TagMap mergedTracerTags; + final TagMap parent; + + TraceLevelTags(final TagMap mergedTracerTags, final TagMap parent) { + this.mergedTracerTags = mergedTracerTags; + this.parent = parent; + } } private volatile LocalRootSpanTags localRootSpanTags; @@ -2130,6 +2163,7 @@ protected static final DDSpanContext buildSpanContext( final CharSequence origin; final TagMap coreTags; final boolean coreTagsNeedsIntercept; + final LocalRootSpanTags localRootSpanTags; final TagMap rootSpanTags; final boolean rootSpanTagsNeedsIntercept; final DDSpanContext context; @@ -2159,6 +2193,7 @@ protected static final DDSpanContext buildSpanContext( origin = null; coreTags = null; coreTagsNeedsIntercept = false; + localRootSpanTags = null; rootSpanTags = null; rootSpanTagsNeedsIntercept = false; parentServiceName = ddsc.getServiceName(); @@ -2233,7 +2268,7 @@ protected static final DDSpanContext buildSpanContext( ciVisibilityContextData = null; } - LocalRootSpanTags localRootSpanTags = tracer.localRootSpanTags; + localRootSpanTags = tracer.localRootSpanTags; rootSpanTags = localRootSpanTags.tags; rootSpanTagsNeedsIntercept = localRootSpanTags.needsIntercept; @@ -2312,12 +2347,22 @@ protected static final DDSpanContext buildSpanContext( final TagMap mergedTracerTags = traceConfig.mergedTracerTags; boolean mergedTracerTagsNeedsIntercept = traceConfig.mergedTracerTagsNeedsIntercept; - final int tagsSize = - mergedTracerTags.size() - + (null == tagLedger ? 0 : tagLedger.estimateSize()) - + (null == coreTags ? 0 : coreTags.size()) - + (null == rootSpanTags ? 0 : rootSpanTags.size()) - + (null == contextualTags ? 0 : contextualTags.size()); + // With builder tags taking precedence, a root span reads its trace-level tags (root-span tags + // over the merged tracer tags) through one shared frozen parent instead of copying the + // root-span tags in. They become the precedence floor: the flag's order already puts them + // below builder and contextual tags, and now inbound header tags and prototype defaults also + // win over them. Tags needing interception are still copied in. + final boolean rootSpanTagsReadThrough = + localRootSpanTags != null + && tracer.builderTagsPrecedence + && !rootSpanTagsNeedsIntercept + && !mergedTracerTagsNeedsIntercept; + final TagMap tagParent; + if (rootSpanTagsReadThrough) { + tagParent = localRootSpanTags.traceLevelParent(mergedTracerTags); + } else { + tagParent = mergedTracerTagsNeedsIntercept ? null : mergedTracerTags; + } if (builderRequestContextDataAppSec != null) { requestContextDataAppSec = builderRequestContextDataAppSec; @@ -2346,7 +2391,6 @@ protected static final DDSpanContext buildSpanContext( w3cBaggage, errorFlag, spanType, - tagsSize, parentTraceCollector, requestContextDataAppSec, requestContextDataIast, @@ -2357,7 +2401,7 @@ protected static final DDSpanContext buildSpanContext( tracer.profilingContextIntegration, tracer.injectBaggageAsTags, tracer.injectLinksAsTags, - mergedTracerTagsNeedsIntercept ? null : mergedTracerTags); + tagParent); // By setting the tags on the context we apply decorators to any tags that have been set via // the builder. The `mergedTracerTags` are always applied first (the precedence floor: @@ -2382,7 +2426,9 @@ protected static final DDSpanContext buildSpanContext( } if (tracer.builderTagsPrecedence) { context.setAllTags(coreTags, coreTagsNeedsIntercept); - context.setAllTags(rootSpanTags, rootSpanTagsNeedsIntercept); + if (!rootSpanTagsReadThrough) { + context.setAllTags(rootSpanTags, rootSpanTagsNeedsIntercept); + } context.setAllTags(contextualTags); context.setAllTags(tagLedger); } else { diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 8fdf11b8a85..61269b4ea01 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -15,6 +15,8 @@ import datadog.trace.api.DDTags; import datadog.trace.api.DDTraceId; import datadog.trace.api.Functions; +import datadog.trace.api.HashStore; +import datadog.trace.api.IdStore; import datadog.trace.api.KnownTagCodec; import datadog.trace.api.KnownTags; import datadog.trace.api.ProcessTags; @@ -57,6 +59,7 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.function.Function; import javax.annotation.Nonnull; +import javax.annotation.concurrent.GuardedBy; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -139,15 +142,48 @@ public class DDSpanContext private CharSequence serviceNameSource; /** - * Tags are associated to the current span, they will not propagate to the children span. + * The span's known tags, by id. Also the lock for all of the span's tag state. Tags belong to + * this span; they don't propagate to its children. * - *

The underlying assumption for using a normal Map with synchronized access instead of a - * ConcurrentHashMap is that even though the tags can be accessed and modified from multiple - * threads, they will rarely, if ever, be read and modified concurrently by multiple threads but - * rather read and accessed in a serial fashion on thread after thread. The synchronization can - * then be wrapped around bulk operations to minimize the costly atomic operations. + *

Storage. The span owns its tag stores directly. This store holds the known tags and + * is created with the span. {@link #customTags} holds the custom tags by name and is created on + * the first custom write. Until then it is null, or the read-through parent's custom store, + * borrowed read-only while {@link #customTagsBorrowed}. Every read and write is routed by name or + * id straight to one of the stores. Internal bulk readers (post-processors, serializers, {@link + * #getTags()}) go through the cached {@link #tagView()} instead. + * + *

Locking. One lock, this store itself, guards both stores, {@link #customTagsBorrowed} + * and {@link #tagView}. It is this store because it always exists, while {@link #customTags} may + * be null or borrowed. + * + *

    + *
  • The stores are unsynchronized; they rely on this lock. + *
  • Read-through parents are frozen, so reading through them needs no lock. + *
  • The lock is reentrant, and that matters: code that runs while it is held may call back + * into the span's setters. The tag interceptor does so from {@code setAllTags}, and + * post-processors do so through the view from {@code processTagsAndBaggage}. + *
  • {@link #unsafeGetTag} and {@code unsafeSetTag} skip the lock. Callers either hold it + * already or guarantee that nothing mutates the span concurrently. + *
+ * + *

A plain lock rather than concurrent structures: tags are rarely read and written by several + * threads at once. They are touched by one thread after another, so a lock around each operation, + * or around a bulk operation, costs less than atomic operations on every access. */ - private final TagMap unsafeTags; + @GuardedBy("knownTags") + private final IdStore knownTags; + + /** Custom tags by name: null, borrowed from the read-through parent, or the span's own. */ + @GuardedBy("knownTags") + private HashStore customTags; + + /** True while {@link #customTags} is the read-through parent's store, borrowed read-only. */ + @GuardedBy("knownTags") + private boolean customTagsBorrowed; + + /** This span's view of its tag stores for its internal readers; see {@link #tagView()}. */ + @GuardedBy("knownTags") + private TagMap tagView; /** The service name is required, otherwise the span are dropped by the agent */ private volatile String serviceName; @@ -217,7 +253,6 @@ public DDSpanContext( final Map baggageItems, final boolean errorFlag, final CharSequence spanType, - final int tagsSize, final TraceCollector traceCollector, final Object requestContextDataAppSec, final Object requestContextDataIast, @@ -239,7 +274,6 @@ public DDSpanContext( null, errorFlag, spanType, - tagsSize, traceCollector, requestContextDataAppSec, requestContextDataIast, @@ -266,7 +300,6 @@ public DDSpanContext( final Map baggageItems, final boolean errorFlag, final CharSequence spanType, - final int tagsSize, final TraceCollector traceCollector, final Object requestContextDataAppSec, final Object requestContextDataIast, @@ -290,7 +323,6 @@ public DDSpanContext( null, errorFlag, spanType, - tagsSize, traceCollector, requestContextDataAppSec, requestContextDataIast, @@ -320,7 +352,6 @@ public DDSpanContext( final Baggage w3cBaggage, final boolean errorFlag, final CharSequence spanType, - final int tagsSize, final TraceCollector traceCollector, final Object requestContextDataAppSec, final Object requestContextDataIast, @@ -346,7 +377,6 @@ public DDSpanContext( w3cBaggage, errorFlag, spanType, - tagsSize, traceCollector, requestContextDataAppSec, requestContextDataIast, @@ -375,7 +405,6 @@ public DDSpanContext( final Baggage w3cBaggage, final boolean errorFlag, final CharSequence spanType, - final int tagsSize, final TraceCollector traceCollector, final Object requestContextDataAppSec, final Object requestContextDataIast, @@ -412,13 +441,11 @@ public DDSpanContext( assert pathwayContext != null; this.pathwayContext = pathwayContext; - // The +1 is the magic number from the tags below that we set at the end, - // and "* 4 / 3" is to make sure that we don't resize immediately - final int capacity = Math.max((tagsSize <= 0 ? 3 : (tagsSize + 1)) * 4 / 3, 8); - this.unsafeTags = - readThroughParent != null - ? TagMap.createFromParent(readThroughParent) - : TagMap.create(capacity); + // Reads through to the frozen tracer-level tags, if any. + TagMap parentTags = TagMap.readThroughParent(readThroughParent); + this.knownTags = IdStore.create(parentTags); + this.customTags = HashStore.inheritedFrom(parentTags); + this.customTagsBorrowed = this.customTags != null; // must set this before setting the service and resource names below this.profilingContextIntegration = profilingContextIntegration; @@ -667,16 +694,15 @@ private void seedAbsentTags(final TagMap map) { if (map == null) { return; } - synchronized (unsafeTags) { + synchronized (knownTags) { map.forEach( this, (ctx, tagEntry) -> { - final String tag = tagEntry.tag(); - if (ctx.unsafeTags.containsKey(tag)) { + if (ctx.isPresent(tagEntry)) { return; } if (!ctx.tagInterceptor.interceptTag(ctx, tagEntry)) { - ctx.unsafeTags.set(tagEntry); + ctx.routePut(tagEntry); } }); } @@ -801,7 +827,7 @@ public int getSamplingPriority() { } public void setSpanSamplingPriority(double rate, int limit) { - synchronized (unsafeTags) { + synchronized (knownTags) { unsafeSetTag(SPAN_SAMPLING_MECHANISM_TAG, SamplingMechanism.SPAN_SAMPLING_RATE); unsafeSetTag(SPAN_SAMPLING_RULE_RATE_TAG, rate); if (limit != Integer.MAX_VALUE) { @@ -930,6 +956,139 @@ public void setSpanKindOrdinal(String kind) { spanKindOrdinal = spanKindOrdinalOf(kind); } + /** + * This span's cached view of its tag stores, for its internal bulk readers (post-processors, the + * serializer, the snapshot copy) -- never its own set/get paths. The view keeps the stores it was + * created over: {@link #customTagsForWrite} drops it when the span creates its own custom store. + * Bulk readers run late in a span's life, after its custom writes, so that is rare. Call under + * the {@code knownTags} lock. + */ + private TagMap tagView() { + TagMap view = this.tagView; + if (view == null) { + // a view handed to the post-processor chain escapes, so allocate it once per span + this.tagView = view = TagMap.viewOf(knownTags, customTags, customTagsBorrowed); + } + return view; + } + + /** + * Adopts the custom store a reader may have created through the view (by writing a custom tag), + * so the span's own paths see it. Call under the {@code knownTags} lock, after using the view. + */ + private void adoptViewStores(TagMap view) { + // Skipped when the span dropped this view during its use: a callback into a span setter + // created the span's own custom store, which must not be replaced by the view's older one. + if (view == this.tagView) { + this.customTags = view.customStore(); + this.customTagsBorrowed = view.isCustomStoreBorrowed(); + } + } + + /** + * The custom store to write to: created on the first custom write, ending any borrowing. A cached + * view still holds the old store, so it is dropped. + */ + private HashStore customTagsForWrite() { + HashStore custom = this.customTags; + if (custom == null || this.customTagsBorrowed) { + custom = HashStore.create(custom); // reads through to the store borrowed till now + this.customTags = custom; + this.customTagsBorrowed = false; + this.tagView = null; + } + return custom; + } + + /** + * Removes the custom tag {@code name}: hiding an inherited one needs a store of the span's own. + */ + private void removeCustomTag(String name) { + HashStore custom = this.customTags; + if (custom == null) { + return; + } + if (this.customTagsBorrowed) { + if (custom.get(name) == null) { + return; // nothing to hide: keep borrowing + } + custom = customTagsForWrite(); + } + custom.remove(name); + } + + /** Removes {@code tag}, known or custom. Call under the {@code knownTags} lock. */ + private void routeRemoval(String tag) { + long key = KnownTagCodec.keyOf(tag); + if (key != 0) { + knownTags.remove(key); + } else { + removeCustomTag(tag); + } + } + + /** + * Copies {@code map}'s tags into the stores, sharing its Entry objects where a store keeps them. + */ + private void routePutAll(TagMap map) { + // straight into the stores: a view created this early would likely be dropped by a later + // custom write + map.forEach(this, (ctx, entryReader) -> ctx.routePut(entryReader)); + } + + /* + * The route* operations send a tag to the store that holds it: the known store by id, else the + * custom store by name. Call them while holding the knownTags lock. + */ + + private void routeSet(long key, String tag, Object value) { + if (key != 0) { + knownTags.set(key, value); + } else { + customTagsForWrite().set(tag, value); + } + } + + private void routeSet(long key, String tag, CharSequence value) { + if (key != 0) { + knownTags.set(key, value); + } else { + customTagsForWrite().set(tag, value); + } + } + + private void routeSet(long key, String tag, int value) { + if (key != 0) { + knownTags.set(key, value); + } else { + customTagsForWrite().set(tag, value); + } + } + + private void routeSet(long key, String tag, long value) { + if (key != 0) { + knownTags.set(key, value); + } else { + customTagsForWrite().set(tag, value); + } + } + + private void routeSet(long key, String tag, float value) { + if (key != 0) { + knownTags.set(key, value); + } else { + customTagsForWrite().set(tag, value); + } + } + + private void routeSet(long key, String tag, double value) { + if (key != 0) { + knownTags.set(key, value); + } else { + customTagsForWrite().set(tag, value); + } + } + static byte spanKindOrdinalOf(String kind) { if (kind == null) { return SPAN_KIND_UNSET; @@ -961,8 +1120,9 @@ public String getSpanKindString() { return SPAN_KIND_VALUES[ordinal]; } // UNSET or CUSTOM -- fall through to tag map - synchronized (unsafeTags) { - return unsafeTags.getString(Tags.SPAN_KIND); + synchronized (knownTags) { + TagMap.EntryReader kind = knownTags.getEntryReader(KnownTags.SPAN_KIND_ID); + return kind == null ? null : kind.stringValue(); } } @@ -973,32 +1133,32 @@ public void setOrigin(final CharSequence origin) { } public void setMetric(final CharSequence key, final Number value) { - synchronized (unsafeTags) { + synchronized (knownTags) { unsafeSetTag(key.toString(), value); } } public void setMetric(final CharSequence key, final int value) { - synchronized (unsafeTags) { - unsafeTags.set(key.toString(), value); + synchronized (knownTags) { + routeSet(key.toString(), value); } } public void setMetric(final CharSequence key, final long value) { - synchronized (unsafeTags) { - unsafeTags.set(key.toString(), value); + synchronized (knownTags) { + routeSet(key.toString(), value); } } public void setMetric(final CharSequence key, final float value) { - synchronized (unsafeTags) { - unsafeTags.set(key.toString(), value); + synchronized (knownTags) { + routeSet(key.toString(), value); } } public void setMetric(final CharSequence key, final double value) { - synchronized (unsafeTags) { - unsafeTags.set(key.toString(), value); + synchronized (knownTags) { + routeSet(key.toString(), value); } } @@ -1007,24 +1167,64 @@ public void setMetric(final TagMap.EntryReader entry) { return; } - synchronized (unsafeTags) { - unsafeTags.set(entry); + synchronized (knownTags) { + routePut(entry); + } + } + + /* + * Route a tag that interception let through, by name: resolved once to the store that holds it. + */ + + private void routeSet(String tag, Object value) { + routeSet(KnownTagCodec.keyOf(tag), tag, value); + } + + private void routeSet(String tag, CharSequence value) { + routeSet(KnownTagCodec.keyOf(tag), tag, value); + } + + private void routeSet(String tag, int value) { + routeSet(KnownTagCodec.keyOf(tag), tag, value); + } + + private void routeSet(String tag, long value) { + routeSet(KnownTagCodec.keyOf(tag), tag, value); + } + + private void routeSet(String tag, float value) { + routeSet(KnownTagCodec.keyOf(tag), tag, value); + } + + private void routeSet(String tag, double value) { + routeSet(KnownTagCodec.keyOf(tag), tag, value); + } + + /** Stores {@code entry}, sharing its Entry where the store keeps Entry objects. */ + private void routePut(TagMap.EntryReader entryReader) { + if (entryReader.tagId() != 0) { + knownTags.put(entryReader); // no Entry built for a dense store, or for a flyweight from one + } else { + TagMap.Entry entry = entryReader.entry(); + if (entry != null) { // a reader with no entry is a no-op, as for TagMap.set + customTagsForWrite().put(entry); + } } } public void removeTag(String tag) { if (tagEquals(tag, Tags.SPAN_KIND)) { - // Clear the cached ordinal; unsafeTags still needs to be updated below. + // Clear the cached ordinal; the tag store still needs to be updated below. spanKindOrdinal = SPAN_KIND_UNSET; } - synchronized (unsafeTags) { - unsafeTags.remove(tag); + synchronized (knownTags) { + routeRemoval(tag); } } /** - * Removes a known tag by id rather than by its name. An id the id-keyed setters ignore is ignored - * here too. + * Removes a known tag by id rather than by its name. An id that names no known tag is ignored, + * like the id-keyed setters do. * * @param tagId a {@code KnownTags.*_ID} constant */ @@ -1035,9 +1235,19 @@ public void removeTag(long tagId) { if (tagId == KnownTags.SPAN_KIND_ID) { spanKindOrdinal = SPAN_KIND_UNSET; } - synchronized (unsafeTags) { - unsafeTags.getAndRemove(tagId); + synchronized (knownTags) { + knownTags.remove(tagId); + } + } + + /** Whether {@code entry}'s tag is already on this span. */ + private boolean isPresent(TagMap.EntryReader entry) { + long key = entry.tagId(); // the reader already carries its id: no name lookup + if (key != 0) { + return knownTags.getValue(key) != null; } + HashStore custom = customTags; + return custom != null && custom.get(entry.tag()) != null; } /** @@ -1062,8 +1272,8 @@ public void setTag(final String tag, final Object value) { } else if (null == value) { removeTag(tag); } else if (!tagInterceptor.interceptCustomTag(this, tag, value)) { - synchronized (unsafeTags) { - unsafeTags.set(tag, value); + synchronized (knownTags) { + customTagsForWrite().set(tag, value); // already known to be custom } } } @@ -1078,8 +1288,8 @@ public void setTag(final String tag, final String value) { } else if (null == value) { removeTag(tag); } else if (!tagInterceptor.interceptCustomTag(this, tag, value)) { - synchronized (unsafeTags) { - unsafeTags.set(tag, value); + synchronized (knownTags) { + customTagsForWrite().set(tag, value); // already known to be custom } } } @@ -1095,10 +1305,11 @@ public void setTag(final long tagId, final Object value) { } if (null == value) { removeTag(tagId); - } else if (!tagInterceptor.needsIntercept(tagId) - || !tagInterceptor.interceptTag(this, tagId, value)) { - synchronized (unsafeTags) { - unsafeTags.internals().setKnown(tagId, value); + return; + } + if (!tagInterceptor.needsIntercept(tagId) || !tagInterceptor.interceptTag(this, tagId, value)) { + synchronized (knownTags) { + knownTags.set(tagId, value); } } } @@ -1109,10 +1320,11 @@ public void setTag(final long tagId, final CharSequence value) { } if (null == value) { removeTag(tagId); - } else if (!tagInterceptor.needsIntercept(tagId) - || !tagInterceptor.interceptTag(this, tagId, value)) { - synchronized (unsafeTags) { - unsafeTags.internals().setKnown(tagId, value); + return; + } + if (!tagInterceptor.needsIntercept(tagId) || !tagInterceptor.interceptTag(this, tagId, value)) { + synchronized (knownTags) { + knownTags.set(tagId, value); } } } @@ -1124,8 +1336,8 @@ public void setTag(final long tagId, final boolean value) { if (tagInterceptor.needsIntercept(tagId)) { this.setBox(tagId, value); } else { - synchronized (unsafeTags) { - unsafeTags.internals().setKnown(tagId, value); + synchronized (knownTags) { + knownTags.set(tagId, value); } } } @@ -1137,8 +1349,8 @@ public void setTag(final long tagId, final int value) { if (tagInterceptor.needsIntercept(tagId)) { this.setBox(tagId, value); } else { - synchronized (unsafeTags) { - unsafeTags.internals().setKnown(tagId, value); + synchronized (knownTags) { + knownTags.set(tagId, value); } } } @@ -1150,8 +1362,8 @@ public void setTag(final long tagId, final long value) { if (tagInterceptor.needsIntercept(tagId)) { this.setBox(tagId, value); } else { - synchronized (unsafeTags) { - unsafeTags.internals().setKnown(tagId, value); + synchronized (knownTags) { + knownTags.set(tagId, value); } } } @@ -1163,8 +1375,8 @@ public void setTag(final long tagId, final float value) { if (tagInterceptor.needsIntercept(tagId)) { this.setBox(tagId, value); } else { - synchronized (unsafeTags) { - unsafeTags.internals().setKnown(tagId, value); + synchronized (knownTags) { + knownTags.set(tagId, value); } } } @@ -1176,8 +1388,8 @@ public void setTag(final long tagId, final double value) { if (tagInterceptor.needsIntercept(tagId)) { this.setBox(tagId, value); } else { - synchronized (unsafeTags) { - unsafeTags.internals().setKnown(tagId, value); + synchronized (knownTags) { + knownTags.set(tagId, value); } } } @@ -1189,8 +1401,8 @@ public void setTag(TagMap.EntryReader entry) { // the interceptor prechecks the entry, to avoid boxing if (!tagInterceptor.interceptTag(this, entry)) { - synchronized (unsafeTags) { - unsafeTags.set(entry); + synchronized (knownTags) { + routePut(entry); } } } @@ -1209,8 +1421,8 @@ public void setTag(TagMap.EntryReader entry) { */ private void setBox(long tagId, Object box) { if (!tagInterceptor.interceptTag(this, tagId, box)) { - synchronized (unsafeTags) { - unsafeTags.internals().setKnown(tagId, box); + synchronized (knownTags) { + knownTags.set(tagId, box); } } } @@ -1224,8 +1436,8 @@ public void setTag(final String tag, final boolean value) { setTag(tagId, value); } else if (!tagInterceptor.needsInterceptCustomTag(tag) || !tagInterceptor.interceptCustomTag(this, tag, value)) { - synchronized (unsafeTags) { - unsafeTags.set(tag, value); + synchronized (knownTags) { + customTagsForWrite().set(tag, value); // already known to be custom } } } @@ -1239,8 +1451,8 @@ public void setTag(final String tag, final int value) { setTag(tagId, value); } else if (!tagInterceptor.needsInterceptCustomTag(tag) || !tagInterceptor.interceptCustomTag(this, tag, value)) { - synchronized (unsafeTags) { - unsafeTags.set(tag, value); + synchronized (knownTags) { + customTagsForWrite().set(tag, value); // already known to be custom } } } @@ -1254,8 +1466,8 @@ public void setTag(final String tag, final long value) { setTag(tagId, value); } else if (!tagInterceptor.needsInterceptCustomTag(tag) || !tagInterceptor.interceptCustomTag(this, tag, value)) { - synchronized (unsafeTags) { - unsafeTags.set(tag, value); + synchronized (knownTags) { + customTagsForWrite().set(tag, value); // already known to be custom } } } @@ -1269,8 +1481,8 @@ public void setTag(final String tag, final float value) { setTag(tagId, value); } else if (!tagInterceptor.needsInterceptCustomTag(tag) || !tagInterceptor.interceptCustomTag(this, tag, value)) { - synchronized (unsafeTags) { - unsafeTags.set(tag, value); + synchronized (knownTags) { + customTagsForWrite().set(tag, value); // already known to be custom } } } @@ -1284,8 +1496,8 @@ public void setTag(final String tag, final double value) { setTag(tagId, value); } else if (!tagInterceptor.needsInterceptCustomTag(tag) || !tagInterceptor.interceptCustomTag(this, tag, value)) { - synchronized (unsafeTags) { - unsafeTags.set(tag, value); + synchronized (knownTags) { + customTagsForWrite().set(tag, value); // already known to be custom } } } @@ -1299,7 +1511,7 @@ void setAllTags(final TagMap map, boolean needsIntercept) { return; } - synchronized (unsafeTags) { + synchronized (knownTags) { if (needsIntercept) { // forEach out-performs the iterator of TagMap // Taking advantage of ability to pass through other context arguments @@ -1308,11 +1520,11 @@ void setAllTags(final TagMap map, boolean needsIntercept) { this, (ctx, tagEntry) -> { if (!ctx.tagInterceptor.interceptTag(ctx, tagEntry)) { - ctx.unsafeTags.set(tagEntry); + ctx.routePut(tagEntry); } }); } else { - unsafeTags.putAll(map); + routePutAll(map); } } } @@ -1322,20 +1534,20 @@ void setAllTags(final TagMap.Ledger ledger) { return; } - synchronized (unsafeTags) { + synchronized (knownTags) { for (final TagMap.EntryChange entryChange : ledger) { String tag = entryChange.tag(); if (entryChange.isRemoval()) { if (tagEquals(tag, Tags.SPAN_KIND)) { - // mirror removeTag(String): keep the cached ordinal in sync with unsafeTags + // mirror removeTag(String): keep the cached ordinal in sync with the tags spanKindOrdinal = SPAN_KIND_UNSET; } - unsafeTags.remove(tag); + routeRemoval(tag); } else { TagMap.Entry entry = (TagMap.Entry) entryChange; if (!tagInterceptor.interceptTag(this, entry)) { - unsafeTags.set(entry); + routePut(entry); } } } @@ -1348,9 +1560,14 @@ void setAllTags(final Map map) { } else if (map instanceof TagMap) { setAllTags((TagMap) map); } else if (!map.isEmpty()) { - synchronized (unsafeTags) { + synchronized (knownTags) { for (final Map.Entry tag : map.entrySet()) { - if (!tagInterceptor.interceptTag(this, tag.getKey(), tag.getValue())) { + long tagId = KnownTagCodec.keyOf(tag.getKey()); + boolean intercepted = + tagId != 0 + ? tagInterceptor.interceptTag(this, tagId, tag.getValue()) + : tagInterceptor.interceptCustomTag(this, tag.getKey(), tag.getValue()); + if (!intercepted) { unsafeSetTag(tag.getKey(), tag.getValue()); } } @@ -1359,19 +1576,19 @@ void setAllTags(final Map map) { } void unsafeSetTag(final String tag, final Object value) { - unsafeTags.set(tag, value); + routeSet(tag, value); } void unsafeSetTag(final String tag, final CharSequence value) { - unsafeTags.set(tag, value); + routeSet(tag, value); } void unsafeSetTag(final String tag, final byte value) { - unsafeTags.set(tag, value); + routeSet(tag, value); } void unsafeSetTag(final String tag, final double value) { - unsafeTags.set(tag, value); + routeSet(tag, value); } Object getTag(final String key) { @@ -1388,7 +1605,7 @@ Object getTag(final String key) { return getSpanKindString(); default: Object value; - synchronized (unsafeTags) { + synchronized (knownTags) { value = unsafeGetTag(key); } // maintain previously observable type of http url :| @@ -1398,27 +1615,45 @@ Object getTag(final String key) { /* * SpanStorage: the span's tags, loaded and stored with no interception. - * Unsynchronized, like unsafeGetTag: callers hold the span's tag lock, as post-processing does. + * Unsynchronized, like unsafeGetTag: callers hold the knownTags lock, as post-processing does. */ @Override public Object load(String tag) { - return unsafeTags.getObject(tag); + long key = KnownTagCodec.keyOf(tag); + if (key != 0) { + return knownTags.getValue(key); + } + HashStore custom = customTags; + TagMap.Entry entry = custom == null ? null : custom.get(tag); + return entry == null ? null : entry.objectValue(); } @Override public String loadString(String tag) { - return unsafeTags.getString(tag); + long key = KnownTagCodec.keyOf(tag); + if (key != 0) { + return knownTags.getString(key); + } + HashStore custom = customTags; + TagMap.Entry entry = custom == null ? null : custom.get(tag); + return entry == null ? null : entry.stringValue(); } @Override public void store(String tag, Object value) { - unsafeTags.set(tag, value); + if (value == null) { + routeRemoval(tag); + } else { + routeSet(tag, value); + } } @Override public void store(TagMap.EntryReader entry) { - unsafeTags.set(entry); + if (entry != null) { + routePut(entry); + } } /** @@ -1429,7 +1664,7 @@ public void store(TagMap.EntryReader entry) { * @return the value associated with the tag */ public Object unsafeGetTag(final String tag) { - Object value = unsafeTags.getObject(tag); + Object value = load(tag); return value != null ? value : unsafeGetDirectionScopedTag(tag); } @@ -1448,8 +1683,7 @@ private Object unsafeGetDirectionScopedTag(final String tag) { if (tagId == 0) { return null; } - TagMap.Entry entry = unsafeTags.getEntry(tagId); - return entry == null ? null : entry.objectValue(); + return knownTags.getValue(tagId); } /** @@ -1472,8 +1706,8 @@ static int directionOfKind(byte spanKindOrdinal) { @Deprecated public TagMap getTags() { - synchronized (unsafeTags) { - TagMap tags = unsafeTags.copy(); + synchronized (knownTags) { + TagMap tags = tagView().copy(); tags.put(DDTags.THREAD_ID, threadId); // maintain previously observable type of the thread name :| @@ -1525,7 +1759,7 @@ public void setMetaStruct(final String field, final T value) { } void earlyProcessTags(AppendableSpanLinks links) { - synchronized (unsafeTags) { + synchronized (knownTags) { TagsPostProcessorFactory.eagerProcessor().processTags(this, this, links); } } @@ -1555,15 +1789,18 @@ void processTagsAndBaggage( // This is a compromise to avoid... // - creating an extra wrapper object that would create significant allocation // - implementing an interface to read the spans that require making the read method public - synchronized (unsafeTags) { + synchronized (knownTags) { + // One live view of the tag stores, for the post-processors and the serializer + TagMap tags = tagView(); + // Tags - TagsPostProcessorFactory.lazyProcessor().processTags(unsafeTags, this, restrictedSpan); + TagsPostProcessorFactory.lazyProcessor().processTags(tags, this, restrictedSpan); // Links if (injectLinksAsTags) { String linksTag = DDSpanLink.toTag(restrictedSpan.getLinks()); if (linksTag != null) { - unsafeTags.set(SPAN_LINKS, linksTag); + tags.set(SPAN_LINKS, linksTag); } } @@ -1572,7 +1809,7 @@ void processTagsAndBaggage( if (injectEventsAsTags) { String eventsTag = spanEventsToTag(events); if (eventsTag != null) { - unsafeTags.set(SPAN_EVENTS, eventsTag); + tags.set(SPAN_EVENTS, eventsTag); } } @@ -1597,7 +1834,7 @@ void processTagsAndBaggage( new Metadata( threadId, threadName, - unsafeTags, + tags, baggageItemsWithPropagationTags, getSamplingPriority(), measured, @@ -1610,6 +1847,7 @@ void processTagsAndBaggage( Config.get().isOtlpTracesExportEnabled() ? OTLP_EXPORT_TRUE : OTLP_EXPORT_FALSE, restrictedSpan.getLinks(), events)); + adoptViewStores(tags); } } @@ -1760,7 +1998,7 @@ public String toString() { s.append(" *measured*"); } - synchronized (unsafeTags) { + synchronized (knownTags) { s.append(" tags=").append(new TreeMap<>(getTags())); } return s.toString(); diff --git a/dd-trace-core/src/test/java/datadog/trace/common/writer/PayloadDispatcherImplTest.java b/dd-trace-core/src/test/java/datadog/trace/common/writer/PayloadDispatcherImplTest.java index cf756003e7c..41b53b7ab5d 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/writer/PayloadDispatcherImplTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/writer/PayloadDispatcherImplTest.java @@ -217,7 +217,6 @@ DDSpan realSpan() throws Exception { Collections.emptyMap(), false, "", - 0, trace, null, null, diff --git a/dd-trace-core/src/test/java/datadog/trace/core/DDCoreJavaSpecification.java b/dd-trace-core/src/test/java/datadog/trace/core/DDCoreJavaSpecification.java index d5fead39ea7..b83d9c16bd4 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/DDCoreJavaSpecification.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/DDCoreJavaSpecification.java @@ -115,7 +115,6 @@ protected DDSpan buildSpan( null, false, spanType, - 0, tracer.createTraceCollector(DDTraceId.ONE), null, null, diff --git a/dd-trace-core/src/test/java/datadog/trace/core/DDSpanContextStoresTest.java b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanContextStoresTest.java new file mode 100644 index 00000000000..d098d638517 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanContextStoresTest.java @@ -0,0 +1,297 @@ +package datadog.trace.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.api.KnownTags; +import datadog.trace.bootstrap.instrumentation.api.SpanPrototype; +import datadog.trace.common.writer.ListWriter; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * How a span routes tags to its own known and custom stores: custom tags by name with every value + * type, primitive sets of intercepted tags by id, map writes, and the custom store borrowed from + * the tracer-level tags until the span's first custom write. + */ +class DDSpanContextStoresTest extends DDCoreJavaSpecification { + private CoreTracer tracer; + + @BeforeEach + void setup() { + tracer = + tracerBuilder() + .writer(new ListWriter()) + .defaultSpanTags(Collections.singletonMap("global.custom", "from-tracer")) + .build(); + } + + @AfterEach + void tearDown() { + tracer.close(); + } + + private DDSpan start() { + return (DDSpan) tracer.startSpan("test-instr", "stores.op"); + } + + @Test + void aReaderWithNoEntryIsANoOp() { + DDSpan span = start(); + try { + span.setTag("http.route", "/users"); + span.setTag("custom.kept", "kept"); + span.spanContext().setTag(new NoEntryReader("http.route", KnownTags.HTTP_ROUTE_ID)); + span.spanContext().setTag(new NoEntryReader("custom.kept", 0L)); + span.spanContext().setMetric(new NoEntryReader("custom.metric", 0L)); + + assertEquals("/users", span.getTag("http.route")); + assertEquals("kept", span.getTag("custom.kept")); + assertNull(span.getTag("custom.metric")); + } finally { + span.finish(); + } + } + + @Test + void customTagsKeepEveryValueType() { + DDSpan span = start(); + try { + span.setTag("custom.chars", (CharSequence) "chars"); + span.setTag("custom.bool", true); + span.setTag("custom.int", 7); + span.setTag("custom.long", 8L); + span.setTag("custom.float", 1.5f); + span.setTag("custom.double", 2.5d); + + assertEquals("chars", span.getTag("custom.chars")); + assertEquals(true, span.getTag("custom.bool")); + assertEquals(7, span.getTag("custom.int")); + assertEquals(8L, span.getTag("custom.long")); + assertEquals(1.5f, span.getTag("custom.float")); + assertEquals(2.5d, span.getTag("custom.double")); + } finally { + span.finish(); + } + } + + @Test + void interceptedTagsSetByIdAsPrimitivesAreRouted() { + DDSpan span = start(); + try { + span.spanContext().setTag(KnownTags.HTTP_STATUS_CODE_ID, 404); + assertEquals(404, span.getHttpStatusCode()); + span.spanContext().setTag(KnownTags.HTTP_STATUS_CODE_ID, 503L); + assertEquals(503, span.getHttpStatusCode()); + + span.spanContext().setTag(KnownTags.DD_MEASURED_ID, true); + assertTrue(span.isMeasured()); + + span.spanContext().setTag(KnownTags.DD1_SR_EAUSR_ID, 0.25d); + span.spanContext().setTag(KnownTags.DD1_SR_EAUSR_ID, 0.5f); + assertEquals(0.5f, ((Number) span.getTag("_dd1.sr.eausr")).floatValue()); + } finally { + span.finish(); + } + } + + @Test + void knownTagsSetByIdAsPrimitivesWithoutInterception() { + DDSpan span = start(); + try { + span.spanContext().setTag(KnownTags.PEER_PORT_ID, 5432L); + span.spanContext().setTag(KnownTags.DB_INSTANCE_ID, 1.5f); + span.spanContext().setTag(KnownTags.DB_POOL_NAME_ID, 2.5d); + assertEquals(5432L, span.getTag("peer.port")); + assertEquals(1.5f, span.getTag("db.instance")); + assertEquals(2.5d, span.getTag("db.pool.name")); + + // an intercepted tag the interceptor declines is still stored: span.kind records its + // ordinal and keeps the value + span.spanContext().setTag(KnownTags.SPAN_KIND_ID, 1); + assertEquals("1", String.valueOf(span.getTag("span.kind"))); + } finally { + span.finish(); + } + } + + @Test + void metricsWithKnownNamesGoToTheKnownStore() { + DDSpan span = start(); + try { + span.setMetric("db.user", 200); + span.setMetric("peer.port", 8080L); + span.setMetric("db.instance", 0.5f); + span.setMetric("db.pool.name", 0.75d); + span.setMetric("custom.metric", 3L); + assertEquals(200, span.getTag("db.user")); + assertEquals(8080L, span.getTag("peer.port")); + assertEquals(0.5f, span.getTag("db.instance")); + assertEquals(0.75d, span.getTag("db.pool.name")); + assertEquals(3L, span.getTag("custom.metric")); + } finally { + span.finish(); + } + } + + @Test + void aMapWriteRoutesKnownAndCustomTags() { + DDSpan span = start(); + try { + Map tags = new LinkedHashMap<>(); + tags.put("http.route", "/users"); + tags.put("custom.from.map", "value"); + tags.put("custom.chars.from.map", new StringBuilder("chars")); + span.setAllTags(tags); + span.spanContext().unsafeSetTag("custom.unsafe", (CharSequence) "unsafe"); + + assertEquals("/users", span.getTag("http.route")); + assertEquals("value", span.getTag("custom.from.map")); + assertEquals("chars", String.valueOf(span.getTag("custom.chars.from.map"))); + assertEquals("unsafe", span.getTag("custom.unsafe")); + } finally { + span.finish(); + } + } + + @Test + void removingACustomTagTheSpanDoesNotHaveKeepsBorrowing() { + DDSpan span = start(); + try { + assertEquals("from-tracer", span.getTag("global.custom")); + span.removeTag("never.set"); // nothing to hide: the borrowed store stays as it is + assertEquals("from-tracer", span.getTag("global.custom")); + } finally { + span.finish(); + } + } + + @Test + void removingABorrowedCustomTagHidesItFromThisSpanOnly() { + DDSpan span = start(); + DDSpan other = start(); + try { + span.removeTag("global.custom"); // copies the borrowed store, then hides the tag + assertNull(span.getTag("global.custom")); + assertEquals("from-tracer", other.getTag("global.custom"), "the tracer-level tag stays"); + } finally { + span.finish(); + other.finish(); + } + } + + @Test + void aPrototypesCustomTagIsSeededOnlyWhereAbsent() { + SpanPrototype prototype = + SpanPrototype.builder() + .initInstrumentationName("test-instr") + .initOperationName("stores.proto") + .initTag("global.custom", "from-prototype") + .initTag("proto.custom", "seeded") + .build(); + DDSpan span = (DDSpan) tracer.startSpan(prototype, null); + try { + // the tracer-level tag is already visible, so the prototype's default doesn't replace it + assertEquals("from-tracer", span.getTag("global.custom")); + assertEquals("seeded", span.getTag("proto.custom")); + } finally { + span.finish(); + } + } + + /** An EntryReader with no entry and no value, as a caller outside the tracer may pass. */ + static final class NoEntryReader implements datadog.trace.api.TagMap.EntryReader { + private final String tag; + private final long tagId; + + NoEntryReader(String tag, long tagId) { + this.tag = tag; + this.tagId = tagId; + } + + @Override + public String tag() { + return tag; + } + + @Override + public long tagId() { + return tagId; + } + + @Override + public byte type() { + return datadog.trace.api.TagMap.EntryReader.OBJECT; + } + + @Override + public boolean is(byte type) { + return false; + } + + @Override + public boolean isNumericPrimitive() { + return false; + } + + @Override + public boolean isNumber() { + return false; + } + + @Override + public boolean isObject() { + return true; + } + + @Override + public Object objectValue() { + return null; + } + + @Override + public String stringValue() { + return null; + } + + @Override + public boolean booleanValue() { + return false; + } + + @Override + public int intValue() { + return 0; + } + + @Override + public long longValue() { + return 0; + } + + @Override + public float floatValue() { + return 0; + } + + @Override + public double doubleValue() { + return 0; + } + + @Override + public java.util.Map.Entry mapEntry() { + return null; + } + + @Override + public datadog.trace.api.TagMap.Entry entry() { + return null; + } + } +} diff --git a/dd-trace-core/src/test/java/datadog/trace/core/DDSpanContextTagViewTest.java b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanContextTagViewTest.java new file mode 100644 index 00000000000..561dfc33bcc --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanContextTagViewTest.java @@ -0,0 +1,60 @@ +package datadog.trace.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import datadog.trace.api.TagMap; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import java.util.Collections; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +/** + * The span's cached view of its tag stores keeps the stores it was created over, so a custom store + * the span creates afterwards must replace the view rather than be missed by it. + */ +class DDSpanContextTagViewTest extends DDCoreJavaSpecification { + private CoreTracer tracer; + + @AfterEach + void cleanup() { + if (tracer != null) { + tracer.close(); + } + } + + @Test + void aCustomTagWrittenAfterTheViewIsCreatedIsSeenThroughANewView() { + tracer = tracerBuilder().build(); + AgentSpan span = tracer.buildSpan("test", "op").start(); + try { + DDSpanContext context = (DDSpanContext) span.spanContext(); + assertEquals(null, context.getTags().get("custom.first")); // creates the view + + span.setTag("custom.first", "a"); // the span's first custom store: the view is dropped + span.setTag("custom.second", "b"); + + TagMap tags = context.getTags(); + assertEquals("a", tags.get("custom.first")); + assertEquals("b", tags.get("custom.second")); + } finally { + span.finish(); + } + } + + @Test + void aCustomTagCopiedInFromAMapIsStoredWithoutAView() { + tracer = tracerBuilder().build(); + AgentSpan span = tracer.buildSpan("test", "op").start(); + try { + DDSpanContext context = (DDSpanContext) span.spanContext(); + context.setAllTags(TagMap.fromMap(Collections.singletonMap("custom.copied", "c"))); + span.setTag("custom.later", "d"); + + TagMap tags = context.getTags(); + assertEquals("c", tags.get("custom.copied")); + assertEquals("d", tags.get("custom.later")); + } finally { + span.finish(); + } + } +} diff --git a/dd-trace-core/src/test/java/datadog/trace/core/DDSpanSerializationTest.java b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanSerializationTest.java index b8ca25a6cc5..1ab43205181 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/DDSpanSerializationTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanSerializationTest.java @@ -217,7 +217,6 @@ void serializeTraceWithBaggageAndTagsCorrectlyV04( baggage, false, null, - tags.size(), tracer.createTraceCollector(DDTraceId.ONE), null, null, @@ -294,7 +293,6 @@ void serializeTraceWithBaggageAndTagsCorrectlyV05( baggage, false, null, - tags.size(), tracer.createTraceCollector(DDTraceId.ONE), null, null, @@ -353,7 +351,7 @@ void serializeTraceWithConfiguredBaggageAndTagsCorrectlyV1(String scenario, bool w3cBaggageItems.put("ignored", "ignored-value"); DDSpanContext context = - createSpanContext(tracer, baggage, Baggage.create(w3cBaggageItems), injectBaggage, true, 1); + createSpanContext(tracer, baggage, Baggage.create(w3cBaggageItems), injectBaggage, true); context.setTag("span-tag", "span-value"); DDSpan span = DDSpan.create("test", 0, context, null); @@ -377,7 +375,7 @@ void serializeTraceWithConfiguredBaggageAndTagsCorrectlyV1(String scenario, bool @Test void serializeTraceWithSpanLinksAsStructuredLinksOnlyV1() throws Exception { CoreTracer tracer = tracerBuilder().writer(new ListWriter()).build(); - DDSpanContext context = createSpanContext(tracer, Collections.emptyMap(), null, true, true, 0); + DDSpanContext context = createSpanContext(tracer, Collections.emptyMap(), null, true, true); DDSpan span = DDSpan.create("test", 0, context, null); Map linkAttributes = new HashMap<>(); @@ -407,7 +405,7 @@ void serializeTraceWithSpanLinksAsStructuredLinksOnlyV1() throws Exception { @Test void serializeTraceWithSpanEventsAsStructuredEventsOnlyV1() throws Exception { CoreTracer tracer = tracerBuilder().writer(new ListWriter()).build(); - DDSpanContext context = createSpanContext(tracer, Collections.emptyMap(), null, true, true, 1); + DDSpanContext context = createSpanContext(tracer, Collections.emptyMap(), null, true, true); DDSpan span = DDSpan.create("test", 0, context, null); span.addEvent("event.one", typedAttributes(), 1234567890L, NANOSECONDS); span.addEvent("event.two", Collections.emptyMap(), 1234567891L, NANOSECONDS); @@ -439,7 +437,7 @@ void serializeTraceWithSpanEventsAsStructuredEventsOnlyV1() throws Exception { }) void serializeTraceWithSpanEventsAsEventsTag(String protocol) throws Exception { CoreTracer tracer = tracerBuilder().writer(new ListWriter()).build(); - DDSpanContext context = createSpanContext(tracer, Collections.emptyMap(), null, true, true, 1); + DDSpanContext context = createSpanContext(tracer, Collections.emptyMap(), null, true, true); DDSpan span = DDSpan.create("test", 0, context, null); span.addEvent("event.one", typedAttributes(), 1234567890L, NANOSECONDS); span.addEvent("event \"two\"", Collections.emptyMap(), 1234567891L, NANOSECONDS); @@ -536,7 +534,6 @@ void serializeTraceWithFlatMapTagV04() throws Exception { null, false, null, - 0, tracer.createTraceCollector(DDTraceId.ONE), null, null, @@ -603,7 +600,6 @@ void serializeTraceWithFlatMapTagV05() throws Exception { null, false, null, - 0, tracer.createTraceCollector(DDTraceId.ONE), null, null, @@ -747,7 +743,6 @@ private DDSpanContext createSpanContext( baggage, false, spanType, - 1, tracer.createTraceCollector(DDTraceId.ONE), null, null, @@ -765,8 +760,7 @@ private DDSpanContext createSpanContext( Map baggage, Baggage w3cBaggage, boolean injectBaggage, - boolean injectLinks, - int tagsSize) { + boolean injectLinks) { return new DDSpanContext( DDTraceId.ONE, 1, @@ -782,7 +776,6 @@ private DDSpanContext createSpanContext( w3cBaggage, false, "fakeType", - tagsSize, tracer.createTraceCollector(DDTraceId.ONE), null, null, diff --git a/dd-trace-core/src/test/java/datadog/trace/core/DDSpanTest.java b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanTest.java index 1d252859739..46659a5e9ad 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/DDSpanTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanTest.java @@ -423,7 +423,6 @@ void inferTopLevelFromParentServiceName( Collections.emptyMap(), false, "fakeType", - 0, tracer.createTraceCollector(DDTraceId.ONE), null, null, diff --git a/dd-trace-core/src/test/java/datadog/trace/core/LongRunningTracesTrackerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/LongRunningTracesTrackerTest.java index 27c8156e27f..aab36487f1e 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/LongRunningTracesTrackerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/LongRunningTracesTrackerTest.java @@ -228,7 +228,6 @@ private static DDSpan newSpanOf(PendingTrace trace, int samplingPriority, long t Collections.emptyMap(), false, "fakeType", - 0, trace, null, null, diff --git a/dd-trace-core/src/test/java/datadog/trace/core/PendingTraceBufferTest.java b/dd-trace-core/src/test/java/datadog/trace/core/PendingTraceBufferTest.java index e72e5fa3a7c..99bc9481c50 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/PendingTraceBufferTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/PendingTraceBufferTest.java @@ -530,7 +530,6 @@ private static DDSpan newSpanOf(PendingTrace trace, int samplingPriority, long t Collections.emptyMap(), false, "fakeType", - 0, trace, null, null, @@ -556,7 +555,6 @@ private static DDSpan newSpanOf(DDSpan parent) { Collections.emptyMap(), false, "fakeType", - 0, traceCollector, null, null, diff --git a/dd-trace-core/src/test/java/datadog/trace/core/PendingTraceTest.java b/dd-trace-core/src/test/java/datadog/trace/core/PendingTraceTest.java index 7d47a0b17c5..e4b003cb688 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/PendingTraceTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/PendingTraceTest.java @@ -65,7 +65,6 @@ private DDSpan createSimpleSpanWithID(PendingTrace trace, long id) { Collections.emptyMap(), false, "", - 0, trace, null, null, diff --git a/dd-trace-core/src/test/java/datadog/trace/core/RootSpanTagsReadThroughTest.java b/dd-trace-core/src/test/java/datadog/trace/core/RootSpanTagsReadThroughTest.java new file mode 100644 index 00000000000..ad9c16a0b9b --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/RootSpanTagsReadThroughTest.java @@ -0,0 +1,90 @@ +package datadog.trace.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import datadog.trace.api.KnownTags; +import datadog.trace.api.config.TracerConfig; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Properties; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +/** + * With builder tags taking precedence, a root span reads its root-span tags through the shared + * trace-level parent instead of holding copies: the same values, none of them stored locally, so + * the span's own store keeps only span-level tags (the slot coloring lets a span-level and a + * trace-level tag share a slot bit). + */ +class RootSpanTagsReadThroughTest extends DDCoreJavaSpecification { + private static final String CUSTOM_ROOT_TAG = "only_root"; + + private CoreTracer tracer; + + @AfterEach + void cleanup() { + if (tracer != null) { + tracer.close(); + } + } + + private CoreTracer tracer(boolean builderTagsPrecedence) { + Map rootTags = new LinkedHashMap<>(); + rootTags.put(KnownTags.LANGUAGE_NAME, "jvm"); + rootTags.put(KnownTags.PROCESS_ID_NAME, 42L); + rootTags.put(CUSTOM_ROOT_TAG, "value"); + Properties properties = new Properties(); + properties.setProperty( + TracerConfig.TRACE_BUILDER_TAGS_PRECEDENCE_ENABLED, String.valueOf(builderTagsPrecedence)); + return tracerBuilder().withProperties(properties).localRootSpanTags(rootTags).build(); + } + + @Test + void aRootSpanReadsItsRootTagsThroughTheTraceLevelParent() throws Exception { + tracer = tracer(false); + int copiedLocally = localKnownTagCount(rootSpanOf(tracer)); + tracer.close(); + + tracer = tracer(true); + DDSpan root = rootSpanOf(tracer); + assertEquals("jvm", root.getTag(KnownTags.LANGUAGE_NAME)); + assertEquals(42L, root.getTag(KnownTags.PROCESS_ID_NAME)); + assertEquals("value", root.getTag(CUSTOM_ROOT_TAG)); + assertEquals(copiedLocally - 2, localKnownTagCount(root)); + } + + @Test + void aChildSpanStillGetsNoRootTags() { + tracer = tracer(true); + AgentSpan root = tracer.buildSpan("test", "root").start(); + AgentSpan child = tracer.buildSpan("test", "child").asChildOf(root.spanContext()).start(); + try { + assertNull(child.getTag(KnownTags.LANGUAGE_NAME)); + assertNull(child.getTag(CUSTOM_ROOT_TAG)); + } finally { + child.finish(); + root.finish(); + } + } + + private static DDSpan rootSpanOf(CoreTracer tracer) { + AgentSpan span = tracer.buildSpan("test", "root").start(); + span.finish(); + return (DDSpan) span; + } + + /** The known tags the span's own id store holds, not counting what it reads through. */ + private static int localKnownTagCount(DDSpan span) throws Exception { + Field knownTags = DDSpanContext.class.getDeclaredField("knownTags"); + knownTags.setAccessible(true); + Object ids = knownTags.get(span.spanContext()); + Method computeSize = + Class.forName("datadog.trace.api.IdStore").getDeclaredMethod("computeSize"); + computeSize.setAccessible(true); + return (Integer) computeSize.invoke(ids); + } +} diff --git a/dd-trace-core/src/test/java/datadog/trace/core/propagation/AbstractHttpInjectorTest.java b/dd-trace-core/src/test/java/datadog/trace/core/propagation/AbstractHttpInjectorTest.java index 3098ab217ec..306984b790f 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/propagation/AbstractHttpInjectorTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/propagation/AbstractHttpInjectorTest.java @@ -51,7 +51,6 @@ protected DDSpanContext mockSpanContext( baggage, false, "fakeType", - 0, this.tracer.createTraceCollector(DDTraceId.ONE), null, null, diff --git a/dd-trace-core/src/test/java/datadog/trace/core/taginterceptor/TagInterceptorTest.java b/dd-trace-core/src/test/java/datadog/trace/core/taginterceptor/TagInterceptorTest.java index 6cb01e76f08..235b21d4799 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/taginterceptor/TagInterceptorTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/taginterceptor/TagInterceptorTest.java @@ -1069,7 +1069,8 @@ void exactlyTheTagsWithTheInterceptedBitHaveACase() throws IllegalAccessExceptio boolean hasCase = consumed || !mockingDetails(span).getInvocations().isEmpty(); assertEquals(KnownTagCodec.isIntercepted(tagId), hasCase, field.getName()); - assertEquals(KnownTagCodec.isIntercepted(tagId), interceptor.needsIntercept(tagId)); + assertEquals( + KnownTagCodec.isIntercepted(tagId), interceptor.needsIntercept(tagId), field.getName()); } } diff --git a/gradle/forbiddenApiFilters/instrumentation.txt b/gradle/forbiddenApiFilters/instrumentation.txt index 385cb45b0b2..102a57de753 100644 --- a/gradle/forbiddenApiFilters/instrumentation.txt +++ b/gradle/forbiddenApiFilters/instrumentation.txt @@ -3,11 +3,6 @@ @defaultMessage Use AgentSpan#setServiceName(String, CharSequence) instead to track the source of the service name. The source should typically be the instrumentation name. datadog.trace.bootstrap.instrumentation.api.AgentSpan#setServiceName(java.lang.String) -# TagMap.internals() skips checks the tracer core has already made; it is @Restricted to the core -# (see datadog.apicontract.Restricted). Instrumentation uses TagMap's checked methods. -@defaultMessage TagMap.internals() is restricted to the tracer core; use TagMap's checked methods instead. -datadog.trace.api.TagMap#internals() - # A span's tag stores, how it gets them from a parent, and the span's view of them skip checks the tracer # core has already made; they are @Restricted to the core (see datadog.apicontract.Restricted). # Instrumentation uses TagMap's checked methods. diff --git a/internal-api/src/main/java/datadog/trace/api/TagMap.java b/internal-api/src/main/java/datadog/trace/api/TagMap.java index 27873667bf0..4d485e86612 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -1,7 +1,6 @@ package datadog.trace.api; import datadog.apicontract.Restricted; -import datadog.trace.api.function.NoEscape; import datadog.trace.api.function.TriConsumer; import datadog.trace.api.internal.VisibleForTesting; import java.util.AbstractCollection; @@ -1715,63 +1714,6 @@ public void set(@Nonnull String tag, double value) { * custom tags have no id and use the String setters. */ - /** - * The tracer core's trusted operations on this map, which skip checks the core has already made. - * Restricted to the core; instrumentation uses the checked methods. Goes away once the span owns - * its stores and writes to them directly. - */ - @Restricted(allowedIn = {"datadog.trace.core"}) - public Internals internals() { - return new Internals(this); - } - - /** - * See {@link TagMap#internals()}. Created per call so escape analysis removes it: use it within - * the expression ({@code map.internals().setKnown(...)}) rather than storing it. - */ - @NoEscape - public static final class Internals { - private final TagMap map; - - Internals(TagMap map) { - this.map = map; - } - - /* - * Sets a known tag by id without validating it, unlike TagMap.set(long, ...): the caller has - * already checked KnownTagCodec.isKnown(tagId). - */ - public void setKnown(long tagId, @Nonnull Object value) { - this.map.putKnown(new Entry(tagId, null, Entry.ANY, 0L, value)); - } - - public void setKnown(long tagId, @Nonnull CharSequence value) { - this.map.putKnown(new Entry(tagId, null, EntryReader.OBJECT, 0L, value)); - } - - public void setKnown(long tagId, boolean value) { - this.map.putKnown( - new Entry( - tagId, null, EntryReader.BOOLEAN, Entry.boolean2Prim(value), Boolean.valueOf(value))); - } - - public void setKnown(long tagId, int value) { - this.map.putKnown(new Entry(tagId, null, EntryReader.INT, Entry.int2Prim(value), null)); - } - - public void setKnown(long tagId, long value) { - this.map.putKnown(new Entry(tagId, null, EntryReader.LONG, Entry.long2Prim(value), null)); - } - - public void setKnown(long tagId, float value) { - this.map.putKnown(new Entry(tagId, null, EntryReader.FLOAT, Entry.float2Prim(value), null)); - } - - public void setKnown(long tagId, double value) { - this.map.putKnown(new Entry(tagId, null, EntryReader.DOUBLE, Entry.double2Prim(value), null)); - } - } - /** A null value removes the tag, as {@code setTag(tag, null)} does on a span. */ public void set(long tagId, @Nullable Object value) { if (value == null) { diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapSetByIdTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapSetByIdTest.java index 4634a02da58..ecef85afdd4 100644 --- a/internal-api/src/test/java/datadog/trace/api/TagMapSetByIdTest.java +++ b/internal-api/src/test/java/datadog/trace/api/TagMapSetByIdTest.java @@ -72,40 +72,4 @@ void anUnknownIdIsRejected() { assertThrows(IllegalArgumentException.class, () -> map.set(0L, "value")); assertThrows(IllegalArgumentException.class, () -> map.set(KnownTagCodec.makeTagId(9999), 1)); } - - @Test - void theCoresTrustedSetsStoreWhatTheCheckedSetsStore() { - TagMap checked = TagMap.create(); - checked.set(KnownTags.HTTP_ROUTE_ID, "/users/{id}"); - checked.set(KnownTags.HTTP_STATUS_CODE_ID, 200); - checked.set(KnownTags.HTTP_RESEND_COUNT_ID, 2L); - checked.set(KnownTags.DD_PROFILING_ENABLED_ID, true); - - TagMap trusted = TagMap.create(); - trusted.internals().setKnown(KnownTags.HTTP_ROUTE_ID, "/users/{id}"); - trusted.internals().setKnown(KnownTags.HTTP_STATUS_CODE_ID, 200); - trusted.internals().setKnown(KnownTags.HTTP_RESEND_COUNT_ID, 2L); - trusted.internals().setKnown(KnownTags.DD_PROFILING_ENABLED_ID, true); - - assertEquals(new HashMap<>(checked), new HashMap<>(trusted)); - assertEquals(KnownTags.HTTP_ROUTE_ID, trusted.getEntry(KnownTags.HTTP_ROUTE_NAME).tagId()); - } - - @Test - void theCoresTrustedSetsCoverTheRemainingValueShapes() { - Object component = new StringBuilder("jdbc"); // not a String: exercises the Object overload - TagMap checked = TagMap.create(); - checked.set(KnownTags.COMPONENT_ID, component); - checked.set(KnownTags.DB_POOL_NAME_ID, 1.5f); - checked.set(KnownTags.DB_INSTANCE_ID, 2.25d); - - TagMap trusted = TagMap.create(); - trusted.internals().setKnown(KnownTags.COMPONENT_ID, component); - trusted.internals().setKnown(KnownTags.DB_POOL_NAME_ID, 1.5f); - trusted.internals().setKnown(KnownTags.DB_INSTANCE_ID, 2.25d); - - assertEquals(new HashMap<>(checked), new HashMap<>(trusted)); - assertEquals(1.5f, trusted.getEntry(KnownTags.DB_POOL_NAME_ID).floatValue()); - assertEquals(2.25d, trusted.getEntry(KnownTags.DB_INSTANCE_ID).doubleValue()); - } }