diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 61524f9355c..4cc52bad3c8 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -13,6 +13,7 @@ /SECURITY.md @DataDog/apm-java /SUPPORT.md @DataDog/apm-java /tag-conventions.yaml @DataDog/apm-java @DataDog/apm-lang-platform-java @DataDog/apm-sdk-capabilities +/tag-conventions-java.yaml @DataDog/apm-java @DataDog/apm-lang-platform-java @DataDog/apm-sdk-capabilities /dd-smoke-tests/README.md @DataDog/apm-java /docs/ @DataDog/apm-java diff --git a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/GenerateKnownTagsTask.kt b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/GenerateKnownTagsTask.kt index bd567a67854..72c2dfc1514 100644 --- a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/GenerateKnownTagsTask.kt +++ b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/GenerateKnownTagsTask.kt @@ -5,6 +5,7 @@ import org.gradle.api.file.DirectoryProperty import org.gradle.api.file.RegularFileProperty import org.gradle.api.tasks.CacheableTask import org.gradle.api.tasks.InputFile +import org.gradle.api.tasks.Optional import org.gradle.api.tasks.OutputDirectory import org.gradle.api.tasks.PathSensitive import org.gradle.api.tasks.PathSensitivity @@ -20,12 +21,18 @@ abstract class GenerateKnownTagsTask : DefaultTask() { @get:PathSensitive(PathSensitivity.NONE) abstract val tagConventionsFile: RegularFileProperty + @get:InputFile + @get:Optional + @get:PathSensitive(PathSensitivity.NONE) + abstract val tracerOverlayFile: RegularFileProperty + @get:OutputDirectory abstract val destinationDirectory: DirectoryProperty @TaskAction fun generate() { val outDir = destinationDirectory.get().asFile - TagRegistryGenerator.generate(tagConventionsFile.get().asFile, outDir) + TagRegistryGenerator.generate( + tagConventionsFile.get().asFile, outDir, tracerOverlayFile.orNull?.asFile) logger.lifecycle("tag-registry: generated -> $outDir") } } diff --git a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/KnownTagsEmitter.kt b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/KnownTagsEmitter.kt index 1f808d8d357..f409bc18bdc 100644 --- a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/KnownTagsEmitter.kt +++ b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/KnownTagsEmitter.kt @@ -6,8 +6,9 @@ import java.util.Locale /** * Emits the generated `KnownTags.java` from a [TagRegistry]. Public API first — per-tag * `_NAME` (string) + `_ID` (encoded long, literal) couplets with a trailing `// makeTagId(...)` - * derivation comment — then the package-private `_SERIAL_NUM` constants, the - * `StringIndex.EmbeddingSupport` keyOf table and the resolver's name switches. + * derivation comment — then the `_SERIAL_NUM` constants, the + * `StringIndex.EmbeddingSupport` keyOf table, the `NAMES_BY_SERIAL` array behind `nameOf`, and the + * resolver's `openTelemetryNameOf` switch. */ object KnownTagsEmitter { @@ -101,16 +102,22 @@ object KnownTagsEmitter { } append("// makeTagId(serial=${t.serial})") if (t.traceLevel) append(" + trace-level") + if (t.intercepted) append(" + intercepted") + if (t.sharedNameDirection != null) append(" + shared-name") if (t.otelName != null) append(" -> ${escape(t.otelName)}") appendLine(" <${escape(t.required)}>") appendLine() } - // Serial numbers (globalSerial per tag) — package-private, consumed by the resolver switch. + // Serial numbers (globalSerial per tag) — public, so a switch on KnownTagCodec.serialNum outside + // this package (the resolver here, TagInterceptor in core) can name them as case labels. appendLine(" // ---- serial numbers ----") for (t in reg.tags) { - appendLine(" static final int ${serialC(t.identity)} = ${t.serial};") + appendLine(" public static final int ${serialC(t.identity)} = ${t.serial};") } + // Every known tag's serial is below SERIAL_LIMIT; serial 0 is no tag. + val serialLimit = (reg.tags.maxOfOrNull { it.serial } ?: 0) + 1 + appendLine(" public static final int SERIAL_LIMIT = $serialLimit;") // OpenTelemetry name -> canonical tag name. Validation ensures aliases are distinct from all // canonical names. Sort by OTel name to keep output deterministic. @@ -163,6 +170,21 @@ object KnownTagsEmitter { KEYOF_IDS = ids; } + /** + * Each tag's Datadog name, indexed by serial; slot 0 (no tag) is null. An array rather than a + * switch keeps nameOf small enough to inline at every caller. + */ + static final String[] NAMES_BY_SERIAL = { + null, + """.trimIndent() + ) + for (t in reg.tags.sortedBy { it.serial }) { + appendLine(" ${nameExpr(t)},") + } + appendLine( + """ + }; + /** * The registry's name↔id tables, as a {@link KnownTagCodec.Resolver}. {@code KnownTagCodec} * reads this field from its own holder, so the two classes complete each other: the codec owns @@ -172,26 +194,12 @@ object KnownTagsEmitter { new KnownTagCodec.Resolver() { @Override public String nameOf(long tagId) { - switch (KnownTagCodec.serialNum(tagId)) { - """.trimIndent() - ) - for (t in reg.tags) { - appendLine( - """ - case ${serialC(t.identity)}: - return ${nameExpr(t)}; - """.trimIndent() - ) - } - // openTelemetryNameOf: canonical id -> OTel-namespace name, null when the tag has none. The - // caller (a serializer) owns any fall-back-to-Datadog-name policy; this stays a pure lookup. - appendLine( - """ - default: - return null; - } + int serial = KnownTagCodec.serialNum(tagId); + return serial < NAMES_BY_SERIAL.length ? NAMES_BY_SERIAL[serial] : null; } + // openTelemetryNameOf: canonical id -> OTel-namespace name, null when the tag has + // none. The caller (a serializer) owns any fall-back-to-Datadog-name policy. @Override public String openTelemetryNameOf(long tagId) { switch (KnownTagCodec.serialNum(tagId)) { diff --git a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagConventions.kt b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagConventions.kt index 54fef064dfb..7b1239fbc40 100644 --- a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagConventions.kt +++ b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagConventions.kt @@ -9,6 +9,8 @@ class TagConventions private constructor( private val spanTypes: Map, private val mixins: Map, private val traceLevel: List, + /** Datadog names the tracer intercepts on the set-path, from the tracer overlay. */ + val interceptedNames: Set = emptySet(), ) { /** * A tag's identity: its Datadog name, plus the direction when that name is declared once per @@ -241,6 +243,16 @@ class TagConventions private constructor( /** The type's own or nearest inherited `span-kind` direction, or null when none is declared. */ private fun directionOf(spanTypes: Map, st: SpanType): Direction? = chainOf(spanTypes, st.name).firstNotNullOfOrNull { it.direction } + /** The tracer overlay's `tags`, as a mixin that applies to no span type; empty when none. */ + private fun overlayMixin(overlay: Map): Map { + val tags = tagList(overlay["tags"]) + if (tags.isEmpty()) return emptyMap() + return mapOf( + OVERLAY_MIXIN to + Mixin(name = OVERLAY_MIXIN, appliesAll = false, appliesTo = emptySet(), tags = tags, refs = emptyList()) + ) + } + /** Reads `span-kind` as the direction it sets, or null when absent. */ private fun parseDirection(m: Map, owner: String): Direction? { val spanKind = m["span-kind"] @@ -250,11 +262,31 @@ class TagConventions private constructor( return (spanKind as String?)?.let { SPAN_KIND_DIRECTIONS.getValue(it) } } + /** The synthetic mixin holding the tracer overlay's tags; it applies to no span type. */ + const val OVERLAY_MIXIN = "tracer overlay" + + /** + * Parses [root], the language-agnostic conventions, plus [overlay], this tracer's own set-path + * routing: `tags` declares keys that exist only to be routed (e.g. `resource.name`), so each is + * intercepted, and `intercepted` adds the tags declared in [root] that the tracer also routes. + */ @Suppress("UNCHECKED_CAST") - fun parse(root: Map): TagConventions { + fun parse(root: Map, overlay: Map = emptyMap()): TagConventions { for (section in listOf("span_types", "mixins", "trace_level")) { require(root[section] == null || root[section] is Map<*, *>) { "$section must be a mapping" } } + require(overlay.keys.all { it == "tags" || it == "intercepted" }) { + "the tracer overlay may only declare `tags` and `intercepted`, not ${overlay.keys - setOf("tags", "intercepted")}" + } + require(refList(overlay["tags"]).isEmpty()) { "tracer overlay tags must be declarations, not refs" } + val interceptedRaw = overlay["intercepted"] + require(interceptedRaw == null || (interceptedRaw is List<*> && interceptedRaw.all { it is String })) { + "intercepted must be a list of Datadog tag names" + } + // An overlay tag exists only to be routed, so it is intercepted without being listed. + val intercepted = + ((interceptedRaw as? List) ?: emptyList()).toSet() + + tagList(overlay["tags"]).map { it.ddName } val spanTypesRaw = (root["span_types"] as? Map) ?: emptyMap() val parsedSpanTypes = spanTypesRaw.mapValues { (name, v) -> @@ -298,7 +330,7 @@ class TagConventions private constructor( refs = refList(m["tags"]), direction = parseDirection(m, "mixin '$name'"), ) - } + } + overlayMixin(overlay) for (spanType in parsedSpanTypes.values) { for (included in spanType.include) { @@ -342,7 +374,12 @@ class TagConventions private constructor( ) } validateOtelNameScope(spanTypes, mixins, traceLevel) - return TagConventions(spanTypes, mixins, traceLevel) + val conv = TagConventions(spanTypes, mixins, traceLevel, intercepted) + val declared = conv.allDeclaredTags().map { it.ddName }.toSet() + for (name in intercepted) { + require(name in declared) { "intercepted tag '$name' is not declared" } + } + return conv } /** diff --git a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistry.kt b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistry.kt index 664ee9f6687..983eaad5ba4 100644 --- a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistry.kt +++ b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistry.kt @@ -4,17 +4,15 @@ package datadog.buildlogic.tagRegistry * Assigns tag ids from a parsed [TagConventions]. The id encoding mirrors KnownTagCodec: [63-48 * serial][47-32 reserved][31-0 flags]. * - *

An id is IDENTITY only: a globally unique serial plus the trace-level classification bit. It + *

An id is a globally unique serial plus two classification bits: trace-level and intercepted. It * carries no storage-layout coordinate -- bits [47-32] are held vacant for the co-occurrence slot * that the dense tag store assigns by graph coloring, which lands with the dense store itself. * Nothing here needs to know how (or whether) a tag is stored. * - *

Nor does anything here know how a tag is SET. Whether the tracer intercepts a tag on the - * set-path (routing it to a span field or a sampling directive instead of tag storage) is a - * property of TagInterceptor, not of the tag's identity, and modelling it was the source of a whole - * class of drift between this registry and the interceptor's actual switch. It arrives with the - * work that consumes it -- the id->handler dispatch table that retires TagInterceptor -- where the - * interceptor can be the authority. Re-adding a classification bit then is purely additive. + *

