8000 Remove AttachableWrapper aspect from scopes (#8534) · DataDog/dd-trace-java@e6fc8c1 · GitHub
[go: up one dir, main page]
Skip to content

Commit e6fc8c1

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 d0b9b46 commit e6fc8c1

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

Lines changed: 0 additions & 10 deletions
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

Lines changed: 2 additions & 17 deletions
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

Lines changed: 0 additions & 13 deletions
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

Lines changed: 2 additions & 20 deletions
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

Lines changed: 0 additions & 13 deletions
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

Lines changed: 2 additions & 20 deletions
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

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,8 @@
55
import datadog.trace.api.scopemanager.ScopeListener;
66
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
77
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
8-
import datadog.trace.bootstrap.instrumentation.api.AttachableWrapper;
9-
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
10-
import javax.annotation.Nonnull;
118

12-
class ContinuableScope implements AgentScope, AttachableWrapper {
9+
class ContinuableScope implements AgentScope {
1310

1411
// different sources of scopes
1512
static final byte INSTRUMENTATION = 0;
@@ -32,10 +29,6 @@ class ContinuableScope implements AgentScope, AttachableWrapper {
3229

3330
private short referenceCount = 1;
3431

35-
private volatile Object wrapper;
36-
private static final AtomicReferenceFieldUpdater<ContinuableScope, Object> WRAPPER_FIELD_UPDATER =
37-
AtomicReferenceFieldUpdater.newUpdater(ContinuableScope.class, Object.class, "wrapper");
38-
3932
private final Stateful scopeState;
4033

4134
ContinuableScope(
@@ -192,14 +185,4 @@ public final void afterActivated() {
192185
public byte source() {
193186
return (byte) (source & 0x7F);
194187
}
195-
196-
@Override
197-
public void attachWrapper(@Nonnull Object wrapper) {
198-
WRAPPER_FIELD_UPDATER.set(this, wrapper);
199-
}
200-
201-
@Override
202-
public Object getWrapper() {
203-
return WRAPPER_FIELD_UPDATER.get(this);
204-
}
205188
}

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

Lines changed: 0 additions & 13 deletions
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

Lines changed: 2 additions & 20 deletions
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)
0