Skip to content

Remove AttachableWrapper aspect from scopes #8647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 2, 2025
Merged

Remove AttachableWrapper aspect from scopes #8647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions ...elemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/TypeConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,6 @@ public Scope toScope(final AgentScope scope) {
if (scope == null) {
return null;
}
if (scope instanceof AttachableWrapper) {
AttachableWrapper attachableScopeWrapper = (AttachableWrapper) scope;
Object wrapper = attachableScopeWrapper.getWrapper();
if (wrapper instanceof Scope) {
return (Scope) wrapper;
}
Scope otScope = new OtelScope(scope);
attachableScopeWrapper.attachWrapper(otScope);
return otScope;
}
if (scope == noopScope()) {
return noopScopeWrapper;
}
Expand Down
19 changes: 2 additions & 17 deletions .../instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.api.DDSpanId
import datadog.trace.api.DDTraceId
import datadog.trace.api.sampling.PrioritySampling
import datadog.trace.api.datastreams.NoopPathwayContext
import datadog.trace.api.sampling.PrioritySampling
import datadog.trace.core.DDSpan
import datadog.trace.core.DDSpanContext
import datadog.trace.core.PendingTrace
import datadog.trace.core.propagation.PropagationTags
import datadog.trace.core.scopemanager.ContinuableScopeManager
import datadog.trace.instrumentation.opentelemetry.TypeConverter

import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext

class TypeConverterTest extends AgentTestRunner {
TypeConverter typeConverter = new TypeConverter()
Expand Down Expand Up @@ -46,20 +45,6 @@ class TypeConverterTest extends AgentTestRunner {
typeConverter.toScope(noopScope) is typeConverter.toScope(noopScope)
}

def "should avoid extra allocation for a scope wrapper"() {
def scopeManager = new ContinuableScopeManager(0, false)
def context = createTestSpanContext()
def span1 = new DDSpan("test", 0, context, null)
def span2 = new DDSpan("test", 0, context, null)
def scope1 = scopeManager.activateManualSpan(span1)
def scope2 = scopeManager.activateManualSpan(span2)
expect:
// return the same wrapper for the same scope
typeConverter.toScope(scope1) is typeConverter.toScope(scope1)
// return distinct wrapper for another context
!typeConverter.toScope(scope1).is(typeConverter.toScope(scope2))
}

def createTestSpanContext() {
def trace = Stub(PendingTrace)
return new DDSpanContext(
Expand Down
13 changes: 0 additions & 13 deletions ...ing/api-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/TypeConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) {
if (scope == null) {
return null;
}
if (scope instanceof AttachableWrapper) {
AttachableWrapper attachableScopeWrapper = (AttachableWrapper) scope;
Object wrapper = attachableScopeWrapper.getWrapper();
if (wrapper instanceof OTScopeManager.OTScope) {
OTScopeManager.OTScope attachedScopeWrapper = (OTScopeManager.OTScope) wrapper;
if (attachedScopeWrapper.isFinishSpanOnClose() == finishSpanOnClose) {
return (Scope) wrapper;
}
}
Scope otScope = new OTScopeManager.OTScope(scope, finishSpanOnClose, this);
attachableScopeWrapper.attachWrapper(otScope);
return otScope;
}
if (scope == noopScope()) {
return noopScopeWrapper;
}
Expand Down
22 changes: 2 additions & 20 deletions dd-java-agent/instrumentation/opentracing/api-0.31/src/test/groovy/TypeConverterTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.api.DDSpanId
import datadog.trace.api.DDTraceId
import datadog.trace.api.sampling.PrioritySampling
import datadog.trace.api.datastreams.NoopPathwayContext
import datadog.trace.api.sampling.PrioritySampling
import datadog.trace.core.DDSpan
import datadog.trace.core.DDSpanContext
import datadog.trace.core.PendingTrace
import datadog.trace.core.propagation.PropagationTags
import datadog.trace.core.scopemanager.ContinuableScopeManager
import datadog.trace.instrumentation.opentracing.DefaultLogHandler
import datadog.trace.instrumentation.opentracing31.TypeConverter

import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext

class TypeConverterTest extends AgentTestRunner {
TypeConverter typeConverter = new TypeConverter(new DefaultLogHandler())
Expand Down Expand Up @@ -51,23 +50,6 @@ class TypeConverterTest extends AgentTestRunner {
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
}

def "should avoid extra allocation for a scope wrapper"() {
def scopeManager = new ContinuableScopeManager(0, false)
def context = createTestSpanContext()
def span1 = new DDSpan("test", 0, context, null)
def span2 = new DDSpan("test", 0, context, null)
def scope1 = scopeManager.activateManualSpan(span1)
def scope2 = scopeManager.activateManualSpan(span2)
expect:
// return the same wrapper for the same scope
typeConverter.toScope(scope1, true) is typeConverter.toScope(scope1, true)
typeConverter.toScope(scope1, false) is typeConverter.toScope(scope1, false)
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope1, false))
!typeConverter.toScope(scope1, false).is(typeConverter.toScope(scope1, true))
// return distinct wrapper for another context
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope2, true))
}