The intercepted bit marks a tag the tracer may route on the set-path (to a span field or a + * sampling directive instead of tag storage), as listed by the tracer overlay. A setter called with + * a constant id then folds the interception test away. TagInterceptor's switch stays the authority + * on what each tag does; a test there keeps the bit and the switch in agreement. */ class TagRegistry private constructor(val tags: List) { data class Tag( @@ -23,6 +21,7 @@ class TagRegistry private constructor(val tags: List) { val required: String, val serial: Int, val traceLevel: Boolean, + val intercepted: Boolean, val id: Long, /** The tag's OpenTelemetry name, in [otelDirection] or every direction; null when not renamed. */ val declaredOtelName: String? = null, @@ -47,14 +46,19 @@ class TagRegistry private constructor(val tags: List) { companion object { const val FIRST_SERIAL = 1 const val LEVEL_TRACE = 1L shl 2 // low-32 carve bit 2; mirrors KnownTagCodec.LEVEL_TRACE + const val INTERCEPTED = 1L shl 1 // low-32 carve bit 1; mirrors KnownTagCodec.INTERCEPTED + const val SHARED_NAME = 1L shl 0 // low-32 carve bit 0; mirrors KnownTagCodec.SHARED_NAME /** - * Mirrors KnownTagCodec.makeTagId(serial) + traceLevel() -- must stay in sync. LEVEL_TRACE at - * bit 2, other low bits and the reserved [47-32] window zero. + * Mirrors KnownTagCodec.makeTagId(serial) + traceLevel() + intercepted() -- must stay in sync. + * LEVEL_TRACE at bit 2, INTERCEPTED at bit 1, SHARED_NAME at bit 0, other low bits and the + * reserved [47-32] window zero. */ - fun encode(serial: Int, traceLevel: Boolean): Long { + fun encode(serial: Int, traceLevel: Boolean, intercepted: Boolean, sharedName: Boolean = false): Long { var id = serial.toLong() shl 48 if (traceLevel) id = id or LEVEL_TRACE + if (intercepted) id = id or INTERCEPTED + if (sharedName) id = id or SHARED_NAME return id } @@ -67,13 +71,15 @@ class TagRegistry private constructor(val tags: List) { conv.allDeclaredTags().sortedBy { it.name }.mapIndexed { i, t -> val serial = FIRST_SERIAL + i val isTraceLevel = t.identity in traceLevel + val isIntercepted = t.ddName in conv.interceptedNames Tag( t.identity, t.type, t.required, serial, isTraceLevel, - id = encode(serial, isTraceLevel), + isIntercepted, + id = encode(serial, isTraceLevel, isIntercepted, t.identity.direction != null), declaredOtelName = renames[t.identity]?.otelName, otelDirection = renames[t.identity]?.direction, ) diff --git a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryExtension.kt b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryExtension.kt index f6d07ca586b..1193de5c3cc 100644 --- a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryExtension.kt +++ b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryExtension.kt @@ -16,6 +16,12 @@ abstract class TagRegistryExtension @Inject constructor( */ abstract val tagConventionsFile: RegularFileProperty + /** + * Optional tracer overlay: this tracer's set-path routing (intercepted tags, and keys that exist + * only to be routed), kept out of the language-agnostic conventions. + */ + abstract val tracerOverlayFile: RegularFileProperty + /** * Destination of the generated sources, by convention under `build/generated/tag-registry`. */ diff --git a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGenerator.kt b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGenerator.kt index 9b69f0fe0b0..50960175cc0 100644 --- a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGenerator.kt +++ b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGenerator.kt @@ -8,17 +8,20 @@ import java.util.Locale /** Emits the Java tag registry and reports in a deterministic order. */ object TagRegistryGenerator { - /** Parses the conventions YAML and writes the full generated tree under [outDir]. */ - fun generate(tagConventionsFile: File, outDir: File) { + /** + * Parses the conventions YAML, plus the tracer overlay when there is one, and writes the full + * generated tree under [outDir]. + */ + fun generate(tagConventionsFile: File, outDir: File, tracerOverlayFile: File? = null) { val mapper = ObjectMapper(YAMLFactory()) - val domain: Map = - tagConventionsFile.inputStream().use { - mapper.readValue(it, object : TypeReference>() {}) - } + fun read(file: File): Map = + file.inputStream().use { mapper.readValue(it, object : TypeReference>() {}) } ?: emptyMap() + val domain = read(tagConventionsFile) + val overlay = tracerOverlayFile?.let(::read) ?: emptyMap() // Validate before touching the destination tree: an invalid domain model must fail loudly, // not after the previous (valid) generated output has already been wiped out. - val conv = TagConventions.parse(domain) + val conv = TagConventions.parse(domain, overlay) val reg = TagRegistry.build(conv) // Remove obsolete generated files when the output changes. @@ -68,18 +71,19 @@ object TagRegistryGenerator { """ # Tag id assignment. tags=${reg.tags.size} - # TAGS serial lvl id required name + # TAGS serial lvl int id required name """.trimIndent() ) - for ((name, _, required, serial, traceLevel, id) in reg.tags) { + for (t in reg.tags) { appendLine( - " %6d %s %-18s %-12s %s".format( + " %6d %s %s %-18s %-12s %s".format( Locale.ROOT, - serial, - if (traceLevel) "T" else "-", - "0x%016X".format(Locale.ROOT, id), - required, - name + t.serial, + if (t.traceLevel) "T" else "-", + if (t.intercepted) "I" else "-", + "0x%016X".format(Locale.ROOT, t.id), + t.required, + t.name ) ) } diff --git a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGeneratorPlugin.kt b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGeneratorPlugin.kt index c350d3c302e..a93b92df9f7 100644 --- a/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGeneratorPlugin.kt +++ b/build-logic/tag-registry/src/main/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGeneratorPlugin.kt @@ -16,6 +16,7 @@ class TagRegistryGeneratorPlugin : Plugin { group = "build" description = "Generates the Java tag registry and assignment reports." tagConventionsFile.convention(ext.tagConventionsFile) + tracerOverlayFile.convention(ext.tracerOverlayFile) destinationDirectory.convention(ext.destinationDirectory) } project.pluginManager.withPlugin("java") { diff --git a/build-logic/tag-registry/src/test/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGeneratorTest.kt b/build-logic/tag-registry/src/test/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGeneratorTest.kt index 1ff22853571..16b899d19ab 100644 --- a/build-logic/tag-registry/src/test/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGeneratorTest.kt +++ b/build-logic/tag-registry/src/test/kotlin/datadog/buildlogic/tagRegistry/TagRegistryGeneratorTest.kt @@ -478,6 +478,56 @@ class TagRegistryGeneratorTest { .withMessageContaining("span-kind-neutral without an otel-name") } + @Test + fun `the tracer overlay declares routed keys and marks intercepted tags`() { + val yaml = directory.conventionsFile( + """ + span_types: + base: + abstract: true + tags: [{dd-name: service, otel-name: service.name}, {dd-name: component}] + """ + ) + val overlay = directory.writeFile( + "overlay.yaml", + """ + tags: [{dd-name: resource.name}] + intercepted: [service] + """ + ) + val output = File(directory, "generated") + + TagRegistryGenerator.generate(yaml, output, overlay) + + val knownTags = contents(output).getValue("java/datadog/trace/api/KnownTags.java") + assertThat(knownTags) + .contains( + "COMPONENT_ID = 0x0001000000000000L", + "RESOURCE_NAME_ID = 0x0002000000000002L", + "SERVICE_ID = 0x0003000000000002L", + "public static final int SERVICE_SERIAL_NUM = 3;", + "public static final int SERIAL_LIMIT = 4;" + ) + } + + @TableTest( + """ + scenario | overlay | message + undeclared name | 'intercepted: [missing]' | is not declared + ref in overlay | 'tags: [{ref: service}]' | must be declarations, not refs + unknown section | 'span_types: {}' | may only declare + duplicate of a tag | 'tags: [{dd-name: service}]' | declared in both + """ + ) + fun `invalid tracer overlays are rejected`(overlay: String, message: String) { + val yaml = directory.conventionsFile("span_types: {base: {tags: [{dd-name: service}]}}") + val overlayFile = directory.writeFile("overlay.yaml", overlay) + + assertThatIllegalArgumentException() + .isThrownBy { TagRegistryGenerator.generate(yaml, File(directory, "generated"), overlayFile) } + .withMessageContaining(message) + } + private fun tagConventions(@Language("yaml") yamlText: String) = TagConventions.parse( ObjectMapper(YAMLFactory()).readValue( directory.conventionsFile(yamlText), diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java index 5c6628c5d35..4621d251729 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java @@ -8,6 +8,7 @@ import datadog.trace.api.Config; import datadog.trace.api.DDTags; import datadog.trace.api.Functions; +import datadog.trace.api.KnownTags; import datadog.trace.api.TagMap; import datadog.trace.api.cache.QualifiedClassNameCache; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; @@ -194,12 +195,12 @@ public final void onPeerConnection( if (remoteAddress != null) { String ip = remoteAddress.getHostAddress(); if (resolved && Config.get().isPeerHostNameEnabled()) { - span.setTag(Tags.PEER_HOSTNAME, hostName(remoteAddress, ip)); + span.setTag(KnownTags.PEER_HOSTNAME_ID, hostName(remoteAddress, ip)); } if (remoteAddress instanceof Inet4Address) { - span.setTag(Tags.PEER_HOST_IPV4, ip); + span.setTag(KnownTags.PEER_IPV4_ID, ip); } else if (remoteAddress instanceof Inet6Address) { - span.setTag(Tags.PEER_HOST_IPV6, ip); + span.setTag(KnownTags.PEER_IPV6_ID, ip); } } } diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy index 3151d655739..a04a75712f0 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy @@ -24,7 +24,16 @@ class BaseDecoratorTest extends DDSpecification { @Shared def errorPriority = null as Byte - def span = Mock(AgentSpan) + def span = mockSpan() + + /** + * Mocks an {@link AgentSpan} that reports every id-keyed {@code setTag} as the name-keyed call, + * so expectations are written by tag name however the code under test sets the tag. See + * {@link NameKeyedAgentSpan}. + */ + AgentSpan mockSpan() { + return Mock(NameKeyedAgentSpan) + } def spanContext = Mock(AgentSpanContext) def "test afterStart"() { diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy index fec5748f089..a1568c81af5 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy @@ -8,7 +8,7 @@ import datadog.trace.bootstrap.instrumentation.api.Tags class ClientDecoratorTest extends BaseDecoratorTest { - def span = Mock(AgentSpan) + def span = mockSpan() def "test afterStart"() { setup: diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DBTypeProcessingDatabaseClientDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DBTypeProcessingDatabaseClientDecoratorTest.groovy index 624100fcc50..efe550ee1bf 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DBTypeProcessingDatabaseClientDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DBTypeProcessingDatabaseClientDecoratorTest.groovy @@ -2,14 +2,13 @@ package datadog.trace.bootstrap.instrumentation.decorator import datadog.trace.api.DDTags import datadog.trace.api.TagMap -import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString import spock.lang.Ignore class DBTypeProcessingDatabaseClientDecoratorTest extends ClientDecoratorTest { - def span = Mock(AgentSpan) + def span = mockSpan() @Ignore("https://github.com/DataDog/dd-trace-java/pull/5213") def "test afterStart"() { diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy index 93852ccc88c..b8d38b32f0d 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy @@ -12,7 +12,7 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST class DatabaseClientDecoratorTest extends ClientDecoratorTest { - def span = Mock(AgentSpan) + def span = mockSpan() def "test afterStart"() { setup: diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecoratorTest.groovy index 1bc83457bd0..92f709a5d79 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecoratorTest.groovy @@ -52,7 +52,7 @@ class HttpClientDecoratorTest extends ClientDecoratorTest { AgentTracer.forceRegister(ORIGINAL_TRACER) } - def span = Mock(AgentSpan) + def span = mockSpan() def "test onRequest"() { setup: diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy index da411dc2431..5ac781b913b 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy @@ -36,7 +36,7 @@ import static datadog.trace.api.gateway.Events.EVENTS class HttpServerDecoratorTest extends ServerDecoratorTest { - def span = Mock(AgentSpan) + def span = mockSpan() static class MapCarrierVisitor implements AgentPropagation.ContextVisitor { diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ServerDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ServerDecoratorTest.groovy index d60c1534627..0e096772976 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ServerDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ServerDecoratorTest.groovy @@ -12,7 +12,7 @@ import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND class ServerDecoratorTest extends BaseDecoratorTest { - def span = Mock(AgentSpan) + def span = mockSpan() def "test afterStart"() { def decorator = newDecorator() diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/NameKeyedAgentSpan.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/NameKeyedAgentSpan.java new file mode 100644 index 00000000000..435ef32cdb5 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/NameKeyedAgentSpan.java @@ -0,0 +1,52 @@ +package datadog.trace.bootstrap.instrumentation.decorator; + +import datadog.trace.api.KnownTagCodec; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; + +/** + * An {@link AgentSpan} to mock in tests whose id-keyed {@code setTag} overloads are {@code final} + * and delegate to the name-keyed ones. A mock cannot override a final method, so it never sees the + * id-keyed call: tests keep expecting {@code setTag(name, value)} whether the code under test sets + * a tag by name or by {@code KnownTags} id. + */ +public abstract class NameKeyedAgentSpan implements AgentSpan { + @Override + public final AgentSpan setTag(long tagId, boolean value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + @Override + public final AgentSpan setTag(long tagId, int value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + @Override + public final AgentSpan setTag(long tagId, long value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + @Override + public final AgentSpan setTag(long tagId, float value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + @Override + public final AgentSpan setTag(long tagId, double value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + @Override + public final AgentSpan setTag(long tagId, String value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + @Override + public final AgentSpan setTag(long tagId, CharSequence value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + @Override + public final AgentSpan setTag(long tagId, Object value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } +} diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/SetTagBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/SetTagBenchmark.java new file mode 100644 index 00000000000..a9d1c25d13e --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/SetTagBenchmark.java @@ -0,0 +1,107 @@ +package datadog.trace.core; + +import static java.util.concurrent.TimeUnit.NANOSECONDS; + +import datadog.trace.api.KnownTags; +import datadog.trace.api.TagMap; +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 {@code DDSpan.setTag} on a live span, overwriting one tag per call, by how the tag is + * named: a constant id ({@code knownTag_byId}, {@code knownIntTag_byId}), a constant intercepted id + * ({@code interceptedTag_byId}, {@code span.kind}), a known tag's name, and a custom name. + * + *

Controls: {@code tagMap_byId} is the same store on a bare {@link TagMap}, with no span, lock, + * or interceptor; {@code tagMap_byId_synchronized} adds an uncontended lock, as the span takes one; + * {@code knownTag_byNonConstantId} reads the id from a non-final field, which C2 never + * constant-folds. + * + *

With a constant id, the interception test folds to the id's INTERCEPTED bit and there is no + * custom-tag path, so comparing {@code knownTag_byId} with {@code tagMap_byId_synchronized} + * isolates the span's own dispatch. The name variants add a registry lookup. + */ +@State(Scope.Thread) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(NANOSECONDS) +@Fork(value = 3) +public class SetTagBenchmark { + static final CoreTracer TRACER = CoreTracer.builder().build(); + + private DDSpan span; + private TagMap tagMap; + private long nonConstantTagId; + + @Setup + public void setup() { + span = (DDSpan) TRACER.startSpan("benchmark", "set.tag"); + tagMap = TagMap.create(); + nonConstantTagId = KnownTags.HTTP_ROUTE_ID; + } + + @TearDown + public void tearDown() { + span.finish(); + } + + @Benchmark + public Object tagMap_byId() { + tagMap.set(KnownTags.HTTP_ROUTE_ID, "/users/{id}"); + return tagMap; + } + + @Benchmark + public Object tagMap_byId_synchronized() { + TagMap tagMap = this.tagMap; + synchronized (tagMap) { + tagMap.set(KnownTags.HTTP_ROUTE_ID, "/users/{id}"); + } + return tagMap; + } + + @Benchmark + public Object knownTag_byNonConstantId() { + return span.setTag(nonConstantTagId, "/users/{id}"); + } + + @Benchmark + public Object knownTag_byId() { + return span.setTag(KnownTags.HTTP_ROUTE_ID, "/users/{id}"); + } + + @Benchmark + public Object interceptedTag_byId() { + return span.setTag(KnownTags.SPAN_KIND_ID, "client"); + } + + @Benchmark + public Object knownTag_byName() { + return span.setTag(KnownTags.HTTP_ROUTE_NAME, "/users/{id}"); + } + + @Benchmark + public Object customTag_byName() { + return span.setTag("my.custom.tag", "/users/{id}"); + } + + @Benchmark + public Object knownIntTag_byId() { + return span.setTag(KnownTags.HTTP_RESEND_COUNT_ID, 2); + } + + @Benchmark + public Object knownIntTag_byName() { + return span.setTag(KnownTags.HTTP_RESEND_COUNT_NAME, 2); + } +} diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java index b0cc812aadc..3427d085306 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java @@ -15,6 +15,7 @@ import datadog.trace.api.DDTags; import datadog.trace.api.DDTraceId; import datadog.trace.api.EndpointTracker; +import datadog.trace.api.KnownTags; import datadog.trace.api.TagMap; import datadog.trace.api.TraceConfig; import datadog.trace.api.debugger.DebuggerConfigBridge; @@ -514,6 +515,67 @@ public DDSpan setTag(final String tag, final Object value) { return this; } + // Id-keyed overrides: same behavior as the String family above, including interception and the + // http.status_code quirk, but the tag is stored by id with no name lookup. + @Override + public DDSpan setTag(final long tagId, final String value) { + if (value == null || value.isEmpty()) { + context.removeTag(tagId); + } else { + context.setTag(tagId, value); + } + return this; + } + + @Override + public DDSpan setTag(final long tagId, final boolean value) { + context.setTag(tagId, value); + return this; + } + + @Override + public DDSpan setTag(final long tagId, final int value) { + if (tagId == KnownTags.HTTP_STATUS_CODE_ID) { + context.setHttpStatusCode((short) value); + } + context.setTag(tagId, value); + return this; + } + + @Override + public DDSpan setTag(final long tagId, final long value) { + context.setTag(tagId, value); + return this; + } + + @Override + public DDSpan setTag(final long tagId, final float value) { + context.setTag(tagId, value); + return this; + } + + @Override + public DDSpan setTag(final long tagId, final double value) { + context.setTag(tagId, value); + return this; + } + + @Override + public DDSpan setTag(final long tagId, final CharSequence value) { + if (value == null || value.length() == 0) { + context.setTag(tagId, (Object) null); + } else { + context.setTag(tagId, value); + } + return this; + } + + @Override + public DDSpan setTag(final long tagId, final Object value) { + context.setTag(tagId, value); + return this; + } + @Override public AgentSpan setAllTags(Map map) { context.setAllTags(map); 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 09698ee0c5c..a3a86d0e669 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 @@ -10,6 +10,7 @@ import datadog.trace.api.DDTags; import datadog.trace.api.DDTraceId; import datadog.trace.api.Functions; +import datadog.trace.api.KnownTagCodec; import datadog.trace.api.KnownTags; import datadog.trace.api.ProcessTags; import datadog.trace.api.TagMap; @@ -610,9 +611,9 @@ public void setSpanType(final CharSequence spanType) { *

This is the shared seam for both the construction path ({@code CoreSpanBuilder}) and * decorator {@code afterStart} (via {@link DDSpan#apply}). The context owns the tag map, so the * eventual cheaper bulk-share path (skipping interception for non-intercepted tags) and the - * identity short-circuit will land here -- deferred to the dense-store / tag-registry work, which - * exposes intercept status at the internal-api level. Until then the constant tags route through - * the interceptor, identical to the per-tag calls this replaces. + * identity short-circuit will land here, with the dense store; a tag's intercept status is + * already on its id ({@link KnownTagCodec#isIntercepted}). Until then each constant tag is + * prechecked by the interceptor, identical to the per-tag calls this replaces. */ public void apply(@Nonnull final SpanPrototype prototype) { if (this.spanType == null) { @@ -647,8 +648,7 @@ private void seedAbsentTags(final TagMap map) { if (ctx.unsafeTags.containsKey(tag)) { return; } - final Object value = tagEntry.objectValue(); - if (!ctx.tagInterceptor.interceptTag(ctx, tag, value)) { + if (!ctx.tagInterceptor.interceptTag(ctx, tagEntry)) { ctx.unsafeTags.set(tagEntry); } }); @@ -995,12 +995,33 @@ public void removeTag(String tag) { } } + /** + * Removes a known tag by id rather than by its name. An id the id-keyed setters ignore is ignored + * here too. + * + * @param tagId a {@code KnownTags.*_ID} constant + */ + public void removeTag(long tagId) { + if (!KnownTagCodec.isKeyableById(tagId)) { + return; + } + if (tagId == KnownTags.SPAN_KIND_ID) { + spanKindOrdinal = SPAN_KIND_UNSET; + } + synchronized (unsafeTags) { + unsafeTags.getAndRemove(tagId); + } + } + /** * Sets a tag to the span. Tags are not propagated to the children. * *

Existing tag value with the same value will be replaced. Setting a tag with a {@code null} * value will remove the tag from the span. * + *

A known tag's name is resolved to its id once, and the id-keyed setter does the rest; only a + * custom tag is handled by name. + * * @param tag The tag name. * @param value The nullable tag value. */ @@ -1008,9 +1029,12 @@ public void setTag(final String tag, final Object value) { if (null == tag) { return; } - if (null == value) { + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTag(tagId, value); + } else if (null == value) { removeTag(tag); - } else if (!tagInterceptor.interceptTag(this, tag, value)) { + } else if (!tagInterceptor.interceptCustomTag(this, tag, value)) { synchronized (unsafeTags) { unsafeTags.set(tag, value); } @@ -1021,25 +1045,124 @@ public void setTag(final String tag, final String value) { if (null == tag) { return; } - if (null == value) { + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTag(tagId, value); + } else if (null == value) { removeTag(tag); - } else if (!tagInterceptor.interceptTag(this, tag, value)) { + } else if (!tagInterceptor.interceptCustomTag(this, tag, value)) { synchronized (unsafeTags) { unsafeTags.set(tag, value); } } } + /* + * Id-keyed setters, mirroring the String setters above. With a constant id, the + * KnownTagCodec.INTERCEPTED test folds, leaving only the split-by-tags table load; an id set never + * takes the custom-tag path. An id that names no known tag, or whose name is shared across + * directions (see KnownTagCodec#isKeyableById), is ignored, like a null tag. + */ + public void setTag(final long tagId, final Object value) { + if (!KnownTagCodec.isKeyableById(tagId)) { + return; + } + if (null == value) { + removeTag(tagId); + } else if (!tagInterceptor.needsIntercept(tagId) + || !tagInterceptor.interceptTag(this, tagId, value)) { + synchronized (unsafeTags) { + unsafeTags.internals().setKnown(tagId, value); + } + } + } + + public void setTag(final long tagId, final CharSequence value) { + if (!KnownTagCodec.isKeyableById(tagId)) { + return; + } + if (null == value) { + removeTag(tagId); + } else if (!tagInterceptor.needsIntercept(tagId) + || !tagInterceptor.interceptTag(this, tagId, value)) { + synchronized (unsafeTags) { + unsafeTags.internals().setKnown(tagId, value); + } + } + } + + public void setTag(final long tagId, final boolean value) { + if (!KnownTagCodec.isKeyableById(tagId)) { + return; + } + if (tagInterceptor.needsIntercept(tagId)) { + this.setBox(tagId, value); + } else { + synchronized (unsafeTags) { + unsafeTags.internals().setKnown(tagId, value); + } + } + } + + public void setTag(final long tagId, final int value) { + if (!KnownTagCodec.isKeyableById(tagId)) { + return; + } + if (tagInterceptor.needsIntercept(tagId)) { + this.setBox(tagId, value); + } else { + synchronized (unsafeTags) { + unsafeTags.internals().setKnown(tagId, value); + } + } + } + + public void setTag(final long tagId, final long value) { + if (!KnownTagCodec.isKeyableById(tagId)) { + return; + } + if (tagInterceptor.needsIntercept(tagId)) { + this.setBox(tagId, value); + } else { + synchronized (unsafeTags) { + unsafeTags.internals().setKnown(tagId, value); + } + } + } + + public void setTag(final long tagId, final float value) { + if (!KnownTagCodec.isKeyableById(tagId)) { + return; + } + if (tagInterceptor.needsIntercept(tagId)) { + this.setBox(tagId, value); + } else { + synchronized (unsafeTags) { + unsafeTags.internals().setKnown(tagId, value); + } + } + } + + public void setTag(final long tagId, final double value) { + if (!KnownTagCodec.isKeyableById(tagId)) { + return; + } + if (tagInterceptor.needsIntercept(tagId)) { + this.setBox(tagId, value); + } else { + synchronized (unsafeTags) { + unsafeTags.internals().setKnown(tagId, value); + } + } + } + public void setTag(TagMap.EntryReader entry) { if (entry == null) { return; } - // pre-check to avoid boxing - boolean intercepted = - precheckIntercept(entry.tag()) - && tagInterceptor.interceptTag(this, entry.tag(), entry.objectValue()); - if (!intercepted) { + // the interceptor prechecks the entry, to avoid boxing + if (!tagInterceptor.interceptTag(this, entry)) { synchronized (unsafeTags) { unsafeTags.set(entry); } @@ -1047,16 +1170,7 @@ public void setTag(TagMap.EntryReader entry) { } /* - * Uses to determine if there's an opportunity to avoid primitve boxing. - * If the underlying map doesn't support efficient primitives, then boxing is used. - * If the tag may be intercepted, then boxing is also used. - */ - private boolean precheckIntercept(String tag) { - return tagInterceptor.needsIntercept(tag); - } - - /* - * Used when precheckIntercept determines that boxing is unavoidable + * Used when the interceptor's precheck says the tag may be intercepted, so boxing is unavoidable * * Either because the tagInterceptor needs to be fully checked (which requires boxing) * In that case, a box has already been created so it makes sense to pass the box @@ -1067,10 +1181,10 @@ private boolean precheckIntercept(String tag) { * The TagMap isn't optimized and will need to box the primitive regardless of * tag interception */ - private void setBox(String tag, Object box) { - if (!tagInterceptor.interceptTag(this, tag, box)) { + private void setBox(long tagId, Object box) { + if (!tagInterceptor.interceptTag(this, tagId, box)) { synchronized (unsafeTags) { - unsafeTags.set(tag, box); + unsafeTags.internals().setKnown(tagId, box); } } } @@ -1079,9 +1193,11 @@ public void setTag(final String tag, final boolean value) { if (null == tag) { return; } - if (precheckIntercept(tag)) { - this.setBox(tag, value); - } else { + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTag(tagId, value); + } else if (!tagInterceptor.needsInterceptCustomTag(tag) + || !tagInterceptor.interceptCustomTag(this, tag, value)) { synchronized (unsafeTags) { unsafeTags.set(tag, value); } @@ -1092,9 +1208,11 @@ public void setTag(final String tag, final int value) { if (null == tag) { return; } - if (precheckIntercept(tag)) { - this.setBox(tag, value); - } else { + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTag(tagId, value); + } else if (!tagInterceptor.needsInterceptCustomTag(tag) + || !tagInterceptor.interceptCustomTag(this, tag, value)) { synchronized (unsafeTags) { unsafeTags.set(tag, value); } @@ -1105,10 +1223,11 @@ public void setTag(final String tag, final long value) { if (null == tag) { return; } - // check needsIntercept first to avoid unnecessary boxing - boolean intercepted = - tagInterceptor.needsIntercept(tag) && tagInterceptor.interceptTag(this, tag, value); - if (!intercepted) { + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTag(tagId, value); + } else if (!tagInterceptor.needsInterceptCustomTag(tag) + || !tagInterceptor.interceptCustomTag(this, tag, value)) { synchronized (unsafeTags) { unsafeTags.set(tag, value); } @@ -1119,9 +1238,11 @@ public void setTag(final String tag, final float value) { if (null == tag) { return; } - if (precheckIntercept(tag)) { - this.setBox(tag, value); - } else { + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTag(tagId, value); + } else if (!tagInterceptor.needsInterceptCustomTag(tag) + || !tagInterceptor.interceptCustomTag(this, tag, value)) { synchronized (unsafeTags) { unsafeTags.set(tag, value); } @@ -1132,9 +1253,11 @@ public void setTag(final String tag, final double value) { if (null == tag) { return; } - if (precheckIntercept(tag)) { - this.setBox(tag, value); - } else { + final long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + setTag(tagId, value); + } else if (!tagInterceptor.needsInterceptCustomTag(tag) + || !tagInterceptor.interceptCustomTag(this, tag, value)) { synchronized (unsafeTags) { unsafeTags.set(tag, value); } @@ -1158,10 +1281,7 @@ void setAllTags(final TagMap map, boolean needsIntercept) { map.forEach( this, (ctx, tagEntry) -> { - String tag = tagEntry.tag(); - Object value = tagEntry.objectValue(); - - if (!ctx.tagInterceptor.interceptTag(ctx, tag, value)) { + if (!ctx.tagInterceptor.interceptTag(ctx, tagEntry)) { ctx.unsafeTags.set(tagEntry); } }); @@ -1188,9 +1308,7 @@ void setAllTags(final TagMap.Ledger ledger) { } else { TagMap.Entry entry = (TagMap.Entry) entryChange; - Object value = entry.objectValue(); - - if (!tagInterceptor.interceptTag(this, tag, value)) { + if (!tagInterceptor.interceptTag(this, entry)) { unsafeTags.set(entry); } } 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 89e5a01adb8..82a6cb16c06 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 @@ -1,19 +1,11 @@ package datadog.trace.core.taginterceptor; import static datadog.trace.api.DDTags.ANALYTICS_SAMPLE_RATE; -import static datadog.trace.api.DDTags.MEASURED; -import static datadog.trace.api.DDTags.ORIGIN_KEY; -import static datadog.trace.api.DDTags.SPAN_TYPE; -import static datadog.trace.api.KnownTags.DB_STATEMENT_OTEL_NAME; -import static datadog.trace.api.KnownTags.HTTP_METHOD_OTEL_NAME; -import static datadog.trace.api.KnownTags.HTTP_STATUS_CODE_OTEL_NAME; -import static datadog.trace.api.KnownTags.HTTP_URL_OTEL_NAME; +import static datadog.trace.api.KnownTags.SERVLET_CONTEXT_ID; import static datadog.trace.api.sampling.PrioritySampling.USER_DROP; -import static datadog.trace.bootstrap.instrumentation.api.InstrumentationTags.SERVLET_CONTEXT; import static datadog.trace.bootstrap.instrumentation.api.ServiceNameSources.SPLIT_BY_SERVLET_CONTEXT; import static datadog.trace.bootstrap.instrumentation.api.ServiceNameSources.SPLIT_BY_TAGS; import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_METHOD; -import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_STATUS; import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_URL; import static datadog.trace.core.taginterceptor.RuleFlags.Feature.FORCE_MANUAL_DROP; import static datadog.trace.core.taginterceptor.RuleFlags.Feature.FORCE_SAMPLING_PRIORITY; @@ -27,6 +19,8 @@ import datadog.trace.api.Config; import datadog.trace.api.ConfigDefaults; import datadog.trace.api.DDTags; +import datadog.trace.api.KnownTagCodec; +import datadog.trace.api.KnownTags; import datadog.trace.api.Pair; import datadog.trace.api.TagMap; import datadog.trace.api.config.GeneralConfig; @@ -41,6 +35,7 @@ import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.core.DDSpanContext; import java.net.URI; +import java.util.HashSet; import java.util.Map; import java.util.Set; import javax.annotation.Nonnull; @@ -54,7 +49,15 @@ public class TagInterceptor { private final boolean isServiceNameSetByUser; private final boolean splitByServletContext; private final String inferredServiceName; - private final Set splitServiceTags; + + /** + * Whether each known tag, indexed by serial, is a split-by-tags entry. Sized to every known + * serial and never changed after construction, so the check is a single load. + */ + private final boolean[] splitServiceSerials; + + /** split-by-tags entries naming custom tags; null when there are none. */ + private final Set splitServiceCustomNames; private final boolean shouldSet404ResourceName; private final boolean shouldSetUrlResourceAsName; @@ -77,9 +80,10 @@ public TagInterceptor( boolean jeeSplitByDeployment) { this.isServiceNameSetByUser = isServiceNameSetByUser; this.inferredServiceName = inferredServiceName; - this.splitServiceTags = splitServiceTags; + this.splitServiceSerials = knownTagSerials(splitServiceTags); + this.splitServiceCustomNames = customTagNames(splitServiceTags); this.ruleFlags = ruleFlags; - splitByServletContext = splitServiceTags.contains(SERVLET_CONTEXT); + splitByServletContext = isSplitServiceTag(SERVLET_CONTEXT_ID); shouldSet404ResourceName = ruleFlags.isEnabled(URL_AS_RESOURCE_NAME) @@ -89,13 +93,68 @@ public TagInterceptor( this.jeeSplitByDeployment = jeeSplitByDeployment; } + /** + * Marks, by serial, each known tag that {@code names} names under any of its names, so a tag set + * by id or by its OpenTelemetry name matches too. A name declared per direction marks each + * direction's tag. + */ + private static boolean[] knownTagSerials(Set names) { + boolean[] serials = new boolean[KnownTagCodec.serialLimit()]; + if (names.isEmpty()) { + return serials; + } + Set canonicalNames = new HashSet<>(); + for (String name : names) { + canonicalNames.add(KnownTagCodec.canonicalTagName(name)); + } + for (int serial = 1; serial < serials.length; ++serial) { + serials[serial] = + canonicalNames.contains(KnownTagCodec.nameOf(KnownTagCodec.makeTagId(serial))); + } + return serials; + } + + /** The names in {@code names} that do not resolve to a known tag, or null when there are none. */ + private static Set customTagNames(Set names) { + Set custom = null; + for (String name : names) { + if (KnownTagCodec.keyOf(name) == 0) { + if (custom == null) { + custom = new HashSet<>(); + } + custom.add(name); + } + } + return custom; + } + + /** Requires a known id, whose serial is always in range of the table. */ + private boolean isSplitServiceTag(long tagId) { + return splitServiceSerials[KnownTagCodec.serialNum(tagId)]; + } + + private boolean isSplitServiceTag(String customTag) { + Set names = splitServiceCustomNames; + return names != null && names.contains(customTag); + } + public boolean needsIntercept(TagMap map) { for (TagMap.EntryReader entry : map) { - if (needsIntercept(entry.tag())) return true; + if (needsIntercept(entry)) return true; } return false; } + public boolean needsIntercept(TagMap.EntryReader entry) { + long tagId = entry.tagId(); + return tagId != 0 ? needsIntercept(tagId) : isSplitServiceTag(entry.tag()); + } + + /** Whether a custom tag (one with no id) may be routed; only split-by-tags routes them. */ + public boolean needsInterceptCustomTag(String customTag) { + return isSplitServiceTag(customTag); + } + public boolean needsIntercept(Map map) { for (String tag : map.keySet()) { if (needsIntercept(tag)) return true; @@ -103,131 +162,137 @@ public boolean needsIntercept(Map map) { return false; } - public boolean needsIntercept(String tag) { - switch (tag) { - case DDTags.RESOURCE_NAME: - case Tags.DB_STATEMENT: - case DB_STATEMENT_OTEL_NAME: - case DDTags.SERVICE_NAME: - case "service": - case Tags.PEER_SERVICE: - case DDTags.MANUAL_KEEP: - case DDTags.MANUAL_DROP: - case Tags.ASM_KEEP: - case Tags.AI_GUARD_KEEP: - case Tags.SAMPLING_PRIORITY: - case Tags.PROPAGATED_TRACE_SOURCE: - case Tags.PROPAGATED_DEBUG: - case SERVLET_CONTEXT: - case SPAN_TYPE: - case ANALYTICS_SAMPLE_RATE: - case Tags.ERROR: - case HTTP_STATUS: - case HTTP_STATUS_CODE_OTEL_NAME: - case HTTP_METHOD: - case HTTP_METHOD_OTEL_NAME: - case HTTP_URL: - case HTTP_URL_OTEL_NAME: - case ORIGIN_KEY: - case MEASURED: - case Tags.SPAN_KIND: - return true; + /** + * Whether {@link #interceptTag(DDSpanContext, long, Object)} may route the tag. With a constant + * id, the {@link KnownTagCodec#INTERCEPTED} test folds to a constant, leaving only the + * split-by-tags table load at run time. {@code tagId} must be a known id. + */ + public boolean needsIntercept(long tagId) { + return KnownTagCodec.isIntercepted(tagId) || isSplitServiceTag(tagId); + } - default: - return splitServiceTags.contains(tag); - } + public boolean needsIntercept(String tag) { + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? needsIntercept(tagId) : isSplitServiceTag(tag); } public boolean interceptTag(DDSpanContext span, String tag, Object value) { - switch (tag) { - case DDTags.RESOURCE_NAME: + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? interceptTag(span, tagId, value) : interceptCustomTag(span, tag, value); + } + + /** + * Routes a known tag to a span field or a sampling directive. Returns true when the tag was + * consumed, false when it should still be stored. Every tag with a case here carries the {@link + * KnownTagCodec#INTERCEPTED} bit, declared in {@code tag-conventions-java.yaml}. + * + *

This method is deliberately too big for C2 to inline, so the handler bodies stay out of + * {@code DDSpanContext.setTag}'s compiled code; {@code TagInterceptorInliningTest} pins that. + */ + public boolean interceptTag(DDSpanContext span, long tagId, Object value) { + switch (KnownTagCodec.serialNum(tagId)) { + case KnownTags.RESOURCE_NAME_SERIAL_NUM: return interceptResourceName(span, value); - case Tags.DB_STATEMENT: - case DB_STATEMENT_OTEL_NAME: + case KnownTags.DB_STATEMENT_SERIAL_NUM: return interceptDbStatement(span, value); - case DDTags.SERVICE_NAME: - case "service": + case KnownTags.SERVICE_SERIAL_NUM: return interceptServiceName(SERVICE_NAME, span, value); - case Tags.PEER_SERVICE: + case KnownTags.PEER_SERVICE_SERIAL_NUM: // we still need to intercept and add this tag when the user manually set span.setTag(DDTags.PEER_SERVICE_SOURCE, Tags.PEER_SERVICE); return interceptServiceName(PEER_SERVICE, span, value); - case DDTags.MANUAL_KEEP: + case KnownTags.MANUAL_KEEP_SERIAL_NUM: if (asBoolean(value)) { span.forceKeep(); return true; } return false; - case DDTags.MANUAL_DROP: + case KnownTags.MANUAL_DROP_SERIAL_NUM: return interceptSamplingPriority( FORCE_MANUAL_DROP, USER_DROP, SamplingMechanism.MANUAL, span, value); - case Tags.ASM_KEEP: + case KnownTags.ASM_KEEP_SERIAL_NUM: if (asBoolean(value)) { span.forceKeep(SamplingMechanism.APPSEC); return true; } return false; - case Tags.AI_GUARD_KEEP: + case KnownTags.AI_GUARD_KEEP_SERIAL_NUM: if (asBoolean(value)) { span.forceKeep(SamplingMechanism.AI_GUARD); return true; } return false; - case Tags.SAMPLING_PRIORITY: + case KnownTags.SAMPLING_PRIORITY_SERIAL_NUM: return interceptSamplingPriority(span, value); - case Tags.PROPAGATED_TRACE_SOURCE: + case KnownTags.DD_P_TS_SERIAL_NUM: if (value instanceof Integer) { span.addPropagatedTraceSource((Integer) value); return true; } return false; - case Tags.PROPAGATED_DEBUG: + case KnownTags.DD_P_DEBUG_SERIAL_NUM: span.updateDebugPropagation(String.valueOf(value)); return true; - case SERVLET_CONTEXT: + case KnownTags.SERVLET_CONTEXT_SERIAL_NUM: return interceptServletContext(span, value); - case SPAN_TYPE: + case KnownTags.SPAN_TYPE_SERIAL_NUM: return interceptSpanType(span, value); - case ANALYTICS_SAMPLE_RATE: + case KnownTags.DD1_SR_EAUSR_SERIAL_NUM: return interceptAnalyticsSampleRate(span, value); - case Tags.ERROR: + case KnownTags.ERROR_SERIAL_NUM: return interceptError(span, value); - case HTTP_STATUS: - case HTTP_STATUS_CODE_OTEL_NAME: + case KnownTags.HTTP_STATUS_CODE_SERIAL_NUM: // not set internally but may come from manual instrumentation return interceptHttpStatusCode(span, value); - case HTTP_METHOD: - case HTTP_METHOD_OTEL_NAME: - case HTTP_URL: - case HTTP_URL_OTEL_NAME: - return interceptUrlResourceAsNameRule(span, tag, value); - case ORIGIN_KEY: + case KnownTags.HTTP_METHOD_SERIAL_NUM: + case KnownTags.HTTP_URL_SERIAL_NUM: + return interceptUrlResourceAsNameRule(span, tagId, value); + case KnownTags.DD_ORIGIN_SERIAL_NUM: return interceptOrigin(span, value); - case MEASURED: + case KnownTags.DD_MEASURED_SERIAL_NUM: return interceptMeasured(span, value); - case Tags.SPAN_KIND: + case KnownTags.SPAN_KIND_SERIAL_NUM: // Cache the ordinal for fast isOutbound() checks. // Return false so the value is still stored in unsafeTags for serialization. span.setSpanKindOrdinal(String.valueOf(value)); return false; default: - return intercept(span, tag, value); + return isSplitServiceTag(tagId) && splitService(span, value); } } - private boolean interceptUrlResourceAsNameRule(DDSpanContext span, String tag, Object value) { + /** + * Routes an entry by its id, or by its name when it is a custom tag. Prechecks, so a primitive + * entry that needs no interception is never boxed. + */ + public boolean interceptTag(DDSpanContext span, TagMap.EntryReader entry) { + long tagId = entry.tagId(); + if (tagId != 0) { + return needsIntercept(tagId) && interceptTag(span, tagId, entry.objectValue()); + } + return isSplitServiceTag(entry.tag()) && splitService(span, entry.objectValue()); + } + + /** Routes a custom tag -- one with no id. */ + public boolean interceptCustomTag(DDSpanContext span, String customTag, Object value) { + return isSplitServiceTag(customTag) && splitService(span, value); + } + + private static boolean splitService(DDSpanContext span, Object value) { + span.setServiceName(String.valueOf(value), SPLIT_BY_TAGS); + return true; + } + + private boolean interceptUrlResourceAsNameRule(DDSpanContext span, long tagId, Object value) { if (shouldSetUrlResourceAsName) { - if (HTTP_METHOD.equals(tag) || HTTP_METHOD_OTEL_NAME.equals(tag)) { + // Read by Datadog name only: entries are stored under it whichever spelling set them. + if (tagId == KnownTags.HTTP_METHOD_ID) { final Object url = span.unsafeGetTag(HTTP_URL); if (url != null) { setResourceFromUrl(span, value.toString(), url); } - } else if (HTTP_URL.equals(tag) || HTTP_URL_OTEL_NAME.equals(tag)) { - // the method may have been set under either spelling -- see HTTP_METHOD_OTEL_NAME. - Object method = span.unsafeGetTag(HTTP_METHOD); - if (method == null) { - method = span.unsafeGetTag(HTTP_METHOD_OTEL_NAME); - } + } else { + final Object method = span.unsafeGetTag(HTTP_METHOD); setResourceFromUrl(span, method != null ? method.toString() : null, value); } } @@ -258,14 +323,6 @@ private static void setResourceFromUrl( } } - private boolean intercept(DDSpanContext span, String tag, Object value) { - if (splitServiceTags.contains(tag)) { - span.setServiceName(String.valueOf(value), SPLIT_BY_TAGS); - return true; - } - return false; - } - private boolean interceptResourceName(DDSpanContext span, Object value) { if (ruleFlags.isEnabled(RESOURCE_NAME)) { if (null == value) { diff --git a/dd-trace-core/src/test/java/datadog/trace/core/DDSpanSetTagByIdTest.java b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanSetTagByIdTest.java new file mode 100644 index 00000000000..34fc1bc2028 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanSetTagByIdTest.java @@ -0,0 +1,112 @@ +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.common.writer.ListWriter; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * {@code DDSpan.setTag(long, ...)}: setting a known tag by id behaves exactly like setting it by + * name, including interception. + */ +class DDSpanSetTagByIdTest extends DDCoreJavaSpecification { + private CoreTracer tracer; + private DDSpan span; + + @BeforeEach + void setup() { + tracer = tracerBuilder().writer(new ListWriter()).build(); + span = (DDSpan) tracer.buildSpan("datadog", "fakeOperation").start(); + } + + @AfterEach + void tearDown() { + span.finish(); + tracer.close(); + } + + @Test + void storesTheTagUnderItsName() { + span.setTag(KnownTags.HTTP_RESEND_COUNT_ID, 2); + span.setTag(KnownTags.PEER_HOSTNAME_ID, "db.internal"); + span.setTag(KnownTags.DD_PROFILING_ENABLED_ID, true); + + assertEquals(2, span.getTag(KnownTags.HTTP_RESEND_COUNT_NAME)); + assertEquals("db.internal", span.getTag(KnownTags.PEER_HOSTNAME_NAME)); + assertEquals(true, span.getTag(KnownTags.DD_PROFILING_ENABLED_NAME)); + } + + @Test + void aNullOrEmptyValueRemovesTheTagById() { + span.setTag(KnownTags.PEER_HOSTNAME_ID, "db.internal"); + span.setTag(KnownTags.COMPONENT_ID, "okhttp"); + + span.setTag(KnownTags.PEER_HOSTNAME_ID, (String) null); + span.setTag(KnownTags.COMPONENT_ID, ""); + + assertNull(span.getTag(KnownTags.PEER_HOSTNAME_NAME)); + assertNull(span.getTag(KnownTags.COMPONENT_NAME)); + } + + @Test + void clearingAnUnknownIdIsIgnored() { + span.setTag(0L, (String) null); + span.setTag(0L, ""); + span.setTag(0L, (Object) null); + + assertNull(span.getTag(KnownTags.PEER_HOSTNAME_NAME)); + } + + @Test + void aSharedNameIdIsIgnoredAndLeavesTheNamedTag() { + span.setTag("peer.port", 8080); + + span.setTag(KnownTags.PEER_PORT_OUTBOUND_ID, 443); + span.setTag(KnownTags.PEER_PORT_INBOUND_ID, (Object) null); + + assertEquals(8080, span.getTag("peer.port")); + } + + @Test + void removingTheSpanKindByIdClearsTheKind() { + span.setTag(KnownTags.SPAN_KIND_ID, "client"); + assertEquals("client", span.getSpanKindString()); + + span.setTag(KnownTags.SPAN_KIND_ID, (String) null); + + assertNull(span.getSpanKindString()); + assertNull(span.getTag(KnownTags.SPAN_KIND_NAME)); + } + + @Test + void interceptedTagsAreStillIntercepted() { + span.setTag(KnownTags.DB_STATEMENT_ID, "select 1"); + span.setTag(KnownTags.SERVICE_ID, "orders-db"); + + assertEquals("select 1", span.getResourceName().toString()); + assertNull(span.getTag(KnownTags.DB_STATEMENT_NAME)); + assertEquals("orders-db", span.getServiceName()); + } + + @Test + void httpStatusCodeSetsTheStatusField() { + span.setTag(KnownTags.HTTP_STATUS_CODE_ID, 503); + + assertEquals(503, span.getHttpStatusCode()); + } + + @Test + void anEmptyOrNullValueRemovesTheTag() { + span.setTag(KnownTags.PEER_HOSTNAME_ID, "db.internal"); + span.setTag(KnownTags.PEER_HOSTNAME_ID, ""); + assertNull(span.getTag(KnownTags.PEER_HOSTNAME_NAME)); + + span.setTag(KnownTags.PEER_HOSTNAME_ID, (CharSequence) "db.internal"); + span.setTag(KnownTags.PEER_HOSTNAME_ID, (CharSequence) null); + assertNull(span.getTag(KnownTags.PEER_HOSTNAME_NAME)); + } +} diff --git a/dd-trace-core/src/test/java/datadog/trace/core/taginterceptor/TagInterceptorInliningTest.java b/dd-trace-core/src/test/java/datadog/trace/core/taginterceptor/TagInterceptorInliningTest.java new file mode 100644 index 00000000000..e11811d641c --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/taginterceptor/TagInterceptorInliningTest.java @@ -0,0 +1,78 @@ +package datadog.trace.core.taginterceptor; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.io.InputStream; +import org.junit.jupiter.api.Test; +import org.objectweb.asm.ClassReader; + +/** + * Pins a code shape that C2's inlining depends on and no behavioral test can see. + * + *

{@code interceptTag(DDSpanContext, long, Object)} must stay too big for C2 to inline: more + * than {@code FreqInlineSize}, 325 bytes of bytecode by default. DDSpanContext.setTag calls it only + * for intercepted tags. If it inlined into setTag's own compiled code, the profiled handler bodies + * would come with it and can push setTag past {@code InlineSmallCode} (2500 bytes of machine code + * by default). Callers then refuse to inline setTag, and a constant id no longer folds its + * interception test away. + */ +class TagInterceptorInliningTest { + private static final int FREQ_INLINE_SIZE = 325; + + @Test + void interceptTagByIdStaysTooBigToInline() throws IOException { + int length = + codeLength("interceptTag", "(Ldatadog/trace/core/DDSpanContext;JLjava/lang/Object;)Z"); + + assertTrue( + length > FREQ_INLINE_SIZE, + "interceptTag(span, long, value) is " + + length + + " bytes of bytecode, small enough for C2 to inline into DDSpanContext.setTag; keep" + + " its handlers in the switch (see this test's Javadoc)"); + } + + /** The {@code code_length} of a method's Code attribute, read from the class file. */ + private static int codeLength(String name, String descriptor) throws IOException { + ClassReader reader; + try (InputStream in = TagInterceptor.class.getResourceAsStream("TagInterceptor.class")) { + reader = new ClassReader(in); + } + char[] buf = new char[reader.getMaxStringLength()]; + int offset = reader.header + 6; // access_flags, this_class, super_class + offset += 2 + 2 * reader.readUnsignedShort(offset); // interfaces + int fields = reader.readUnsignedShort(offset); + offset += 2; + for (int i = 0; i < fields; ++i) { + offset = skipAttributes(reader, offset + 6); // access_flags, name, descriptor + } + int methods = reader.readUnsignedShort(offset); + offset += 2; + for (int i = 0; i < methods; ++i) { + if (reader.readUTF8(offset + 2, buf).equals(name) + && reader.readUTF8(offset + 4, buf).equals(descriptor)) { + int attributes = reader.readUnsignedShort(offset + 6); + int attribute = offset + 8; + for (int j = 0; j < attributes; ++j) { + if ("Code".equals(reader.readUTF8(attribute, buf))) { + return reader.readInt(attribute + 10); // name, length, max_stack, max_locals + } + attribute += 6 + reader.readInt(attribute + 2); + } + } + offset = skipAttributes(reader, offset + 6); + } + throw new AssertionError(name + descriptor + " not found"); + } + + /** Skips the attributes whose count is at {@code offset}; returns the offset after them. */ + private static int skipAttributes(ClassReader reader, int offset) { + int attributes = reader.readUnsignedShort(offset); + offset += 2; + for (int i = 0; i < attributes; ++i) { + offset += 6 + reader.readInt(offset + 2); // name, length, info + } + return offset; + } +} 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 91f3da0910f..1b1a925b856 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 @@ -18,6 +18,7 @@ import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockingDetails; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -25,6 +26,8 @@ import datadog.trace.api.DDSpanTypes; import datadog.trace.api.DDTags; import datadog.trace.api.DDTraceId; +import datadog.trace.api.KnownTagCodec; +import datadog.trace.api.KnownTags; import datadog.trace.api.ProductTraceSource; import datadog.trace.api.env.CapturedEnvironment; import datadog.trace.api.remoteconfig.ServiceNameCollector; @@ -34,6 +37,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags; +import datadog.trace.bootstrap.instrumentation.api.ServiceNameSources; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.common.sampling.AllSampler; import datadog.trace.common.writer.ListWriter; @@ -48,6 +52,7 @@ import datadog.trace.test.junit.utils.config.WithConfig; import datadog.trace.test.junit.utils.converter.ConfigDefaultsConverter; import datadog.trace.test.junit.utils.converter.TagsConverter; +import java.lang.reflect.Field; import java.util.Collections; import java.util.Map; import java.util.function.Function; @@ -905,4 +910,80 @@ void whenInterceptsProductTraceSourcePropagationTagUpdatePropagatedTraceSourceIs verify(context, times(1)).addPropagatedTraceSource(ProductTraceSource.ASM); } + + @Test + void exactlyTheTagsWithTheInterceptedBitHaveACase() throws IllegalAccessException { + RuleFlags ruleFlags = mock(RuleFlags.class); + when(ruleFlags.isEnabled(any())).thenReturn(true); + TagInterceptor interceptor = + new TagInterceptor(false, "my-service", emptySet(), ruleFlags, false); + + for (Field field : KnownTags.class.getFields()) { + if (field.getType() != long.class) { + continue; + } + long tagId = field.getLong(null); + DDSpanContext span = mock(DDSpanContext.class); + when(span.getServiceName()).thenReturn(""); + + // Each case consumes one of these values or acts on the span for it; with no split-by-tags, + // the default case does neither. + boolean consumed = false; + for (Object value : new Object[] {true, 1, "1"}) { + consumed |= interceptor.interceptTag(span, tagId, value); + } + + boolean hasCase = consumed || !mockingDetails(span).getInvocations().isEmpty(); + assertEquals(KnownTagCodec.isIntercepted(tagId), hasCase, field.getName()); + assertEquals(KnownTagCodec.isIntercepted(tagId), interceptor.needsIntercept(tagId)); + } + } + + @TableTest({ + "scenario | configured | tag ", + "configured by Datadog name | db.type | db.type ", + "configured by OTel name | db.system | db.type ", + "set by OTel name | db.type | db.system" + }) + void splitByTagsMatchesAKnownTagByIdUnderAnyName(String configured, String tag) { + TagInterceptor interceptor = splittingInterceptor(configured); + DDSpanContext span = mock(DDSpanContext.class); + + assertTrue(interceptor.needsIntercept(KnownTags.DB_TYPE_ID)); + assertTrue(interceptor.needsIntercept(tag)); + assertFalse(interceptor.needsIntercept(KnownTags.COMPONENT_ID)); + assertTrue(interceptor.interceptTag(span, KnownTags.DB_TYPE_ID, "postgres")); + assertTrue(interceptor.interceptTag(span, tag, "mysql")); + + verify(span).setServiceName("postgres", ServiceNameSources.SPLIT_BY_TAGS); + verify(span).setServiceName("mysql", ServiceNameSources.SPLIT_BY_TAGS); + } + + @Test + void splitByTagsMatchesACustomTagByName() { + TagInterceptor interceptor = splittingInterceptor("my.custom.tag"); + DDSpanContext span = mock(DDSpanContext.class); + + assertTrue(interceptor.needsIntercept("my.custom.tag")); + assertFalse(interceptor.needsIntercept("other.custom.tag")); + assertFalse(interceptor.needsIntercept(KnownTags.COMPONENT_ID)); + assertTrue(interceptor.interceptTag(span, "my.custom.tag", "split")); + assertFalse(interceptor.interceptTag(span, "other.custom.tag", "split")); + + verify(span).setServiceName("split", ServiceNameSources.SPLIT_BY_TAGS); + } + + @Test + void splitByTagsOnANameDeclaredPerDirectionMatchesEachDirection() { + TagInterceptor interceptor = splittingInterceptor(Tags.PEER_PORT); + + assertTrue(interceptor.needsIntercept(KnownTags.PEER_PORT_INBOUND_ID)); + assertTrue(interceptor.needsIntercept(KnownTags.PEER_PORT_OUTBOUND_ID)); + assertTrue(interceptor.needsIntercept(Tags.PEER_PORT)); + } + + private static TagInterceptor splittingInterceptor(String splitByTag) { + return new TagInterceptor( + true, "my-service", Collections.singleton(splitByTag), new RuleFlags(), false); + } } diff --git a/gradle/forbiddenApiFilters/instrumentation.txt b/gradle/forbiddenApiFilters/instrumentation.txt index de3910a8ac8..5ffb1f7868d 100644 --- a/gradle/forbiddenApiFilters/instrumentation.txt +++ b/gradle/forbiddenApiFilters/instrumentation.txt @@ -2,3 +2,8 @@ # Use AgentSpan#setServiceName(String, CharSequence) instead to track the source of the service name @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() diff --git a/internal-api/build.gradle.kts b/internal-api/build.gradle.kts index a9d82cebc72..9c877c4562d 100644 --- a/internal-api/build.gradle.kts +++ b/internal-api/build.gradle.kts @@ -266,6 +266,7 @@ extra["excludedClassesInstructionCoverage"] = listOf("datadog.trace.util.stacktr tagRegistry { tagConventionsFile.set(rootProject.layout.projectDirectory.file("tag-conventions.yaml")) + tracerOverlayFile.set(rootProject.layout.projectDirectory.file("tag-conventions-java.yaml")) } dependencies { diff --git a/internal-api/src/jmh/java/datadog/trace/api/TagMapInsertBenchmark.java b/internal-api/src/jmh/java/datadog/trace/api/TagMapInsertBenchmark.java new file mode 100644 index 00000000000..628d711a9e6 --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/api/TagMapInsertBenchmark.java @@ -0,0 +1,108 @@ +package datadog.trace.api; + +import java.util.concurrent.TimeUnit; +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.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * Inserting a span's worth of tags into a fresh {@link TagMap}, the way span decoration does: known + * tags by Datadog name, the same known tags by id, and custom tags by name. + * + *

Single-threaded: each invocation builds its own map, so more threads would mostly measure + * allocation bandwidth. Run with {@code -prof gc} for allocation per operation. Tag names are + * string constants, as the tracer's are. + * + *

Results (MacBook, JDK 21, default flags, {@code @Fork(2)} triage), 12 tags per op: + * + * 

+ * Benchmark      ns/op         B/op
+ * knownByName    121.1 +- 3.8  736
+ * knownById       94.7 +- 1.7  736
+ * customByName    80.6 +- 2.4  736
+ *
+ */ +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(2) +@Warmup(iterations = 5, time = 1) +@Measurement(iterations = 5, time = 1) +@Threads(1) +@State(Scope.Benchmark) +public class TagMapInsertBenchmark { + // Per-span tags a client or server decorator typically sets. + static final String[] KNOWN_NAMES = { + KnownTags.COMPONENT_NAME, + KnownTags.SPAN_KIND_NAME, + KnownTags.HTTP_METHOD_NAME, + KnownTags.HTTP_URL_NAME, + KnownTags.HTTP_ROUTE_NAME, + KnownTags.HTTP_HOSTNAME_NAME, + KnownTags.HTTP_USERAGENT_NAME, + KnownTags.HTTP_CLIENT_IP_NAME, + KnownTags.PEER_HOSTNAME_NAME, + KnownTags.PEER_IPV4_NAME, + KnownTags.PEER_IPV6_NAME, + KnownTags.DD_INTEGRATION_NAME, + }; + + static final long[] KNOWN_IDS = { + KnownTags.COMPONENT_ID, + KnownTags.SPAN_KIND_ID, + KnownTags.HTTP_METHOD_ID, + KnownTags.HTTP_URL_ID, + KnownTags.HTTP_ROUTE_ID, + KnownTags.HTTP_HOSTNAME_ID, + KnownTags.HTTP_USERAGENT_ID, + KnownTags.HTTP_CLIENT_IP_ID, + KnownTags.PEER_HOSTNAME_ID, + KnownTags.PEER_IPV4_ID, + KnownTags.PEER_IPV6_ID, + KnownTags.DD_INTEGRATION_ID, + }; + + static final String[] CUSTOM_NAMES = new String[KNOWN_NAMES.length]; + + static final Object[] VALUES = new Object[KNOWN_NAMES.length]; + + static { + for (int i = 0; i < KNOWN_NAMES.length; ++i) { + CUSTOM_NAMES[i] = ("app.custom.tag." + i).intern(); + VALUES[i] = "value-" + i; + } + } + + @Benchmark + public TagMap knownByName() { + TagMap map = TagMap.create(); + for (int i = 0; i < KNOWN_NAMES.length; ++i) { + map.set(KNOWN_NAMES[i], VALUES[i]); + } + return map; + } + + @Benchmark + public TagMap knownById() { + TagMap map = TagMap.create(); + for (int i = 0; i < KNOWN_IDS.length; ++i) { + map.set(KNOWN_IDS[i], VALUES[i]); + } + return map; + } + + @Benchmark + public TagMap customByName() { + TagMap map = TagMap.create(); + for (int i = 0; i < CUSTOM_NAMES.length; ++i) { + map.set(CUSTOM_NAMES[i], VALUES[i]); + } + return map; + } +} diff --git a/internal-api/src/main/java/datadog/apicontract/Restricted.java b/internal-api/src/main/java/datadog/apicontract/Restricted.java new file mode 100644 index 00000000000..ab0372fa06f --- /dev/null +++ b/internal-api/src/main/java/datadog/apicontract/Restricted.java @@ -0,0 +1,24 @@ +package datadog.apicontract; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks a member that is public only because Java has no narrower visibility across modules: it may + * be used only from the packages in {@link #allowedIn}. It typically trades a safety check for + * speed, trusting its caller to have made the check already. + * + *

This marker changes no runtime behavior. Instrumentation modules are kept out by the + * forbidden-APIs signatures in {@code gradle/forbiddenApiFilters/instrumentation.txt}, which must + * list every {@code @Restricted} member. + */ +@Documented +@Retention(RetentionPolicy.CLASS) +@Target({ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.TYPE}) +public @interface Restricted { + /** Package prefixes allowed to use the member, such as {@code datadog.trace.core}. */ + String[] allowedIn(); +} diff --git a/internal-api/src/main/java/datadog/trace/api/KnownTagCodec.java b/internal-api/src/main/java/datadog/trace/api/KnownTagCodec.java index d84e1e6a537..314fe6d67dc 100644 --- a/internal-api/src/main/java/datadog/trace/api/KnownTagCodec.java +++ b/internal-api/src/main/java/datadog/trace/api/KnownTagCodec.java @@ -15,19 +15,21 @@ public final class KnownTagCodec { * openTelemetryNameOf switch on it, and the generator emits each id as a literal. Bits [47-32] * are RESERVED and always zero here: they are the window the dense tag store uses for its * co-occurrence slot coordinate, which arrives with that store. Of the low 32 flag bits, bit 2 is - * the trace/span LEVEL bit (set ⟹ trace-level); bits 1-0 are reserved. Unknown (string-only) - * custom tags are NOT known ids — {@code keyOf} returns 0 for them. + * the trace/span LEVEL bit (set ⟹ trace-level), bit 1 is the INTERCEPTED bit, and bit 0 is the + * SHARED_NAME bit. Unknown (string-only) custom tags are NOT known ids — {@code keyOf} returns 0 for + * them. * - *

An id says what a tag IS, not how it is SET. Whether the tracer intercepts a tag on the - * set-path — routing it to a span field or a sampling directive instead of tag storage — belongs - * to TagInterceptor, whose {@code needsIntercept} switch is the authority; mirroring it here as a - * classification bit and a serial-range tier only created drift between the two. That - * classification returns with the work that consumes it (the id→handler dispatch table that - * retires TagInterceptor), and re-adding a bit then is purely additive. + *

The INTERCEPTED bit marks a tag TagInterceptor may route on the set-path, to a span field or + * a sampling directive instead of tag storage. It is a hint, not a decision: the interceptor's + * switch decides, and may still store the tag. A setter called with a constant id tests the bit + * at JIT time, so the interception path folds away for every tag without it. * *

There is deliberately NO OpenTelemetry-applicability flag: an absent otel-name means * pass-through (the tag is emitted under its Datadog name), so today every known tag has an * OpenTelemetry name and such a flag would be constant. It returns once a Datadog-only tag exists. + * + *

Serials are assigned at build time and are NOT stable across releases: they follow the tags' + * names in order, so adding a tag renumbers others. Never persist or transmit a raw id. */ public static int serialNum(long tagId) { return (int) (tagId >>> 48); @@ -46,6 +48,38 @@ public static boolean isTraceLevel(long tagId) { return (tagId & LEVEL_TRACE) != 0L; } + /** + * INTERCEPTED bit (low-32 carve, bit 1): marks a tag TagInterceptor may route on the set-path. + * The tracer overlay, {@code tag-conventions-java.yaml}, lists these tags. + */ + public static final long INTERCEPTED = 1L << 1; + + /** True if the tagId names a tag TagInterceptor may route on the set-path. */ + public static boolean isIntercepted(long tagId) { + return (tagId & INTERCEPTED) != 0L; + } + + /** + * SHARED_NAME bit (low-32 carve, bit 0): marks one direction of a tag declared once per direction + * under a shared Datadog name, such as {@code peer.port}. {@link #keyOf} resolves that name to + * neither id, so the name alone cannot find an entry stored under one of them. + */ + public static final long SHARED_NAME = 1L << 0; + + /** True if the tagId is one direction of a tag whose Datadog name is shared across directions. */ + public static boolean hasSharedName(long tagId) { + return (tagId & SHARED_NAME) != 0L; + } + + /** + * True if {@code tagId} can key a {@link TagMap} entry: a known tag whose name resolves back to + * it. A {@link #hasSharedName shared-name} id cannot, until name resolution knows the span's + * direction. Like {@link #isKnown}, this folds away for a constant id. + */ + public static boolean isKeyableById(long tagId) { + return isKnown(tagId) && !hasSharedName(tagId); + } + /** Returns the tagId with the {@link #LEVEL_TRACE} flag set. */ public static long traceLevel(long tagId) { return tagId | LEVEL_TRACE; @@ -116,6 +150,23 @@ public static String openTelemetryTagOf(long tagId) { return otelName != null ? otelName : resolver.nameOf(tagId); } + /** + * One past the largest known serial: an array of this length, indexed by {@link #serialNum}, has + * a slot for every known tag (slot 0 is no tag). + */ + public static int serialLimit() { + return KnownTags.SERIAL_LIMIT; + } + + /** + * True if {@code tagId} names a known tag. Gives the same answer as {@code nameOf(tagId) != + * null}, but as a range check on the serial, which folds away for a constant id. + */ + public static boolean isKnown(long tagId) { + int serial = serialNum(tagId); + return serial != 0 && serial < serialLimit(); // serial 0 is no tag + } + /** The id for {@code name} in any namespace, or 0 when it is not a known tag. */ public static long keyOf(String name) { return Installed.RESOLVER.keyOf(name); 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 52d2ef32a2b..0dbef18896b 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -1,5 +1,7 @@ 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; @@ -250,38 +252,76 @@ public static boolean isEmptyValue(Object value) { */ @Nullable public static final Entry create(@Nonnull String tag, Object value) { - return isEmptyValue(value) ? null : TagMap.Entry.newAnyEntry(tag, value); + return isEmptyValue(value) ? null : TagMap.anyEntryFor(tag, value); } /** If value is non-null, returns a new TagMap.Entry If value is null or empty, returns null */ @Nullable public static final Entry create(@Nonnull String tag, CharSequence value) { // NOTE: From the static typing, we know that value is not a primitive box - return isEmptyValue(value) ? null : TagMap.Entry.newObjectEntry(tag, value); + return isEmptyValue(value) ? null : TagMap.objectEntryFor(tag, value); } public static final Entry create(@Nonnull String tag, boolean value) { - return TagMap.Entry.newBooleanEntry(tag, value); + return TagMap.booleanEntryFor(tag, value); } public static final Entry create(@Nonnull String tag, int value) { - return TagMap.Entry.newIntEntry(tag, value); + return TagMap.intEntryFor(tag, value); } public static final Entry create(@Nonnull String tag, long value) { - return TagMap.Entry.newLongEntry(tag, value); + return TagMap.longEntryFor(tag, value); } public static final Entry create(@Nonnull String tag, float value) { - return TagMap.Entry.newFloatEntry(tag, value); + return TagMap.floatEntryFor(tag, value); } public static final Entry create(@Nonnull String tag, double value) { - return TagMap.Entry.newDoubleEntry(tag, value); + return TagMap.doubleEntryFor(tag, value); + } + + /* + * Id-keyed counterparts of the create overloads above, for a KnownTags.*_ID: same contract (a + * null or empty value yields no entry), without the name lookup. An id that names no known tag + * is rejected. + */ + + /** Entry for a known tag id, or null when {@code value} is null or an empty CharSequence. */ + @Nullable + public static final Entry create(long tagId, Object value) { + return isEmptyValue(value) ? null : TagMap.Entry.newAnyEntry(tagId, value); + } + + /** Entry for a known tag id, or null when {@code value} is null or empty. */ + @Nullable + public static final Entry create(long tagId, CharSequence value) { + return isEmptyValue(value) ? null : TagMap.Entry.newObjectEntry(tagId, value); + } + + public static final Entry create(long tagId, boolean value) { + return TagMap.Entry.newBooleanEntry(tagId, value); + } + + public static final Entry create(long tagId, int value) { + return TagMap.Entry.newIntEntry(tagId, value); + } + + public static final Entry create(long tagId, long value) { + return TagMap.Entry.newLongEntry(tagId, value); + } + + public static final Entry create(long tagId, float value) { + return TagMap.Entry.newFloatEntry(tagId, value); + } + + public static final Entry create(long tagId, double value) { + return TagMap.Entry.newDoubleEntry(tagId, value); } static Entry newAnyEntry(Map.Entry entry) { - return newAnyEntry(entry.getKey(), entry.getValue()); + return anyEntryFor(entry.getKey(), entry.getValue()); } static Entry newAnyEntry(String tag, Object value) { @@ -299,6 +339,36 @@ static Entry newObjectEntry(String tag, Object value) { return new Entry(tag, OBJECT, 0, value); } + // Id-keyed factories: the tag id already names a canonical tag, so these skip the + // canonicalizing name lookup the String factories pay. + static Entry newAnyEntry(long tagId, Object value) { + return new Entry(tagId, ANY, 0L, value); + } + + static Entry newObjectEntry(long tagId, Object value) { + return new Entry(tagId, OBJECT, 0, value); + } + + static Entry newBooleanEntry(long tagId, boolean value) { + return new Entry(tagId, BOOLEAN, boolean2Prim(value), Boolean.valueOf(value)); + } + + static Entry newIntEntry(long tagId, int value) { + return new Entry(tagId, INT, int2Prim(value), null); + } + + static Entry newLongEntry(long tagId, long value) { + return new Entry(tagId, LONG, long2Prim(value), null); + } + + static Entry newFloatEntry(long tagId, float value) { + return new Entry(tagId, FLOAT, float2Prim(value), null); + } + + static Entry newDoubleEntry(long tagId, double value) { + return new Entry(tagId, DOUBLE, double2Prim(value), null); + } + static Entry newBooleanEntry(String tag, boolean value) { return new Entry(tag, BOOLEAN, boolean2Prim(value), Boolean.valueOf(value)); } @@ -340,10 +410,12 @@ static Entry newDoubleEntry(String tag, Double box) { } /* - * hash is stored in line for fast handling of Entry-s coming from another TagMap - * However, hash is lazily computed using the same trick as {@link java.lang.String}. + * The tag's 64-bit hash, computed once at construction. For a known tag it is the tag id, whose + * serial sits in the high bits, so the upper 32 bits are never zero; for a custom tag it is the + * name's hash in the low 32 bits, with the upper 32 bits zero. The two can never collide, and + * tagId() is a field read. See tagHashOf. */ - int lazyTagHash; + final long tagHash; // To optimize construction of Entry around boxed primitives and Object entries, // no type checks are done during construction. @@ -365,38 +437,81 @@ static Entry newDoubleEntry(String tag, Double box) { volatile String strCache = null; + /** + * A custom tag's entry: no registry lookup. A known tag is built from its id instead -- callers + * that start from a name resolve it once and pick the id or name factory (see {@code + * TagMap.anyEntryFor}). + */ private Entry(String tag, byte type, long prim, Object obj) { - /* - * Canonicalize known names at the single Entry construction point, so Datadog and - * OpenTelemetry spellings use the same TagMap key. This adds a StringIndex lookup to every - * new entry, including on the application thread. - */ - super(KnownTagCodec.canonicalTagName(tag)); - this.lazyTagHash = 0; // lazily computed + this(0L, tag, type, prim, obj); + assert KnownTagCodec.keyOf(tag) == 0 : "'" + tag + "' is a known tag; build it from its id"; + } + + private Entry(long tagId, byte type, long prim, Object obj) { + // Resolve the name once: it both validates the id and names the entry. + super(requireKnownName(tagId)); + this.tagHash = tagId; this.rawType = type; this.rawPrim = prim; this.rawObj = obj; } - int hash() { - // If value of hash read in this thread is zero, then hash is computed. - // hash is not held as a volatile, since this computation can safely be repeated as any time - int hash = this.lazyTagHash; - if (hash != 0) return hash; + /** {@code tagId} is a known id, or 0 for the custom tag {@code customTag}. */ + private Entry(long tagId, String customTag, byte type, long prim, Object obj) { + super(tagId != 0 ? KnownTagCodec.nameOf(tagId) : customTag); + this.tagHash = tagId != 0 ? tagId : customHash(customTag); + + this.rawType = type; + this.rawPrim = prim; + this.rawObj = obj; + } - hash = _hash(this.tag); - this.lazyTagHash = hash; - return hash; + private static String requireKnownName(long tagId) { + String name = tagId == 0 ? null : KnownTagCodec.nameOf(tagId); + if (name == null) { + throw new IllegalArgumentException("not a known tag id: " + Long.toHexString(tagId)); + } + if (KnownTagCodec.hasSharedName(tagId)) { + // Name-keyed access hashes the shared name as a custom tag, so it could never find an entry + // keyed by this id; set the tag by name until resolution knows the span's direction. + throw new IllegalArgumentException("tag id has a shared name, set it by name: " + name); + } + return name; + } + + /** The tag hash a name maps to: its id when known, else its {@link #customHash}. */ + static long tagHashOf(String tag) { + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? tagId : customHash(tag); + } + + /** A custom tag's hash: the name hash in the low 32 bits, never colliding with a tag id. */ + static long customHash(String tag) { + return _hash(tag) & 0xFFFFFFFFL; + } + + /** + * Folds a tag hash to the 32-bit bucket hash. A tag id's serial is in bits 63-48, so it is + * folded down from there rather than from bit 32, where a bucket mask would discard it. + * + *

Never zero: BucketGroup treats a zero hash as a vacant slot. The fold can produce zero + * when a serial equals the id's flag bits (serial 4 with the trace-level bit, 4), so zero maps + * to the same nonzero sentinel {@link #_hash} uses. + */ + static int bucketHash(long tagHash) { + int hash = (int) (tagHash >>> 48) ^ (int) tagHash; + return hash == 0 ? 0xDD06 : hash; + } + + int hash() { + return bucketHash(this.tagHash); } @Override public long tagId() { - /* - * Resolve on demand. Only OTLP serialization currently needs the ID, so caching it here - * would add a field to every Entry to save a lookup on exported tags. - */ - return KnownTagCodec.keyOf(this.tag); + // A known tag's hash is its id, whose upper 32 bits are never zero; a custom tag's are. + return (this.tagHash >>> 32) != 0 ? this.tagHash : 0L; } @Override @@ -889,31 +1004,31 @@ public boolean containsRemovals() { } public Ledger set(String tag, Object value) { - return this.recordEntry(Entry.newAnyEntry(tag, value)); + return this.recordEntry(anyEntryFor(tag, value)); } public Ledger set(String tag, CharSequence value) { - return this.recordEntry(Entry.newObjectEntry(tag, value)); + return this.recordEntry(objectEntryFor(tag, value)); } public Ledger set(String tag, boolean value) { - return this.recordEntry(Entry.newBooleanEntry(tag, value)); + return this.recordEntry(booleanEntryFor(tag, value)); } public Ledger set(String tag, int value) { - return this.recordEntry(Entry.newIntEntry(tag, value)); + return this.recordEntry(intEntryFor(tag, value)); } public Ledger set(String tag, long value) { - return this.recordEntry(Entry.newLongEntry(tag, value)); + return this.recordEntry(longEntryFor(tag, value)); } public Ledger set(String tag, float value) { - return this.recordEntry(Entry.newFloatEntry(tag, value)); + return this.recordEntry(floatEntryFor(tag, value)); } public Ledger set(String tag, double value) { - return this.recordEntry(Entry.newDoubleEntry(tag, value)); + return this.recordEntry(doubleEntryFor(tag, value)); } public Ledger set(Entry entry) { @@ -1301,9 +1416,18 @@ public Entry getEntry(String tag) { // Entries are stored under their canonical Datadog name (see Entry's constructor); a lookup by // an OpenTelemetry rename must canonicalize the same way, or it would hash to the wrong bucket // and silently miss the entry stored under the Datadog name. - String canonicalTag = KnownTagCodec.canonicalTagName(tag); + long tagHash = Entry.tagHashOf(tag); + String canonicalTag = (tagHash >>> 32) != 0 ? KnownTagCodec.nameOf(tagHash) : tag; + return this.getEntry(canonicalTag, tagHash); + } + + /** The entry for a known tag id, found by the id itself rather than by resolving a name. */ + public Entry getEntry(long tagId) { + return this.getEntry(Entry.requireKnownName(tagId), tagId); + } - Entry local = this.getLocalEntry(canonicalTag); + private Entry getEntry(String canonicalTag, long tagHash) { + Entry local = this.getLocalEntry(canonicalTag, tagHash); if (local != null) { // Local entry shadows the parent (local-wins) — unchanged hot path. return local; @@ -1318,13 +1442,13 @@ public Entry getEntry(String tag) { if (this.removedFromParent != null && this.removedFromParent.contains(canonicalTag)) { return null; // tombstoned: removed locally, do not read through } - return parent.getEntry(canonicalTag); + return parent.getEntry(canonicalTag, tagHash); } /** Looks up an entry in this map's own buckets only — no read-through to the parent. */ - private Entry getLocalEntry(String tag) { + private Entry getLocalEntry(String tag, long tagHash) { Object[] thisBuckets = this.buckets; - int hash = TagMap.Entry._hash(tag); + int hash = Entry.bucketHash(tagHash); return findInBucket(thisBuckets[hash & (thisBuckets.length - 1)], hash, tag); } @@ -1373,7 +1497,7 @@ private boolean parentEntryVisible(Entry parentEntry, TagMap fromAncestor) { @Deprecated @Override public Object put(@Nonnull String tag, Object value) { - TagMap.Entry entry = this.getAndSet(Entry.newAnyEntry(tag, value)); + TagMap.Entry entry = this.getAndSet(anyEntryFor(tag, value)); return entry == null ? null : entry.objectValue(); } @@ -1388,32 +1512,195 @@ public void set(@Nullable TagMap.EntryReader newEntryReader) { } } + /* + * Entry factories for callers that start from a name: resolve it once, then build a known tag + * from its id or a custom tag from its name. (Entry's own name factories are custom-only.) + */ + static Entry anyEntryFor(String tag, Object value) { + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? Entry.newAnyEntry(tagId, value) : Entry.newAnyEntry(tag, value); + } + + static Entry objectEntryFor(String tag, CharSequence value) { + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? Entry.newObjectEntry(tagId, value) : Entry.newObjectEntry(tag, value); + } + + static Entry booleanEntryFor(String tag, boolean value) { + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? Entry.newBooleanEntry(tagId, value) : Entry.newBooleanEntry(tag, value); + } + + static Entry intEntryFor(String tag, int value) { + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? Entry.newIntEntry(tagId, value) : Entry.newIntEntry(tag, value); + } + + static Entry longEntryFor(String tag, long value) { + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? Entry.newLongEntry(tagId, value) : Entry.newLongEntry(tag, value); + } + + static Entry floatEntryFor(String tag, float value) { + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? Entry.newFloatEntry(tagId, value) : Entry.newFloatEntry(tag, value); + } + + static Entry doubleEntryFor(String tag, double value) { + long tagId = KnownTagCodec.keyOf(tag); + return tagId != 0 ? Entry.newDoubleEntry(tagId, value) : Entry.newDoubleEntry(tag, value); + } + public void set(@Nonnull String tag, @Nonnull Object value) { - this.putEntry(Entry.newAnyEntry(tag, value)); + long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + this.set(tagId, value); + } else { + this.putEntry(Entry.newAnyEntry(tag, value)); + } } public void set(@Nonnull String tag, @Nonnull CharSequence value) { - this.putEntry(Entry.newObjectEntry(tag, value)); + long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + this.set(tagId, value); + } else { + this.putEntry(Entry.newObjectEntry(tag, value)); + } } public void set(@Nonnull String tag, boolean value) { - this.putEntry(Entry.newBooleanEntry(tag, value)); + long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + this.set(tagId, value); + } else { + this.putEntry(Entry.newBooleanEntry(tag, value)); + } } public void set(@Nonnull String tag, int value) { - this.putEntry(Entry.newIntEntry(tag, value)); + long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + this.set(tagId, value); + } else { + this.putEntry(Entry.newIntEntry(tag, value)); + } } public void set(@Nonnull String tag, long value) { - this.putEntry(Entry.newLongEntry(tag, value)); + long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + this.set(tagId, value); + } else { + this.putEntry(Entry.newLongEntry(tag, value)); + } } public void set(@Nonnull String tag, float value) { - this.putEntry(Entry.newFloatEntry(tag, value)); + long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + this.set(tagId, value); + } else { + this.putEntry(Entry.newFloatEntry(tag, value)); + } } public void set(@Nonnull String tag, double value) { - this.putEntry(Entry.newDoubleEntry(tag, value)); + long tagId = KnownTagCodec.keyOf(tag); + if (tagId != 0) { + this.set(tagId, value); + } else { + this.putEntry(Entry.newDoubleEntry(tag, value)); + } + } + + /* + * Id-keyed setters: the caller passes a KnownTags.*_ID, so the entry is built under the tag's + * canonical name without the name lookup the String setters pay. The id must name a known tag; + * 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. + */ + @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; + + private 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.isKeyableById(tagId). + */ + public void setKnown(long tagId, @Nonnull Object value) { + this.map.putEntry(new Entry(tagId, null, Entry.ANY, 0L, value)); + } + + public void setKnown(long tagId, @Nonnull CharSequence value) { + this.map.putEntry(new Entry(tagId, null, EntryReader.OBJECT, 0L, value)); + } + + public void setKnown(long tagId, boolean value) { + this.map.putEntry( + new Entry( + tagId, null, EntryReader.BOOLEAN, Entry.boolean2Prim(value), Boolean.valueOf(value))); + } + + public void setKnown(long tagId, int value) { + this.map.putEntry(new Entry(tagId, null, EntryReader.INT, Entry.int2Prim(value), null)); + } + + public void setKnown(long tagId, long value) { + this.map.putEntry(new Entry(tagId, null, EntryReader.LONG, Entry.long2Prim(value), null)); + } + + public void setKnown(long tagId, float value) { + this.map.putEntry(new Entry(tagId, null, EntryReader.FLOAT, Entry.float2Prim(value), null)); + } + + public void setKnown(long tagId, double value) { + this.map.putEntry(new Entry(tagId, null, EntryReader.DOUBLE, Entry.double2Prim(value), null)); + } + } + + public void set(long tagId, @Nonnull Object value) { + this.putEntry(Entry.newAnyEntry(tagId, value)); + } + + public void set(long tagId, @Nonnull CharSequence value) { + this.putEntry(Entry.newObjectEntry(tagId, value)); + } + + public void set(long tagId, boolean value) { + this.putEntry(Entry.newBooleanEntry(tagId, value)); + } + + public void set(long tagId, int value) { + this.putEntry(Entry.newIntEntry(tagId, value)); + } + + public void set(long tagId, long value) { + this.putEntry(Entry.newLongEntry(tagId, value)); + } + + public void set(long tagId, float value) { + this.putEntry(Entry.newFloatEntry(tagId, value)); + } + + public void set(long tagId, double value) { + this.putEntry(Entry.newDoubleEntry(tagId, value)); } /** @@ -1504,31 +1791,31 @@ private Entry putEntry(@Nonnull Entry newEntry) { } public Entry getAndSet(@Nonnull String tag, Object value) { - return this.getAndSet(Entry.newAnyEntry(tag, value)); + return this.getAndSet(anyEntryFor(tag, value)); } public Entry getAndSet(@Nonnull String tag, CharSequence value) { - return this.getAndSet(Entry.newObjectEntry(tag, value)); + return this.getAndSet(objectEntryFor(tag, value)); } public TagMap.Entry getAndSet(@Nonnull String tag, boolean value) { - return this.getAndSet(Entry.newBooleanEntry(tag, value)); + return this.getAndSet(booleanEntryFor(tag, value)); } public TagMap.Entry getAndSet(@Nonnull String tag, int value) { - return this.getAndSet(Entry.newIntEntry(tag, value)); + return this.getAndSet(intEntryFor(tag, value)); } public TagMap.Entry getAndSet(@Nonnull String tag, long value) { - return this.getAndSet(Entry.newLongEntry(tag, value)); + return this.getAndSet(longEntryFor(tag, value)); } public TagMap.Entry getAndSet(@Nonnull String tag, float value) { - return this.getAndSet(Entry.newFloatEntry(tag, value)); + return this.getAndSet(floatEntryFor(tag, value)); } public TagMap.Entry getAndSet(@Nonnull String tag, double value) { - return this.getAndSet(Entry.newDoubleEntry(tag, value)); + return this.getAndSet(doubleEntryFor(tag, value)); } public void putAll(Map map) { @@ -1785,9 +2072,20 @@ public Entry getAndRemove(String tag) { // See getEntry: entries are stored under their canonical Datadog name, so a removal by an // OpenTelemetry rename must canonicalize first to find (and tombstone) the right entry. - String canonicalTag = KnownTagCodec.canonicalTagName(tag); + long tagHash = Entry.tagHashOf(tag); + String canonicalTag = (tagHash >>> 32) != 0 ? KnownTagCodec.nameOf(tagHash) : tag; + return this.getAndRemove(canonicalTag, tagHash); + } + + /** Removes the entry for a known tag id; see {@link #getEntry(long)}. */ + public Entry getAndRemove(long tagId) { + this.checkWriteAccess(); + + return this.getAndRemove(Entry.requireKnownName(tagId), tagId); + } - Entry localRemoved = this.removeLocal(canonicalTag); + private Entry getAndRemove(String canonicalTag, long tagHash) { + Entry localRemoved = this.removeLocal(canonicalTag, tagHash); TagMap parent = this.parent; if (parent != null) { @@ -1798,7 +2096,7 @@ public Entry getAndRemove(String tag) { boolean alreadyTombstoned = this.removedFromParent != null && this.removedFromParent.contains(canonicalTag); if (!alreadyTombstoned) { - Entry parentEntry = parent.getEntry(canonicalTag); + Entry parentEntry = parent.getEntry(canonicalTag, tagHash); if (parentEntry != null) { if (this.removedFromParent == null) { // Small initial capacity: this set is rare and almost always holds only a handful of @@ -1814,10 +2112,10 @@ public Entry getAndRemove(String tag) { } /** Removes an entry from this map's own buckets only — no parent/tombstone handling. */ - private Entry removeLocal(String tag) { + private Entry removeLocal(String tag, long tagHash) { Object[] thisBuckets = this.buckets; - int hash = TagMap.Entry._hash(tag); + int hash = Entry.bucketHash(tagHash); int bucketIndex = hash & (thisBuckets.length - 1); Object bucket = thisBuckets[bucketIndex]; @@ -3001,7 +3299,7 @@ public Object objectValue() { @Override public TagMap.Entry entry() { - return TagMap.Entry.newAnyEntry(this.tag, this.value); + return TagMap.anyEntryFor(this.tag, this.value); } @Override diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java index 94c0f80572a..af3aa211c15 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java @@ -9,6 +9,7 @@ import datadog.context.ImplicitContextKeyed; import datadog.trace.api.DDSpanId; import datadog.trace.api.DDTraceId; +import datadog.trace.api.KnownTagCodec; import datadog.trace.api.TagMap; import datadog.trace.api.TraceConfig; import datadog.trace.api.gateway.IGSpanInfo; @@ -103,6 +104,43 @@ default boolean isValid() { AgentSpan setTag(String key, Object value); + /* + * Id-keyed setTag family: the caller passes a KnownTags.*_ID instead of a name. The default + * resolves the id to its name and delegates to the String setter, which is correct for every + * implementation; the core span overrides these to skip the name lookup when storing the tag. + */ + default AgentSpan setTag(long tagId, boolean value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + default AgentSpan setTag(long tagId, int value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + default AgentSpan setTag(long tagId, long value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + default AgentSpan setTag(long tagId, float value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + default AgentSpan setTag(long tagId, double value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + default AgentSpan setTag(long tagId, String value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + default AgentSpan setTag(long tagId, CharSequence value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + + default AgentSpan setTag(long tagId, Object value) { + return setTag(KnownTagCodec.nameOf(tagId), value); + } + /** entry may be null - in which case the tags remained unchanged */ AgentSpan setTag(TagMap.EntryReader entry); diff --git a/internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java b/internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java index f2022391203..08570b034fa 100644 --- a/internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java +++ b/internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java @@ -169,6 +169,10 @@ void aNameDeclaredPerDirectionIsOneTagPerDirection() { assertEquals(Tags.PEER_PORT, KnownTagCodec.nameOf(KnownTags.PEER_PORT_INBOUND_ID)); assertEquals(Tags.PEER_PORT, KnownTagCodec.nameOf(KnownTags.PEER_PORT_OUTBOUND_ID)); assertNotEquals(KnownTags.PEER_PORT_INBOUND_ID, KnownTags.PEER_PORT_OUTBOUND_ID); + assertTrue(KnownTagCodec.hasSharedName(KnownTags.PEER_PORT_INBOUND_ID)); + assertTrue(KnownTagCodec.hasSharedName(KnownTags.PEER_PORT_OUTBOUND_ID)); + assertFalse(KnownTagCodec.isKeyableById(KnownTags.PEER_PORT_OUTBOUND_ID)); + assertTrue(KnownTagCodec.isKeyableById(KnownTags.PEER_HOSTNAME_ID)); assertEquals(0L, KnownTagCodec.keyOf(Tags.PEER_PORT)); } diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapEntryCreateByIdTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapEntryCreateByIdTest.java new file mode 100644 index 00000000000..f970855c212 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/TagMapEntryCreateByIdTest.java @@ -0,0 +1,59 @@ +package datadog.trace.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +/** {@link TagMap.Entry#create(long, Object)} and its overloads: creating an entry by tag id. */ +class TagMapEntryCreateByIdTest { + @Test + void matchesCreateByNameForEveryValueType() { + assertEquivalent( + TagMap.Entry.create(KnownTags.COMPONENT_NAME, (Object) "okhttp"), + TagMap.Entry.create(KnownTags.COMPONENT_ID, (Object) "okhttp")); + assertEquivalent( + TagMap.Entry.create(KnownTags.PEER_HOSTNAME_NAME, (CharSequence) "db.internal"), + TagMap.Entry.create(KnownTags.PEER_HOSTNAME_ID, (CharSequence) "db.internal")); + assertEquivalent( + TagMap.Entry.create(KnownTags.DD_PROFILING_ENABLED_NAME, true), + TagMap.Entry.create(KnownTags.DD_PROFILING_ENABLED_ID, true)); + assertEquivalent( + TagMap.Entry.create(KnownTags.HTTP_STATUS_CODE_NAME, 5432), + TagMap.Entry.create(KnownTags.HTTP_STATUS_CODE_ID, 5432)); + assertEquivalent( + TagMap.Entry.create(KnownTags.HTTP_RESEND_COUNT_NAME, 2L), + TagMap.Entry.create(KnownTags.HTTP_RESEND_COUNT_ID, 2L)); + assertEquivalent( + TagMap.Entry.create(KnownTags.DB_USER_NAME, 1.5f), + TagMap.Entry.create(KnownTags.DB_USER_ID, 1.5f)); + assertEquivalent( + TagMap.Entry.create(KnownTags.DB_POOL_NAME, 2.5d), + TagMap.Entry.create(KnownTags.DB_POOL_NAME_ID, 2.5d)); + } + + @Test + void aNullOrEmptyValueCreatesNoEntry() { + assertNull(TagMap.Entry.create(KnownTags.COMPONENT_ID, (Object) null)); + assertNull(TagMap.Entry.create(KnownTags.COMPONENT_ID, (Object) "")); + assertNull(TagMap.Entry.create(KnownTags.COMPONENT_ID, (CharSequence) null)); + assertNull(TagMap.Entry.create(KnownTags.COMPONENT_ID, (CharSequence) "")); + } + + @Test + void anUnknownIdIsRejected() { + assertThrows(IllegalArgumentException.class, () -> TagMap.Entry.create(0L, (Object) "value")); + assertThrows( + IllegalArgumentException.class, + () -> TagMap.Entry.create(KnownTagCodec.makeTagId(9999), 1)); + } + + /** Same tag, id, type and value -- the two creation paths build equivalent entries. */ + private static void assertEquivalent(TagMap.Entry byName, TagMap.Entry byId) { + assertEquals(byName.tag(), byId.tag()); + assertEquals(byName.tagId(), byId.tagId()); + assertEquals(byName.type(), byId.type()); + assertEquals(byName.objectValue(), byId.objectValue()); + } +} diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapSetByIdTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapSetByIdTest.java new file mode 100644 index 00000000000..9bc0edc80e9 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/TagMapSetByIdTest.java @@ -0,0 +1,103 @@ +package datadog.trace.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.HashMap; +import org.junit.jupiter.api.Test; + +/** {@link TagMap#set(long, Object)} and its overloads: setting a known tag by its id. */ +class TagMapSetByIdTest { + @Test + void storesUnderTheCanonicalName() { + TagMap map = TagMap.create(); + + map.set(KnownTags.PEER_HOSTNAME_ID, "db.internal"); + + assertEquals("db.internal", map.getObject(KnownTags.PEER_HOSTNAME_NAME)); + assertEquals(KnownTags.PEER_HOSTNAME_ID, map.getEntry(KnownTags.PEER_HOSTNAME_NAME).tagId()); + } + + @Test + void anOpenTelemetryRenameIsStoredUnderTheDatadogName() { + TagMap byId = TagMap.create(); + TagMap byOtelName = TagMap.create(); + + byId.set(KnownTags.HTTP_METHOD_ID, "GET"); + byOtelName.set(KnownTags.HTTP_METHOD_OTEL_NAME, "GET"); + + assertEquals(new HashMap<>(byOtelName), new HashMap<>(byId)); + } + + @Test + void matchesTheNameKeyedSettersForEveryValueType() { + TagMap byId = TagMap.create(); + byId.set(KnownTags.HTTP_STATUS_CODE_ID, 5432); + byId.set(KnownTags.HTTP_RESEND_COUNT_ID, 2L); + byId.set(KnownTags.PEER_HOSTNAME_ID, (CharSequence) "db.internal"); + byId.set(KnownTags.DB_INSTANCE_ID, (Object) "orders"); + byId.set(KnownTags.DD_PROFILING_ENABLED_ID, true); + byId.set(KnownTags.DB_USER_ID, 1.5f); + byId.set(KnownTags.DB_POOL_NAME_ID, 2.5d); + + TagMap byName = TagMap.create(); + byName.set(KnownTags.HTTP_STATUS_CODE_NAME, 5432); + byName.set(KnownTags.HTTP_RESEND_COUNT_NAME, 2L); + byName.set(KnownTags.PEER_HOSTNAME_NAME, (CharSequence) "db.internal"); + byName.set(KnownTags.DB_INSTANCE_NAME, (Object) "orders"); + byName.set(KnownTags.DD_PROFILING_ENABLED_NAME, true); + byName.set(KnownTags.DB_USER_NAME, 1.5f); + byName.set(KnownTags.DB_POOL_NAME, 2.5d); + + assertEquals(new HashMap<>(byName), new HashMap<>(byId)); + assertEquals(5432, byId.getEntry(KnownTags.HTTP_STATUS_CODE_NAME).intValue()); + } + + @Test + void getsAndRemovesById() { + TagMap map = TagMap.create(); + map.set(KnownTags.HTTP_METHOD_NAME, "GET"); + + assertEquals("GET", map.getEntry(KnownTags.HTTP_METHOD_ID).stringValue()); + assertEquals("GET", map.getAndRemove(KnownTags.HTTP_METHOD_ID).stringValue()); + assertNull(map.getEntry(KnownTags.HTTP_METHOD_ID)); + assertNull(map.getAndRemove(KnownTags.HTTP_METHOD_ID)); + } + + @Test + void anUnknownIdIsRejected() { + TagMap map = TagMap.create(); + + assertThrows(IllegalArgumentException.class, () -> map.set(0L, "value")); + assertThrows(IllegalArgumentException.class, () -> map.set(KnownTagCodec.makeTagId(9999), 1)); + } + + @Test + void rejectsSharedNameIds() { + TagMap map = TagMap.create(); + assertThrows(IllegalArgumentException.class, () -> map.set(KnownTags.PEER_PORT_INBOUND_ID, 80)); + assertThrows( + IllegalArgumentException.class, () -> map.set(KnownTags.PEER_PORT_OUTBOUND_ID, 443)); + assertThrows( + IllegalArgumentException.class, () -> map.getEntry(KnownTags.PEER_PORT_OUTBOUND_ID)); + } + + @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()); + } +} diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapTagHashTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapTagHashTest.java new file mode 100644 index 00000000000..dd477db0eb9 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/TagMapTagHashTest.java @@ -0,0 +1,114 @@ +package datadog.trace.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** + * The 64-bit tag hash on {@link TagMap.Entry}: a known tag's hash is its id, a custom tag's is its + * name hash in the low 32 bits. + */ +class TagMapTagHashTest { + @Test + void aKnownTagHashesToItsIdHoweverItIsSet() { + TagMap.Entry byId = TagMap.Entry.newAnyEntry(KnownTags.HTTP_METHOD_ID, "GET"); + TagMap.Entry byName = TagMap.anyEntryFor(KnownTags.HTTP_METHOD_NAME, "GET"); + TagMap.Entry byOtelName = TagMap.anyEntryFor(KnownTags.HTTP_METHOD_OTEL_NAME, "GET"); + + assertEquals(KnownTags.HTTP_METHOD_ID, byId.tagHash); + assertEquals(KnownTags.HTTP_METHOD_ID, byName.tagHash); + assertEquals(KnownTags.HTTP_METHOD_ID, byOtelName.tagHash); + assertEquals(KnownTags.HTTP_METHOD_ID, byName.tagId()); + } + + @Test + void theNameFactoriesAreForCustomTagsOnly() { + // Entry's name factories skip the registry, so a known name must come in through its id; + // anyEntryFor and friends resolve a name first. (Tests run with assertions enabled.) + assertThrows( + AssertionError.class, () -> TagMap.Entry.newAnyEntry(KnownTags.HTTP_METHOD_NAME, "GET")); + assertEquals(0L, TagMap.Entry.newAnyEntry("my.custom.tag", "value").tagId()); + } + + @Test + void aCustomTagHashesToItsNameWithNoId() { + TagMap.Entry custom = TagMap.Entry.newAnyEntry("my.custom.tag", "value"); + + assertEquals(0L, custom.tagHash >>> 32); + assertEquals(0L, custom.tagId()); + assertEquals(TagMap.Entry.tagHashOf("my.custom.tag"), custom.tagHash); + } + + @Test + void theBucketHashOfACustomTagIsUnchanged() { + assertEquals( + TagMap.Entry._hash("my.custom.tag"), + TagMap.Entry.bucketHash(TagMap.Entry.tagHashOf("my.custom.tag"))); + } + + @Test + void theBucketHashOfAKnownTagCarriesItsSerial() { + // Folding from bit 32 would leave the serial above any bucket mask; it must come from bit 48. + assertNotEquals( + TagMap.Entry.bucketHash(KnownTags.HTTP_METHOD_ID) & 0xFF, + TagMap.Entry.bucketHash(KnownTags.HTTP_ROUTE_ID) & 0xFF); + } + + @Test + void noKnownTagFoldsToTheVacantSlotHash() throws IllegalAccessException { + // BucketGroup treats a zero hash as a vacant slot; _dd.djm.enabled once folded to zero. + for (long tagId : knownTagIds()) { + assertNotEquals(0, TagMap.Entry.bucketHash(tagId), Long.toHexString(tagId)); + } + assertNotEquals(0, TagMap.Entry.bucketHash(KnownTags.DD_DJM_ENABLED_ID)); + } + + @Test + void everyKnownTagSurvivesBucketCollisionsAndCopies() throws IllegalAccessException { + List tagIds = knownTagIds(); + TagMap map = TagMap.create(); + for (long tagId : tagIds) { + map.set(tagId, "value"); + } + TagMap copy = map.copy(); + + assertEquals(tagIds.size(), map.size()); + assertEquals(tagIds.size(), copy.size()); + for (long tagId : tagIds) { + String name = KnownTagCodec.nameOf(tagId); + assertEquals("value", map.getObject(name), name); + assertEquals("value", copy.getObject(name), name); + } + } + + private static List knownTagIds() throws IllegalAccessException { + List tagIds = new ArrayList<>(); + for (Field field : KnownTags.class.getFields()) { + if (field.getType() == long.class && Modifier.isStatic(field.getModifiers())) { + long tagId = field.getLong(null); + if (KnownTagCodec.isKeyableById(tagId)) { + tagIds.add(tagId); + } + } + } + return tagIds; + } + + @Test + void lookupsFindEntriesSetEitherWay() { + TagMap map = TagMap.create(); + map.set(KnownTags.HTTP_STATUS_CODE_ID, 5432); + map.set("my.custom.tag", "value"); + + assertEquals(5432, map.getEntry(KnownTags.HTTP_STATUS_CODE_NAME).intValue()); + assertEquals("value", map.getString("my.custom.tag")); + assertEquals(5432, map.getAndRemove(KnownTags.HTTP_STATUS_CODE_NAME).intValue()); + assertEquals(null, map.getEntry(KnownTags.HTTP_STATUS_CODE_NAME)); + } +} diff --git a/tag-conventions-java.yaml b/tag-conventions-java.yaml new file mode 100644 index 00000000000..2f3d89e39ed --- /dev/null +++ b/tag-conventions-java.yaml @@ -0,0 +1,40 @@ +# Tracer overlay for tag-conventions.yaml — dd-trace-java's set-path routing. +# --------------------------------------------------------------------------- +# tag-conventions.yaml is language-agnostic; which keys a tracer intercepts on the set-path (routing +# them to a span field or a sampling directive instead of tag storage) is this tracer's concern, so +# it lives here. +# +# tags — keys that exist only to be routed, so the conventions do not declare them. They get +# ids like any other tag (instrumentation can set them by id) but belong to no span type. +# Each is intercepted, so it is not listed again below. Same declaration syntax as +# tag-conventions.yaml; refs are not allowed. +# intercepted — tags declared in tag-conventions.yaml that TagInterceptor also routes. Each one's id carries +# the INTERCEPTED bit, so a setter called with a constant id folds the interception test +# away. TagInterceptor's switch is the authority on what each one does; a test there +# keeps this list and the switch in agreement. +# --------------------------------------------------------------------------- + +tags: + - { dd-name: resource.name, type: string } + - { dd-name: span.type, type: string } + - { dd-name: error, type: boolean } + - { dd-name: manual.keep, type: boolean } + - { dd-name: manual.drop, type: boolean } + - { dd-name: asm.keep, type: boolean } + - { dd-name: ai_guard.keep, type: boolean } + - { dd-name: sampling.priority, type: int } + - { dd-name: _dd.p.ts, type: int } + - { dd-name: _dd.p.debug, type: string } + - { dd-name: _dd.origin, type: string } + - { dd-name: _dd.measured, type: boolean } + - { dd-name: _dd1.sr.eausr, type: double } + +intercepted: + - service + - peer.service + - span.kind + - servlet.context + - db.statement + - http.method + - http.url + - http.status_code diff --git a/tag-conventions.yaml b/tag-conventions.yaml index b6cdfe5deb4..c4a5f9f9b70 100644 --- a/tag-conventions.yaml +++ b/tag-conventions.yaml @@ -200,8 +200,9 @@ mixins: # - span.kind enumerates: server | client | producer | consumer | internal | broker. # - Some keys (resource.name, error, sampling.priority, ...) are accepted by setTag but routed to a # span field or a trace directive instead of tag storage. That routing is a per-language tracer -# concern, so it is NOT modelled here; such a key appears above only when it also needs an id and -# a name (service does, for OpenTelemetry's service.name). +# concern, so it is NOT modelled here: dd-trace-java declares those keys, and marks every tag it +# intercepts, in its own overlay (tag-conventions-java.yaml). Such a key appears above only when it +# also needs an id and a name across tracers (service does, for OpenTelemetry's service.name). # - Tags with no otel-name pass through under their Datadog name (RFC "retain"). A `# TODO(otel)` note # marks a pending OpenTelemetry-team review of a mapping that is not yet a settled rename. # - db.statement/db.query.text is deliberately never re-exposed as a stored tag once consumed into