diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/SpanCreationBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/SpanCreationBenchmark.java index c2bcd4a1cfb..c31ad846ee1 100644 --- a/dd-trace-core/src/jmh/java/datadog/trace/core/SpanCreationBenchmark.java +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/SpanCreationBenchmark.java @@ -2,6 +2,7 @@ import static java.util.concurrent.TimeUnit.MICROSECONDS; +import datadog.trace.api.KnownTags; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.SpanPrototype; import datadog.trace.bootstrap.instrumentation.api.Tags; @@ -222,6 +223,26 @@ public void webServerSpanViaBuilder() { span.finish(); } + /** + * {@link #webServerSpanViaBuilder}, setting each builder tag by its {@link KnownTags} id: no name + * lookup when the tag is recorded, and the ledger stores the id directly. + */ + @Benchmark + public void webServerSpanViaBuilderById() { + AgentSpan span = + tracer + .buildSpan(INSTRUMENTATION_NAME, SERVER_OPERATION_NAME) + .withTag(KnownTags.COMPONENT_ID, COMPONENT_VALUE) + .withTag(KnownTags.SPAN_KIND_ID, Tags.SPAN_KIND_SERVER) + .withTag(KnownTags.HTTP_METHOD_ID, HTTP_METHOD_VALUE) + .withTag(KnownTags.HTTP_ROUTE_ID, HTTP_ROUTE_VALUE) + .withTag(KnownTags.HTTP_URL_ID, HTTP_URL_VALUE) + .withTag(KnownTags.HTTP_STATUS_CODE_ID, HTTP_STATUS_VALUE) + .withTag(KnownTags.PEER_PORT_ID, PEER_PORT_VALUE) + .start(); + span.finish(); + } + /** JDBC/DB-client-shaped span: create -> set the typical DB known tags (9) -> finish. */ @Benchmark public void jdbcClientSpan() { 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 9ed3f34aceb..ccc867858e1 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 @@ -43,6 +43,7 @@ import datadog.trace.api.EndpointTracker; import datadog.trace.api.IdGenerationStrategy; import datadog.trace.api.InstrumenterConfig; +import datadog.trace.api.KnownTagCodec; import datadog.trace.api.Pair; import datadog.trace.api.TagMap; import datadog.trace.api.TraceConfig; @@ -2075,6 +2076,24 @@ public final CoreSpanBuilder withTag(final String tag, final Object value) { return this; } + @Override + public final CoreSpanBuilder withTag(final long tagId, final Object value) { + if (!KnownTagCodec.isKnown(tagId)) { + return this; + } + TagMap.Ledger tagLedger = this.tagLedger; + if (tagLedger == null) { + this.tagLedger = tagLedger = TagMap.ledger(); + } + if (value == null) { + // see withTag(String, Object): only a removal of a tag this builder set is recorded + tagLedger.smartRemove(tagId); + } else { + tagLedger.set(tagId, value); + } + return this; + } + @Override public final AgentTracer.SpanBuilder withRequestContextData( RequestContextSlot slot, T data) { 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 9281c377afb..13b12e5d079 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 @@ -1322,22 +1322,41 @@ void setAllTags(final TagMap.Ledger ledger) { } synchronized (unsafeTags) { - 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 - spanKindOrdinal = SPAN_KIND_UNSET; - } - unsafeTags.remove(tag); - } else { - TagMap.Entry entry = (TagMap.Entry) entryChange; + // Interception needs an intercepted id in the ledger, or a split-by-tags entry, which is + // config rather than an id bit. Without either, every change goes straight to storage. + if (ledger.hasInterceptedTags() || tagInterceptor.hasSplitServiceTags()) { + ledger.forEach(this, DDSpanContext::interceptOrApplyLedgerChange); + } else { + ledger.forEach(this, DDSpanContext::applyLedgerChange); + } + } + } - if (!tagInterceptor.interceptTag(this, entry)) { - unsafeTags.set(entry); - } - } + /** Call while holding the {@code unsafeTags} lock. */ + private static void interceptOrApplyLedgerChange( + DDSpanContext ctx, TagMap.EntryChangeReader change) { + if (change.isRemoval() || !ctx.tagInterceptor.interceptTag(ctx, change)) { + applyLedgerChange(ctx, change); + } + } + + /** Call while holding the {@code unsafeTags} lock. */ + private static void applyLedgerChange(DDSpanContext ctx, TagMap.EntryChangeReader change) { + long tagId = change.tagId(); + if (change.isRemoval()) { + if (tagId == KnownTags.SPAN_KIND_ID) { + // mirror removeTag(String): keep the cached ordinal in sync with unsafeTags + ctx.spanKindOrdinal = SPAN_KIND_UNSET; } + if (tagId != 0) { + ctx.unsafeTags.getAndRemove(tagId); + } else { + ctx.unsafeTags.remove(change.tag()); + } + } else if (tagId != 0) { + ctx.unsafeTags.internals().setKnown(tagId, change.objectValue()); + } else { + ctx.unsafeTags.set(change); } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index b487d4665b7..b651d4e9407 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -61,6 +61,9 @@ public class TagInterceptor { /** split-by-tags entries naming custom tags; null when there are none. */ private final Set splitServiceCustomNames; + /** Whether any split-by-tags entry is configured; see {@link #hasSplitServiceTags}. */ + private final boolean hasSplitServiceTags; + private final boolean shouldSet404ResourceName; private final boolean shouldSetUrlResourceAsName; private final boolean jeeSplitByDeployment; @@ -84,6 +87,7 @@ public TagInterceptor( this.inferredServiceName = inferredServiceName; this.splitServiceSerials = knownTagSerials(splitServiceTags); this.splitServiceCustomNames = customTagNames(splitServiceTags); + this.hasSplitServiceTags = !splitServiceTags.isEmpty(); this.ruleFlags = ruleFlags; splitByServletContext = isSplitServiceTag(SERVLET_CONTEXT_ID); @@ -129,6 +133,15 @@ private static Set customTagNames(Set names) { return custom; } + /** + * Whether any split-by-tags entry is configured. split-by-tags is tracer config, not an id bit, + * so a batch of tags needs no interception only when this is false and no id in the batch has + * {@link KnownTagCodec#INTERCEPTED}. + */ + public boolean hasSplitServiceTags() { + return hasSplitServiceTags; + } + /** Requires a known id, whose serial is always in range of the table. */ private boolean isSplitServiceTag(long tagId) { return splitServiceSerials[KnownTagCodec.serialNum(tagId)]; diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreSpanBuilderWithTagByIdTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreSpanBuilderWithTagByIdTest.java new file mode 100644 index 00000000000..7a316930662 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreSpanBuilderWithTagByIdTest.java @@ -0,0 +1,75 @@ +package datadog.trace.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import datadog.trace.api.KnownTagCodec; +import datadog.trace.api.KnownTags; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.common.writer.ListWriter; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** {@code SpanBuilder.withTag(long, ...)}: a builder tag set by id lands like one set by name. */ +class CoreSpanBuilderWithTagByIdTest extends DDCoreJavaSpecification { + private CoreTracer tracer; + + @BeforeEach + void setup() { + tracer = tracerBuilder().writer(new ListWriter()).build(); + } + + @AfterEach + void tearDown() { + tracer.close(); + } + + @Test + void aTagSetByIdIsStoredUnderItsName() { + AgentSpan span = + tracer + .buildSpan("datadog", "op") + .withTag(KnownTags.HTTP_ROUTE_ID, "/users/{id}") + .withTag(KnownTags.HTTP_STATUS_CODE_ID, 200) + .withTag("my.custom.tag", "x") + .start(); + + assertEquals("/users/{id}", span.getTag(KnownTags.HTTP_ROUTE_NAME)); + assertEquals(200, span.getTag(KnownTags.HTTP_STATUS_CODE_NAME)); + assertEquals("x", span.getTag("my.custom.tag")); + span.finish(); + } + + @Test + void anInterceptedTagSetByIdIsRouted() { + AgentSpan span = + tracer.buildSpan("datadog", "op").withTag(KnownTags.RESOURCE_NAME_ID, "GET /x").start(); + + assertEquals("GET /x", span.getResourceName().toString()); + span.finish(); + } + + @Test + void aNullValueRemovesOnlyATagTheBuilderSet() { + AgentSpan span = + tracer + .buildSpan("datadog", "op") + .withTag(KnownTags.HTTP_ROUTE_ID, "/users") + .withTag(KnownTags.HTTP_ROUTE_ID, null) + .withTag(KnownTags.HTTP_METHOD_ID, null) + .start(); + + assertNull(span.getTag(KnownTags.HTTP_ROUTE_NAME)); + span.finish(); + } + + @Test + void anUnknownIdIsIgnored() { + AgentSpan span = + tracer.buildSpan("datadog", "op").withTag(KnownTagCodec.makeTagId(9999), "x").start(); + + assertNull(span.getTag("x")); + span.finish(); + } +} 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 be07fadbeeb..c56f6cba3cc 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -236,6 +236,14 @@ default String openTelemetryTag(int direction) { Entry entry(); } + /** + * A change in a {@link Ledger}: a set, readable as an {@link EntryReader}, or a removal, for + * which only {@link #tag()} and {@link #tagId()} apply. + */ + public interface EntryChangeReader extends EntryReader { + boolean isRemoval(); + } + public static final class Entry extends EntryChange implements Map.Entry, EntryReader { /* @@ -977,17 +985,59 @@ static int _hash(String tag) { * An in-order ledger of changes to be made to a TagMap. * Ledger can also serves as a builder for TagMap-s via build & buildImmutable. */ - public static final class Ledger implements Iterable { - EntryChange[] entryChanges; + /** + * An ordered log of tag changes -- sets and removals -- replayed in order onto a span's tags. + * Changes are kept in parallel arrays rather than as {@link Entry} objects, so recording one + * allocates nothing once the arrays are sized, and a builder that reuses its ledger ({@link + * #reset}) allocates nothing at all. Consumers read changes through a reused {@link + * EntryChangeReader}. + * + *

A known tag is recorded under its id, a custom tag under its name. Two summaries ride along: + * + *

    + *
  • {@code occupancy}, the OR of {@code 1L << slotOf(id)} over recorded known tags, with bit + * 63 also standing for every custom tag. Slots are colored so tags on one span type never + * share one, so {@link #smartRemove} of a tag the ledger never recorded is a one-bit miss. + *
  • {@code idFlags}, the OR of recorded ids' flag bits, so {@link #hasInterceptedTags} tells + * a consumer whether any change may need interception by id. + *
+ * + *

Not thread-safe, like the span builder that owns it. + */ + public static final class Ledger implements Iterable { + /** The value of a recorded removal; never a real tag value. */ + static final Object REMOVED = new Object(); + + /** The occupancy bit every custom tag shares, with the known tags that have no slot. */ + static final long CUSTOM_OCCUPANCY = 1L << 63; + + /** A known tag's id, or a custom tag's {@link Entry#customHash} (upper 32 bits zero). */ + long[] tagHashes; + + /** + * A custom tag's name; null for a known tag, whose name is {@code nameOf(id)}. The array itself + * is allocated on the first custom tag, so a ledger of known tags never allocates it. + */ + String[] tags; + + /** The value set, or {@link #REMOVED}. */ + Object[] values; + int nextPos = 0; boolean containsRemovals = false; + long occupancy = 0L; + long idFlags = 0L; + + /** The reader {@link #forEach} reuses; created on first use. */ + LedgerReader reader; Ledger() { this(8); } Ledger(int size) { - this.entryChanges = new EntryChange[size]; + this.tagHashes = new long[size]; + this.values = new Object[size]; } public boolean isDefinitelyEmpty() { @@ -1008,81 +1058,174 @@ public boolean containsRemovals() { return this.containsRemovals; } + /** + * Whether a recorded known tag carries {@link KnownTagCodec#INTERCEPTED}. Custom tags and + * split-by-tags entries are routed by tracer config, which the caller checks separately. + */ + public boolean hasInterceptedTags() { + return (this.idFlags & KnownTagCodec.INTERCEPTED) != 0L; + } + public Ledger set(String tag, Object value) { - return this.recordEntry(anyEntryFor(tag, value)); + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? this.recordKnown(tagId, value) : this.recordCustom(tag, value); } public Ledger set(String tag, CharSequence value) { - return this.recordEntry(objectEntryFor(tag, value)); + return this.set(tag, (Object) value); } public Ledger set(String tag, boolean value) { - return this.recordEntry(booleanEntryFor(tag, value)); + return this.set(tag, (Object) Boolean.valueOf(value)); } public Ledger set(String tag, int value) { - return this.recordEntry(intEntryFor(tag, value)); + return this.set(tag, (Object) Integer.valueOf(value)); } public Ledger set(String tag, long value) { - return this.recordEntry(longEntryFor(tag, value)); + return this.set(tag, (Object) Long.valueOf(value)); } public Ledger set(String tag, float value) { - return this.recordEntry(floatEntryFor(tag, value)); + return this.set(tag, (Object) Float.valueOf(value)); } public Ledger set(String tag, double value) { - return this.recordEntry(doubleEntryFor(tag, value)); + return this.set(tag, (Object) Double.valueOf(value)); } - public Ledger set(Entry entry) { - return this.recordEntry(entry); + /** Records a known tag by id; rejects an id that names no known tag. */ + public Ledger set(long tagId, Object value) { + return this.recordKnown(requireKnownLedgerId(tagId), value); } - public Ledger remove(String tag) { - return this.recordRemoval(EntryChange.newRemoval(tag)); + public Ledger set(long tagId, CharSequence value) { + return this.set(tagId, (Object) value); } - private Ledger recordEntry(Entry entry) { - this.recordChange(entry); - return this; + public Ledger set(long tagId, boolean value) { + return this.set(tagId, (Object) Boolean.valueOf(value)); } - private Ledger recordRemoval(EntryRemoval entry) { - this.recordChange(entry); - this.containsRemovals = true; + public Ledger set(long tagId, int value) { + return this.set(tagId, (Object) Integer.valueOf(value)); + } - return this; + public Ledger set(long tagId, long value) { + return this.set(tagId, (Object) Long.valueOf(value)); } - private void recordChange(EntryChange entryChange) { - if (this.nextPos >= this.entryChanges.length) { - this.entryChanges = Arrays.copyOf(this.entryChanges, this.entryChanges.length << 1); - } + public Ledger set(long tagId, float value) { + return this.set(tagId, (Object) Float.valueOf(value)); + } - this.entryChanges[this.nextPos++] = entryChange; + public Ledger set(long tagId, double value) { + return this.set(tagId, (Object) Double.valueOf(value)); } + public Ledger set(Entry entry) { + long tagId = entry.tagId(); + return tagId != 0 + ? this.recordKnown(tagId, entry.objectValue()) + : this.recordCustom(entry.tag(), entry.objectValue()); + } + + public Ledger remove(String tag) { + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? this.recordKnown(tagId, REMOVED) : this.recordCustom(tag, REMOVED); + } + + public Ledger remove(long tagId) { + return this.recordKnown(requireKnownLedgerId(tagId), REMOVED); + } + + /** + * Records a removal only if the ledger already holds a change for {@code tag}, so a removal of + * a tag set elsewhere (another map applied to the same span) isn't clobbered. + */ public Ledger smartRemove(String tag) { - if (this.contains(tag)) { - this.remove(tag); + long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + return this.smartRemove(tagId); + } + if (this.containsCustom(tag)) { + this.recordCustom(tag, REMOVED); } return this; } - private boolean contains(String tag) { - // Entries and removals are both recorded under their canonical Datadog name (see newAnyEntry - // et al. and EntryRemoval's own constructor); canonicalize the query the same way or it - // would miss. - String canonicalTag = KnownTagCodec.canonicalTagName(tag); + /** {@link #smartRemove(String)} by id; a tag the ledger never recorded is a one-bit miss. */ + public Ledger smartRemove(long tagId) { + if ((this.occupancy & (1L << KnownTagCodec.slotOf(tagId))) != 0L + && this.containsKnown(tagId)) { + this.recordKnown(tagId, REMOVED); + } + return this; + } - EntryChange[] thisChanges = this.entryChanges; + private Ledger recordKnown(long tagId, Object value) { + this.occupancy |= 1L << KnownTagCodec.slotOf(tagId); + this.idFlags |= tagId & 0xFFFFFFFFL; + return this.record(tagId, null, value); + } + + private Ledger recordCustom(String tag, Object value) { + this.occupancy |= CUSTOM_OCCUPANCY; + return this.record(Entry.customHash(tag), tag, value); + } + private Ledger record(long tagHash, String tag, Object value) { + int pos = this.nextPos; + if (pos >= this.tagHashes.length) { + int newCap = this.tagHashes.length << 1; + this.tagHashes = Arrays.copyOf(this.tagHashes, newCap); + this.values = Arrays.copyOf(this.values, newCap); + if (this.tags != null) { + this.tags = Arrays.copyOf(this.tags, newCap); + } + } + this.tagHashes[pos] = tagHash; + if (tag != null) { + String[] names = this.tags; + if (names == null) { + this.tags = names = new String[this.tagHashes.length]; + } + names[pos] = tag; + } else if (this.tags != null) { + this.tags[pos] = null; + } + this.values[pos] = value; + this.nextPos = pos + 1; + if (value == REMOVED) { + this.containsRemovals = true; + } + return this; + } + + private boolean containsKnown(long tagId) { + long[] hashes = this.tagHashes; // min is to clamp, so bounds check elimination optimization works - int lenClamp = Math.min(this.nextPos, thisChanges.length); + int lenClamp = Math.min(this.nextPos, hashes.length); for (int i = 0; i < lenClamp; ++i) { - if (thisChanges[i].matches(canonicalTag)) return true; + if (hashes[i] == tagId) return true; + } + return false; + } + + private boolean containsCustom(String tag) { + if ((this.occupancy & CUSTOM_OCCUPANCY) == 0L) { + return false; + } + String[] names = this.tags; + if (names == null) { + return false; // bit 63 came from a known tag with no slot + } + long hash = Entry.customHash(tag); + long[] hashes = this.tagHashes; + int lenClamp = Math.min(this.nextPos, Math.min(hashes.length, names.length)); + for (int i = 0; i < lenClamp; ++i) { + if (hashes[i] == hash && tag.equals(names[i])) return true; } return false; } @@ -1091,27 +1234,56 @@ private boolean contains(String tag) { * Just for testing */ Entry findLastEntry(String tag) { - String canonicalTag = KnownTagCodec.canonicalTagName(tag); - EntryChange[] thisChanges = this.entryChanges; - - // min is to clamp, so ArrayBoundsCheckElimination optimization works - int clampLen = Math.min(this.nextPos, thisChanges.length) - 1; - for (int i = clampLen; i >= 0; --i) { - EntryChange thisChange = thisChanges[i]; - if (!thisChange.isRemoval() && thisChange.matches(canonicalTag)) return (Entry) thisChange; + long tagId = KnownTagCodec.keyOf(tag); + long hash = tagId != 0 ? tagId : Entry.customHash(tag); + for (int i = Math.min(this.nextPos, this.tagHashes.length) - 1; i >= 0; --i) { + if (this.tagHashes[i] == hash + && (tagId != 0 || tag.equals(this.tags[i])) + && this.values[i] != REMOVED) { + return tagId != 0 + ? Entry.newAnyEntry(tagId, this.values[i]) + : Entry.newAnyEntry(tag, this.values[i]); + } } return null; } public void reset() { - Arrays.fill(this.entryChanges, null); + int size = this.nextPos; + if (this.tags != null) { + Arrays.fill(this.tags, 0, size, null); + } + Arrays.fill(this.values, 0, size, null); this.nextPos = 0; this.containsRemovals = false; + this.occupancy = 0L; + this.idFlags = 0L; } + /** + * Iterates the changes in order. Every {@code next()} returns the same reader, repositioned: + * read it before advancing, and call {@link EntryReader#entry()} to keep a change. + */ @Override - public Iterator iterator() { - return new IteratorImpl(this.entryChanges, this.nextPos); + public Iterator iterator() { + return new LedgerReader(this); + } + + /** + * Calls {@code consumer} with each change in order, through one reader that this ledger reuses + * across calls: no allocation once the ledger has been read once. Not reentrant: don't iterate + * the same ledger from inside {@code consumer}. The reader is only valid during the call. + */ + public void forEach(T thisObj, BiConsumer consumer) { + LedgerReader reader = this.reader; + if (reader == null) { + this.reader = reader = new LedgerReader(this); + } + int size = this.nextPos; + for (int i = 0; i < size; ++i) { + reader.pos = i; + consumer.accept(thisObj, reader); + } } public TagMap build() { @@ -1121,15 +1293,21 @@ public TagMap build() { } void fill(TagMap map) { - EntryChange[] entryChanges = this.entryChanges; - int size = this.nextPos; - for (int i = 0; i < size && i < entryChanges.length; ++i) { - EntryChange change = entryChanges[i]; - - if (change.isRemoval()) { - map.remove(change.tag()); + int size = Math.min(this.nextPos, this.tagHashes.length); + for (int i = 0; i < size; ++i) { + long tagHash = this.tagHashes[i]; + Object value = this.values[i]; + boolean known = (tagHash >>> 32) != 0; + if (value == REMOVED) { + if (known) { + map.getAndRemove(tagHash); + } else { + map.remove(this.tags[i]); + } + } else if (known) { + map.set(tagHash, value); } else { - map.set((Entry) change); + map.putEntry(Entry.newAnyEntry(this.tags[i], value)); } } } @@ -1142,30 +1320,137 @@ public TagMap buildImmutable() { } } - static final class IteratorImpl implements Iterator { - private final EntryChange[] entryChanges; - private final int size; + static long requireKnownLedgerId(long tagId) { + if (!KnownTagCodec.isKnown(tagId)) { + throw new IllegalArgumentException("not a known tag id: " + Long.toHexString(tagId)); + } + return tagId; + } + } - private int pos; + /** + * A reusable cursor over a {@link Ledger}'s changes: both the iterator and the {@link + * EntryChangeReader} it returns. For a removal only {@link #tag()} and {@link #tagId()} apply. + */ + static final class LedgerReader implements EntryChangeReader, Iterator { + final Ledger ledger; + int pos = -1; - IteratorImpl(EntryChange[] entryChanges, int size) { - this.entryChanges = entryChanges; - this.size = size; + LedgerReader(Ledger ledger) { + this.ledger = ledger; + } - this.pos = -1; - } + @Override + public boolean hasNext() { + return this.pos + 1 < this.ledger.nextPos; + } - @Override - public boolean hasNext() { - return (this.pos + 1 < this.size); - } + @Override + public EntryChangeReader next() { + if (!this.hasNext()) throw new NoSuchElementException("no next"); + ++this.pos; + return this; + } + + @Override + public boolean isRemoval() { + return this.ledger.values[this.pos] == Ledger.REMOVED; + } + + @Override + public String tag() { + String[] names = this.ledger.tags; + String tag = names != null ? names[this.pos] : null; + return tag != null ? tag : KnownTagCodec.nameOf(this.ledger.tagHashes[this.pos]); + } + + @Override + public long tagId() { + long tagHash = this.ledger.tagHashes[this.pos]; + // a known tag's hash is its id, whose upper 32 bits are never zero; a custom tag's are + return (tagHash >>> 32) != 0 ? tagHash : 0L; + } + + private Object value() { + return this.ledger.values[this.pos]; + } + + @Override + public byte type() { + return TagValueConversions.typeOf(this.value()); + } + + @Override + public boolean is(byte type) { + return TagValueConversions.isA(this.value(), type); + } + + @Override + public boolean isNumber() { + return TagValueConversions.isNumber(this.value()); + } - @Override - public EntryChange next() { - if (!this.hasNext()) throw new NoSuchElementException("no next"); + @Override + public boolean isNumericPrimitive() { + return TagValueConversions.isNumericPrimitive(this.value()); + } + + @Override + public boolean isObject() { + return TagValueConversions.isObject(this.value()); + } + + @Override + public boolean booleanValue() { + return TagValueConversions.toBoolean(this.value()); + } - return this.entryChanges[++this.pos]; + @Override + public int intValue() { + return TagValueConversions.toInt(this.value()); + } + + @Override + public long longValue() { + return TagValueConversions.toLong(this.value()); + } + + @Override + public float floatValue() { + return TagValueConversions.toFloat(this.value()); + } + + @Override + public double doubleValue() { + return TagValueConversions.toDouble(this.value()); + } + + @Override + public String stringValue() { + return TagValueConversions.toString(this.value()); + } + + @Override + public Object objectValue() { + return this.value(); + } + + /** The change as an {@link Entry}, or null for a removal. */ + @Override + public Entry entry() { + Object value = this.value(); + if (value == Ledger.REMOVED) { + return null; } + long tagId = this.tagId(); + return tagId != 0 + ? Entry.newAnyEntry(tagId, value) + : Entry.newAnyEntry(this.ledger.tags[this.pos], value); + } + + @Override + public Map.Entry mapEntry() { + return this.entry(); } } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java index 540a4a2481a..fae09e379cc 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java @@ -436,6 +436,12 @@ public interface SpanBuilder { SpanBuilder withTag(String tag, Object value); + /** + * Sets a known tag by its {@code KnownTags} id; a null value removes it. An id that names no + * known tag is ignored. + */ + SpanBuilder withTag(long tagId, Object value); + SpanBuilder withStartTimestamp(long microseconds); SpanBuilder withServiceName(String serviceName); diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapFuzzTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapFuzzTest.java index 685dc1d9cdf..3d31bf57f13 100644 --- a/internal-api/src/test/java/datadog/trace/api/TagMapFuzzTest.java +++ b/internal-api/src/test/java/datadog/trace/api/TagMapFuzzTest.java @@ -1405,10 +1405,9 @@ protected void _applyToTestMap(TagMap testMap) { @Override protected void _applyToExpectedMap(Map expectedMap) { - for (TagMap.EntryChange change : this.ledger) { - // ledgerOf - doesn't produce / removes, so this is safe - TagMap.Entry entry = (TagMap.Entry) change; - expectedMap.put(entry.tag(), entry.objectValue()); + for (TagMap.EntryChangeReader change : this.ledger) { + // ledgerOf doesn't produce removals + expectedMap.put(change.tag(), change.objectValue()); } } diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapLedgerByIdTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapLedgerByIdTest.java new file mode 100644 index 00000000000..405165785fb --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/TagMapLedgerByIdTest.java @@ -0,0 +1,195 @@ +package datadog.trace.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** + * {@link TagMap.Ledger} keyed by id: known tags recorded under their ids, custom tags under their + * names, read back through {@link TagMap.EntryChangeReader}, with the slot-occupancy and + * intercepted-id summaries. + */ +class TagMapLedgerByIdTest { + @Test + void aKnownTagByIdAndByNameRecordTheSameChange() { + TagMap byId = TagMap.ledger().set(KnownTags.HTTP_ROUTE_ID, "/users").build(); + TagMap byName = TagMap.ledger().set(KnownTags.HTTP_ROUTE_NAME, "/users").build(); + + assertEquals(new HashMap<>(byName), new HashMap<>(byId)); + assertEquals(KnownTags.HTTP_ROUTE_ID, byId.getEntry(KnownTags.HTTP_ROUTE_NAME).tagId()); + } + + @Test + void anOpenTelemetryNameRecordsItsTagsId() { + TagMap.Ledger ledger = TagMap.ledger().set(KnownTags.HTTP_METHOD_OTEL_NAME, "GET"); + + for (TagMap.EntryChangeReader change : ledger) { + assertEquals(KnownTags.HTTP_METHOD_ID, change.tagId()); + assertEquals(KnownTags.HTTP_METHOD_NAME, change.tag()); + } + } + + @Test + void anUnknownIdIsRejected() { + TagMap.Ledger ledger = TagMap.ledger(); + + assertThrows( + IllegalArgumentException.class, () -> ledger.set(KnownTagCodec.makeTagId(9999), "x")); + assertThrows( + IllegalArgumentException.class, () -> ledger.remove(KnownTagCodec.makeTagId(9999))); + } + + @Test + void theReaderReplaysSetsAndRemovalsInOrder() { + TagMap.Ledger ledger = + TagMap.ledger() + .set(KnownTags.HTTP_ROUTE_ID, "/users") + .set("my.custom.tag", 1) + .remove(KnownTags.HTTP_ROUTE_ID) + .remove("my.custom.tag"); + + List seen = new ArrayList<>(); + ledger.forEach( + seen, + (out, change) -> + out.add( + change.tag() + + (change.isRemoval() ? " removed" : "=" + change.objectValue()) + + " #" + + Long.toHexString(change.tagId()))); + + assertEquals( + Arrays.asList( + KnownTags.HTTP_ROUTE_NAME + "=/users #" + Long.toHexString(KnownTags.HTTP_ROUTE_ID), + "my.custom.tag=1 #0", + KnownTags.HTTP_ROUTE_NAME + " removed #" + Long.toHexString(KnownTags.HTTP_ROUTE_ID), + "my.custom.tag removed #0"), + seen); + assertTrue(ledger.containsRemovals()); + assertTrue(ledger.build().isEmpty()); + } + + @Test + void forEachReusesOneReader() { + TagMap.Ledger ledger = TagMap.ledger().set(KnownTags.HTTP_ROUTE_ID, "/a").set("b", "c"); + List readers = new ArrayList<>(); + + ledger.forEach(readers, List::add); + ledger.forEach(readers, List::add); + + assertEquals(4, readers.size()); + for (TagMap.EntryChangeReader reader : readers) { + assertSame(readers.get(0), reader); + } + } + + @Test + void aRemovalReadsAsNoEntry() { + TagMap.Ledger ledger = TagMap.ledger().remove(KnownTags.HTTP_ROUTE_ID); + + for (TagMap.EntryChangeReader change : ledger) { + assertTrue(change.isRemoval()); + assertNull(change.entry()); + } + } + + @Test + void smartRemoveOfATagNeverRecordedRecordsNothing() { + TagMap.Ledger ledger = TagMap.ledger().set(KnownTags.HTTP_ROUTE_ID, "/users"); + + ledger.smartRemove(KnownTags.HTTP_STATUS_CODE_ID); + ledger.smartRemove(KnownTags.HTTP_STATUS_CODE_NAME); + ledger.smartRemove("never.set"); + + assertEquals(1, ledger.estimateSize()); + assertFalse(ledger.containsRemovals()); + } + + @Test + void smartRemoveOfARecordedTagRecordsTheRemoval() { + TagMap.Ledger ledger = + TagMap.ledger() + .set(KnownTags.HTTP_ROUTE_ID, "/users") + .set(KnownTags.HTTP_METHOD_ID, "GET") + .set("my.custom.tag", "x"); + + ledger.smartRemove(KnownTags.HTTP_ROUTE_ID); + ledger.smartRemove(KnownTags.HTTP_METHOD_OTEL_NAME); // by its OpenTelemetry name + ledger.smartRemove("my.custom.tag"); + + assertEquals(6, ledger.estimateSize()); + assertTrue(ledger.build().isEmpty()); + } + + @Test + void smartRemoveOfACustomTagMatchesByName() { + TagMap.Ledger ledger = TagMap.ledger().set("my.custom.tag", "x"); + + ledger.smartRemove("my.other.tag"); + + assertEquals(1, ledger.estimateSize()); + } + + @Test + void hasInterceptedTagsTracksTheRecordedIds() { + TagMap.Ledger ledger = TagMap.ledger().set(KnownTags.HTTP_ROUTE_ID, "/users").set("x", "y"); + assertFalse(ledger.hasInterceptedTags()); + + ledger.set(KnownTags.SPAN_KIND_ID, "server"); + assertTrue(ledger.hasInterceptedTags()); + } + + @Test + void resetClearsTheChangesAndTheSummaries() { + TagMap.Ledger ledger = + TagMap.ledger() + .set(KnownTags.SPAN_KIND_ID, "server") + .set("my.custom.tag", "x") + .remove(KnownTags.HTTP_ROUTE_ID); + + ledger.reset(); + + assertTrue(ledger.isDefinitelyEmpty()); + assertFalse(ledger.containsRemovals()); + assertFalse(ledger.hasInterceptedTags()); + ledger.smartRemove(KnownTags.SPAN_KIND_ID).smartRemove("my.custom.tag"); + assertTrue(ledger.isDefinitelyEmpty()); + } + + @Test + void aLedgerOfKnownTagsNeverAllocatesItsNamesArray() { + TagMap.Ledger ledger = + TagMap.ledger(2) + .set(KnownTags.HTTP_ROUTE_ID, "/a") + .set(KnownTags.HTTP_METHOD_NAME, "GET") + .set(KnownTags.HTTP_URL_ID, "http://x") // grows past 2 + .smartRemove("never.set"); + + assertNull(ledger.tags); + + ledger.set("my.custom.tag", "x"); + assertEquals("x", ledger.build().getString("my.custom.tag")); + assertEquals("/a", ledger.build().getString(KnownTags.HTTP_ROUTE_NAME)); + } + + @Test + void theArraysGrowPastTheirInitialSize() { + TagMap.Ledger ledger = TagMap.ledger(2); + for (int i = 0; i < 20; ++i) { + ledger.set("tag" + i, i); + } + + TagMap map = ledger.build(); + assertEquals(20, map.size()); + assertEquals(19, map.getInt("tag19")); + } +} diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapLedgerTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapLedgerTest.java index ddd7dccf2b4..6311cd39816 100644 --- a/internal-api/src/test/java/datadog/trace/api/TagMapLedgerTest.java +++ b/internal-api/src/test/java/datadog/trace/api/TagMapLedgerTest.java @@ -23,11 +23,10 @@ public void inOrder() { assertEquals(SIZE, ledger.estimateSize()); int i = 0; - for (TagMap.EntryChange entryChange : ledger) { - TagMap.Entry entry = (TagMap.Entry) entryChange; - - assertEquals(key(i), entry.tag()); - assertEquals(value(i), entry.stringValue()); + for (TagMap.EntryChangeReader change : ledger) { + assertFalse(change.isRemoval()); + assertEquals(key(i), change.tag()); + assertEquals(value(i), change.stringValue()); ++i; } @@ -44,13 +43,14 @@ public void testTypes() { ledger.set("object", (Object) "string"); ledger.set("string", "string"); - assertEntryRawType(TagMap.Entry.BOOLEAN, ledger, "bool"); - assertEntryRawType(TagMap.Entry.INT, ledger, "int"); - assertEntryRawType(TagMap.Entry.LONG, ledger, "long"); - assertEntryRawType(TagMap.Entry.FLOAT, ledger, "float"); - assertEntryRawType(TagMap.Entry.DOUBLE, ledger, "double"); - assertEntryRawType(TagMap.Entry.ANY, ledger, "object"); - assertEntryRawType(TagMap.Entry.OBJECT, ledger, "string"); + // The ledger keeps values, not typed entries; what a reader sees is the resolved type. + assertEntryType(TagMap.Entry.BOOLEAN, ledger, "bool"); + assertEntryType(TagMap.Entry.INT, ledger, "int"); + assertEntryType(TagMap.Entry.LONG, ledger, "long"); + assertEntryType(TagMap.Entry.FLOAT, ledger, "float"); + assertEntryType(TagMap.Entry.DOUBLE, ledger, "double"); + assertEntryType(TagMap.Entry.OBJECT, ledger, "object"); + assertEntryType(TagMap.Entry.OBJECT, ledger, "string"); } @Test @@ -220,9 +220,13 @@ static final String value(int i) { return "value-" + i; } - static final void assertEntryRawType(byte expectedType, TagMap.Ledger ledger, String tag) { - TagMap.Entry entry = ledger.findLastEntry(tag); - assertEquals(expectedType, entry.rawType); + static final void assertEntryType(byte expectedType, TagMap.Ledger ledger, String tag) { + assertEquals(expectedType, ledger.findLastEntry(tag).type()); + for (TagMap.EntryChangeReader change : ledger) { + if (tag.equals(change.tag())) { + assertEquals(expectedType, change.type()); + } + } } static final void assertFrozen(TagMap map) {