def createTestSpanContext() {
def trace = Stub(PendingTrace)
return new DDSpanContext(
Expand Down
13 changes: 0 additions & 13 deletions ...ing/api-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/TypeConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) {
if (scope == null) {
return null;
}
if (scope instanceof AttachableWrapper) {
AttachableWrapper attachableScopeWrapper = (AttachableWrapper) scope;
Object wrapper = attachableScopeWrapper.getWrapper();
if (wrapper instanceof OTScopeManager.OTScope) {
OTScopeManager.OTScope attachedScopeWrapper = (OTScopeManager.OTScope) wrapper;
if (attachedScopeWrapper.isFinishSpanOnClose() == finishSpanOnClose) {
return (Scope) wrapper;
}
}
Scope otScope = new OTScopeManager.OTScope(scope, finishSpanOnClose, this);
attachableScopeWrapper.attachWrapper(otScope);
return otScope;
}
if (scope == noopScope()) {
return noopScopeWrapper;
}
Expand Down
22 changes: 2 additions & 20 deletions dd-java-agent/instrumentation/opentracing/api-0.32/src/test/groovy/TypeConverterTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.api.DDSpanId
import datadog.trace.api.DDTraceId
import datadog.trace.api.sampling.PrioritySampling
import datadog.trace.api.datastreams.NoopPathwayContext
import datadog.trace.api.sampling.PrioritySampling
import datadog.trace.core.DDSpan
import datadog.trace.core.DDSpanContext
import datadog.trace.core.PendingTrace
import datadog.trace.core.propagation.PropagationTags
import datadog.trace.core.scopemanager.ContinuableScopeManager
import datadog.trace.instrumentation.opentracing.DefaultLogHandler
import datadog.trace.instrumentation.opentracing32.TypeConverter

import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext

class TypeConverterTest extends AgentTestRunner {
TypeConverter typeConverter = new TypeConverter(new DefaultLogHandler())
Expand Down Expand Up @@ -51,23 +50,6 @@ class TypeConverterTest extends AgentTestRunner {
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
}

def "should avoid extra allocation for a scope wrapper"() {
def scopeManager = new ContinuableScopeManager(0, false)
def context = createTestSpanContext()
def span1 = new DDSpan("test", 0, context, null)
def span2 = new DDSpan("test", 0, context, null)
def scope1 = scopeManager.activateManualSpan(span1)
def scope2 = scopeManager.activateManualSpan(span2)
expect:
// return the same wrapper for the same scope
typeConverter.toScope(scope1, true) is typeConverter.toScope(scope1, true)
typeConverter.toScope(scope1, false) is typeConverter.toScope(scope1, false)
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope1, false))
!typeConverter.toScope(scope1, false).is(typeConverter.toScope(scope1, true))
// return distinct wrapper for another context
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope2, true))
}

