Skip to content

Commit 01b5e2c

Browse files
mccullsdevflow.devflow-routing-intake
andauthored
Drop noopScope() wrapper caching from OT/OTel TypeConverters (#12011)
Drop noopScope() wrapper caching from OT/OTel TypeConverters The cached noopScopeWrapper only optimized an already-rare path (scope-manager depth-limit overflow); drop the special-casing and the now-unneeded dependency on AgentTracer.noopScope() identity. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 4a5a1e2 commit 01b5e2c

8 files changed

Lines changed: 25 additions & 63 deletions

File tree

dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/TypeConverter.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package datadog.trace.instrumentation.opentelemetry;
22

3-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
43
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan;
54
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext;
65

@@ -16,12 +15,10 @@
1615
public class TypeConverter {
1716
private final Span noopSpanWrapper;
1817
private final SpanContext noopContextWrapper;
19-
private final OtelScope noopScopeWrapper;
2018

2119
public TypeConverter() {
2220
noopSpanWrapper = new OtelSpan(noopSpan(), this);
2321
noopContextWrapper = new OtelSpanContext(noopSpanContext());
24-
noopScopeWrapper = new OtelScope(noopScope());
2522
}
2623

2724
public AgentSpan toAgentSpan(final Span span) {
@@ -55,9 +52,6 @@ public Scope toScope(final AgentScope scope) {
5552
if (scope == null) {
5653
return null;
5754
}
58-
if (scope == noopScope()) {
59-
return noopScopeWrapper;
60-
}
6155
return new OtelScope(scope);
6256
}
6357

dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import datadog.trace.core.PendingTrace
99
import datadog.trace.core.propagation.PropagationTags
1010
import datadog.trace.instrumentation.opentelemetry.TypeConverter
1111

12-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
1312
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
1413
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1514

@@ -39,12 +38,6 @@ class TypeConverterTest extends InstrumentationSpecification {
3938
typeConverter.toSpanContext(noopContext) is typeConverter.toSpanContext(noopContext)
4039
}
4140

42-
def "should avoid the noop scope wrapper allocation"() {
43-
def noopScope = noopScope()
44-
expect:
45-
typeConverter.toScope(noopScope) is typeConverter.toScope(noopScope)
46-
}
47-
4841
def createTestSpanContext() {
4942
def trace = Stub(PendingTrace)
5043
return new DDSpanContext(

dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/TypeConverter.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package datadog.trace.instrumentation.opentracing31;
22

3-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
43
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan;
54
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext;
65

@@ -18,13 +17,11 @@ public class TypeConverter {
1817
private final LogHandler logHandler;
1918
private final OTSpan noopSpanWrapper;
2019
private final OTSpanContext noopContextWrapper;
21-
private final OTScopeManager.OTScope noopScopeWrapper;
2220

2321
public TypeConverter(final LogHandler logHandler) {
2422
this.logHandler = logHandler;
2523
noopSpanWrapper = new OTSpan(noopSpan(), this, logHandler);
2624
noopContextWrapper = new OTSpanContext(noopSpanContext());
27-
noopScopeWrapper = new OTScopeManager.OTScope(noopScope(), false, this);
2825
}
2926

3027
public AgentSpan toAgentSpan(final Span span) {
@@ -58,9 +55,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) {
5855
if (scope == null) {
5956
return null;
6057
}
61-
if (scope == noopScope()) {
62-
return noopScopeWrapper;
63-
}
6458
return new OTScopeManager.OTScope(scope, finishSpanOnClose, this);
6559
}
6660

dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/test/groovy/TypeConverterTest.groovy

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ import datadog.trace.core.DDSpanContext
88
import datadog.trace.core.PendingTrace
99
import datadog.trace.core.propagation.PropagationTags
1010
import datadog.trace.instrumentation.opentracing.DefaultLogHandler
11+
import datadog.trace.bootstrap.instrumentation.api.AgentScope
1112
import datadog.trace.instrumentation.opentracing31.TypeConverter
1213

13-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
1414
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
1515
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1616

@@ -40,14 +40,14 @@ class TypeConverterTest extends InstrumentationSpecification {
4040
typeConverter.toSpanContext(noopContext) is typeConverter.toSpanContext(noopContext)
4141
}
4242

43-
def "should avoid the noop scope wrapper allocation"() {
44-
def noopScope = noopScope()
43+
def "should reuse the noop span wrapper via scope"() {
44+
def noopScope = Stub(AgentScope) {
45+
span() >> noopSpan()
46+
}
47+
def noopSpanWrapper = typeConverter.toSpan(noopSpan())
4548
expect:
46-
typeConverter.toScope(noopScope, true) is typeConverter.toScope(noopScope, true)
47-
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, false)
48-
// noop scopes expected to be the same despite the finishSpanOnClose flag
49-
typeConverter.toScope(noopScope, true) is typeConverter.toScope(noopScope, false)
50-
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
49+
typeConverter.toScope(noopScope, true).span() is noopSpanWrapper
50+
typeConverter.toScope(noopScope, false).span() is noopSpanWrapper
5151
}
5252

5353
def createTestSpanContext() {

dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/TypeConverter.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package datadog.trace.instrumentation.opentracing32;
22

3-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
43
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan;
54
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext;
65

@@ -18,13 +17,11 @@ public class TypeConverter {
1817
private final LogHandler logHandler;
1918
private final OTSpan noopSpanWrapper;
2019
private final OTSpanContext noopContextWrapper;
21-
private final OTScopeManager.OTScope noopScopeWrapper;
2220

2321
public TypeConverter(final LogHandler logHandler) {
2422
this.logHandler = logHandler;
2523
noopSpanWrapper = new OTSpan(noopSpan(), this, logHandler);
2624
noopContextWrapper = new OTSpanContext(noopSpanContext());
27-
noopScopeWrapper = new OTScopeManager.OTScope(noopScope(), false, this);
2825
}
2926

3027
public AgentSpan toAgentSpan(final Span span) {
@@ -58,9 +55,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) {
5855
if (scope == null) {
5956
return null;
6057
}
61-
if (scope == noopScope()) {
62-
return noopScopeWrapper;
63-
}
6458
return new OTScopeManager.OTScope(scope, finishSpanOnClose, this);
6559
}
6660

dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/test/groovy/TypeConverterTest.groovy

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ import datadog.trace.core.DDSpanContext
88
import datadog.trace.core.PendingTrace
99
import datadog.trace.core.propagation.PropagationTags
1010
import datadog.trace.instrumentation.opentracing.DefaultLogHandler
11+
import datadog.trace.bootstrap.instrumentation.api.AgentScope
1112
import datadog.trace.instrumentation.opentracing32.TypeConverter
1213

13-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
1414
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
1515
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1616

@@ -40,14 +40,14 @@ class TypeConverterTest extends InstrumentationSpecification {
4040
typeConverter.toSpanContext(noopContext) is typeConverter.toSpanContext(noopContext)
4141
}
4242

43-
def "should avoid the noop scope wrapper allocation"() {
44-
def noopScope = noopScope()
43+
def "should reuse the noop span wrapper via scope"() {
44+
def noopScope = Stub(AgentScope) {
45+
span() >> noopSpan()
46+
}
47+
def noopSpanWrapper = typeConverter.toSpan(noopSpan())
4548
expect:
46-
typeConverter.toScope(noopScope, true) is typeConverter.toScope(noopScope, true)
47-
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, false)
48-
// noop scopes expected to be the same despite the finishSpanOnClose flag
49-
typeConverter.toScope(noopScope, true) is typeConverter.toScope(noopScope, false)
50-
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
49+
typeConverter.toScope(noopScope, true).span() is noopSpanWrapper
50+
typeConverter.toScope(noopScope, false).span() is noopSpanWrapper
5151
}
5252

5353
def createTestSpanContext() {

dd-trace-ot/src/main/java/datadog/opentracing/TypeConverter.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package datadog.opentracing;
22

3-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
43
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan;
54
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext;
65

@@ -17,13 +16,11 @@ class TypeConverter {
1716
private final LogHandler logHandler;
1817
private final OTSpan noopSpanWrapper;
1918
private final OTSpanContext noopContextWrapper;
20-
private final OTScopeManager.OTScope noopScopeWrapper;
2119

2220
public TypeConverter(final LogHandler logHandler) {
2321
this.logHandler = logHandler;
2422
noopSpanWrapper = new OTSpan(noopSpan(), this, logHandler);
2523
noopContextWrapper = new OTSpanContext(noopSpanContext());
26-
noopScopeWrapper = new OTScopeManager.OTScope(noopScope(), false, this);
2724
}
2825

2926
public AgentSpan toAgentSpan(final Span span) {
@@ -61,9 +58,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) {
6158
if (scope == null) {
6259
return null;
6360
}
64-
if (scope == noopScope()) {
65-
return noopScopeWrapper;
66-
}
6761
return new OTScopeManager.OTScope(scope, finishSpanOnClose, this);
6862
}
6963

dd-trace-ot/src/test/java/datadog/opentracing/TypeConverterTest.java

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package datadog.opentracing;
22

3-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
43
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan;
54
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext;
65
import static org.junit.jupiter.api.Assertions.assertNotSame;
76
import static org.junit.jupiter.api.Assertions.assertSame;
7+
import static org.mockito.Mockito.mock;
8+
import static org.mockito.Mockito.when;
89

10+
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
911
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1012
import datadog.trace.common.writer.ListWriter;
1113
import datadog.trace.core.CoreTracer;
@@ -45,20 +47,11 @@ void shouldAvoidTheNoopContextWrapperAllocation() {
4547
}
4648

4749
@Test
48-
void shouldAvoidTheNoopScopeWrapperAllocation() {
49-
datadog.trace.bootstrap.instrumentation.api.AgentScope noopScopeInstance = noopScope();
50-
assertSame(
51-
typeConverter.toScope(noopScopeInstance, true),
52-
typeConverter.toScope(noopScopeInstance, true));
53-
assertSame(
54-
typeConverter.toScope(noopScopeInstance, false),
55-
typeConverter.toScope(noopScopeInstance, false));
56-
// noop scopes expected to be the same despite the finishSpanOnClose flag
57-
assertSame(
58-
typeConverter.toScope(noopScopeInstance, true),
59-
typeConverter.toScope(noopScopeInstance, false));
60-
assertSame(
61-
typeConverter.toScope(noopScopeInstance, false),
62-
typeConverter.toScope(noopScopeInstance, true));
50+
void shouldReuseNoopSpanWrapperViaScope() {
51+
AgentScope noopScope = mock(AgentScope.class);
52+
when(noopScope.span()).thenReturn(noopSpan());
53+
OTSpan noopSpanWrapper = typeConverter.toSpan(noopSpan());
54+
assertSame(typeConverter.toScope(noopScope, true).span(), noopSpanWrapper);
55+
assertSame(typeConverter.toScope(noopScope, false).span(), noopSpanWrapper);
6356
}
6457
}

0 commit comments

Comments
 (0)