From e38e5f2915b3e627f53dba8b0eea343905f0d92e Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Mon, 24 Apr 2023 14:23:05 +0100 Subject: [PATCH] Add initial DynamicConfig API and use it to maintain service mappings --- .../java/datadog/trace/core/CoreTracer.java | 17 ++--- .../datadog/trace/core/DDSpanContext.java | 2 +- .../java/datadog/trace/core/PendingTrace.java | 8 +++ .../writer/DDAgentWriterCombinedTest.groovy | 2 +- .../common/writer/DDAgentWriterTest.groovy | 2 +- .../writer/DDIntakeWriterCombinedTest.groovy | 2 +- .../common/writer/DDIntakeWriterTest.groovy | 2 +- .../writer/PayloadDispatcherTest.groovy | 4 +- .../datadog/trace/core/CoreTracerTest.groovy | 2 +- .../trace/core/PendingTraceBufferTest.groovy | 22 +++++-- .../trace/core/PendingTraceTest.groovy | 4 ++ .../opentracing/TypeConverterTest.groovy | 2 +- internal-api/build.gradle | 3 + .../java/datadog/trace/api/DynamicConfig.java | 62 +++++++++++++++++++ .../java/datadog/trace/api/TraceConfig.java | 9 +++ 15 files changed, 119 insertions(+), 24 deletions(-) create mode 100644 internal-api/src/main/java/datadog/trace/api/DynamicConfig.java create mode 100644 internal-api/src/main/java/datadog/trace/api/TraceConfig.java diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 7804b2422e2..174107fcf0e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -24,12 +24,14 @@ import datadog.trace.api.Config; import datadog.trace.api.DDSpanId; import datadog.trace.api.DDTraceId; +import datadog.trace.api.DynamicConfig; import datadog.trace.api.EndpointCheckpointer; import datadog.trace.api.EndpointCheckpointerHolder; import datadog.trace.api.EndpointTracker; import datadog.trace.api.IdGenerationStrategy; import datadog.trace.api.InstrumenterConfig; import datadog.trace.api.StatsDClient; +import datadog.trace.api.TraceConfig; import datadog.trace.api.TracePropagationStyle; import datadog.trace.api.config.GeneralConfig; import datadog.trace.api.experimental.DataStreamsCheckpointer; @@ -151,12 +153,12 @@ public static CoreTracerBuilder builder() { final MetricsAggregator metricsAggregator; + /** Maintains dynamic configuration associated with the tracer */ + private final DynamicConfig dynamicConfig; /** A set of tags that are added only to the application's root span */ private final Map localRootSpanTags; /** A set of tags that are added to every span */ private final Map defaultSpanTags; - /** A configured mapping of service names to update with new values */ - private final Map serviceNameMappings; /** number of spans in a pending trace before they get flushed */ private final int partialFlushMinSpans; @@ -205,6 +207,10 @@ public static CoreTracerBuilder builder() { private final PropagationTags.Factory propagationTagsFactory; + TraceConfig captureTraceConfig() { + return dynamicConfig.captureTraceConfig(); + } + PropagationTags.Factory getPropagationTagsFactory() { return propagationTagsFactory; } @@ -479,13 +485,13 @@ private CoreTracer( endpointCheckpointer = EndpointCheckpointerHolder.create(); this.serviceName = serviceName; + this.dynamicConfig = DynamicConfig.create().setServiceMapping(serviceNameMappings).apply(); this.sampler = sampler; this.injector = injector; this.extractor = extractor; this.logs128bTraceIdEnabled = InstrumenterConfig.get().isLogs128bTraceIdEnabled(); this.localRootSpanTags = localRootSpanTags; this.defaultSpanTags = defaultSpanTags; - this.serviceNameMappings = serviceNameMappings; this.partialFlushMinSpans = partialFlushMinSpans; this.idGenerationStrategy = null == idGenerationStrategy @@ -636,11 +642,6 @@ public PendingTrace createTrace(DDTraceId id) { return pendingTraceFactory.create(id); } - public String mapServiceName(String serviceName) { - String mapped = serviceNameMappings.get(serviceName); - return null == mapped ? serviceName : mapped; - } - /** * If an application is using a non-system classloader, that classloader should be registered * here. Due to the way Spring Boot structures its' executable jar, this might log some warnings. diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index c5c47f111c1..89d0653d3fa 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -340,7 +340,7 @@ public String getServiceName() { } public void setServiceName(final String serviceName) { - this.serviceName = trace.getTracer().mapServiceName(serviceName); + this.serviceName = trace.mapServiceName(serviceName); this.topLevel = isTopLevel(parentServiceName, this.serviceName); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/PendingTrace.java b/dd-trace-core/src/main/java/datadog/trace/core/PendingTrace.java index a650f4c3385..523261e5cb4 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/PendingTrace.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/PendingTrace.java @@ -2,6 +2,7 @@ import datadog.communication.monitor.Recording; import datadog.trace.api.DDTraceId; +import datadog.trace.api.TraceConfig; import datadog.trace.api.time.TimeSource; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentTrace; @@ -72,6 +73,8 @@ PendingTrace create(@Nonnull DDTraceId traceId) { private final TimeSource timeSource; private final boolean strictTraceWrites; private final HealthMetrics healthMetrics; + private final TraceConfig traceConfig; + private final ConcurrentLinkedDeque finishedSpans = new ConcurrentLinkedDeque<>(); // We must maintain a separate count because ConcurrentLinkedDeque.size() is a linear operation. @@ -121,12 +124,17 @@ private PendingTrace( this.timeSource = timeSource; this.strictTraceWrites = strictTraceWrites; this.healthMetrics = healthMetrics; + this.traceConfig = tracer.captureTraceConfig(); } CoreTracer getTracer() { return tracer; } + String mapServiceName(String serviceName) { + return traceConfig.getServiceMapping().getOrDefault(serviceName, serviceName); + } + /** * Current timestamp in nanoseconds; 'touches' the trace by updating {@link #lastReferenced}. * diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDAgentWriterCombinedTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDAgentWriterCombinedTest.groovy index e83144e62ac..d49d8f4ece6 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDAgentWriterCombinedTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDAgentWriterCombinedTest.groovy @@ -263,8 +263,8 @@ class DDAgentWriterCombinedTest extends DDCoreSpecification { def createMinimalContext() { def tracer = Mock(CoreTracer) - tracer.mapServiceName(_) >> { String serviceName -> serviceName } def trace = Mock(PendingTrace) + trace.mapServiceName(_) >> { String serviceName -> serviceName } trace.getTracer() >> tracer return new DDSpanContext( DDTraceId.ONE, diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDAgentWriterTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDAgentWriterTest.groovy index 906bc01585e..642558d6af5 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDAgentWriterTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDAgentWriterTest.groovy @@ -191,8 +191,8 @@ class DDAgentWriterTest extends DDCoreSpecification { def newSpan() { CoreTracer tracer = Mock(CoreTracer) - tracer.mapServiceName(_) >> { String serviceName -> serviceName } PendingTrace trace = Mock(PendingTrace) + trace.mapServiceName(_) >> { String serviceName -> serviceName } trace.getTracer() >> tracer def context = new DDSpanContext( DDTraceId.ONE, diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDIntakeWriterCombinedTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDIntakeWriterCombinedTest.groovy index d0d0c26f1fa..172c27b2c59 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDIntakeWriterCombinedTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDIntakeWriterCombinedTest.groovy @@ -730,8 +730,8 @@ class DDIntakeWriterCombinedTest extends DDCoreSpecification { def createMinimalContext() { def tracer = Mock(CoreTracer) - tracer.mapServiceName(_) >> { String serviceName -> serviceName } def trace = Mock(PendingTrace) + trace.mapServiceName(_) >> { String serviceName -> serviceName } trace.getTracer() >> tracer return new DDSpanContext( DDTraceId.ONE, diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDIntakeWriterTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDIntakeWriterTest.groovy index 954e518cdec..65a82b0707c 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDIntakeWriterTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDIntakeWriterTest.groovy @@ -181,8 +181,8 @@ class DDIntakeWriterTest extends DDCoreSpecification{ def newSpan() { CoreTracer tracer = Mock(CoreTracer) - tracer.mapServiceName(_) >> { String serviceName -> serviceName } PendingTrace trace = Mock(PendingTrace) + trace.mapServiceName(_) >> { String serviceName -> serviceName } trace.getTracer() >> tracer def context = new DDSpanContext( DDTraceId.ONE, diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/PayloadDispatcherTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/PayloadDispatcherTest.groovy index 051cab077d0..1736a81eb80 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/writer/PayloadDispatcherTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/writer/PayloadDispatcherTest.groovy @@ -149,11 +149,9 @@ class PayloadDispatcherTest extends DDSpecification { def realSpan() { CoreTracer tracer = Mock(CoreTracer) - tracer.mapServiceName(_) >> { String serviceName -> - serviceName - } PendingTrace trace = Mock(PendingTrace) trace.getTracer() >> tracer + trace.mapServiceName(_) >> { String serviceName -> serviceName } def context = new DDSpanContext( DDTraceId.ONE, 1, diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy index 0b11ea8d752..fc1b47c0f10 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy @@ -173,7 +173,7 @@ class CoreTracerTest extends DDCoreSpecification { then: tracer.defaultSpanTags == map - tracer.serviceNameMappings == map + tracer.captureTraceConfig().serviceMapping == map taggedHeaders == map cleanup: diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/PendingTraceBufferTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/PendingTraceBufferTest.groovy index e686664f98d..4f0e43c3f79 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/PendingTraceBufferTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/PendingTraceBufferTest.groovy @@ -4,6 +4,7 @@ import datadog.communication.monitor.Monitoring import datadog.trace.SamplingPriorityMetadataChecker import datadog.trace.api.DDSpanId import datadog.trace.api.DDTraceId +import datadog.trace.api.TraceConfig import datadog.trace.api.sampling.PrioritySampling import datadog.trace.api.time.SystemTimeSource import datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopPathwayContext @@ -29,10 +30,16 @@ class PendingTraceBufferTest extends DDSpecification { def bufferSpy = Spy(buffer) def tracer = Mock(CoreTracer) + def traceConfig = Mock(TraceConfig) def scopeManager = new ContinuableScopeManager(10, true, true) def factory = new PendingTrace.Factory(tracer, bufferSpy, SystemTimeSource.INSTANCE, false, HealthMetrics.NO_OP) List continuations = [] + def setup() { + tracer.captureTraceConfig() >> traceConfig + traceConfig.getServiceMapping() >> [:] + } + def cleanup() { buffer.close() buffer.worker.join(1000) @@ -142,7 +149,7 @@ class PendingTraceBufferTest extends DDSpecification { then: _ * tracer.getPartialFlushMinSpans() >> 10 - _ * tracer.mapServiceName(_) + _ * traceConfig.getServiceMapping() >> [:] _ * tracer.getTimeWithNanoTicks(_) 1 * tracer.writeTimer() >> Monitoring.DISABLED.newTimer("") 1 * tracer.write(_) >> { List> spans -> @@ -162,10 +169,11 @@ class PendingTraceBufferTest extends DDSpecification { } then: + _ * tracer.captureTraceConfig() >> traceConfig buffer.queue.size() == BUFFER_SIZE buffer.queue.capacity() * bufferSpy.enqueue(_) _ * tracer.getPartialFlushMinSpans() >> 10 - _ * tracer.mapServiceName(_) + _ * traceConfig.getServiceMapping() >> [:] _ * tracer.getTimeWithNanoTicks(_) 0 * _ @@ -174,11 +182,12 @@ class PendingTraceBufferTest extends DDSpecification { addContinuation(newSpanOf(pendingTrace)).finish() then: + 1 * tracer.captureTraceConfig() >> traceConfig 1 * bufferSpy.enqueue(_) 1 * tracer.writeTimer() >> Monitoring.DISABLED.newTimer("") 1 * tracer.write({ it.size() == 1 }) _ * tracer.getPartialFlushMinSpans() >> 10 - _ * tracer.mapServiceName(_) + _ * traceConfig.getServiceMapping() >> [:] 2 * tracer.getTimeWithNanoTicks(_) 0 * _ pendingTrace.isEnqueued == 0 @@ -273,7 +282,7 @@ class PendingTraceBufferTest extends DDSpecification { 1 * tracer.write({ it.size() == 1 }) >> { childLatch.countDown() } - _ * tracer.mapServiceName(_) + _ * traceConfig.getServiceMapping() >> [:] 2 * tracer.getTimeWithNanoTicks(_) 0 * _ } @@ -335,13 +344,14 @@ class PendingTraceBufferTest extends DDSpecification { span.finish() then: + 1 * tracer.captureTraceConfig() >> traceConfig pendingTrace.rootSpanWritten pendingTrace.isEnqueued == 0 buffer.queue.size() == 0 1 * tracer.writeTimer() >> Monitoring.DISABLED.newTimer("") 1 * tracer.write({ it.size() == 1 }) 1 * tracer.getPartialFlushMinSpans() >> 10000 - 1 * tracer.mapServiceName(_) + 1 * traceConfig.getServiceMapping() >> [:] 2 * tracer.getTimeWithNanoTicks(_) 0 * _ @@ -355,7 +365,7 @@ class PendingTraceBufferTest extends DDSpecification { buffer.queue.size() == 1 buffer.queue.capacity() * bufferSpy.enqueue(_) _ * tracer.getPartialFlushMinSpans() >> 10000 - _ * tracer.mapServiceName(_) + _ * traceConfig.getServiceMapping() >> [:] _ * tracer.getTimeWithNanoTicks(_) 0 * _ diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/PendingTraceTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/PendingTraceTest.groovy index 6624656a655..6b1e048b270 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/PendingTraceTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/PendingTraceTest.groovy @@ -1,6 +1,7 @@ package datadog.trace.core import datadog.trace.api.DDTraceId +import datadog.trace.api.TraceConfig import datadog.trace.api.sampling.PrioritySampling import datadog.trace.api.time.TimeSource import datadog.trace.bootstrap.instrumentation.api.AgentTracer @@ -66,8 +67,11 @@ class PendingTraceTest extends PendingTraceTestBase { def "verify healthmetrics called"() { setup: def tracer = Mock(CoreTracer) + def traceConfig = Mock(TraceConfig) def buffer = Mock(PendingTraceBuffer) def healthMetrics = Mock(HealthMetrics) + tracer.captureTraceConfig() >> traceConfig + traceConfig.getServiceMapping() >> [:] PendingTrace trace = new PendingTrace(tracer,DDTraceId.from(0),buffer,Mock(TimeSource),false,healthMetrics) when: rootSpan = createSimpleSpan(trace) diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/TypeConverterTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/TypeConverterTest.groovy index 1e86a9743e9..439c89637b4 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/TypeConverterTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/TypeConverterTest.groovy @@ -70,8 +70,8 @@ class TypeConverterTest extends DDSpecification { def createTestSpanContext() { def tracer = Mock(CoreTracer) - tracer.mapServiceName(_) >> { String serviceName -> serviceName } def trace = Mock(PendingTrace) + trace.mapServiceName(_) >> { String serviceName -> serviceName } trace.getTracer() >> tracer return new DDSpanContext( diff --git a/internal-api/build.gradle b/internal-api/build.gradle index da16f04e320..37d97c6175a 100644 --- a/internal-api/build.gradle +++ b/internal-api/build.gradle @@ -114,6 +114,9 @@ excludedClassesCoverage += [ "datadog.trace.api.ConfigCollector.Holder", "datadog.trace.api.Config.HostNameHolder", "datadog.trace.api.Config.RuntimeIdHolder", + "datadog.trace.api.DynamicConfig", + "datadog.trace.api.DynamicConfig.Builder", + "datadog.trace.api.DynamicConfig.State", "datadog.trace.api.InstrumenterConfig", // can't reliably force same identity hash for different instance to cover branch "datadog.trace.api.cache.FixedSizeCache.IdentityHash", diff --git a/internal-api/src/main/java/datadog/trace/api/DynamicConfig.java b/internal-api/src/main/java/datadog/trace/api/DynamicConfig.java new file mode 100644 index 00000000000..de154bf8668 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/DynamicConfig.java @@ -0,0 +1,62 @@ +package datadog.trace.api; + +import static datadog.trace.util.CollectionUtils.tryMakeImmutableMap; + +import java.util.Collections; +import java.util.Map; + +/** Manages dynamic configuration for a particular {@link Tracer} instance. */ +public final class DynamicConfig { + private volatile State currentState; + + private DynamicConfig() {} + + public static Builder create() { + return new DynamicConfig().prepare(); + } + + /** Captures a snapshot of the configuration at the start of a trace. */ + public TraceConfig captureTraceConfig() { + return currentState; + } + + public Builder prepare() { + return new Builder(currentState); + } + + public final class Builder { + Map serviceMapping; + + Builder(State state) { + if (null == state) { + this.serviceMapping = Collections.emptyMap(); + } else { + this.serviceMapping = state.serviceMapping; + } + } + + public Builder setServiceMapping(Map serviceMapping) { + this.serviceMapping = tryMakeImmutableMap(serviceMapping); + return this; + } + + /** Overwrites the current configuration with a new snapshot. */ + public DynamicConfig apply() { + DynamicConfig.this.currentState = new State(this); + return DynamicConfig.this; + } + } + + /** Immutable snapshot of the configuration. */ + static final class State implements TraceConfig { + final Map serviceMapping; + + State(Builder builder) { + this.serviceMapping = builder.serviceMapping; + } + + public Map getServiceMapping() { + return serviceMapping; + } + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/TraceConfig.java b/internal-api/src/main/java/datadog/trace/api/TraceConfig.java new file mode 100644 index 00000000000..17f1e0a1a42 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/TraceConfig.java @@ -0,0 +1,9 @@ +package datadog.trace.api; + +import java.util.Map; + +/** Snapshot of dynamic configuration; valid for the duration of a trace. */ +public interface TraceConfig { + + Map getServiceMapping(); +}