def createTestSpanContext() {
def trace = Stub(PendingTrace)
return new DDSpanContext(
Expand Down
19 changes: 1 addition & 18 deletions dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,8 @@
import datadog.trace.api.scopemanager.ScopeListener;
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.AttachableWrapper;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
import javax.annotation.Nonnull;

class ContinuableScope implements AgentScope, AttachableWrapper {
class ContinuableScope implements AgentScope {

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

private short referenceCount = 1;

private volatile Object wrapper;
private static final AtomicReferenceFieldUpdater<ContinuableScope, Object> WRAPPER_FIELD_UPDATER =
AtomicReferenceFieldUpdater.newUpdater(ContinuableScope.class, Object.class, "wrapper");

private final Stateful scopeState;

ContinuableScope(
Expand Down Expand Up @@ -207,14 +200,4 @@ public final void afterActivated() {
public byte source() {
return (byte) (source & 0x7F);
}

@Override
public void attachWrapper(@Nonnull Object wrapper) {
WRAPPER_FIELD_UPDATER.set(this, wrapper);
}

@Override
public Object getWrapper() {
return WRAPPER_FIELD_UPDATER.get(this);
}
}
13 changes: 0 additions & 13 deletions dd-trace-ot/src/main/java/datadog/opentracing/TypeConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) {
if (scope == null) {
return null;
}
if (scope instanceof AttachableWrapper) {
AttachableWrapper attachableScopeWrapper = (AttachableWrapper) scope;
Object wrapper = attachableScopeWrapper.getWrapper();
if (wrapper instanceof OTScopeManager.OTScope) {
OTScopeManager.OTScope attachedScopeWrapper = (OTScopeManager.OTScope) wrapper;
if (attachedScopeWrapper.isFinishSpanOnClose() == finishSpanOnClose) {
return (Scope) wrapper;
}
}
Scope otScope = new OTScopeManager.OTScope(scope, finishSpanOnClose, this);
attachableScopeWrapper.attachWrapper(otScope);
return otScope;
}
if (scope == noopScope()) {
return noopScopeWrapper;
}
Expand Down
22 changes: 2 additions & 20 deletions dd-trace-ot/src/test/groovy/datadog/opentracing/TypeConverterTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,18 @@ package datadog.opentracing

import datadog.trace.api.DDSpanId
import datadog.trace.api.DDTraceId
import datadog.trace.api.sampling.PrioritySampling
import datadog.trace.api.datastreams.NoopPathwayContext
import datadog.trace.api.sampling.PrioritySampling
import datadog.trace.core.CoreTracer
import datadog.trace.core.DDSpan
import datadog.trace.core.DDSpanContext
import datadog.trace.core.PendingTrace
import datadog.trace.core.propagation.PropagationTags
import datadog.trace.core.scopemanager.ContinuableScopeManager
import datadog.trace.test.util.DDSpecification

import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext

class TypeConverterTest extends DDSpecification {
TypeConverter typeConverter = new TypeConverter(new DefaultLogHandler())
Expand Down Expand Up @@ -52,23 +51,6 @@ class TypeConverterTest extends DDSpecification {
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
}

def "should avoid extra allocation for a scope wrapper"() {
def scopeManager = new ContinuableScopeManager(0, false)
def context = createTestSpanContext()
def span1 = new DDSpan("test", 0, context, null)
def span2 = new DDSpan("test", 0, context, null)
def scope1 = scopeManager.activateManualSpan(span1)
def scope2 = scopeManager.activateManualSpan(span2)
expect:
// return the same wrapper for the same scope
typeConverter.toScope(scope1, true) is typeConverter.toScope(scope1, true)
typeConverter.toScope(scope1, false) is typeConverter.toScope(scope1, false)
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope1, false))
!typeConverter.toScope(scope1, false).is(typeConverter.toScope(scope1, true))
// return distinct wrapper for another context
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope2, true))
}

def createTestSpanContext() {
def tracer = Stub(CoreTracer)
def trace = Stub(PendingTrace)
Expand Down