10000 Remove AttachableWrapper aspect from scopes · DataDog/dd-trace-java@d4278e3 · GitHub
[go: up one dir, main page]
Skip to content

Commit d4278e3

Browse files
mccullsmcculls
committed
Remove AttachableWrapper aspect from scopes
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 8122037 commit d4278e3

File tree

9 files changed

+9
-148
lines changed

9 files changed

+9
-148
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 & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +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
6-
import datadog.trace.bootstrap.instrumentation.api.ScopeSource
5+
import datadog.trace.api.sampling.PrioritySampling
76
import datadog.trace.core.DDSpan
87
import datadog.trace.core.DDSpanContext
98
import datadog.trace.core.PendingTrace
109
import datadog.trace.core.propagation.PropagationTags
11-
import datadog.trace.core.scopemanager.ContinuableScopeManager
1210
import datadog.trace.instrumentation.opentelemetry.TypeConverter
1311

1412
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
15-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1613
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
14+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1715

1816
class TypeConverterTest extends AgentTestRunner {
1917
TypeConverter typeConverter = new TypeConverter()
@@ -47,20 +45,6 @@ class TypeConverterTest extends AgentTestRunner {
4745
typeConverter.toScope(noopScope) is typeConverter.toScope(noopScope)
4846
}
4947

50-
def "should avoid extra allocation for a scope wrapper"() {
51-
def scopeManager = new ContinuableScopeManager(0, false)
52-
def context = createTestSpanContext()
53-
def span1 = new DDSpan("test", 0, context, null)
54-
def span2 = new DDSpan("test", 0, context, null)
55-
def scope1 = scopeManager.activate(span1, ScopeSource.MANUAL)
56-
def scope2 = scopeManager.activate(span2, ScopeSource.MANUAL)
57-
expect:
58-
// return the same wrapper for the same scope
59-
typeConverter.toScope(scope1) is typeConverter.toScope(scope1)
60-
// return distinct wrapper for another context
61-
!typeConverter.toScope(scope1).is(typeConverter.toScope(scope2))
62-
}
63-
6448
def createTestSpanContext() {
6549
def trace = Stub(PendingTrace)
6650
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 & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +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
6-
import datadog.trace.bootstrap.instrumentation.api.ScopeSource
5+
import datadog.trace.api.sampling.PrioritySampling
76
import datadog.trace.core.DDSpan
87
import datadog.trace.core.DDSpanContext
98
import datadog.trace.core.PendingTrace
109
import datadog.trace.core.propagation.PropagationTags
11-
import datadog.trace.core.scopemanager.ContinuableScopeManager
1210
import datadog.trace.instrumentation.opentracing.DefaultLogHandler
1311
import datadog.trace.instrumentation.opentracing31.TypeConverter
1412

1513
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
16-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1714
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
15+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1816

1917
class TypeConverterTest extends AgentTestRunner {
2018
TypeConverter typeConverter = new TypeConverter(new DefaultLogHandler())
@@ -52,23 +50,6 @@ class TypeConverterTest extends AgentTestRunner {
5250
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
5351
}
5452

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.activate(span1, ScopeSource.MANUAL)
61-
def scope2 = scopeManager.activate(span2, ScopeSource.MANUAL)
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-
7253
def createTestSpanContext() {
7354
def trace = Stub(PendingTrace)
7455
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 & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +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
6-
import datadog.trace.bootstrap.instrumentation.api.ScopeSource
5+
import datadog.trace.api.sampling.PrioritySampling
76
import datadog.trace.core.DDSpan
87
import datadog.trace.core.DDSpanContext
98
import datadog.trace.core.PendingTrace
109
import datadog.trace.core.propagation.PropagationTags
11-
import datadog.trace.core.scopemanager.ContinuableScopeManager
1210
import datadog.trace.instrumentation.opentracing.DefaultLogHandler
1311
import datadog.trace.instrumentation.opentracing32.TypeConverter
1412

1513
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
16-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1714
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
15+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1816

1917
class TypeConverterTest extends AgentTestRunner {
2018
TypeConverter typeConverter = new TypeConverter(new DefaultLogHandler())
@@ -52,23 +50,6 @@ class TypeConverterTest extends AgentTestRunner {
5250
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
5351
}
5452

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.activate(span1, ScopeSource.MANUAL)
61-
def scope2 = scopeManager.activate(span2, ScopeSource.MANUAL)
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-
7253
def createTestSpanContext() {
7354
def trace = Stub(PendingTrace)
7455
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,12 +5,9 @@
55
import datadog.trace.api.scopemanager.ScopeListener;
66
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
77
import datadog 10000 .trace.bootstrap.instrumentation.api.AgentSpan;
8-
import datadog.trace.bootstrap.instrumentation.api.AttachableWrapper;
98
import datadog.trace.bootstrap.instrumentation.api.ScopeSource;
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
private final ContinuableScopeManager scopeManager;
1512

1613
final AgentSpan span; // package-private so scopeManager can access it directly
@@ -27,10 +24,6 @@ class ContinuableScope implements AgentScope, AttachableWrapper {
2724

2825
private short referenceCount = 1;
2926

30-
private volatile Object wrapper;
31-
private static final AtomicReferenceFieldUpdater<ContinuableScope, Object> WRAPPER_FIELD_UPDATER =
32-
AtomicReferenceFieldUpdater.newUpdater(ContinuableScope.class, Object.class, "wrapper");
33-
3427
private final Stateful scopeState;
3528

3629
ContinuableScope(
@@ -188,14 +181,4 @@ public final void afterActivated() {
188181
public byte source() {
189182
return (byte) (source & 0x7F);
190183
}
191-
192-
@Override
193-
public void attachWrapper(@Nonnull Object wrapper) {
194-
WRAPPER_FIELD_UPDATER.set(this, wrapper);
195-
}
196-
197-
@Override
198-
public Object getWrapper() {
199-
return WRAPPER_FIELD_UPDATER.get(this);
200-
}
201184
}

‎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 & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +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
7-
import datadog.trace.bootstrap.instrumentation.api.ScopeSource
6+
import datadog.trace.api.sampling.PrioritySampling
87
import datadog.trace.core.CoreTracer
98
import datadog.trace.core.DDSpan
109
import datadog.trace.core.DDSpanContext
1110
import datadog.trace.core.PendingTrace
1211
import datadog.trace.core.propagation.PropagationTags
13-
import datadog.trace.core.scopemanager.ContinuableScopeManager
1412
import datadog.trace.test.util.DDSpecification
1513

1614
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
17-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1815
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
16+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1917

2018
class TypeConverterTest extends DDSpecification {
2119
TypeConverter typeConverter = new TypeConverter(new DefaultLogHandler())
@@ -53,23 +51,6 @@ class TypeConverterTest extends DDSpecification {
5351
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
5452
}
5553

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

0 commit comments

Comments
 (0)
0