Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import datadog.instrument.utils.ClassLoaderValue;
import datadog.metrics.api.statsd.StatsDClientManager;
import datadog.trace.api.Config;
import datadog.trace.api.InstrumenterConfig;
import datadog.trace.api.Platform;
import datadog.trace.api.WithGlobalTracer;
import datadog.trace.api.appsec.AppSecEventTracker;
Expand Down Expand Up @@ -337,6 +338,10 @@ public static void start(
StaticEventLogger.end("crashtracking");
}

if (InstrumenterConfig.get().isLegacyContextManagerEnabled()) {
AgentTracer.installLegacyContextManager();
}
Comment thread
mcculls marked this conversation as resolved.

startDatadogAgent(initTelemetry, inst);

final EnumSet<Library> libraries = detectLibraries(log);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import datadog.trace.agent.tooling.MeterInstaller;
import datadog.trace.agent.tooling.ProfilerInstaller;
import datadog.trace.agent.tooling.TracerInstaller;
import datadog.trace.api.InstrumenterConfig;
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -18,6 +20,10 @@ public final class TracerActivation {

public static void activate() {
try {
// Bind legacy context manager (if enabled)
if (InstrumenterConfig.get().isLegacyContextManagerEnabled()) {
AgentTracer.installLegacyContextManager();
}
// Initialize meter
MeterInstaller.installMeter();
// Initialize tracer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import com.datadog.debugger.sink.DebuggerSink
import com.datadog.debugger.sink.ProbeStatusSink
import com.google.common.collect.Sets
import datadog.communication.ddagent.DDAgentFeaturesDiscovery
import datadog.context.ContextManager
import datadog.instrument.classinject.ClassInjector
import datadog.metrics.agent.AgentMeter
import datadog.metrics.api.Monitoring
Expand All @@ -43,6 +44,7 @@ import datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers
import datadog.trace.agent.tooling.bytebuddy.matcher.GlobalIgnores
import datadog.trace.api.Config
import datadog.trace.api.IdGenerationStrategy
import datadog.trace.api.InstrumenterConfig
import datadog.trace.api.Pair
import datadog.trace.api.ProcessTags
import datadog.trace.api.TraceConfig
Expand Down Expand Up @@ -351,18 +353,22 @@ abstract class InstrumentationSpecification extends DDSpecification implements A
void setupSpec() {
InstrumentationErrors.resetErrors()

AgentMeter.registerIfAbsent(
STATS_D_CLIENT,
new MonitoringImpl(STATS_D_CLIENT, 10, TimeUnit.SECONDS),
DDSketchHistograms.FACTORY
)

// If this fails, it's likely the result of another test loading Config before it can be
// injected into the bootstrap classpath. If one test extends AgentTestRunner in a module, all tests must extend
assert Config.getClassLoader() == null: "Config must load on the bootstrap classpath."

configurePreAgent()

if (InstrumenterConfig.get().isLegacyContextManagerEnabled()) {
AgentTracer.installLegacyContextManager()
Comment thread
mcculls marked this conversation as resolved.
}
Comment thread
mcculls marked this conversation as resolved.

AgentMeter.registerIfAbsent(
STATS_D_CLIENT,
new MonitoringImpl(STATS_D_CLIENT, 10, TimeUnit.SECONDS),
DDSketchHistograms.FACTORY
)

TEST_DATA_STREAMS_WRITER = new RecordingDatastreamsPayloadWriter()
DDAgentFeaturesDiscovery features = new MockFeaturesDiscovery(true)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
import datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers;
import datadog.trace.api.Config;
import datadog.trace.api.IdGenerationStrategy;
import datadog.trace.api.InstrumenterConfig;
import datadog.trace.bootstrap.InstrumentationErrors;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI;
import datadog.trace.common.writer.ListWriter;
import datadog.trace.core.CoreTracer;
Expand Down Expand Up @@ -75,6 +77,10 @@ static void initAll() {
// injected into the bootstrap classpath.
assertNull(Config.class.getClassLoader(), "Config must load on the bootstrap classpath.");

if (InstrumenterConfig.get().isLegacyContextManagerEnabled()) {
AgentTracer.installLegacyContextManager();
}

Comment on lines +80 to +83

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❔ question: ‏Is this supposed to stay? I would rather not query Config/InstrumentationConfig at this level. I might need to think about how to use some JUnit extension mechanism.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically, I would recommend creating a dedicated temporary extension, based on the one like AllowContextTestingExtension, but near to the test base, and register it after the current extensions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How else can we test the feature-flag?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this needs to be after any config mutation - for forked JUnit tests are we setting config overrides outside the process or in an extension?

// Create shared test writer and tracer
writer = new ListWriter();
CoreTracer coreTracer =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider;
import software.amazon.awssdk.core.SdkBytes;
Expand Down Expand Up @@ -529,6 +530,7 @@ boolean isDataStreamsEnabled() {
}
}

@Disabled("TODO(APMLP-829): non-legacy ContextManager migration still in-progress")
@WithConfig(key = GeneralConfig.DATA_STREAMS_ENABLED, value = "false")
@WithConfig(key = TraceInstrumentationConfig.LEGACY_CONTEXT_MANAGER_ENABLED, value = "false")
class SqsClientV0ContextSwapForkedTest extends SqsClientV0TestBase {}
Expand Down Expand Up @@ -756,6 +758,7 @@ class SqsClientV0ReceiveIterationTest extends SqsClientV0ReceiveIterationTestBas
@WithConfig(key = GeneralConfig.DATA_STREAMS_ENABLED, value = "true")
class SqsClientV0DataStreamsReceiveIterationTest extends SqsClientV0ReceiveIterationTestBase {}

@Disabled("TODO(APMLP-829): non-legacy ContextManager migration still in-progress")
@WithConfig(key = GeneralConfig.DATA_STREAMS_ENABLED, value = "false")
@WithConfig(key = TraceInstrumentationConfig.LEGACY_CONTEXT_MANAGER_ENABLED, value = "false")
class SqsClientV0ContextSwapReceiveIterationForkedTest extends SqsClientV0ReceiveIterationTestBase {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import javax.jms.TopicSession
import jms10mock.Jms10ConnectionFactory
import org.apache.activemq.command.ActiveMQTextMessage
import org.apache.activemq.junit.EmbeddedActiveMQBroker
import spock.lang.Ignore
import spock.lang.Shared

abstract class JMS1Test extends VersionedNamingTestBase {
Expand Down Expand Up @@ -1061,6 +1062,7 @@ class JMS1V0Test extends JMS1Test {
}
}

@Ignore("TODO(APMLP-829): non-legacy ContextManager migration still in-progress")
class JMSContextSwapForkedTest extends JMS1V0Test {
@Override
protected void configurePreAgent() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import org.springframework.kafka.listener.MessageListener
import org.springframework.kafka.test.rule.KafkaEmbedded
import org.springframework.kafka.test.utils.ContainerTestUtils
import org.springframework.kafka.test.utils.KafkaTestUtils
import spock.lang.Ignore
import spock.lang.Shared

import java.util.concurrent.ExecutionException
Expand Down Expand Up @@ -1545,6 +1546,7 @@ class KafkaClientDataStreamsDisabledForkedTest extends KafkaClientTestBase {
}
}

@Ignore("TODO(APMLP-829): non-legacy ContextManager migration still in-progress")
class KafkaClientContextSwapForkedTest extends KafkaClientV0ForkedTest {
void configurePreAgent() {
super.configurePreAgent()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.isAsyncPropagationEnabled

import spock.lang.Ignore

abstract class KafkaClientTestBase extends VersionedNamingTestBase {
static final SHARED_TOPIC = "shared.topic"

Expand Down Expand Up @@ -1211,6 +1213,7 @@ class KafkaClientDataStreamsDisabledForkedTest extends KafkaClientTestBase {
}
}
@Ignore("TODO(APMLP-829): non-legacy ContextManager migration still in-progress")
class KafkaClientContextSwapForkedTest extends KafkaClientV0ForkedTest {
void configurePreAgent() {
super.configurePreAgent()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,34 @@ public void setup() {
}

static ContextManager createManager(String type) {
return "Continuable".equals(type)
? new ContinuableScopeManager(0, false)
: ThreadLocalContextManager.INSTANCE;
if ("Continuable".equals(type)) {
ContinuableScopeManager csm = new ContinuableScopeManager(0, false);
return new ContextManager() {
@Override
public Context current() {
return csm.current();
}

@Override
public ContextScope attach(Context ctx) {
return csm.attach(ctx);
}

@Override
public Context swap(Context ctx) {
return csm.swap(ctx);
}

@Override
public ContextContinuation capture(Context ctx) {
return csm.capture(ctx);
}

@Override
public void addListener(ContextListener l) {}
};
}
return ThreadLocalContextManager.INSTANCE;
}

static Context[] createContexts() {
Expand Down
23 changes: 23 additions & 0 deletions dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
import datadog.communication.ddagent.DDAgentFeaturesDiscovery;
import datadog.communication.ddagent.ExternalAgentLauncher;
import datadog.communication.ddagent.SharedCommunicationObjects;
import datadog.context.Context;
import datadog.context.ContextContinuation;
import datadog.context.ContextScope;
import datadog.context.propagation.Propagators;
import datadog.environment.ThreadSupport;
import datadog.logging.RatelimitedLogger;
Expand Down Expand Up @@ -2465,4 +2468,24 @@ static TagMap withTracerTags(
}
return result.freeze();
}

@Override
public Context currentContext() {
return scopeManager.current();
}

@Override
public ContextScope attach(Context context) {
return scopeManager.attach(context);
}

@Override
public Context swap(Context context) {
return scopeManager.swap(context);
}

@Override
public ContextContinuation capture(Context context) {
return scopeManager.capture(context);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

import datadog.context.Context;
import datadog.context.ContextContinuation;
import datadog.context.ContextListener;
import datadog.context.ContextManager;
import datadog.context.ContextScope;
import datadog.logging.RatelimitedLogger;
import datadog.trace.api.Config;
Expand Down Expand Up @@ -47,7 +45,7 @@
* from being reported even if all related spans are finished. It also delegates to other
* ScopeInterceptors to provide additional functionality.
*/
public final class ContinuableScopeManager implements ContextManager {
public final class ContinuableScopeManager {

static final Logger log = LoggerFactory.getLogger(ContinuableScopeManager.class);
static final RatelimitedLogger ratelimitedLog = new RatelimitedLogger(log, 1, MINUTES);
Expand Down Expand Up @@ -95,8 +93,6 @@ public ContinuableScopeManager(
this.profilingContextIntegration = profilingContextIntegration;
this.profilingEnabled =
!(profilingContextIntegration instanceof ProfilingContextIntegration.NoOp);

ContextManager.register(this);
}

public AgentScope activateSpan(final AgentSpan span) {
Expand Down Expand Up @@ -370,18 +366,15 @@ ScopeStack scopeStack() {
return this.tlsScopeStack.get();
}

@Override
public Context current() {
final ContinuableScope active = scopeStack().active();
return active == null ? Context.root() : active.context;
}

@Override
public ContextScope attach(@NonNull Context context) {
return activate(context);
}

@Override
public Context swap(@NonNull Context context) {
ScopeStack oldStack = tlsScopeStack.get();
ContinuableScope oldScope = oldStack.top;
Expand Down Expand Up @@ -412,7 +405,6 @@ public Context swap(@NonNull Context context) {
return new ScopeContext(oldStack);
}

@Override
public ContextContinuation capture(@NonNull Context context) {
// respect async propagation flag for Context.current().capture()
ContinuableScope activeScope = scopeStack().active();
Expand All @@ -431,12 +423,6 @@ public ContextContinuation capture(@NonNull Context context) {
return new ScopeContinuation(this, context, CONTEXT, traceCollector).register();
}

@Override
public void addListener(@NonNull ContextListener unused) {
// this new API is not expected to be used in legacy mode...
log.warn("Unexpected call to ContextManager.addListener(...)");
}

static final class ScopeStackThreadLocal extends ThreadLocal<ScopeStack> {

private final ProfilingContextIntegration profilingContextIntegration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopContinuation;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan;
import static datadog.trace.core.scopemanager.ScopeManagerTest.EVENT.ACTIVATE;
import static datadog.trace.core.scopemanager.ScopeManagerTest.EVENT.CLOSE;
import static datadog.trace.core.scopemanager.ScopeManagerForkedTest.EVENT.ACTIVATE;
import static datadog.trace.core.scopemanager.ScopeManagerForkedTest.EVENT.CLOSE;
import static datadog.trace.test.util.GCUtils.awaitGC;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -36,6 +36,7 @@
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.AgentTracer;
import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration;
import datadog.trace.common.writer.ListWriter;
import datadog.trace.context.TraceScope;
Expand All @@ -56,13 +57,14 @@
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.tabletest.junit.TableTest;

class ScopeManagerTest extends DDCoreJavaSpecification {
class ScopeManagerForkedTest extends DDCoreJavaSpecification {

enum EVENT {
ACTIVATE,
Expand All @@ -83,6 +85,11 @@ protected boolean useStrictTraceWrites() {
ProfilingContextIntegration profilingContext;
Stateful state;

@BeforeAll
static void installLegacyContextManager() {
AgentTracer.installLegacyContextManager();
}

@BeforeEach
void setup() {
state = mock(Stateful.class);
Expand All @@ -91,6 +98,7 @@ void setup() {
when(profilingContext.name()).thenReturn("mock");
writer = new ListWriter();
tracer = tracerBuilder().writer(writer).profilingContextIntegration(profilingContext).build();
AgentTracer.forceRegister(tracer);
scopeManager = ScopeManagerTestBridge.getScopeManager(tracer);
Comment thread
mcculls marked this conversation as resolved.
eventCountingListener = new EventCountingListener();
scopeManager.addScopeListener(eventCountingListener);
Expand Down
1 change: 1 addition & 0 deletions internal-api/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ extra["excludedClassesCoverage"] = listOf(
"datadog.trace.bootstrap.instrumentation.api.AgentSpan",
"datadog.trace.bootstrap.instrumentation.api.AgentSpanContext",
"datadog.trace.bootstrap.instrumentation.api.AgentTracer",
"datadog.trace.bootstrap.instrumentation.api.AgentTracer.LegacyContextManager",
"datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopAgentHistogram",
"datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopAgentTraceCollector",
"datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopTraceConfig",
Expand Down
Loading
Loading