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 0c48adc20e5..6a8767e523f 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 @@ -7,6 +7,7 @@ import datadog.trace.api.Config; import datadog.trace.api.DDTags; import datadog.trace.api.Functions; +import datadog.trace.api.TagMap; import datadog.trace.api.cache.QualifiedClassNameCache; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; @@ -40,17 +41,29 @@ public String apply(Class clazz) { Functions.PrefixJoin.of(".")); protected final boolean traceAnalyticsEnabled; - protected final Double traceAnalyticsSampleRate; + protected final double traceAnalyticsSampleRate; + + private final TagMap.Entry traceAnalyticsEntry; + + // Deliberately not volatile, reading null and repeating the calculation is safe + private TagMap.Entry cachedComponentEntry = null; protected BaseDecorator() { final Config config = Config.get(); final String[] instrumentationNames = instrumentationNames(); + this.traceAnalyticsEnabled = instrumentationNames.length > 0 && config.isTraceAnalyticsIntegrationEnabled( traceAnalyticsDefault(), instrumentationNames); + this.traceAnalyticsSampleRate = (double) config.getInstrumentationAnalyticsSampleRate(instrumentationNames); + + this.traceAnalyticsEntry = + this.traceAnalyticsEnabled + ? TagMap.Entry.create(DDTags.ANALYTICS_SAMPLE_RATE, traceAnalyticsSampleRate) + : null; } protected abstract String[] instrumentationNames(); @@ -59,6 +72,20 @@ protected BaseDecorator() { protected abstract CharSequence component(); + /** Caches the component TagMap.Entry, so it isn't recreated for every trace */ + protected final TagMap.Entry componentEntry() { + // DQH = Tried calling component() in the constructor, but that had issues with static + // field ordering. That was caught be an integration test, but I didn't want to risk + // breaking other integrations where the test is not as thorough. + + // This approach while more complicated doesn't have any field initialization ordering issues. + TagMap.Entry componentEntry = cachedComponentEntry; + if (componentEntry == null) { + cachedComponentEntry = componentEntry = TagMap.Entry.create(Tags.COMPONENT, component()); + } + return componentEntry; + } + protected boolean traceAnalyticsDefault() { return false; } @@ -67,12 +94,17 @@ public AgentSpan afterStart(final AgentSpan span) { if (spanType() != null) { span.setSpanType(spanType()); } + + span.setTag(componentEntry()); + + // DQH - Could retrieve the value from componentEntry and cast to avoid the virtual call, + // unclear which option is better here final CharSequence component = component(); - span.setTag(Tags.COMPONENT, component); span.context().setIntegrationName(component); - if (traceAnalyticsEnabled) { - span.setMetric(DDTags.ANALYTICS_SAMPLE_RATE, traceAnalyticsSampleRate); - } + + // null handled by setMetric + span.setMetric(traceAnalyticsEntry); + return span; } 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 9135441c735..6f397845a98 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 @@ -1,6 +1,6 @@ package datadog.trace.bootstrap.instrumentation.decorator - +import datadog.trace.api.TagMap import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities @@ -25,11 +25,13 @@ class BaseDecoratorTest extends DDSpecification { then: 1 * span.setSpanType(decorator.spanType()) - 1 * span.setTag(Tags.COMPONENT, "test-component") + 1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component")) 1 * span.context() >> spanContext 1 * spanContext.setIntegrationName("test-component") + _ * span.setTag(_) _ * span.setTag(_, _) // Want to allow other calls from child implementations. _ * span.setMeasured(true) + _ * span.setMetric(_) _ * span.setMetric(_, _) _ * span.setServiceName(_, _) _ * span.setOperationName(_) 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 fd9c3744463..fea72bb5714 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 @@ -1,6 +1,7 @@ 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.AgentSpanContext import datadog.trace.bootstrap.instrumentation.api.Tags @@ -22,12 +23,13 @@ class ClientDecoratorTest extends BaseDecoratorTest { 1 * span.setServiceName(serviceName, "test-component") } 1 * span.setMeasured(true) - 1 * span.setTag(Tags.COMPONENT, "test-component") + 1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component")) 1 * span.context() >> spanContext 1 * spanContext.setIntegrationName("test-component") 1 * span.setTag(Tags.SPAN_KIND, "client") 1 * span.setSpanType(decorator.spanType()) - 1 * span.setMetric(DDTags.ANALYTICS_SAMPLE_RATE, 1.0) + 1 * span.setMetric(TagMap.Entry.create(DDTags.ANALYTICS_SAMPLE_RATE, 1.0)) + _ * span.setTag(_) _ * span.setTag(_, _) // Want to allow other calls from child implementations. _ * span.setServiceName(_) _ * span.setOperationName(_) 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 a861d718ef3..c40fbbd007b 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 @@ -1,6 +1,7 @@ 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.AgentSpanContext import datadog.trace.bootstrap.instrumentation.api.Tags @@ -26,12 +27,12 @@ class DatabaseClientDecoratorTest extends ClientDecoratorTest { 1 * span.setServiceName(serviceName, "test-component") } 1 * span.setMeasured(true) - 1 * span.setTag(Tags.COMPONENT, "test-component") + 1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component")) 1 * span.context() >> spanContext 1 * spanContext.setIntegrationName("test-component") 1 * span.setTag(Tags.SPAN_KIND, "client") 1 * span.setSpanType("test-type") - 1 * span.setMetric(DDTags.ANALYTICS_SAMPLE_RATE, 1.0) + 1 * span.setMetric(TagMap.Entry.create(DDTags.ANALYTICS_SAMPLE_RATE, 1.0)) 0 * _ where: 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 9eeb1b19e77..bde0bd7927f 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 @@ -1,6 +1,6 @@ package datadog.trace.bootstrap.instrumentation.decorator - +import datadog.trace.api.TagMap import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext @@ -23,13 +23,15 @@ class ServerDecoratorTest extends BaseDecoratorTest { then: 1 * span.setTag(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE) - 1 * span.setTag(COMPONENT, "test-component") + 1 * span.setTag(TagMap.Entry.create(COMPONENT, "test-component")) 1 * span.context() >> spanContext 1 * spanContext.setIntegrationName("test-component") 1 * span.setTag(SPAN_KIND, "server") 1 * span.setSpanType(decorator.spanType()) if (decorator.traceAnalyticsEnabled) { - 1 * span.setMetric(ANALYTICS_SAMPLE_RATE, 1.0) + 1 * span.setMetric(TagMap.Entry.create(ANALYTICS_SAMPLE_RATE, 1.0)) + } else { + 1 * span.setMetric(null) } 0 * _ } diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapEntryTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapEntryTest.java index 9eef1cb40c9..cf8e9114a5d 100644 --- a/internal-api/src/test/java/datadog/trace/api/TagMapEntryTest.java +++ b/internal-api/src/test/java/datadog/trace/api/TagMapEntryTest.java @@ -155,7 +155,7 @@ public void createBoolean(boolean value) { @ValueSource(booleans = {false, true}) public void newBooleanEntry(boolean value) { test( - () -> TagMap.Entry.newBooleanEntry("foo", value), + () -> TagMap.Entry.create("foo", value), TagMap.Entry.BOOLEAN, (entry) -> multiCheck( @@ -498,6 +498,21 @@ public void createDouble(double value) { @ParameterizedTest @DisplayName("newDoubleEntry: double") + @ValueSource( + doubles = {Double.MIN_VALUE, Float.MIN_VALUE, -1D, 0D, 1D, Math.E, Math.PI, Double.MAX_VALUE}) + public void doubleEntry_via_create(double value) { + test( + () -> TagMap.Entry.create("foo", value), + TagMap.Entry.DOUBLE, + (entry) -> + multiCheck( + checkKey("foo", entry), + checkValue(value, entry), + checkIsNumericPrimitive(entry), + checkType(TagMap.Entry.DOUBLE, entry))); + } + + @ParameterizedTest @ValueSource( doubles = {Double.MIN_VALUE, Float.MIN_VALUE, -1D, 0D, 1D, Math.E, Math.PI, Double.MAX_VALUE}) public void doubleEntry(double value) {