diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/TypeConverter.java b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/TypeConverter.java index 1e3714d6a2c..66f5602c50e 100644 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/TypeConverter.java +++ b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/TypeConverter.java @@ -1,6 +1,5 @@ package datadog.trace.instrumentation.opentelemetry; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext; @@ -16,12 +15,10 @@ public class TypeConverter { private final Span noopSpanWrapper; private final SpanContext noopContextWrapper; - private final OtelScope noopScopeWrapper; public TypeConverter() { noopSpanWrapper = new OtelSpan(noopSpan(), this); noopContextWrapper = new OtelSpanContext(noopSpanContext()); - noopScopeWrapper = new OtelScope(noopScope()); } public AgentSpan toAgentSpan(final Span span) { @@ -55,9 +52,6 @@ public Scope toScope(final AgentScope scope) { if (scope == null) { return null; } - if (scope == noopScope()) { - return noopScopeWrapper; - } return new OtelScope(scope); } diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy index 6852423dd22..60c0f43d9be 100644 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy +++ b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy @@ -9,7 +9,6 @@ import datadog.trace.core.PendingTrace import datadog.trace.core.propagation.PropagationTags import datadog.trace.instrumentation.opentelemetry.TypeConverter -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext @@ -39,12 +38,6 @@ class TypeConverterTest extends InstrumentationSpecification { typeConverter.toSpanContext(noopContext) is typeConverter.toSpanContext(noopContext) } - def "should avoid the noop scope wrapper allocation"() { - def noopScope = noopScope() - expect: - typeConverter.toScope(noopScope) is typeConverter.toScope(noopScope) - } - def createTestSpanContext() { def trace = Stub(PendingTrace) return new DDSpanContext( diff --git a/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/TypeConverter.java b/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/TypeConverter.java index 09693fda11b..9d89a0c9320 100644 --- a/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/TypeConverter.java +++ b/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/TypeConverter.java @@ -1,6 +1,5 @@ package datadog.trace.instrumentation.opentracing31; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext; @@ -18,13 +17,11 @@ public class TypeConverter { private final LogHandler logHandler; private final OTSpan noopSpanWrapper; private final OTSpanContext noopContextWrapper; - private final OTScopeManager.OTScope noopScopeWrapper; public TypeConverter(final LogHandler logHandler) { this.logHandler = logHandler; noopSpanWrapper = new OTSpan(noopSpan(), this, logHandler); noopContextWrapper = new OTSpanContext(noopSpanContext()); - noopScopeWrapper = new OTScopeManager.OTScope(noopScope(), false, this); } public AgentSpan toAgentSpan(final Span span) { @@ -58,9 +55,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) { if (scope == null) { return null; } - if (scope == noopScope()) { - return noopScopeWrapper; - } return new OTScopeManager.OTScope(scope, finishSpanOnClose, this); } diff --git a/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/test/groovy/TypeConverterTest.groovy b/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/test/groovy/TypeConverterTest.groovy index 9aba9eabaa8..fcdd67b9e4b 100644 --- a/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/test/groovy/TypeConverterTest.groovy +++ b/dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/test/groovy/TypeConverterTest.groovy @@ -8,9 +8,9 @@ import datadog.trace.core.DDSpanContext import datadog.trace.core.PendingTrace import datadog.trace.core.propagation.PropagationTags import datadog.trace.instrumentation.opentracing.DefaultLogHandler +import datadog.trace.bootstrap.instrumentation.api.AgentScope import datadog.trace.instrumentation.opentracing31.TypeConverter -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext @@ -40,14 +40,14 @@ class TypeConverterTest extends InstrumentationSpecification { typeConverter.toSpanContext(noopContext) is typeConverter.toSpanContext(noopContext) } - def "should avoid the noop scope wrapper allocation"() { - def noopScope = noopScope() + def "should reuse the noop span wrapper via scope"() { + def noopScope = Stub(AgentScope) { + span() >> noopSpan() + } + def noopSpanWrapper = typeConverter.toSpan(noopSpan()) expect: - typeConverter.toScope(noopScope, true) is typeConverter.toScope(noopScope, true) - typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, false) - // noop scopes expected to be the same despite the finishSpanOnClose flag - typeConverter.toScope(noopScope, true) is typeConverter.toScope(noopScope, false) - typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true) + typeConverter.toScope(noopScope, true).span() is noopSpanWrapper + typeConverter.toScope(noopScope, false).span() is noopSpanWrapper } def createTestSpanContext() { diff --git a/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/TypeConverter.java b/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/TypeConverter.java index 6067d9c2be3..2d2b78b8146 100644 --- a/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/TypeConverter.java +++ b/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/TypeConverter.java @@ -1,6 +1,5 @@ package datadog.trace.instrumentation.opentracing32; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext; @@ -18,13 +17,11 @@ public class TypeConverter { private final LogHandler logHandler; private final OTSpan noopSpanWrapper; private final OTSpanContext noopContextWrapper; - private final OTScopeManager.OTScope noopScopeWrapper; public TypeConverter(final LogHandler logHandler) { this.logHandler = logHandler; noopSpanWrapper = new OTSpan(noopSpan(), this, logHandler); noopContextWrapper = new OTSpanContext(noopSpanContext()); - noopScopeWrapper = new OTScopeManager.OTScope(noopScope(), false, this); } public AgentSpan toAgentSpan(final Span span) { @@ -58,9 +55,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) { if (scope == null) { return null; } - if (scope == noopScope()) { - return noopScopeWrapper; - } return new OTScopeManager.OTScope(scope, finishSpanOnClose, this); } diff --git a/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/test/groovy/TypeConverterTest.groovy b/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/test/groovy/TypeConverterTest.groovy index cb05e4b1737..795964ec8eb 100644 --- a/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/test/groovy/TypeConverterTest.groovy +++ b/dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/test/groovy/TypeConverterTest.groovy @@ -8,9 +8,9 @@ import datadog.trace.core.DDSpanContext import datadog.trace.core.PendingTrace import datadog.trace.core.propagation.PropagationTags import datadog.trace.instrumentation.opentracing.DefaultLogHandler +import datadog.trace.bootstrap.instrumentation.api.AgentScope import datadog.trace.instrumentation.opentracing32.TypeConverter -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext @@ -40,14 +40,14 @@ class TypeConverterTest extends InstrumentationSpecification { typeConverter.toSpanContext(noopContext) is typeConverter.toSpanContext(noopContext) } - def "should avoid the noop scope wrapper allocation"() { - def noopScope = noopScope() + def "should reuse the noop span wrapper via scope"() { + def noopScope = Stub(AgentScope) { + span() >> noopSpan() + } + def noopSpanWrapper = typeConverter.toSpan(noopSpan()) expect: - typeConverter.toScope(noopScope, true) is typeConverter.toScope(noopScope, true) - typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, false) - // noop scopes expected to be the same despite the finishSpanOnClose flag - typeConverter.toScope(noopScope, true) is typeConverter.toScope(noopScope, false) - typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true) + typeConverter.toScope(noopScope, true).span() is noopSpanWrapper + typeConverter.toScope(noopScope, false).span() is noopSpanWrapper } def createTestSpanContext() { diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/TypeConverter.java b/dd-trace-ot/src/main/java/datadog/opentracing/TypeConverter.java index b02f96a3272..2193eeee980 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/TypeConverter.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/TypeConverter.java @@ -1,6 +1,5 @@ package datadog.opentracing; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext; @@ -17,13 +16,11 @@ class TypeConverter { private final LogHandler logHandler; private final OTSpan noopSpanWrapper; private final OTSpanContext noopContextWrapper; - private final OTScopeManager.OTScope noopScopeWrapper; public TypeConverter(final LogHandler logHandler) { this.logHandler = logHandler; noopSpanWrapper = new OTSpan(noopSpan(), this, logHandler); noopContextWrapper = new OTSpanContext(noopSpanContext()); - noopScopeWrapper = new OTScopeManager.OTScope(noopScope(), false, this); } public AgentSpan toAgentSpan(final Span span) { @@ -61,9 +58,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) { if (scope == null) { return null; } - if (scope == noopScope()) { - return noopScopeWrapper; - } return new OTScopeManager.OTScope(scope, finishSpanOnClose, this); } diff --git a/dd-trace-ot/src/test/java/datadog/opentracing/TypeConverterTest.java b/dd-trace-ot/src/test/java/datadog/opentracing/TypeConverterTest.java index 839c1470792..e506bc44616 100644 --- a/dd-trace-ot/src/test/java/datadog/opentracing/TypeConverterTest.java +++ b/dd-trace-ot/src/test/java/datadog/opentracing/TypeConverterTest.java @@ -1,11 +1,13 @@ package datadog.opentracing; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.common.writer.ListWriter; import datadog.trace.core.CoreTracer; @@ -45,20 +47,11 @@ void shouldAvoidTheNoopContextWrapperAllocation() { } @Test - void shouldAvoidTheNoopScopeWrapperAllocation() { - datadog.trace.bootstrap.instrumentation.api.AgentScope noopScopeInstance = noopScope(); - assertSame( - typeConverter.toScope(noopScopeInstance, true), - typeConverter.toScope(noopScopeInstance, true)); - assertSame( - typeConverter.toScope(noopScopeInstance, false), - typeConverter.toScope(noopScopeInstance, false)); - // noop scopes expected to be the same despite the finishSpanOnClose flag - assertSame( - typeConverter.toScope(noopScopeInstance, true), - typeConverter.toScope(noopScopeInstance, false)); - assertSame( - typeConverter.toScope(noopScopeInstance, false), - typeConverter.toScope(noopScopeInstance, true)); + void shouldReuseNoopSpanWrapperViaScope() { + AgentScope noopScope = mock(AgentScope.class); + when(noopScope.span()).thenReturn(noopSpan()); + OTSpan noopSpanWrapper = typeConverter.toSpan(noopSpan()); + assertSame(typeConverter.toScope(noopScope, true).span(), noopSpanWrapper); + assertSame(typeConverter.toScope(noopScope, false).span(), noopSpanWrapper); } }