Skip to content

Commit ccb5be3

Browse files
mccullsmcculls
committed
Remove AttachableWrapper aspect from scopes (#8534)
In the majority of cases the scope wrapper will only be accessed once when activating a span or context. The other cases are deprecated calls to check the active scope, where the returned scope wrapper is short-lived and cheap to recreate.
1 parent c4cb2e9 commit ccb5be3

File tree

9 files changed

+9
-144
lines changed

9 files changed

+9
-144
lines changed

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

-10
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,6 @@ public Scope toScope(final AgentScope scope) {
5555
if (scope == null) {
5656
return null;
5757
}
58-
if (scope instanceof AttachableWrapper) {
59-
AttachableWrapper attachableScopeWrapper = (AttachableWrapper) scope;
60-
Object wrapper = attachableScopeWrapper.getWrapper();
61-
if (wrapper instanceof Scope) {
62-
return (Scope) wrapper;
63-
}
64-
Scope otScope = new OtelScope(scope);
65-
attachableScopeWrapper.attachWrapper(otScope);
66-
return otScope;
67-
}
6858
if (scope == noopScope()) {
6959
return noopScopeWrapper;
7060
}

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

+2-17
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,17 @@
11
import datadog.trace.agent.test.AgentTestRunner
22
import datadog.trace.api.DDSpanId
33
import datadog.trace.api.DDTraceId
4-
import datadog.trace.api.sampling.PrioritySampling
54
import datadog.trace.api.datastreams.NoopPathwayContext
5+
import datadog.trace.api.sampling.PrioritySampling
66
import datadog.trace.core.DDSpan
77
import datadog.trace.core.DDSpanContext
88
import datadog.trace.core.PendingTrace
99
import datadog.trace.core.propagation.PropagationTags
10-
import datadog.trace.core.scopemanager.ContinuableScopeManager
1110
import datadog.trace.instrumentation.opentelemetry.TypeConverter
1211

1312
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
14-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1513
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
14+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1615

1716
class TypeConverterTest extends AgentTestRunner {
1817
TypeConverter typeConverter = new TypeConverter()
@@ -46,20 +45,6 @@ class TypeConverterTest extends AgentTestRunner {
4645
typeConverter.toScope(noopScope) is typeConverter.toScope(noopScope)
4746
}
4847

49-
def "should avoid extra allocation for a scope wrapper"() {
50-
def scopeManager = new ContinuableScopeManager(0, false)
51-
def context = createTestSpanContext()
52-
def span1 = new DDSpan("test", 0, context, null)
53-
def span2 = new DDSpan("test", 0, context, null)
54-
def scope1 = scopeManager.activateManualSpan(span1)
55-
def scope2 = scopeManager.activateManualSpan(span2)
56-
expect:
57-
// return the same wrapper for the same scope
58-
typeConverter.toScope(scope1) is typeConverter.toScope(scope1)
59-
// return distinct wrapper for another context
60-
!typeConverter.toScope(scope1).is(typeConverter.toScope(scope2))
61-
}
62-
6348
def createTestSpanContext() {
6449
def trace = Stub(PendingTrace)
6550
return new DDSpanContext(

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

-13
Original file line numberDiff line numberDiff line change
@@ -58,19 +58,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) {
5858
if (scope == null) {
5959
return null;
6060
}
61-
if (scope instanceof AttachableWrapper) {
62-
AttachableWrapper attachableScopeWrapper = (AttachableWrapper) scope;
63-
Object wrapper = attachableScopeWrapper.getWrapper();
64-
if (wrapper instanceof OTScopeManager.OTScope) {
65-
OTScopeManager.OTScope attachedScopeWrapper = (OTScopeManager.OTScope) wrapper;
66-
if (attachedScopeWrapper.isFinishSpanOnClose() == finishSpanOnClose) {
67-
return (Scope) wrapper;
68-
}
69-
}
70-
Scope otScope = new OTScopeManager.OTScope(scope, finishSpanOnClose, this);
71-
attachableScopeWrapper.attachWrapper(otScope);
72-
return otScope;
73-
}
7461
if (scope == noopScope()) {
7562
return noopScopeWrapper;
7663
}

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

+2-20
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,18 @@
11
import datadog.trace.agent.test.AgentTestRunner
22
import datadog.trace.api.DDSpanId
33
import datadog.trace.api.DDTraceId
4-
import datadog.trace.api.sampling.PrioritySampling
54
import datadog.trace.api.datastreams.NoopPathwayContext
5+
import datadog.trace.api.sampling.PrioritySampling
66
import datadog.trace.core.DDSpan
77
import datadog.trace.core.DDSpanContext
88
import datadog.trace.core.PendingTrace
99
import datadog.trace.core.propagation.PropagationTags
10-
import datadog.trace.core.scopemanager.ContinuableScopeManager
1110
import datadog.trace.instrumentation.opentracing.DefaultLogHandler
1211
import datadog.trace.instrumentation.opentracing31.TypeConverter
1312

1413
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
15-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1614
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
15+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1716

1817
class TypeConverterTest extends AgentTestRunner {
1918
TypeConverter typeConverter = new TypeConverter(new DefaultLogHandler())
@@ -51,23 +50,6 @@ class TypeConverterTest extends AgentTestRunner {
5150
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
5251
}
5352

54-
def "should avoid extra allocation for a scope wrapper"() {
55-
def scopeManager = new ContinuableScopeManager(0, false)
56-
def context = createTestSpanContext()
57-
def span1 = new DDSpan("test", 0, context, null)
58-
def span2 = new DDSpan("test", 0, context, null)
59-
def scope1 = scopeManager.activateManualSpan(span1)
60-
def scope2 = scopeManager.activateManualSpan(span2)
61-
expect:
62-
// return the same wrapper for the same scope
63-
typeConverter.toScope(scope1, true) is typeConverter.toScope(scope1, true)
64-
typeConverter.toScope(scope1, false) is typeConverter.toScope(scope1, false)
65-
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope1, false))
66-
!typeConverter.toScope(scope1, false).is(typeConverter.toScope(scope1, true))
67-
// return distinct wrapper for another context
68-
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope2, true))
69-
}
70-
7153
def createTestSpanContext() {
7254
def trace = Stub(PendingTrace)
7355
return new DDSpanContext(

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

-13
Original file line numberDiff line numberDiff line change
@@ -58,19 +58,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) {
5858
if (scope == null) {
5959
return null;
6060
}
61-
if (scope instanceof AttachableWrapper) {
62-
AttachableWrapper attachableScopeWrapper = (AttachableWrapper) scope;
63-
Object wrapper = attachableScopeWrapper.getWrapper();
64-
if (wrapper instanceof OTScopeManager.OTScope) {
65-
OTScopeManager.OTScope attachedScopeWrapper = (OTScopeManager.OTScope) wrapper;
66-
if (attachedScopeWrapper.isFinishSpanOnClose() == finishSpanOnClose) {
67-
return (Scope) wrapper;
68-
}
69-
}
70-
Scope otScope = new OTScopeManager.OTScope(scope, finishSpanOnClose, this);
71-
attachableScopeWrapper.attachWrapper(otScope);
72-
return otScope;
73-
}
7461
if (scope == noopScope()) {
7562
return noopScopeWrapper;
7663
}

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

+2-20
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,18 @@
11
import datadog.trace.agent.test.AgentTestRunner
22
import datadog.trace.api.DDSpanId
33
import datadog.trace.api.DDTraceId
4-
import datadog.trace.api.sampling.PrioritySampling
54
import datadog.trace.api.datastreams.NoopPathwayContext
5+
import datadog.trace.api.sampling.PrioritySampling
66
import datadog.trace.core.DDSpan
77
import datadog.trace.core.DDSpanContext
88
import datadog.trace.core.PendingTrace
99
import datadog.trace.core.propagation.PropagationTags
10-
import datadog.trace.core.scopemanager.ContinuableScopeManager
1110
import datadog.trace.instrumentation.opentracing.DefaultLogHandler
1211
import datadog.trace.instrumentation.opentracing32.TypeConverter
1312

1413
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
15-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1614
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
15+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1716

1817
class TypeConverterTest extends AgentTestRunner {
1918
TypeConverter typeConverter = new TypeConverter(new DefaultLogHandler())
@@ -51,23 +50,6 @@ class TypeConverterTest extends AgentTestRunner {
5150
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
5251
}
5352

54-
def "should avoid extra allocation for a scope wrapper"() {
55-
def scopeManager = new ContinuableScopeManager(0, false)
56-
def context = createTestSpanContext()
57-
def span1 = new DDSpan("test", 0, context, null)
58-
def span2 = new DDSpan("test", 0, context, null)
59-
def scope1 = scopeManager.activateManualSpan(span1)
60-
def scope2 = scopeManager.activateManualSpan(span2)
61-
expect:
62-
// return the same wrapper for the same scope
63-
typeConverter.toScope(scope1, true) is typeConverter.toScope(scope1, true)
64-
typeConverter.toScope(scope1, false) is typeConverter.toScope(scope1, false)
65-
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope1, false))
66-
!typeConverter.toScope(scope1, false).is(typeConverter.toScope(scope1, true))
67-
// return distinct wrapper for another context
68-
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope2, true))
69-
}
70-
7153
def createTestSpanContext() {
7254
def trace = Stub(PendingTrace)
7355
return new DDSpanContext(

‎dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java

+1-18
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,8 @@
66
import datadog.trace.api.scopemanager.ScopeListener;
77
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
88
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
9-
import datadog.trace.bootstrap.instrumentation.api.AttachableWrapper;
10-
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
11-
import javax.annotation.Nonnull;
129

13-
class ContinuableScope implements AgentScope, AttachableWrapper {
10+
class ContinuableScope implements AgentScope {
1411

1512
// different sources of scopes
1613
static final byte INSTRUMENTATION = 0;
@@ -34,10 +31,6 @@ class ContinuableScope implements AgentScope, AttachableWrapper {
3431

3532
private short referenceCount = 1;
3633

37-
private volatile Object wrapper;
38-
private static final AtomicReferenceFieldUpdater<ContinuableScope, Object> WRAPPER_FIELD_UPDATER =
39-
AtomicReferenceFieldUpdater.newUpdater(ContinuableScope.class, Object.class, "wrapper");
40-
4134
private final Stateful scopeState;
4235

4336
ContinuableScope(
@@ -207,14 +200,4 @@ public final void afterActivated() {
207200
public byte source() {
208201
return (byte) (source & 0x7F);
209202
}
210-
211-
@Override
212-
public void attachWrapper(@Nonnull Object wrapper) {
213-
WRAPPER_FIELD_UPDATER.set(this, wrapper);
214-
}
215-
216-
@Override
217-
public Object getWrapper() {
218-
return WRAPPER_FIELD_UPDATER.get(this);
219-
}
220203
}

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

-13
Original file line numberDiff line numberDiff line change
@@ -61,19 +61,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) {
6161
if (scope == null) {
6262
return null;
6363
}
64-
if (scope instanceof AttachableWrapper) {
65-
AttachableWrapper attachableScopeWrapper = (AttachableWrapper) scope;
66-
Object wrapper = attachableScopeWrapper.getWrapper();
67-
if (wrapper instanceof OTScopeManager.OTScope) {
68-
OTScopeManager.OTScope attachedScopeWrapper = (OTScopeManager.OTScope) wrapper;
69-
if (attachedScopeWrapper.isFinishSpanOnClose() == finishSpanOnClose) {
70-
return (Scope) wrapper;
71-
}
72-
}
73-
Scope otScope = new OTScopeManager.OTScope(scope, finishSpanOnClose, this);
74-
attachableScopeWrapper.attachWrapper(otScope);
75-
return otScope;
76-
}
7764
if (scope == noopScope()) {
7865
return noopScopeWrapper;
7966
}

‎dd-trace-ot/src/test/groovy/datadog/opentracing/TypeConverterTest.groovy

+2-20
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,18 @@ package datadog.opentracing
22

33
import datadog.trace.api.DDSpanId
44
import datadog.trace.api.DDTraceId
5-
import datadog.trace.api.sampling.PrioritySampling
65
import datadog.trace.api.datastreams.NoopPathwayContext
6+
import datadog.trace.api.sampling.PrioritySampling
77
import datadog.trace.core.CoreTracer
88
import datadog.trace.core.DDSpan
99
import datadog.trace.core.DDSpanContext
1010
import datadog.trace.core.PendingTrace
1111
import datadog.trace.core.propagation.PropagationTags
12-
import datadog.trace.core.scopemanager.ContinuableScopeManager
1312
import datadog.trace.test.util.DDSpecification
1413

1514
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
16-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1715
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
16+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1817

1918
class TypeConverterTest extends DDSpecification {
2019
TypeConverter typeConverter = new TypeConverter(new DefaultLogHandler())
@@ -52,23 +51,6 @@ class TypeConverterTest extends DDSpecification {
5251
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
5352
}
5453

55-
def "should avoid extra allocation for a scope wrapper"() {
56-
def scopeManager = new ContinuableScopeManager(0, false)
57-
def context = createTestSpanContext()
58-
def span1 = new DDSpan("test", 0, context, null)
59-
def span2 = new DDSpan("test", 0, context, null)
60-
def scope1 = scopeManager.activateManualSpan(span1)
61-
def scope2 = scopeManager.activateManualSpan(span2)
62-
expect:
63-
// return the same wrapper for the same scope
64-
typeConverter.toScope(scope1, true) is typeConverter.toScope(scope1, true)
65-
typeConverter.toScope(scope1, false) is typeConverter.toScope(scope1, false)
66-
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope1, false))
67-
!typeConverter.toScope(scope1, false).is(typeConverter.toScope(scope1, true))
68-
// return distinct wrapper for another context
69-
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope2, true))
70-
}
71-
7254
def createTestSpanContext() {
7355
def tracer = Stub(CoreTracer)
7456
def trace = Stub(PendingTrace)

0 commit comments

Comments
 (0)