From 2076f6f8e6c6c8abdfe21d911cbe74e1456c72d5 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 9 May 2024 14:02:54 +0200 Subject: [PATCH 01/62] add new config property --- internal-api/src/main/java/datadog/trace/api/Config.java | 4 ++++ .../main/java/datadog/trace/api/InstrumenterConfig.java | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 554f6e47196..0ac8e168fff 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -2257,6 +2257,10 @@ public boolean isTraceEnabled() { return instrumenterConfig.isTraceEnabled(); } + public boolean isApmTracingEnabled() { + return instrumenterConfig.isApmTracingEnabled(); + } + public boolean isLongRunningTraceEnabled() { return longRunningTraceEnabled; } diff --git a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java index b92f6f93f63..9950908b20f 100644 --- a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java +++ b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java @@ -154,6 +154,8 @@ public class InstrumenterConfig { private final Collection additionalJaxRsAnnotations; + private final boolean apmTracinEnabled; + private InstrumenterConfig() { this(ConfigProvider.createDefault()); } @@ -263,6 +265,8 @@ private InstrumenterConfig() { this.additionalJaxRsAnnotations = tryMakeImmutableSet(configProvider.getList(JAX_RS_ADDITIONAL_ANNOTATIONS)); + + apmTracinEnabled = !isTraceEnabled() && (getAppSecActivation() == ProductActivation.FULLY_ENABLED || getIastActivation() == ProductActivation.FULLY_ENABLED); } public boolean isSpanOriginEnabled() { @@ -292,6 +296,10 @@ public boolean isTraceEnabled() { return traceEnabled; } + public boolean isApmTracingEnabled() { + return apmTracinEnabled; + } + public boolean isTraceOtelEnabled() { return traceOtelEnabled; } From 6ec447faed85ca158300394e4b72babc57f1cad3 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 13 May 2024 12:13:59 +0200 Subject: [PATCH 02/62] Add _dd.apm.enabled:0 tag --- .../java/datadog/trace/agent/tooling/AgentInstaller.java | 2 +- .../java/datadog/trace/agent/tooling/TracerInstaller.java | 4 +++- dd-trace-api/src/main/java/datadog/trace/api/DDTags.java | 1 + internal-api/src/main/java/datadog/trace/api/Config.java | 6 ++++++ 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java b/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java index 6fef809fef7..ed6a0c2a28c 100644 --- a/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java +++ b/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java @@ -293,7 +293,7 @@ public static Set getEnabledSystems() { EnumSet enabledSystems = EnumSet.noneOf(InstrumenterModule.TargetSystem.class); InstrumenterConfig cfg = InstrumenterConfig.get(); - if (cfg.isTraceEnabled()) { + if (cfg.isTraceEnabled() || cfg.isApmTracingEnabled()) { enabledSystems.add(InstrumenterModule.TargetSystem.TRACING); } if (cfg.isProfilingEnabled()) { diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java index 6bd27c31bb1..56b3d8d43ed 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java @@ -15,7 +15,9 @@ public class TracerInstaller { public static synchronized void installGlobalTracer( SharedCommunicationObjects sharedCommunicationObjects, ProfilingContextIntegration profilingContextIntegration) { - if (Config.get().isTraceEnabled() || Config.get().isCiVisibilityEnabled()) { + if (Config.get().isTraceEnabled() + || Config.get().isCiVisibilityEnabled() + || Config.get().isApmTracingEnabled()) { if (!(GlobalTracer.get() instanceof CoreTracer)) { CoreTracer tracer = CoreTracer.builder() diff --git a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java index 19b599f76a7..de82229bbcc 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java @@ -72,4 +72,5 @@ public class DDTags { public static final String PROFILING_CONTEXT_ENGINE = "_dd.profiling.ctx"; public static final String BASE_SERVICE = "_dd.base_service"; public static final String PARENT_ID = "_dd.parent_id"; + public static final String APM_ENABLED = "_dd.apm.enabled"; } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 0ac8e168fff..3a206113912 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -130,6 +130,7 @@ import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_RESOLVER_ENABLED; import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_X_DATADOG_TAGS_MAX_LENGTH; import static datadog.trace.api.ConfigDefaults.DEFAULT_WRITER_BAGGAGE_INJECT; +import static datadog.trace.api.DDTags.APM_ENABLED; import static datadog.trace.api.DDTags.HOST_TAG; import static datadog.trace.api.DDTags.INTERNAL_HOST_NAME; import static datadog.trace.api.DDTags.LANGUAGE_TAG_KEY; @@ -3601,6 +3602,11 @@ public Map getLocalRootSpanTags() { result.put(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE); result.put(SCHEMA_VERSION_TAG_KEY, SpanNaming.instance().version()); result.put(PROFILING_ENABLED, isProfilingEnabled() ? 1 : 0); + result.put(DSM_ENABLED, isDataStreamsEnabled() ? 1 : 0); + result.put(DJM_ENABLED, isDataJobsEnabled() ? 1 : 0); + if (isApmTracingEnabled()) { + result.put(APM_ENABLED, 0); + } if (reportHostName) { final String hostName = getHostName(); From ecdd7318966f8e91154563a9bee4faf0796e8a70 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 13 May 2024 12:15:14 +0200 Subject: [PATCH 03/62] Add _dd.apm.enabled:0 tag --- .../src/main/java/datadog/trace/api/InstrumenterConfig.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java index 9950908b20f..9658c3f11d1 100644 --- a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java +++ b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java @@ -266,7 +266,10 @@ private InstrumenterConfig() { this.additionalJaxRsAnnotations = tryMakeImmutableSet(configProvider.getList(JAX_RS_ADDITIONAL_ANNOTATIONS)); - apmTracinEnabled = !isTraceEnabled() && (getAppSecActivation() == ProductActivation.FULLY_ENABLED || getIastActivation() == ProductActivation.FULLY_ENABLED); + apmTracinEnabled = + !isTraceEnabled() + && (getAppSecActivation() == ProductActivation.FULLY_ENABLED + || getIastActivation() == ProductActivation.FULLY_ENABLED); } public boolean isSpanOriginEnabled() { From 7bb2586d32a60ad51087782c213cd9ac84c5cf56 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 13 May 2024 12:46:35 +0200 Subject: [PATCH 04/62] Disabling the computation agent-side of the APM trace metrics by pretending it was already done by the library. --- .../datadog/trace/common/writer/ddagent/DDAgentApi.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java index e9a3e73a81a..94d38a76cb4 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java @@ -9,6 +9,7 @@ import datadog.communication.monitor.Counter; import datadog.communication.monitor.Monitoring; import datadog.communication.monitor.Recording; +import datadog.trace.api.Config; import datadog.trace.common.writer.Payload; import datadog.trace.common.writer.RemoteApi; import datadog.trace.common.writer.RemoteResponseListener; @@ -108,7 +109,10 @@ public Response sendSerializedTraces(final Payload payload) { .addHeader(DATADOG_DROPPED_SPAN_COUNT, Long.toString(payload.droppedSpans())) .addHeader( DATADOG_CLIENT_COMPUTED_STATS, - metricsEnabled && featuresDiscovery.supportsMetrics() ? "true" : "") + (metricsEnabled && featuresDiscovery.supportsMetrics()) + || Config.get().isApmTracingEnabled() + ? "true" + : "") .put(payload.toRequest()) .build(); this.totalTraces += payload.traceCount(); From 8c752a3831b7588c4fa720eb36fa79562398db88 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 14 May 2024 08:02:06 +0200 Subject: [PATCH 05/62] Disabling the computation library-side of the APM trace metrics --- .../datadog/trace/common/metrics/MetricsAggregatorFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java index 491cc12b1c0..43e9532b792 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java @@ -10,7 +10,7 @@ public class MetricsAggregatorFactory { public static MetricsAggregator createMetricsAggregator( Config config, SharedCommunicationObjects sharedCommunicationObjects) { - if (config.isTracerMetricsEnabled()) { + if (config.isTraceEnabled() && config.isTracerMetricsEnabled()) { log.debug("tracer metrics enabled"); return new ConflatingMetricsAggregator(config, sharedCommunicationObjects); } From e1efe75dc3b3da7da68e858688357f151f01695a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 16 May 2024 08:14:15 +0200 Subject: [PATCH 06/62] refactor config to have a property that determines if tracing is needed with apm disabled --- .../datadog/trace/agent/tooling/AgentInstaller.java | 2 +- .../trace/agent/tooling/TracerInstaller.java | 2 +- .../src/main/java/datadog/trace/api/DDTags.java | 2 ++ .../trace/common/writer/ddagent/DDAgentApi.java | 3 ++- .../src/main/java/datadog/trace/api/Config.java | 6 +++--- .../java/datadog/trace/api/InstrumenterConfig.java | 13 ++++++------- 6 files changed, 15 insertions(+), 13 deletions(-) diff --git a/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java b/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java index ed6a0c2a28c..5879e7da89a 100644 --- a/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java +++ b/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java @@ -293,7 +293,7 @@ public static Set getEnabledSystems() { EnumSet enabledSystems = EnumSet.noneOf(InstrumenterModule.TargetSystem.class); InstrumenterConfig cfg = InstrumenterConfig.get(); - if (cfg.isTraceEnabled() || cfg.isApmTracingEnabled()) { + if (cfg.isTraceEnabled() || cfg.areTracingDependantProductsEnabled()) { enabledSystems.add(InstrumenterModule.TargetSystem.TRACING); } if (cfg.isProfilingEnabled()) { diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java index 56b3d8d43ed..66303332410 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java @@ -17,7 +17,7 @@ public static synchronized void installGlobalTracer( ProfilingContextIntegration profilingContextIntegration) { if (Config.get().isTraceEnabled() || Config.get().isCiVisibilityEnabled() - || Config.get().isApmTracingEnabled()) { + || Config.get().areTracingDependantProductsEnabled()) { if (!(GlobalTracer.get() instanceof CoreTracer)) { CoreTracer tracer = CoreTracer.builder() diff --git a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java index de82229bbcc..5056ef276d1 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java @@ -73,4 +73,6 @@ public class DDTags { public static final String BASE_SERVICE = "_dd.base_service"; public static final String PARENT_ID = "_dd.parent_id"; public static final String APM_ENABLED = "_dd.apm.enabled"; + + public static final String APPSEC = "_dd.p.appsec"; } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java index 94d38a76cb4..f294511b5d8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java @@ -110,7 +110,8 @@ public Response sendSerializedTraces(final Payload payload) { .addHeader( DATADOG_CLIENT_COMPUTED_STATS, (metricsEnabled && featuresDiscovery.supportsMetrics()) - || Config.get().isApmTracingEnabled() + || (!Config.get().isTraceEnabled() + && Config.get().areTracingDependantProductsEnabled()) ? "true" : "") .put(payload.toRequest()) diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 3a206113912..bed3d5aec87 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -2258,8 +2258,8 @@ public boolean isTraceEnabled() { return instrumenterConfig.isTraceEnabled(); } - public boolean isApmTracingEnabled() { - return instrumenterConfig.isApmTracingEnabled(); + public boolean areTracingDependantProductsEnabled() { + return instrumenterConfig.areTracingDependantProductsEnabled(); } public boolean isLongRunningTraceEnabled() { @@ -3604,7 +3604,7 @@ public Map getLocalRootSpanTags() { result.put(PROFILING_ENABLED, isProfilingEnabled() ? 1 : 0); result.put(DSM_ENABLED, isDataStreamsEnabled() ? 1 : 0); result.put(DJM_ENABLED, isDataJobsEnabled() ? 1 : 0); - if (isApmTracingEnabled()) { + if (areTracingDependantProductsEnabled()) { result.put(APM_ENABLED, 0); } diff --git a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java index 9658c3f11d1..35b66891931 100644 --- a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java +++ b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java @@ -154,7 +154,7 @@ public class InstrumenterConfig { private final Collection additionalJaxRsAnnotations; - private final boolean apmTracinEnabled; + private final boolean tracingDependantProductsEnabled; private InstrumenterConfig() { this(ConfigProvider.createDefault()); @@ -266,10 +266,9 @@ private InstrumenterConfig() { this.additionalJaxRsAnnotations = tryMakeImmutableSet(configProvider.getList(JAX_RS_ADDITIONAL_ANNOTATIONS)); - apmTracinEnabled = - !isTraceEnabled() - && (getAppSecActivation() == ProductActivation.FULLY_ENABLED - || getIastActivation() == ProductActivation.FULLY_ENABLED); + tracingDependantProductsEnabled = + getAppSecActivation() == ProductActivation.FULLY_ENABLED + || getIastActivation() == ProductActivation.FULLY_ENABLED; } public boolean isSpanOriginEnabled() { @@ -299,8 +298,8 @@ public boolean isTraceEnabled() { return traceEnabled; } - public boolean isApmTracingEnabled() { - return apmTracinEnabled; + public boolean areTracingDependantProductsEnabled() { + return tracingDependantProductsEnabled; } public boolean isTraceOtelEnabled() { From 07af6b8dbe02daf8e20ce4518aed0ded77cab813 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 21 May 2024 08:34:39 +0200 Subject: [PATCH 07/62] tracing libraries must configure the tracer sampling mechanism so that, by default, 1 trace per minute gets sent. --- .../trace/common/sampling/Sampler.java | 4 ++ .../sampling/ServiceAsmTimeTraceSampler.java | 45 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 dd-trace-core/src/main/java/datadog/trace/common/sampling/ServiceAsmTimeTraceSampler.java diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java index 9efc911d54f..744612271da 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java @@ -34,6 +34,10 @@ final class Builder { public static Sampler forConfig(final Config config, final TraceConfig traceConfig) { Sampler sampler; if (config != null) { + if(!config.isTraceEnabled() && config.areTracingDependantProductsEnabled()){ + log.debug("APM is disabled. Only 1 trace per minute will be sent."); + return new ServiceAsmTimeTraceSampler(); + } final Map serviceRules = config.getTraceSamplingServiceRules(); final Map operationRules = config.getTraceSamplingOperationRules(); List traceSamplingRules; diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ServiceAsmTimeTraceSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ServiceAsmTimeTraceSampler.java new file mode 100644 index 00000000000..cf881b29d40 --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ServiceAsmTimeTraceSampler.java @@ -0,0 +1,45 @@ +package datadog.trace.common.sampling; + +import datadog.trace.api.sampling.SamplingMechanism; +import datadog.trace.core.CoreSpan; +import datadog.trace.core.util.SimpleRateLimiter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + + +public class ServiceAsmTimeTraceSampler implements Sampler, PrioritySampler { + + private static final Logger log = LoggerFactory.getLogger(ServiceAsmTimeTraceSampler.class); + + private final SimpleRateLimiter rateLimiter; + + public ServiceAsmTimeTraceSampler() { + this.rateLimiter = new SimpleRateLimiter(60); //one per minute + } + + @Override + public > boolean sample(final T span) { + // Priority sampling sends all traces to the core agent, including traces marked dropped. + // This allows the core agent to collect stats on all traces. + return true; + } + + @Override + public > void setSamplingPriority(final T span) { + + //TODO check how to short circuit this for ASM + + if (rateLimiter.tryAcquire()) { + span.setSamplingPriority( + PrioritySampling.SAMPLER_KEEP, + SamplingMechanism.DEFAULT); + } else { + span.setSamplingPriority( + PrioritySampling.SAMPLER_DROP, + SamplingMechanism.DEFAULT); + } + + } + +} From 31f75b026c9323b330ad5a8c42984bf9d77f8424 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 21 May 2024 08:37:52 +0200 Subject: [PATCH 08/62] tracing libraries must configure the tracer sampling mechanism so that, by default, 1 trace per minute gets sent. --- .../datadog/trace/common/sampling/Sampler.java | 2 +- .../sampling/ServiceAsmTimeTraceSampler.java | 16 ++++------------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java index 744612271da..329a632dbeb 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java @@ -34,7 +34,7 @@ final class Builder { public static Sampler forConfig(final Config config, final TraceConfig traceConfig) { Sampler sampler; if (config != null) { - if(!config.isTraceEnabled() && config.areTracingDependantProductsEnabled()){ + if (!config.isTraceEnabled() && config.areTracingDependantProductsEnabled()) { log.debug("APM is disabled. Only 1 trace per minute will be sent."); return new ServiceAsmTimeTraceSampler(); } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ServiceAsmTimeTraceSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ServiceAsmTimeTraceSampler.java index cf881b29d40..a2aad60459c 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ServiceAsmTimeTraceSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ServiceAsmTimeTraceSampler.java @@ -6,8 +6,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; - - public class ServiceAsmTimeTraceSampler implements Sampler, PrioritySampler { private static final Logger log = LoggerFactory.getLogger(ServiceAsmTimeTraceSampler.class); @@ -15,7 +13,7 @@ public class ServiceAsmTimeTraceSampler implements Sampler, PrioritySampler { private final SimpleRateLimiter rateLimiter; public ServiceAsmTimeTraceSampler() { - this.rateLimiter = new SimpleRateLimiter(60); //one per minute + this.rateLimiter = new SimpleRateLimiter(60); // one per minute } @Override @@ -28,18 +26,12 @@ public > boolean sample(final T span) { @Override public > void setSamplingPriority(final T span) { - //TODO check how to short circuit this for ASM + // TODO check how to short circuit this for ASM if (rateLimiter.tryAcquire()) { - span.setSamplingPriority( - PrioritySampling.SAMPLER_KEEP, - SamplingMechanism.DEFAULT); + span.setSamplingPriority(PrioritySampling.SAMPLER_KEEP, SamplingMechanism.DEFAULT); } else { - span.setSamplingPriority( - PrioritySampling.SAMPLER_DROP, - SamplingMechanism.DEFAULT); + span.setSamplingPriority(PrioritySampling.SAMPLER_DROP, SamplingMechanism.DEFAULT); } - } - } From cd5e0be9def9506a113393edadcca8f0ebba9eef Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 21 May 2024 09:06:42 +0200 Subject: [PATCH 09/62] fix merge issue --- internal-api/src/main/java/datadog/trace/api/Config.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index bed3d5aec87..c9b5751a0a2 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -3602,8 +3602,6 @@ public Map getLocalRootSpanTags() { result.put(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE); result.put(SCHEMA_VERSION_TAG_KEY, SpanNaming.instance().version()); result.put(PROFILING_ENABLED, isProfilingEnabled() ? 1 : 0); - result.put(DSM_ENABLED, isDataStreamsEnabled() ? 1 : 0); - result.put(DJM_ENABLED, isDataJobsEnabled() ? 1 : 0); if (areTracingDependantProductsEnabled()) { result.put(APM_ENABLED, 0); } From d750c8a98ffaa68ceec061d781be676022bb4ffe Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 21 May 2024 09:53:50 +0200 Subject: [PATCH 10/62] Add test to areTracingDependantProductsEnabled --- .../datadog/trace/api/ConfigTest.groovy | 76 +++++++++++++------ 1 file changed, 52 insertions(+), 24 deletions(-) diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 232352b40cb..dcab96aec00 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -26,6 +26,7 @@ import static datadog.trace.api.TracePropagationStyle.B3SINGLE import static datadog.trace.api.TracePropagationStyle.DATADOG import static datadog.trace.api.TracePropagationStyle.HAYSTACK import static datadog.trace.api.TracePropagationStyle.TRACECONTEXT +import static datadog.trace.api.config.AppSecConfig.APPSEC_ENABLED import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_AGENTLESS_ENABLED import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_ENABLED import static datadog.trace.api.config.DebuggerConfig.DEBUGGER_CLASSFILE_DUMP_ENABLED @@ -56,6 +57,7 @@ import static datadog.trace.api.config.GeneralConfig.SITE import static datadog.trace.api.config.GeneralConfig.TAGS import static datadog.trace.api.config.GeneralConfig.TRACER_METRICS_IGNORED_RESOURCES import static datadog.trace.api.config.GeneralConfig.VERSION +import static datadog.trace.api.config.IastConfig.IAST_ENABLED import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_CHECK_PERIOD import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_ENABLED import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_METRICS_CONFIGS @@ -1188,8 +1190,8 @@ class ConfigTest extends DDSpecification { value | expected // null means default value // spotless:off "1" | [1] - "3,13,400-403" | [3,13,400,401,402,403] - "2,10,13-15" | [2,10,13,14,15] + "3,13,400-403" | [3, 13, 400, 401, 402, 403] + "2,10,13-15" | [2, 10, 13, 14, 15] "a" | null "" | null "1000" | null @@ -1492,12 +1494,12 @@ class ConfigTest extends DDSpecification { then: config.mergedSpanTags == [a: "1", b: "2", c: "3", (ENV): "eu-east", (VERSION): "43"] config.mergedJmxTags == [a : "1", b: "2", d: "4", (ENV): "eu-east", (VERSION): "43", - (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName] + (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName] config.requestHeaderTags == [e: "five"] config.mergedProfilingTags == [a : "1", b: "2", f: "6", (ENV): "eu-east", (VERSION): "43", - (HOST_TAG) : config.getHostName(), (RUNTIME_ID_TAG): config.getRuntimeId(), (RUNTIME_VERSION_TAG): config.getRuntimeVersion(), - (SERVICE_TAG): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] + (HOST_TAG) : config.getHostName(), (RUNTIME_ID_TAG): config.getRuntimeId(), (RUNTIME_VERSION_TAG): config.getRuntimeVersion(), + (SERVICE_TAG): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] } def "verify dd.tags overrides global tags in system properties"() { @@ -1695,7 +1697,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-env-var" config.mergedSpanTags == [version: "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): 'dd-service-env-var', - version : "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] + version : "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] } def "merge env from dd.trace.global.tags and DD_SERVICE and DD_VERSION"() { @@ -1711,7 +1713,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-env-var" config.mergedSpanTags == [version: "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): 'dd-service-env-var', - version : "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] + version : "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] } def "set of dd.trace.global.tags.env exclusively by java properties and without DD_ENV"() { @@ -1774,7 +1776,7 @@ class ConfigTest extends DDSpecification { config.serviceName == DEFAULT_SERVICE_NAME config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, - 'service.version': 'my-svc-vers'] + 'service.version': 'my-svc-vers'] } def "DD_SERVICE precedence over 'dd.service.name' java property is set; 'dd.service' overwrites DD_SERVICE"() { @@ -1792,7 +1794,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-java-prop" config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, - 'service.version': 'my-svc-vers'] + 'service.version': 'my-svc-vers'] } def "DD_SERVICE precedence over 'DD_SERVICE_NAME' environment var is set"() { @@ -1808,7 +1810,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-env-var" config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, - 'service.version': 'my-svc-vers'] + 'service.version': 'my-svc-vers'] } def "dd.service overwrites DD_SERVICE"() { @@ -1824,7 +1826,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-java-prop" config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, - 'service.version': 'my-svc-vers'] + 'service.version': 'my-svc-vers'] } def "set servicenaem by DD_SERVICE"() { @@ -1840,7 +1842,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-env-var" config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, - 'service.version': 'my-svc-vers'] + 'service.version': 'my-svc-vers'] } def "explicit service name is not overridden by captured environment"() { @@ -2388,12 +2390,12 @@ class ConfigTest extends DDSpecification { where: configuredFlushInterval | flushInterval - "invalid" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL - "-1" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL - "9" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL - "451" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL - "10" | 10 - "450" | 450 + "invalid" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL + "-1" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL + "9" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL + "451" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL + "10" | 10 + "450" | 450 } def "long running trace invalid flush_interval set to default: #configuredFlushInterval"() { @@ -2409,12 +2411,12 @@ class ConfigTest extends DDSpecification { where: configuredFlushInterval | flushInterval - "invalid" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL - "-1" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL - "19" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL - "451" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL - "20" | 20 - "450" | 450 + "invalid" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL + "-1" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL + "19" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL + "451" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL + "20" | 20 + "450" | 450 } def "partial flush and min spans interaction"() { @@ -2440,4 +2442,30 @@ class ConfigTest extends DDSpecification { true | 11 | 11 false | 17 | 0 } + + void "test areTracingDependantProductsEnabled"() { + setup: + def prop = new Properties() + prop.setProperty(TRACE_ENABLED, apmEnabled.toString()) // TRACE_ENABLED == APM enabled + prop.setProperty(APPSEC_ENABLED, appsecEnabled.toString()) + prop.setProperty(IAST_ENABLED, iastEnabled.toString()) + + when: + Config config = Config.get(prop) + + then: + config.isTraceEnabled() == apmEnabled + config.areTracingDependantProductsEnabled() == (appsecEnabled || iastEnabled) + + where: + apmEnabled | appsecEnabled | iastEnabled + true | true | true + true | true | false + true | false | true + true | false | false + false | true | true + false | true | false + false | false | true + + } } From 6c89550fcdda5e266423994d5d4aec73a31c9c3a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 21 May 2024 10:32:53 +0200 Subject: [PATCH 11/62] rollback --- .../datadog/trace/api/ConfigTest.groovy | 76 ++++++------------- 1 file changed, 24 insertions(+), 52 deletions(-) diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index dcab96aec00..232352b40cb 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -26,7 +26,6 @@ import static datadog.trace.api.TracePropagationStyle.B3SINGLE import static datadog.trace.api.TracePropagationStyle.DATADOG import static datadog.trace.api.TracePropagationStyle.HAYSTACK import static datadog.trace.api.TracePropagationStyle.TRACECONTEXT -import static datadog.trace.api.config.AppSecConfig.APPSEC_ENABLED import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_AGENTLESS_ENABLED import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_ENABLED import static datadog.trace.api.config.DebuggerConfig.DEBUGGER_CLASSFILE_DUMP_ENABLED @@ -57,7 +56,6 @@ import static datadog.trace.api.config.GeneralConfig.SITE import static datadog.trace.api.config.GeneralConfig.TAGS import static datadog.trace.api.config.GeneralConfig.TRACER_METRICS_IGNORED_RESOURCES import static datadog.trace.api.config.GeneralConfig.VERSION -import static datadog.trace.api.config.IastConfig.IAST_ENABLED import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_CHECK_PERIOD import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_ENABLED import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_METRICS_CONFIGS @@ -1190,8 +1188,8 @@ class ConfigTest extends DDSpecification { value | expected // null means default value // spotless:off "1" | [1] - "3,13,400-403" | [3, 13, 400, 401, 402, 403] - "2,10,13-15" | [2, 10, 13, 14, 15] + "3,13,400-403" | [3,13,400,401,402,403] + "2,10,13-15" | [2,10,13,14,15] "a" | null "" | null "1000" | null @@ -1494,12 +1492,12 @@ class ConfigTest extends DDSpecification { then: config.mergedSpanTags == [a: "1", b: "2", c: "3", (ENV): "eu-east", (VERSION): "43"] config.mergedJmxTags == [a : "1", b: "2", d: "4", (ENV): "eu-east", (VERSION): "43", - (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName] + (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName] config.requestHeaderTags == [e: "five"] config.mergedProfilingTags == [a : "1", b: "2", f: "6", (ENV): "eu-east", (VERSION): "43", - (HOST_TAG) : config.getHostName(), (RUNTIME_ID_TAG): config.getRuntimeId(), (RUNTIME_VERSION_TAG): config.getRuntimeVersion(), - (SERVICE_TAG): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] + (HOST_TAG) : config.getHostName(), (RUNTIME_ID_TAG): config.getRuntimeId(), (RUNTIME_VERSION_TAG): config.getRuntimeVersion(), + (SERVICE_TAG): config.serviceName, (LANGUAGE_TAG_KEY): LANGUAGE_TAG_VALUE] } def "verify dd.tags overrides global tags in system properties"() { @@ -1697,7 +1695,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-env-var" config.mergedSpanTags == [version: "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): 'dd-service-env-var', - version : "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] + version : "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] } def "merge env from dd.trace.global.tags and DD_SERVICE and DD_VERSION"() { @@ -1713,7 +1711,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-env-var" config.mergedSpanTags == [version: "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): 'dd-service-env-var', - version : "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] + version : "3.2.1", "service.version": "my-svc-vers", "env": "us-barista-test", other_tag: "test"] } def "set of dd.trace.global.tags.env exclusively by java properties and without DD_ENV"() { @@ -1776,7 +1774,7 @@ class ConfigTest extends DDSpecification { config.serviceName == DEFAULT_SERVICE_NAME config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, - 'service.version': 'my-svc-vers'] + 'service.version': 'my-svc-vers'] } def "DD_SERVICE precedence over 'dd.service.name' java property is set; 'dd.service' overwrites DD_SERVICE"() { @@ -1794,7 +1792,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-java-prop" config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, - 'service.version': 'my-svc-vers'] + 'service.version': 'my-svc-vers'] } def "DD_SERVICE precedence over 'DD_SERVICE_NAME' environment var is set"() { @@ -1810,7 +1808,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-env-var" config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, - 'service.version': 'my-svc-vers'] + 'service.version': 'my-svc-vers'] } def "dd.service overwrites DD_SERVICE"() { @@ -1826,7 +1824,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-java-prop" config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, - 'service.version': 'my-svc-vers'] + 'service.version': 'my-svc-vers'] } def "set servicenaem by DD_SERVICE"() { @@ -1842,7 +1840,7 @@ class ConfigTest extends DDSpecification { config.serviceName == "dd-service-env-var" config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, - 'service.version': 'my-svc-vers'] + 'service.version': 'my-svc-vers'] } def "explicit service name is not overridden by captured environment"() { @@ -2390,12 +2388,12 @@ class ConfigTest extends DDSpecification { where: configuredFlushInterval | flushInterval - "invalid" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL - "-1" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL - "9" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL - "451" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL - "10" | 10 - "450" | 450 + "invalid" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL + "-1" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL + "9" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL + "451" | DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL + "10" | 10 + "450" | 450 } def "long running trace invalid flush_interval set to default: #configuredFlushInterval"() { @@ -2411,12 +2409,12 @@ class ConfigTest extends DDSpecification { where: configuredFlushInterval | flushInterval - "invalid" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL - "-1" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL - "19" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL - "451" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL - "20" | 20 - "450" | 450 + "invalid" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL + "-1" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL + "19" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL + "451" | DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL + "20" | 20 + "450" | 450 } def "partial flush and min spans interaction"() { @@ -2442,30 +2440,4 @@ class ConfigTest extends DDSpecification { true | 11 | 11 false | 17 | 0 } - - void "test areTracingDependantProductsEnabled"() { - setup: - def prop = new Properties() - prop.setProperty(TRACE_ENABLED, apmEnabled.toString()) // TRACE_ENABLED == APM enabled - prop.setProperty(APPSEC_ENABLED, appsecEnabled.toString()) - prop.setProperty(IAST_ENABLED, iastEnabled.toString()) - - when: - Config config = Config.get(prop) - - then: - config.isTraceEnabled() == apmEnabled - config.areTracingDependantProductsEnabled() == (appsecEnabled || iastEnabled) - - where: - apmEnabled | appsecEnabled | iastEnabled - true | true | true - true | true | false - true | false | true - true | false | false - false | true | true - false | true | false - false | false | true - - } } From ec850f292fb74ad48e706198c950486954a8b9fb Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 21 May 2024 10:35:10 +0200 Subject: [PATCH 12/62] Standalone ASM, disabling APM tracing and its billing, must be enabled with the boolean environment variable DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED=true. --- .../datadog/trace/agent/tooling/AgentInstaller.java | 2 +- .../datadog/trace/agent/tooling/TracerInstaller.java | 4 +--- .../java/datadog/trace/api/config/AppSecConfig.java | 3 +++ .../trace/common/metrics/MetricsAggregatorFactory.java | 6 +++++- ...r.java => ExperimentalAppSecStandaloneSampler.java} | 7 ++++--- .../java/datadog/trace/common/sampling/Sampler.java | 4 ++-- .../trace/common/writer/ddagent/DDAgentApi.java | 3 +-- .../src/main/java/datadog/trace/api/Config.java | 10 +++++++++- .../java/datadog/trace/api/InstrumenterConfig.java | 10 ---------- 9 files changed, 26 insertions(+), 23 deletions(-) rename dd-trace-core/src/main/java/datadog/trace/common/sampling/{ServiceAsmTimeTraceSampler.java => ExperimentalAppSecStandaloneSampler.java} (80%) diff --git a/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java b/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java index 5879e7da89a..6fef809fef7 100644 --- a/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java +++ b/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java @@ -293,7 +293,7 @@ public static Set getEnabledSystems() { EnumSet enabledSystems = EnumSet.noneOf(InstrumenterModule.TargetSystem.class); InstrumenterConfig cfg = InstrumenterConfig.get(); - if (cfg.isTraceEnabled() || cfg.areTracingDependantProductsEnabled()) { + if (cfg.isTraceEnabled()) { enabledSystems.add(InstrumenterModule.TargetSystem.TRACING); } if (cfg.isProfilingEnabled()) { diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java index 66303332410..6bd27c31bb1 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java @@ -15,9 +15,7 @@ public class TracerInstaller { public static synchronized void installGlobalTracer( SharedCommunicationObjects sharedCommunicationObjects, ProfilingContextIntegration profilingContextIntegration) { - if (Config.get().isTraceEnabled() - || Config.get().isCiVisibilityEnabled() - || Config.get().areTracingDependantProductsEnabled()) { + if (Config.get().isTraceEnabled() || Config.get().isCiVisibilityEnabled()) { if (!(GlobalTracer.get() instanceof CoreTracer)) { CoreTracer tracer = CoreTracer.builder() diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java index 861b410ae54..a842b68503c 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java @@ -32,5 +32,8 @@ public final class AppSecConfig { public static final String APPSEC_MAX_STACK_TRACES = "appsec.max.stacktraces"; public static final String APPSEC_MAX_STACK_TRACE_DEPTH = "appsec.max.stacktrace.depth"; + public static final String EXPERIMENTAL_APPSEC_STANDALONE_ENABLED = + "experimental.appsec.standalone.enabled"; + private AppSecConfig() {} } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java index 43e9532b792..5f27548814e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java @@ -10,7 +10,11 @@ public class MetricsAggregatorFactory { public static MetricsAggregator createMetricsAggregator( Config config, SharedCommunicationObjects sharedCommunicationObjects) { - if (config.isTraceEnabled() && config.isTracerMetricsEnabled()) { + if (config.isTracerMetricsEnabled() + && !config.isExperimentalAppSecStandaloneEnabled() // When APM is disabled, libraries must + // completely disable the generation of APM + // trace metrics + ) { log.debug("tracer metrics enabled"); return new ConflatingMetricsAggregator(config, sharedCommunicationObjects); } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ServiceAsmTimeTraceSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ExperimentalAppSecStandaloneSampler.java similarity index 80% rename from dd-trace-core/src/main/java/datadog/trace/common/sampling/ServiceAsmTimeTraceSampler.java rename to dd-trace-core/src/main/java/datadog/trace/common/sampling/ExperimentalAppSecStandaloneSampler.java index a2aad60459c..f8aef8ef4d8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ServiceAsmTimeTraceSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ExperimentalAppSecStandaloneSampler.java @@ -6,13 +6,14 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class ServiceAsmTimeTraceSampler implements Sampler, PrioritySampler { +public class ExperimentalAppSecStandaloneSampler implements Sampler, PrioritySampler { - private static final Logger log = LoggerFactory.getLogger(ServiceAsmTimeTraceSampler.class); + private static final Logger log = + LoggerFactory.getLogger(ExperimentalAppSecStandaloneSampler.class); private final SimpleRateLimiter rateLimiter; - public ServiceAsmTimeTraceSampler() { + public ExperimentalAppSecStandaloneSampler() { this.rateLimiter = new SimpleRateLimiter(60); // one per minute } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java index 329a632dbeb..353a1b23b3d 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java @@ -34,9 +34,9 @@ final class Builder { public static Sampler forConfig(final Config config, final TraceConfig traceConfig) { Sampler sampler; if (config != null) { - if (!config.isTraceEnabled() && config.areTracingDependantProductsEnabled()) { + if (config.isExperimentalAppSecStandaloneEnabled()) { log.debug("APM is disabled. Only 1 trace per minute will be sent."); - return new ServiceAsmTimeTraceSampler(); + return new ExperimentalAppSecStandaloneSampler(); } final Map serviceRules = config.getTraceSamplingServiceRules(); final Map operationRules = config.getTraceSamplingOperationRules(); diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java index f294511b5d8..14f124748af 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java @@ -110,8 +110,7 @@ public Response sendSerializedTraces(final Payload payload) { .addHeader( DATADOG_CLIENT_COMPUTED_STATS, (metricsEnabled && featuresDiscovery.supportsMetrics()) - || (!Config.get().isTraceEnabled() - && Config.get().areTracingDependantProductsEnabled()) + || Config.get().isExperimentalAppSecStandaloneEnabled() ? "true" : "") .put(payload.toRequest()) diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index c9b5751a0a2..f3744b07b7d 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -163,6 +163,7 @@ import static datadog.trace.api.config.AppSecConfig.APPSEC_TRACE_RATE_LIMIT; import static datadog.trace.api.config.AppSecConfig.APPSEC_WAF_METRICS; import static datadog.trace.api.config.AppSecConfig.APPSEC_WAF_TIMEOUT; +import static datadog.trace.api.config.AppSecConfig.EXPERIMENTAL_APPSEC_STANDALONE_ENABLED; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_ADDITIONAL_CHILD_PROCESS_JVM_ARGS; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_AGENTLESS_ENABLED; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_AGENTLESS_URL; @@ -757,6 +758,7 @@ static class HostNameHolder { private final boolean appSecStackTraceEnabled; private final int appSecMaxStackTraces; private final int appSecMaxStackTraceDepth; + private final boolean experimentalAppSecStandaloneEnabled; private final boolean apiSecurityEnabled; private final float apiSecurityRequestSampleRate; @@ -1670,6 +1672,8 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) configProvider.getStringNotEmpty( APPSEC_AUTOMATED_USER_EVENTS_TRACKING, SAFE.toString())); appSecScaEnabled = configProvider.getBoolean(APPSEC_SCA_ENABLED); + experimentalAppSecStandaloneEnabled = + configProvider.getBoolean(EXPERIMENTAL_APPSEC_STANDALONE_ENABLED, false); appSecRaspEnabled = configProvider.getBoolean(APPSEC_RASP_ENABLED, DEFAULT_APPSEC_RASP_ENABLED); appSecStackTraceEnabled = configProvider.getBoolean(APPSEC_STACK_TRACE_ENABLED, DEFAULT_APPSEC_STACK_TRACE_ENABLED); @@ -3602,7 +3606,7 @@ public Map getLocalRootSpanTags() { result.put(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE); result.put(SCHEMA_VERSION_TAG_KEY, SpanNaming.instance().version()); result.put(PROFILING_ENABLED, isProfilingEnabled() ? 1 : 0); - if (areTracingDependantProductsEnabled()) { + if (areTracingDependantProductsEnabled() || isExperimentalAppSecStandaloneEnabled()) { result.put(APM_ENABLED, 0); } @@ -4086,6 +4090,10 @@ public Boolean getAppSecScaEnabled() { return appSecScaEnabled; } + public boolean isExperimentalAppSecStandaloneEnabled() { + return experimentalAppSecStandaloneEnabled; + } + public boolean isAppSecRaspEnabled() { return appSecRaspEnabled; } diff --git a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java index 35b66891931..b92f6f93f63 100644 --- a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java +++ b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java @@ -154,8 +154,6 @@ public class InstrumenterConfig { private final Collection additionalJaxRsAnnotations; - private final boolean tracingDependantProductsEnabled; - private InstrumenterConfig() { this(ConfigProvider.createDefault()); } @@ -265,10 +263,6 @@ private InstrumenterConfig() { this.additionalJaxRsAnnotations = tryMakeImmutableSet(configProvider.getList(JAX_RS_ADDITIONAL_ANNOTATIONS)); - - tracingDependantProductsEnabled = - getAppSecActivation() == ProductActivation.FULLY_ENABLED - || getIastActivation() == ProductActivation.FULLY_ENABLED; } public boolean isSpanOriginEnabled() { @@ -298,10 +292,6 @@ public boolean isTraceEnabled() { return traceEnabled; } - public boolean areTracingDependantProductsEnabled() { - return tracingDependantProductsEnabled; - } - public boolean isTraceOtelEnabled() { return traceOtelEnabled; } From 993c70803191468a09043110cd4c437eafe9d56b Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 22 May 2024 10:27:57 +0200 Subject: [PATCH 13/62] Refactor required sampler and add test --- .../ExperimentalAppSecStandaloneSampler.java | 38 --------------- .../trace/common/sampling/Sampler.java | 2 +- .../trace/common/sampling/TimeSampler.java | 46 +++++++++++++++++++ .../common/sampling/TimeSamplerTest.groovy | 44 ++++++++++++++++++ 4 files changed, 91 insertions(+), 39 deletions(-) delete mode 100644 dd-trace-core/src/main/java/datadog/trace/common/sampling/ExperimentalAppSecStandaloneSampler.java create mode 100644 dd-trace-core/src/main/java/datadog/trace/common/sampling/TimeSampler.java create mode 100644 dd-trace-core/src/test/groovy/datadog/trace/common/sampling/TimeSamplerTest.groovy diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ExperimentalAppSecStandaloneSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ExperimentalAppSecStandaloneSampler.java deleted file mode 100644 index f8aef8ef4d8..00000000000 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ExperimentalAppSecStandaloneSampler.java +++ /dev/null @@ -1,38 +0,0 @@ -package datadog.trace.common.sampling; - -import datadog.trace.api.sampling.SamplingMechanism; -import datadog.trace.core.CoreSpan; -import datadog.trace.core.util.SimpleRateLimiter; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class ExperimentalAppSecStandaloneSampler implements Sampler, PrioritySampler { - - private static final Logger log = - LoggerFactory.getLogger(ExperimentalAppSecStandaloneSampler.class); - - private final SimpleRateLimiter rateLimiter; - - public ExperimentalAppSecStandaloneSampler() { - this.rateLimiter = new SimpleRateLimiter(60); // one per minute - } - - @Override - public > boolean sample(final T span) { - // Priority sampling sends all traces to the core agent, including traces marked dropped. - // This allows the core agent to collect stats on all traces. - return true; - } - - @Override - public > void setSamplingPriority(final T span) { - - // TODO check how to short circuit this for ASM - - if (rateLimiter.tryAcquire()) { - span.setSamplingPriority(PrioritySampling.SAMPLER_KEEP, SamplingMechanism.DEFAULT); - } else { - span.setSamplingPriority(PrioritySampling.SAMPLER_DROP, SamplingMechanism.DEFAULT); - } - } -} diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java index 353a1b23b3d..134f3d5d6fc 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java @@ -36,7 +36,7 @@ public static Sampler forConfig(final Config config, final TraceConfig traceConf if (config != null) { if (config.isExperimentalAppSecStandaloneEnabled()) { log.debug("APM is disabled. Only 1 trace per minute will be sent."); - return new ExperimentalAppSecStandaloneSampler(); + return new TimeSampler(60000); // 1 trace per minute } final Map serviceRules = config.getTraceSamplingServiceRules(); final Map operationRules = config.getTraceSamplingOperationRules(); diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/TimeSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/TimeSampler.java new file mode 100644 index 00000000000..9757576e960 --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/TimeSampler.java @@ -0,0 +1,46 @@ +package datadog.trace.common.sampling; + +import datadog.trace.api.sampling.SamplingMechanism; +import datadog.trace.core.CoreSpan; +import java.util.concurrent.atomic.AtomicLong; + +/** Sampler that samples traces based on a fixed time rate in milliseconds. */ +public class TimeSampler implements Sampler, PrioritySampler { + + private final AtomicLong lastSampleTime; + private final int rateInMilliseconds; + + public TimeSampler(int rateInMilliseconds) { + this.rateInMilliseconds = rateInMilliseconds; + this.lastSampleTime = new AtomicLong(-1); + } + + @Override + public > boolean sample(final T span) { + // Priority sampling sends all traces to the core agent, including traces marked dropped. + // This allows the core agent to collect stats on all traces. + return true; + } + + @Override + public > void setSamplingPriority(final T span) { + + if (shouldSample()) { + span.setSamplingPriority(PrioritySampling.SAMPLER_KEEP, SamplingMechanism.DEFAULT); + } else { + span.setSamplingPriority(PrioritySampling.SAMPLER_DROP, SamplingMechanism.DEFAULT); + } + } + + private boolean shouldSample() { + long now = System.currentTimeMillis(); + long lastTime = lastSampleTime.get(); + + if (lastTime == -1 || now - lastTime >= rateInMilliseconds) { + if (lastSampleTime.compareAndSet(lastTime, now)) { + return true; + } + } + return false; + } +} diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/TimeSamplerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/TimeSamplerTest.groovy new file mode 100644 index 00000000000..e5724e35146 --- /dev/null +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/TimeSamplerTest.groovy @@ -0,0 +1,44 @@ +package datadog.trace.common.sampling + +import datadog.trace.common.writer.ListWriter +import datadog.trace.core.test.DDCoreSpecification +import datadog.trace.api.sampling.PrioritySampling + +class TimeSamplerTest extends DDCoreSpecification{ + + def writer = new ListWriter() + + void "test setSamplingPriority"(){ + setup: + final rate = 1000 // 1 trace per second + def sampler = new TimeSampler(rate) + def tracer = tracerBuilder().writer(writer).sampler(sampler).build() + + when: + def span1 = tracer.buildSpan("test").start() + sampler.setSamplingPriority(span1) + + then: + span1.getSamplingPriority() == PrioritySampling.SAMPLER_KEEP + + when: + def span2 = tracer.buildSpan("test2").start() + sampler.setSamplingPriority(span2) + + then: + span2.getSamplingPriority() == PrioritySampling.SAMPLER_DROP + + when: + def span3 = tracer.buildSpan("test3").start() + + then: "we wait for one second" + Thread.sleep(rate) + sampler.setSamplingPriority(span3) + + and: + span3.getSamplingPriority() == PrioritySampling.SAMPLER_KEEP + + cleanup: + tracer.close() + } +} From 5fb0d2a6ab0b5d8db4f12acfe7a1bf211f28442b Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 22 May 2024 11:45:52 +0200 Subject: [PATCH 14/62] Add Sampler test --- .../trace/common/sampling/SamplerTest.groovy | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy new file mode 100644 index 00000000000..fa4b21c2e11 --- /dev/null +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy @@ -0,0 +1,20 @@ +package datadog.trace.common.sampling + +import datadog.trace.api.Config +import datadog.trace.test.util.DDSpecification + +class SamplerTest extends DDSpecification{ + + void "test that TimeSampler is selected when experimentalAppSecStandalone is enabled"() { + setup: + System.setProperty("dd.experimental.appsec.standalone.enabled", "true") + Config config = new Config() + + when: + Sampler sampler = Sampler.Builder.forConfig(config, null) + + then: + sampler instanceof TimeSampler + ((TimeSampler)sampler).rateInMilliseconds == 60000 //1 minute + } +} From 87df43d5fc1f7436fb9393b82280fd4b8353b894 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 22 May 2024 13:57:30 +0200 Subject: [PATCH 15/62] introduce a new propagated span tag _dd.p.appsec: 1 providing the knowledge to downstream services that the current distributed trace is containing at least one ASM event and must inherit from the given force-keep priority indeed. --- .../appsec/gateway/AppSecRequestContext.java | 14 +++++++------ .../java/datadog/trace/core/PendingTrace.java | 21 +++++++++++++++---- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java index f01cdf7009b..7d4c0a0f31e 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java @@ -6,7 +6,6 @@ import com.datadog.appsec.stack_trace.StackTraceCollection; import com.datadog.appsec.stack_trace.StackTraceEvent; import com.datadog.appsec.util.StandardizedLogging; -import datadog.trace.api.Config; import datadog.trace.api.http.StoredBodySupplier; import datadog.trace.api.internal.TraceSegment; import io.sqreen.powerwaf.Additive; @@ -418,11 +417,14 @@ public void reportEvents(Collection appSecEvents) { for (AppSecEvent event : appSecEvents) { StandardizedLogging.attackDetected(log, event); } - if (this.appSecEvents == null) { - synchronized (this) { - if (this.appSecEvents == null) { - this.appSecEvents = new ConcurrentLinkedQueue<>(); - } + synchronized (this) { + if (this.collectedEvents == null) { + this.collectedEvents = new ArrayList<>(); + } + try { + this.collectedEvents.addAll(events); + } catch (UnsupportedOperationException e) { + throw new IllegalStateException("Events cannot be added anymore"); } } this.appSecEvents.addAll(appSecEvents); 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 5f8c4c9813c..5452e23860c 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 @@ -1,6 +1,7 @@ package datadog.trace.core; import datadog.communication.monitor.Recording; +import datadog.trace.api.Config; import datadog.trace.api.DDTraceId; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.time.TimeSource; @@ -489,14 +490,26 @@ public void setSamplingPriorityIfNecessary() { // This check skips potential complex sampling priority logic when we know its redundant // Locks inside DDSpanContext ensure the correct behavior in the race case - if (traceConfig.sampler instanceof PrioritySampler - && rootSpan != null - && rootSpan.context().getSamplingPriority() == PrioritySampling.UNSET) { + if (traceConfig.sampler instanceof PrioritySampler && rootSpan != null) { - ((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan); + // TODO check if this is the place to ignore the force-keep priority in the absence of + // propagated _dd.p.appsec span tag. + if (Config.get().isExperimentalAppSecStandaloneEnabled() + && rootSpan.getTag("_dd.p.appsec") != null) { + setSamplingPriority(); + return; + } + + if (rootSpan.context().getSamplingPriority() == PrioritySampling.UNSET) { + setSamplingPriority(); + } } } + private void setSamplingPriority() { + ((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan); + } + public boolean sample(DDSpan spanToSample) { return traceConfig.sampler.sample(spanToSample); } From 3dfeb580861b06413ec7d3dc48f1674267dae0c3 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 22 May 2024 18:02:15 +0200 Subject: [PATCH 16/62] self code review corrections --- dd-trace-api/src/main/java/datadog/trace/api/DDTags.java | 2 +- .../src/main/java/datadog/trace/core/PendingTrace.java | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java index 5056ef276d1..ee09eb80bad 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java @@ -74,5 +74,5 @@ public class DDTags { public static final String PARENT_ID = "_dd.parent_id"; public static final String APM_ENABLED = "_dd.apm.enabled"; - public static final String APPSEC = "_dd.p.appsec"; + public static final String PROPAGATED_APPSEC = "_dd.p.appsec"; } 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 5452e23860c..8ebcc2cf0b6 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 @@ -1,5 +1,7 @@ package datadog.trace.core; +import static datadog.trace.api.DDTags.PROPAGATED_APPSEC; + import datadog.communication.monitor.Recording; import datadog.trace.api.Config; import datadog.trace.api.DDTraceId; @@ -492,10 +494,9 @@ public void setSamplingPriorityIfNecessary() { if (traceConfig.sampler instanceof PrioritySampler && rootSpan != null) { - // TODO check if this is the place to ignore the force-keep priority in the absence of - // propagated _dd.p.appsec span tag. + // Ignore the force-keep priority in the absence of propagated _dd.p.appsec span tag. if (Config.get().isExperimentalAppSecStandaloneEnabled() - && rootSpan.getTag("_dd.p.appsec") != null) { + && rootSpan.getTag(PROPAGATED_APPSEC) == null) { setSamplingPriority(); return; } From 950cf681c998501985fba3689ea6a62adfb67345 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 30 May 2024 10:21:43 +0200 Subject: [PATCH 17/62] Add new appsec ad hoc propagation tag --- .../main/java/com/datadog/iast/Reporter.java | 1 + .../trace/common/sampling/TimeSampler.java | 6 ++++++ .../java/datadog/trace/core/DDSpanContext.java | 4 ++++ .../java/datadog/trace/core/PendingTrace.java | 4 +--- .../trace/core/propagation/PropagationTags.java | 5 +++++ .../core/propagation/ptags/PTagsCodec.java | 9 +++++++++ .../core/propagation/ptags/PTagsFactory.java | 17 +++++++++++++++++ .../core/taginterceptor/TagInterceptor.java | 3 +++ 8 files changed, 46 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java index 0e1a22c5a3a..cde0684a935 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java @@ -98,6 +98,7 @@ private VulnerabilityBatch getOrCreateVulnerabilityBatch(final AgentSpan span) { // TODO: We need to check if we can have an API with more fine-grained semantics on why traces // are kept. segment.setTagTop(Tags.ASM_KEEP, true); + segment.setTagTop(Tags.PROPAGATED_APPSEC, "1"); return batch; } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/TimeSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/TimeSampler.java index 9757576e960..eb6a09e1a99 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/TimeSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/TimeSampler.java @@ -3,10 +3,14 @@ import datadog.trace.api.sampling.SamplingMechanism; import datadog.trace.core.CoreSpan; import java.util.concurrent.atomic.AtomicLong; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** Sampler that samples traces based on a fixed time rate in milliseconds. */ public class TimeSampler implements Sampler, PrioritySampler { + private static final Logger log = LoggerFactory.getLogger(TimeSampler.class); + private final AtomicLong lastSampleTime; private final int rateInMilliseconds; @@ -26,8 +30,10 @@ public > boolean sample(final T span) { public > void setSamplingPriority(final T span) { if (shouldSample()) { + log.debug("Set SAMPLER_KEEP for span {}", span.getSpanId()); span.setSamplingPriority(PrioritySampling.SAMPLER_KEEP, SamplingMechanism.DEFAULT); } else { + log.debug("Set SAMPLER_DROP for span {}", span.getSpanId()); span.setSamplingPriority(PrioritySampling.SAMPLER_DROP, SamplingMechanism.DEFAULT); } } 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 ddbe110fb5d..cd73494e60b 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 @@ -511,6 +511,10 @@ private void forceKeepThisSpan(byte samplingMechanism) { } } + public void addAppsecPropagationTag() { + propagationTags.updateAppsecPropagation(true); + } + /** @return if sampling priority was set by this method invocation */ public boolean setSamplingPriority(final int newPriority, final int newMechanism) { DDSpanContext spanContext = getRootSpanContextOrThis(); 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 8ebcc2cf0b6..1d86dc21455 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 @@ -1,7 +1,5 @@ package datadog.trace.core; -import static datadog.trace.api.DDTags.PROPAGATED_APPSEC; - import datadog.communication.monitor.Recording; import datadog.trace.api.Config; import datadog.trace.api.DDTraceId; @@ -496,7 +494,7 @@ public void setSamplingPriorityIfNecessary() { // Ignore the force-keep priority in the absence of propagated _dd.p.appsec span tag. if (Config.get().isExperimentalAppSecStandaloneEnabled() - && rootSpan.getTag(PROPAGATED_APPSEC) == null) { + && !rootSpan.context().getPropagationTags().isAppsecPropagationEnabled()) { setSamplingPriority(); return; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java index c898381576e..55c4d1d79d9 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java @@ -100,6 +100,11 @@ public interface Factory { */ public abstract void fillTagMap(Map tagMap); + /** Add the appsec propagation tag to the propagation tags. */ + public abstract void updateAppsecPropagation(boolean enabled); + + public abstract boolean isAppsecPropagationEnabled(); + public HashMap createTagMap() { HashMap result = new HashMap<>(); fillTagMap(result); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java index 2379c39d360..b7aa7486dbb 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java @@ -15,6 +15,7 @@ abstract class PTagsCodec { protected static final TagKey DECISION_MAKER_TAG = TagKey.from("dm"); protected static final TagKey TRACE_ID_TAG = TagKey.from("tid"); + protected static final TagKey APPSEC_TAG = TagKey.from("appsec"); protected static final String PROPAGATION_ERROR_MALFORMED_TID = "malformed_tid "; protected static final String PROPAGATION_ERROR_INCONSISTENT_TID = "inconsistent_tid "; protected static final TagKey UPSTREAM_SERVICES_DEPRECATED_TAG = TagKey.from("upstream_services"); @@ -35,6 +36,9 @@ static String headerValue(PTagsCodec codec, PTags ptags) { if (ptags.getTraceIdHighOrderBitsHexTagValue() != null) { size = codec.appendTag(sb, TRACE_ID_TAG, ptags.getTraceIdHighOrderBitsHexTagValue(), size); } + if (ptags.isAppsecPropagationEnabled()) { + size = codec.appendTag(sb, APPSEC_TAG, TagValue.from("1"), size); + } Iterator it = ptags.getTagPairs().iterator(); while (it.hasNext() && !codec.isTooLarge(sb, size)) { TagElement tagKey = it.next(); @@ -78,6 +82,11 @@ static void fillTagMap(PTags propagationTags, Map tagMap) { DECISION_MAKER_TAG.forType(Encoding.DATADOG).toString(), propagationTags.getDecisionMakerTagValue().forType(Encoding.DATADOG).toString()); } + if (propagationTags.isAppsecPropagationEnabled()) { + tagMap.put( + APPSEC_TAG.forType(Encoding.DATADOG).toString(), + TagValue.from("1").forType(Encoding.DATADOG).toString()); + } if (propagationTags.getTraceIdHighOrderBitsHexTagValue() != null) { tagMap.put( TRACE_ID_TAG.forType(Encoding.DATADOG).toString(), diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index e60643a73f2..8ac6e49506a 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -75,6 +75,8 @@ static class PTags extends PropagationTags { // extracted decision maker tag for easier updates private volatile TagValue decisionMakerTagValue; + private volatile boolean appsecPropagationEnabled; + // xDatadogTagsSize of the tagPairs, does not include the decision maker tag private volatile int xDatadogTagsSize = -1; @@ -186,6 +188,21 @@ public void updateTraceSamplingPriority(int samplingPriority, int samplingMechan } } + @Override + public void updateAppsecPropagation(boolean enabled) { + if (appsecPropagationEnabled != enabled) { + // This should invalidate any cached w3c and datadog header + clearCachedHeader(DATADOG); + clearCachedHeader(W3C); + } + appsecPropagationEnabled = enabled; + } + + @Override + public boolean isAppsecPropagationEnabled() { + return appsecPropagationEnabled; + } + @Override public int getSamplingPriority() { return samplingPriority; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index e7fda0f8ce4..27f139485f9 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -108,6 +108,9 @@ public boolean interceptTag(DDSpanContext span, String tag, Object value) { return false; case Tags.SAMPLING_PRIORITY: return interceptSamplingPriority(span, value); + case DDTags.PROPAGATED_APPSEC: + span.addAppsecPropagationTag(); + return true; case InstrumentationTags.SERVLET_CONTEXT: return interceptServletContext(span, value); case SPAN_TYPE: From 03da7bc92292bf6bf8061ed1e21138fd7a888127 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 30 May 2024 10:56:35 +0200 Subject: [PATCH 18/62] move p.appsec from DDtags to Tags --- dd-trace-api/src/main/java/datadog/trace/api/DDTags.java | 2 -- .../java/datadog/trace/core/taginterceptor/TagInterceptor.java | 2 +- .../java/datadog/trace/bootstrap/instrumentation/api/Tags.java | 2 ++ 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java index ee09eb80bad..de82229bbcc 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java @@ -73,6 +73,4 @@ public class DDTags { public static final String BASE_SERVICE = "_dd.base_service"; public static final String PARENT_ID = "_dd.parent_id"; public static final String APM_ENABLED = "_dd.apm.enabled"; - - public static final String PROPAGATED_APPSEC = "_dd.p.appsec"; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index 27f139485f9..3efbaa0524e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -108,7 +108,7 @@ public boolean interceptTag(DDSpanContext span, String tag, Object value) { return false; case Tags.SAMPLING_PRIORITY: return interceptSamplingPriority(span, value); - case DDTags.PROPAGATED_APPSEC: + case Tags.PROPAGATED_APPSEC: span.addAppsecPropagationTag(); return true; case InstrumentationTags.SERVLET_CONTEXT: diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java index 0dccae4c366..beda142f321 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java @@ -129,4 +129,6 @@ public class Tags { /** ASM force tracer to keep the trace */ public static final String ASM_KEEP = "asm.keep"; + + public static final String PROPAGATED_APPSEC = "p.appsec"; } From 5ac2827aa79c92d39ecd8471cfe81c1a2c9a0562 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 30 May 2024 10:58:48 +0200 Subject: [PATCH 19/62] Add unit test to TagInterceptor --- .../core/taginterceptor/TagInterceptorTest.groovy | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy index d7e4d5ed4ce..0765ece9c0f 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy @@ -729,4 +729,18 @@ class TagInterceptorTest extends DDCoreSpecification { "/test" | "test" "test" | "test" } + + void "When intercepts appsec propagation tag addAppsecPropagationTag is called"(){ + setup: + final ruleFlags = Mock(RuleFlags) + ruleFlags.isEnabled(_) >> true + final interceptor = new TagInterceptor(ruleFlags) + final context = Mock(DDSpanContext) + + when: + interceptor.interceptTag(context, Tags.PROPAGATED_APPSEC, "1") + + then: + 1 * context.addAppsecPropagationTag() + } } From f991aa159f7cd6c2581eb8748bcbb9dfd3aa6b3f Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 31 May 2024 12:43:59 +0200 Subject: [PATCH 20/62] Add smoke tests --- .../asm-standalone-billing/build.gradle | 31 +++++ .../asmstandalonebilling/AppConfig.java | 25 ++++ .../asmstandalonebilling/Controller.java | 82 +++++++++++++ .../SpringbootApplication.java | 11 ++ ...stractAsmStandaloneBillingSmokeTest.groovy | 63 ++++++++++ ...mStandaloneBillingSamplingSmokeTest.groovy | 69 +++++++++++ .../AsmStandaloneBillingSmokeTest.groovy | 115 ++++++++++++++++++ settings.gradle | 1 + 8 files changed, 397 insertions(+) create mode 100644 dd-smoke-tests/asm-standalone-billing/build.gradle create mode 100644 dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/AppConfig.java create mode 100644 dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java create mode 100644 dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/SpringbootApplication.java create mode 100644 dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy create mode 100644 dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy create mode 100644 dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy diff --git a/dd-smoke-tests/asm-standalone-billing/build.gradle b/dd-smoke-tests/asm-standalone-billing/build.gradle new file mode 100644 index 00000000000..d5f5774ef34 --- /dev/null +++ b/dd-smoke-tests/asm-standalone-billing/build.gradle @@ -0,0 +1,31 @@ +plugins { + id 'java' + id 'org.springframework.boot' version '2.7.15' + id 'io.spring.dependency-management' version '1.0.15.RELEASE' + id 'java-test-fixtures' +} + +apply from: "$rootDir/gradle/java.gradle" +description = 'ASM Standalone Billing Tests.' + +java { + sourceCompatibility = '1.8' +} + +repositories { + mavenCentral() +} + +dependencies { + implementation 'org.springframework.boot:spring-boot-starter-web' + implementation group: 'io.opentracing', name: 'opentracing-api', version: '0.32.0' + implementation group: 'io.opentracing', name: 'opentracing-util', version: '0.32.0' + implementation project(':dd-trace-api') + testImplementation project(':dd-smoke-tests') + testImplementation(testFixtures(project(":dd-smoke-tests:iast-util"))) +} + +tasks.withType(Test).configureEach { + dependsOn "bootJar" + jvmArgs "-Ddatadog.smoketest.springboot.shadowJar.path=${tasks.bootJar.archiveFile.get()}" +} diff --git a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/AppConfig.java b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/AppConfig.java new file mode 100644 index 00000000000..3db04b37581 --- /dev/null +++ b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/AppConfig.java @@ -0,0 +1,25 @@ +package datadog.smoketest.asmstandalonebilling; + +import java.util.EnumSet; +import javax.servlet.ServletContext; +import javax.servlet.ServletException; +import javax.servlet.SessionTrackingMode; +import org.springframework.boot.web.servlet.ServletContextInitializer; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class AppConfig { + @Bean + public ServletContextInitializer servletContextInitializer() { + return new SessionTrackingConfig(); + } + + private class SessionTrackingConfig implements ServletContextInitializer { + @Override + public void onStartup(ServletContext servletContext) throws ServletException { + EnumSet sessionTrackingModes = EnumSet.of(SessionTrackingMode.COOKIE); + servletContext.setSessionTrackingModes(sessionTrackingModes); + } + } +} diff --git a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java new file mode 100644 index 00000000000..e8db5f60eda --- /dev/null +++ b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java @@ -0,0 +1,82 @@ +package datadog.smoketest.asmstandalonebilling; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.opentracing.Span; +import io.opentracing.util.GlobalTracer; +import java.io.IOException; +import javax.servlet.http.HttpServletResponse; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.client.RestTemplate; + +@RestController +@RequestMapping("/rest-api") +public class Controller { + + @GetMapping("/greetings") + public String greetings(@RequestParam(name = "forceKeep", required = false) boolean forceKeep) { + if (forceKeep) { + forceKeepSpan(); + } + return "Hello I'm service " + System.getProperty("dd.service.name"); + } + + @GetMapping("/appsec/{id}") + public String pathParam( + @PathVariable("id") String id, + @RequestParam(name = "url", required = false) String url, + @RequestParam(name = "forceKeep", required = false) boolean forceKeep) { + if (forceKeep) { + forceKeepSpan(); + } + if (url != null) { + RestTemplate restTemplate = new RestTemplate(); + return restTemplate.getForObject(url, String.class); + } + return id; + } + + @GetMapping("/iast") + @SuppressFBWarnings + public void write( + @RequestParam(name = "injection", required = false) String injection, + @RequestParam(name = "url", required = false) String url, + @RequestParam(name = "forceKeep", required = false) boolean forceKeep, + final HttpServletResponse response) { + if (forceKeep) { + forceKeepSpan(); + } + if (injection != null) { + try { + response.getWriter().write(injection); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + if (url != null) { + RestTemplate restTemplate = new RestTemplate(); + restTemplate.getForObject(url, String.class); + } + } + + /** + * @GetMapping("/forcekeep") public String forceKeep() { return "Span " + forceKeepSpan() + " will + * be kept alive"; } @GetMapping("/call") public String call( @RequestParam(name = "url", required + * = false) String url, @RequestParam(name = "forceKeep", required = false) boolean forceKeep) { + * if (forceKeep) { forceKeepSpan(); } if (url != null) { RestTemplate restTemplate = new + * RestTemplate(); return restTemplate.getForObject(url, String.class); } return "No url + * provided"; } + */ + private String forceKeepSpan() { + // TODO: Configure the keep alive in dd-trace-api + final Span span = GlobalTracer.get().activeSpan(); + if (span != null) { + span.setTag("manual.keep", true); + return span.context().toSpanId(); + } + return null; + } +} diff --git a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/SpringbootApplication.java b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/SpringbootApplication.java new file mode 100644 index 00000000000..0ac36d7d2f7 --- /dev/null +++ b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/SpringbootApplication.java @@ -0,0 +1,11 @@ +package datadog.smoketest.asmstandalonebilling; + +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; + +@SpringBootApplication +public class SpringbootApplication { + public static void main(String[] args) { + SpringApplication.run(SpringbootApplication.class, args); + } +} diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy new file mode 100644 index 00000000000..201236f9023 --- /dev/null +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy @@ -0,0 +1,63 @@ +package datadog.smoketest.asmstandalonebilling + +import datadog.smoketest.AbstractServerSmokeTest +import datadog.trace.api.sampling.PrioritySampling +import datadog.trace.test.agent.decoder.DecodedTrace + +abstract class AbstractAsmStandaloneBillingSmokeTest extends AbstractServerSmokeTest { + + @Override + File createTemporaryFile(int processIndex) { + return null + } + + @Override + String logLevel() { + return 'debug' + } + + @Override + Closure decodedTracesCallback() { + return {} // force traces decoding + } + + protected ProcessBuilder createProcess(String[] properties){ + createProcess(-1, properties) + } + + + protected ProcessBuilder createProcess(int processIndex, String[] properties){ + def port = processIndex == -1 ? httpPort : httpPorts[processIndex] + String springBootShadowJar = System.getProperty("datadog.smoketest.springboot.shadowJar.path") + List command = [] + command.add(javaPath()) + command.addAll(defaultJavaProperties) + command.addAll(properties) + command.addAll((String[]) ['-jar', springBootShadowJar, "--server.port=${port}"]) + ProcessBuilder processBuilder = new ProcessBuilder(command) + processBuilder.directory(new File(buildDirectory)) + // Spring will print all environment variables to the log, which may pollute it and affect log assertions. + processBuilder.environment().clear() + return processBuilder + } + + protected DecodedTrace getServiceTrace(String serviceName) { + return traces.find { trace -> + trace.spans.find { span -> + span.service == serviceName + } + } + } + + protected checkRootSpanPrioritySampling(DecodedTrace trace, byte priority) { + return trace.spans[0].metrics['_sampling_priority_v1'] == priority + } + + protected hasAppsecPropagationTag(DecodedTrace trace) { + return trace.spans[0].meta['_dd.p.appsec'] == "1" + } + + protected hasApmDisabledTag(DecodedTrace trace) { + return trace.spans[0].metrics['_dd.apm.enabled'] == 0 + } +} diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy new file mode 100644 index 00000000000..981624a7bda --- /dev/null +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy @@ -0,0 +1,69 @@ +package datadog.smoketest.asmstandalonebilling + +import datadog.trace.api.sampling.PrioritySampling +import okhttp3.Request + +class AsmStandaloneBillingSamplingSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { + + @Override + ProcessBuilder createProcessBuilder(){ + final String[] processProperties = [ + "-Ddd.experimental.appsec.standalone.enabled=true", + "-Ddd.iast.enabled=true", + "-Ddd.iast.detection.mode=FULL", + "-Ddd.iast.debug.enabled=true", + "-Ddd.service.name=asm-standalone-billing-sampling-spring-smoketest-app", + ] + return createProcess(processProperties) + } + + void 'test force keep call using time sampling'() { + setup: + final vulnerableUrl = "http://localhost:${httpPorts[0]}/rest-api/iast?injection=xss" + final vulnerableRequest = new Request.Builder().url(vulnerableUrl).get().build() + final forceKeepUrl = "http://localhost:${httpPorts[0]}/rest-api/greetings?forceKeep=true" + final forceKeepRequest = new Request.Builder().url(forceKeepUrl).get().build() + final noForceKeepUrl = "http://localhost:${httpPorts[0]}/rest-api/greetings" + final noForceKeepRequest = new Request.Builder().url(noForceKeepUrl).get().build() + + when: "firs request with ASM events" + final vulnerableResponse = client.newCall(vulnerableRequest).execute() + + then: "First trace should have a root span with USER_KEEP sampling priority due to ASM events" + vulnerableResponse.successful + waitForTraceCount(1) + assert traces.size() == 1 + checkRootSpanPrioritySampling(traces[0], PrioritySampling.USER_KEEP) + hasAppsecPropagationTag(traces[0]) + + when: "Request without ASM events and no force kept span" + final noForceKeepResponse = client.newCall(noForceKeepRequest).execute() + + then: "This trace should enter into the sampling mechanism and have a root span with SAMPLER_KEEP sampling priority as it's the first span checked in a minute" + noForceKeepResponse.successful + waitForTraceCount(2) + assert traces.size() == 2 + checkRootSpanPrioritySampling(traces[1], PrioritySampling.SAMPLER_KEEP) + !hasAppsecPropagationTag(traces[1]) + + when: "Request without ASM events and force kept span" + final forceKeepResponse = client.newCall(forceKeepRequest).execute() + + then: "This trace should have a root span with USER_KEEP sampling priority as although it's not the first span checked in a minute, it's force kept'" + forceKeepResponse.successful + waitForTraceCount(3) + assert traces.size() == 3 + checkRootSpanPrioritySampling(traces[2], PrioritySampling.USER_KEEP) + !hasAppsecPropagationTag(traces[2]) + + when: "Second request without ASM events and no force kept span" + final noForceKeepResponse2 = client.newCall(noForceKeepRequest).execute() + + then: "This trace should enter into the sampling mechanism and have a root span with SAMPLER_DROP sampling priority as it's not the first span checked in a minute" + noForceKeepResponse2.successful + waitForTraceCount(4) + assert traces.size() == 4 + checkRootSpanPrioritySampling(traces[3], PrioritySampling.SAMPLER_DROP) + !hasAppsecPropagationTag(traces[3]) + } +} diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy new file mode 100644 index 00000000000..afd5310d7f8 --- /dev/null +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy @@ -0,0 +1,115 @@ +package datadog.smoketest.asmstandalonebilling + +import okhttp3.Request + +class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { + + private static final String standAloneBillingServiceName = "asm-standalone-billing-smoketest-app" + private static final String apmEnabledServiceName = "apm-enabled-smoketest-app" + + static final String[] standAloneBillingProperties = [ + "-Ddd.experimental.appsec.standalone.enabled=true", + "-Ddd.iast.enabled=true", + "-Ddd.iast.detection.mode=FULL", + "-Ddd.iast.debug.enabled=true", + "-Ddd.appsec.enabled=true", + "-Ddd.trace.tracer.metrics.enabled=true", + "-Ddd.service.name=${standAloneBillingServiceName}", + ] + + static final String[] apmEnabledProperties = ["-Ddd.service.name=${apmEnabledServiceName}", "-Ddd.trace.tracer.metrics.enabled=true",] + + protected int numberOfProcesses() { + return 2 + } + + @Override + ProcessBuilder createProcessBuilder(int processIndex) { + if(processIndex == 0){ + return createProcess(processIndex, standAloneBillingProperties) + } else { + return createProcess(processIndex, apmEnabledProperties) + } + } + + void 'When APM is disabled, numeric tag _dd.apm.enabled:0 must be added to the metrics map of the service entry spans.'() { + setup: + final url1 = "http://localhost:${httpPorts[0]}/rest-api/greetings" + final request1 = new Request.Builder().url(url1).get().build() + final url2 = "http://localhost:${httpPorts[1]}/rest-api/greetings" + final request2 = new Request.Builder().url(url2).get().build() + + when: + final response1 = client.newCall(request1).execute() + final response2 = client.newCall(request2).execute() + + then: + response1.successful + response2.successful + waitForTraceCount(2) + hasApmDisabledTag(getServiceTrace(standAloneBillingServiceName)) + !hasApmDisabledTag(getServiceTrace(apmEnabledServiceName)) + } + + void 'When APM is disabled, libraries must completely disable the generation of APM trace metrics'(){ + setup: + final url1 = "http://localhost:${httpPorts[0]}/rest-api/greetings" + final request1 = new Request.Builder().url(url1).get().build() + + when: + client.newCall(request1).execute() + + then: 'Check if Datadog-Client-Computed-Stats header is present and set to true, Disabling the metrics computation agent-side' + waitForTraceCount(1) + def computedStatsHeader = server.lastRequest.headers.get('Datadog-Client-Computed-Stats') + assert computedStatsHeader != null && computedStatsHeader == 'true' + + then:'metrics should be disabled' + checkLogPostExit { log -> + if (log.contains('datadog.trace.agent.common.metrics.MetricsAggregatorFactory - tracer metrics disabled')) { + return true + } + return false + } + } + + void 'test _dd.p.appsec propagation for appsec event'() { + setup: + final downstreamUrl = "http://localhost:${httpPorts[1]}/rest-api/greetings" + final url = localUrl + "url=${downstreamUrl}" + final request = new Request.Builder().url(url).get().build() + + when: "Request to an endpoint that triggers ASM events and then calls another endpoint" + final response1 = client.newCall(request).execute() + + then: "Both traces should have a root span with _dd.p.appsec=1 tag" + response1.successful + waitForTraceCount(2) + assert traces.size() == 2 + hasAppsecPropagationTag(traces.get(0)) + hasAppsecPropagationTag(traces.get(1)) + + where: + localUrl << [ + "http://localhost:${httpPorts[0]}/rest-api/appsec/appscan_fingerprint?", + "http://localhost:${httpPorts[0]}/rest-api/iast?injection=vulnerable&" + ] + } + + /* + void 'test _dd.p.appsec propagation for iast event'() { + setup: + final downstreamUrl = "http://localhost:${httpPorts[1]}/rest-api/greetings" + final url = "http://localhost:${httpPorts[0]}/rest-api/iast?injection=vulnerable&url=${downstreamUrl}" + final request = new Request.Builder().url(url1).get().build() + when: "Request to an endpoint that triggers ASM events and then calls another endpoint" + final response1 = client.newCall(request1).execute() + then: "Both traces should have a root span with _dd.p.appsec=1 tag" + response1.successful + waitForTraceCount(2) + assert traces.size() == 2 + hasAppsecPropagationTag(traces.get(0)) + hasAppsecPropagationTag(traces.get(1)) + } + */ +} diff --git a/settings.gradle b/settings.gradle index 0d21b007e57..310ea686bed 100644 --- a/settings.gradle +++ b/settings.gradle @@ -91,6 +91,7 @@ include ':utils:version-utils' // smoke tests include ':dd-smoke-tests:armeria-grpc' +include ':dd-smoke-tests:asm-standalone-billing' include ':dd-smoke-tests:backend-mock' include ':dd-smoke-tests:cli' include ':dd-smoke-tests:custom-systemloader' From 132eb2f6126191146b7d5417c18ca5538505ef85 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 31 May 2024 12:55:02 +0200 Subject: [PATCH 21/62] clean Controller --- .../smoketest/asmstandalonebilling/Controller.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java index e8db5f60eda..6d2e673a7e5 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java +++ b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java @@ -62,16 +62,7 @@ public void write( } } - /** - * @GetMapping("/forcekeep") public String forceKeep() { return "Span " + forceKeepSpan() + " will - * be kept alive"; } @GetMapping("/call") public String call( @RequestParam(name = "url", required - * = false) String url, @RequestParam(name = "forceKeep", required = false) boolean forceKeep) { - * if (forceKeep) { forceKeepSpan(); } if (url != null) { RestTemplate restTemplate = new - * RestTemplate(); return restTemplate.getForObject(url, String.class); } return "No url - * provided"; } - */ private String forceKeepSpan() { - // TODO: Configure the keep alive in dd-trace-api final Span span = GlobalTracer.get().activeSpan(); if (span != null) { span.setTag("manual.keep", true); From 082e43338930570a0c047ca4a19cc71c875d67f2 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 31 May 2024 12:57:49 +0200 Subject: [PATCH 22/62] clean test --- .../AsmStandaloneBillingSmokeTest.groovy | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy index afd5310d7f8..797d91bfb09 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy @@ -95,21 +95,4 @@ class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTes "http://localhost:${httpPorts[0]}/rest-api/iast?injection=vulnerable&" ] } - - /* - void 'test _dd.p.appsec propagation for iast event'() { - setup: - final downstreamUrl = "http://localhost:${httpPorts[1]}/rest-api/greetings" - final url = "http://localhost:${httpPorts[0]}/rest-api/iast?injection=vulnerable&url=${downstreamUrl}" - final request = new Request.Builder().url(url1).get().build() - when: "Request to an endpoint that triggers ASM events and then calls another endpoint" - final response1 = client.newCall(request1).execute() - then: "Both traces should have a root span with _dd.p.appsec=1 tag" - response1.successful - waitForTraceCount(2) - assert traces.size() == 2 - hasAppsecPropagationTag(traces.get(0)) - hasAppsecPropagationTag(traces.get(1)) - } - */ } From 687a713f4ca9135b37076d97cbdd01866f516297 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 31 May 2024 13:02:53 +0200 Subject: [PATCH 23/62] Avoid possible cast exception --- .../main/java/datadog/trace/core/PendingTrace.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) 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 1d86dc21455..c77a3da9339 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 @@ -489,26 +489,19 @@ public void setSamplingPriorityIfNecessary() { // There's a race where multiple threads can see PrioritySampling.UNSET here // This check skips potential complex sampling priority logic when we know its redundant // Locks inside DDSpanContext ensure the correct behavior in the race case - if (traceConfig.sampler instanceof PrioritySampler && rootSpan != null) { - // Ignore the force-keep priority in the absence of propagated _dd.p.appsec span tag. if (Config.get().isExperimentalAppSecStandaloneEnabled() && !rootSpan.context().getPropagationTags().isAppsecPropagationEnabled()) { - setSamplingPriority(); + ((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan); return; } - if (rootSpan.context().getSamplingPriority() == PrioritySampling.UNSET) { - setSamplingPriority(); + ((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan); } } } - private void setSamplingPriority() { - ((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan); - } - public boolean sample(DDSpan spanToSample) { return traceConfig.sampler.sample(spanToSample); } From 8080ba5f329a26bacb3352bd32454691fc743f3c Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 31 May 2024 15:06:04 +0200 Subject: [PATCH 24/62] New . smoke test to cover RFC scenarios with the 1st one --- .../asmstandalonebilling/Controller.java | 8 +- ...stractAsmStandaloneBillingSmokeTest.groovy | 8 ++ ...AsmStandaloneBillingMatrixSmokeTest.groovy | 85 +++++++++++++++++++ 3 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy diff --git a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java index 6d2e673a7e5..38862ad2f26 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java +++ b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java @@ -17,10 +17,16 @@ public class Controller { @GetMapping("/greetings") - public String greetings(@RequestParam(name = "forceKeep", required = false) boolean forceKeep) { + public String greetings( + @RequestParam(name = "url", required = false) String url, + @RequestParam(name = "forceKeep", required = false) boolean forceKeep) { if (forceKeep) { forceKeepSpan(); } + if (url != null) { + RestTemplate restTemplate = new RestTemplate(); + return restTemplate.getForObject(url, String.class); + } return "Hello I'm service " + System.getProperty("dd.service.name"); } diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy index 201236f9023..d3f3e000f5b 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy @@ -53,6 +53,14 @@ abstract class AbstractAsmStandaloneBillingSmokeTest extends AbstractServerSmoke return trace.spans[0].metrics['_sampling_priority_v1'] == priority } + protected isSampledBySampler(DecodedTrace trace) { + def samplingPriority = trace.spans[0].metrics['_sampling_priority_v1'] + if(samplingPriority == PrioritySampling.SAMPLER_KEEP || samplingPriority == PrioritySampling.SAMPLER_DROP) { + return true + } + return false + } + protected hasAppsecPropagationTag(DecodedTrace trace) { return trace.spans[0].meta['_dd.p.appsec'] == "1" } diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy new file mode 100644 index 00000000000..b1daec419d2 --- /dev/null +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy @@ -0,0 +1,85 @@ +package datadog.smoketest.asmstandalonebilling + +import okhttp3.Request + +class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { + + static final String standAloneBillingServiceName = "asm-standalone-billing-matrix-smoketest-app" + static final String apmEnabledServiceName = "apm-enabled-matrix-smoketest-app" + static final String asmEnabledServiceName = "asm-enabled-matrix-smoketest-app" + + static final String[] standAloneBillingProperties = [ + "-Ddd.experimental.appsec.standalone.enabled=true", + "-Ddd.iast.enabled=true", + "-Ddd.iast.detection.mode=FULL", + "-Ddd.iast.debug.enabled=true", + //"-Ddd.appsec.enabled=true", + "-Ddd.trace.tracer.metrics.enabled=true", + "-Ddd.service.name=${standAloneBillingServiceName}", + ] + + static final String[] apmEnabledProperties = ["-Ddd.service.name=${apmEnabledServiceName}", "-Ddd.trace.tracer.metrics.enabled=true",] + + static final String[] asmEnabledProperties = [ + "-Ddd.iast.enabled=true", + "-Ddd.iast.detection.mode=FULL", + "-Ddd.iast.debug.enabled=true", + //"-Ddd.appsec.enabled=true", + "-Ddd.trace.tracer.metrics.enabled=true", + "-Ddd.service.name=${asmEnabledServiceName}", + ] + + protected int numberOfProcesses() { + return 3 + } + + @Override + ProcessBuilder createProcessBuilder(int processIndex) { + switch (processIndex) { + case 0: + return createProcess(processIndex, standAloneBillingProperties) + case 1: + return createProcess(processIndex, apmEnabledProperties) + case 2: + return createProcess(processIndex, asmEnabledProperties) + default: + throw new IllegalArgumentException("Invalid process index: ${processIndex}") + } + } + + void 'Test RFC Scenario 1'() { + setup: + //No ASM events + final downstreamUrl = "http://localhost:${httpPorts[2]}/rest-api/greetings" + //No ASM events + final standAloneBillingUrl = "http://localhost:${httpPorts[0]}/rest-api/greetings?url=${downstreamUrl}" + //No traced upstream service + final upstreamUrl = "http://localhost:${httpPorts[1]}/rest-api/greetings?url=${standAloneBillingUrl}" + final request = new Request.Builder().url(upstreamUrl).get().build() + + when: + final response = client.newCall(request).execute() + + then: + response.successful + waitForTraceCount(3) + + and: "No traced upstream service, resulting in no propagated sampling decision" + def upstreamTrace = getServiceTrace(apmEnabledServiceName) + isSampledBySampler(upstreamTrace) + !hasAppsecPropagationTag (upstreamTrace) + !hasApmDisabledTag (upstreamTrace) + + and:"No ASM events, resulting in the local sampling decision" + def standAloneBillingTrace = getServiceTrace(standAloneBillingServiceName) + isSampledBySampler(standAloneBillingTrace) + !hasAppsecPropagationTag (standAloneBillingTrace) + hasApmDisabledTag (standAloneBillingTrace) + + and:"No assumption can be done about the setup of the downstream services, so the default APM tracing behavior must be kept" + def downstreamTrace = getServiceTrace(asmEnabledServiceName) + isSampledBySampler(downstreamTrace) + !hasAppsecPropagationTag (downstreamTrace) + !hasApmDisabledTag (downstreamTrace) + } +} From a913cc64c4696e2e47da4d6a1f6b36e282faf21a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Sat, 1 Jun 2024 11:07:36 +0200 Subject: [PATCH 25/62] All scenarios implemented --- ...AsmStandaloneBillingMatrixSmokeTest.groovy | 129 +++++++++++++++++- 1 file changed, 128 insertions(+), 1 deletion(-) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy index b1daec419d2..56668fc4e5f 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy @@ -1,10 +1,12 @@ package datadog.smoketest.asmstandalonebilling +import datadog.trace.api.sampling.PrioritySampling import okhttp3.Request class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { static final String standAloneBillingServiceName = "asm-standalone-billing-matrix-smoketest-app" + static final String standAloneBillingServiceName2 = "asm-standalone-billing-matrix-smoketest-app2" static final String apmEnabledServiceName = "apm-enabled-matrix-smoketest-app" static final String asmEnabledServiceName = "asm-enabled-matrix-smoketest-app" @@ -18,6 +20,16 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm "-Ddd.service.name=${standAloneBillingServiceName}", ] + static final String[] standAloneBillingProperties2 = [ + "-Ddd.experimental.appsec.standalone.enabled=true", + "-Ddd.iast.enabled=true", + "-Ddd.iast.detection.mode=FULL", + "-Ddd.iast.debug.enabled=true", + //"-Ddd.appsec.enabled=true", + "-Ddd.trace.tracer.metrics.enabled=true", + "-Ddd.service.name=${standAloneBillingServiceName2}", + ] + static final String[] apmEnabledProperties = ["-Ddd.service.name=${apmEnabledServiceName}", "-Ddd.trace.tracer.metrics.enabled=true",] static final String[] asmEnabledProperties = [ @@ -30,7 +42,7 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm ] protected int numberOfProcesses() { - return 3 + return 4 } @Override @@ -42,6 +54,8 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm return createProcess(processIndex, apmEnabledProperties) case 2: return createProcess(processIndex, asmEnabledProperties) + case 3: + return createProcess(processIndex, standAloneBillingProperties2) default: throw new IllegalArgumentException("Invalid process index: ${processIndex}") } @@ -82,4 +96,117 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm !hasAppsecPropagationTag (downstreamTrace) !hasApmDisabledTag (downstreamTrace) } + + void 'Test RFC Scenario 2'() { + setup: + //No ASM events + final downstreamUrl = "http://localhost:${httpPorts[2]}/rest-api/greetings" + //ASM event + final standAloneBillingUrl = "http://localhost:${httpPorts[0]}/rest-api/iast?injection=vulnerable%26url=${downstreamUrl}" + //No traced upstream service + final upstreamUrl = "http://localhost:${httpPorts[1]}/rest-api/greetings?url=${standAloneBillingUrl}" + final request = new Request.Builder().url(upstreamUrl).get().build() + + when: + final response = client.newCall(request).execute() + + then: + response.successful + waitForTraceCount(3) + + and: "No traced upstream service, resulting in no propagated sampling decision" + def upstreamTrace = getServiceTrace(apmEnabledServiceName) + isSampledBySampler(upstreamTrace) + !hasAppsecPropagationTag (upstreamTrace) + !hasApmDisabledTag (upstreamTrace) + + and:"ASM events" + def standAloneBillingTrace = getServiceTrace(standAloneBillingServiceName) + checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) + hasAppsecPropagationTag (standAloneBillingTrace) + hasApmDisabledTag (standAloneBillingTrace) + + and:"ASM requires the distributed trace" + def downstreamTrace = getServiceTrace(asmEnabledServiceName) + checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) + hasAppsecPropagationTag (standAloneBillingTrace) + !hasApmDisabledTag (downstreamTrace) + } + + void 'Test RFC Scenario 3'() { + setup: + //No ASM events + final downstreamUrl = "http://localhost:${httpPorts[2]}/rest-api/greetings" + //No ASM events + final standAloneBillingUrl = "http://localhost:${httpPorts[0]}/rest-api/greetings?url=${downstreamUrl}" + //upstream service with force keep span + final upstreamUrl = "http://localhost:${httpPorts[1]}/rest-api/greetings?forceKeep=true&url=${standAloneBillingUrl}" + final request = new Request.Builder().url(upstreamUrl).get().build() + + when: + final response = client.newCall(request).execute() + + then: + response.successful + waitForTraceCount(3) + + and: "Upstream APM service propagating the force keep" + def upstreamTrace = getServiceTrace(apmEnabledServiceName) + checkRootSpanPrioritySampling(upstreamTrace, PrioritySampling.USER_KEEP) + !hasAppsecPropagationTag (upstreamTrace) + !hasApmDisabledTag (upstreamTrace) + + and:"No ASM events, resulting in the local sampling decision" + def standAloneBillingTrace = getServiceTrace(standAloneBillingServiceName) + //TODO Check RFC as it said isSampledBySampler(standAloneBillingTrace) but sampling priority it's set in the upstream service and locked for all downstream processes + checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) + !hasAppsecPropagationTag (standAloneBillingTrace) + hasApmDisabledTag (standAloneBillingTrace) + + and:"Only the local trace is expected to be “muted” and no assumption must be done about the downstream one, so the sampling decision propagated by upstream services must be honored" + def downstreamTrace = getServiceTrace(asmEnabledServiceName) + checkRootSpanPrioritySampling(downstreamTrace, PrioritySampling.USER_KEEP) + !hasAppsecPropagationTag (downstreamTrace) + !hasApmDisabledTag (downstreamTrace) + } + + void 'Test RFC Scenario 4'() { + setup: + //No ASM events + final downstreamUrl = "http://localhost:${httpPorts[2]}/rest-api/greetings" + //Stand alone without events + final standAloneBillingUrl = "http://localhost:${httpPorts[0]}/rest-api/greetings?url=${downstreamUrl}" + //StandAlone with iast event + final upstreamUrl = "http://localhost:${httpPorts[3]}/rest-api/iast?injection=vulnerable&url=${standAloneBillingUrl}" + final request = new Request.Builder().url(upstreamUrl).get().build() + + when: + final response = client.newCall(request).execute() + + then: + response.successful + waitForTraceCount(3) + + and: "Upstream standalone ASM service having ASM events result in force keep and propagation of the tag" + def upstreamTrace = getServiceTrace(standAloneBillingServiceName2) + checkRootSpanPrioritySampling(upstreamTrace, PrioritySampling.USER_KEEP) + hasAppsecPropagationTag (upstreamTrace) + hasApmDisabledTag (upstreamTrace) + + and:"standalone service must keep the local trace with the local sampling priority" + def standAloneBillingTrace = getServiceTrace(standAloneBillingServiceName) + checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) + //TODO Check RFC as it said !hasAppsecPropagationTag (standAloneBillingTrace) but the tag is propagated from the upstream service + hasAppsecPropagationTag (standAloneBillingTrace) + hasApmDisabledTag (standAloneBillingTrace) + + and:"Only the local trace is expected to be “muted” and no assumption must be done about the downstream one, so the sampling decision propagated by upstream services must be honored" + def downstreamTrace = getServiceTrace(asmEnabledServiceName) + checkRootSpanPrioritySampling(downstreamTrace, PrioritySampling.USER_KEEP) + hasAppsecPropagationTag (downstreamTrace) + !hasApmDisabledTag (downstreamTrace) + } + + + } From cd9b73f823dead5b0819647acb3d3fe9a46bb529 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 3 Jun 2024 07:49:59 +0200 Subject: [PATCH 26/62] change Tags.PROPAGATED_APPSEC "1" for true --- .../agent-iast/src/main/java/com/datadog/iast/Reporter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java index cde0684a935..ea5f3d2174f 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java @@ -98,7 +98,7 @@ private VulnerabilityBatch getOrCreateVulnerabilityBatch(final AgentSpan span) { // TODO: We need to check if we can have an API with more fine-grained semantics on why traces // are kept. segment.setTagTop(Tags.ASM_KEEP, true); - segment.setTagTop(Tags.PROPAGATED_APPSEC, "1"); + segment.setTagTop(Tags.PROPAGATED_APPSEC, true); return batch; } From 4eb01125c34d51961258f972b382c7158da79b59 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 3 Jun 2024 07:57:55 +0200 Subject: [PATCH 27/62] change Tags.PROPAGATED_APPSEC "1" for true --- .../src/main/java/datadog/trace/core/DDSpanContext.java | 4 ++-- .../datadog/trace/core/taginterceptor/TagInterceptor.java | 2 +- .../trace/core/taginterceptor/TagInterceptorTest.groovy | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) 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 cd73494e60b..1b2f3252c3b 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 @@ -511,8 +511,8 @@ private void forceKeepThisSpan(byte samplingMechanism) { } } - public void addAppsecPropagationTag() { - propagationTags.updateAppsecPropagation(true); + public void updateAppsecPropagation(boolean value) { + propagationTags.updateAppsecPropagation(value); } /** @return if sampling priority was set by this method invocation */ diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index 3efbaa0524e..8ff86d6a49b 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -109,7 +109,7 @@ public boolean interceptTag(DDSpanContext span, String tag, Object value) { case Tags.SAMPLING_PRIORITY: return interceptSamplingPriority(span, value); case Tags.PROPAGATED_APPSEC: - span.addAppsecPropagationTag(); + span.updateAppsecPropagation(asBoolean(value)); return true; case InstrumentationTags.SERVLET_CONTEXT: return interceptServletContext(span, value); diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy index 0765ece9c0f..b91cb2210b6 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy @@ -738,9 +738,9 @@ class TagInterceptorTest extends DDCoreSpecification { final context = Mock(DDSpanContext) when: - interceptor.interceptTag(context, Tags.PROPAGATED_APPSEC, "1") + interceptor.interceptTag(context, Tags.PROPAGATED_APPSEC, true) then: - 1 * context.addAppsecPropagationTag() + 1 * context.updateAppsecPropagation(true) } } From b7de9375767be9bfa46ce6066479327fb537f636 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 3 Jun 2024 10:04:16 +0200 Subject: [PATCH 28/62] Fix codenarc --- ...stractAsmStandaloneBillingSmokeTest.groovy | 5 +- ...AsmStandaloneBillingMatrixSmokeTest.groovy | 54 +++++++++---------- .../AsmStandaloneBillingSmokeTest.groovy | 24 ++++----- 3 files changed, 38 insertions(+), 45 deletions(-) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy index d3f3e000f5b..ad02462a28f 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy @@ -55,10 +55,7 @@ abstract class AbstractAsmStandaloneBillingSmokeTest extends AbstractServerSmoke protected isSampledBySampler(DecodedTrace trace) { def samplingPriority = trace.spans[0].metrics['_sampling_priority_v1'] - if(samplingPriority == PrioritySampling.SAMPLER_KEEP || samplingPriority == PrioritySampling.SAMPLER_DROP) { - return true - } - return false + return samplingPriority == PrioritySampling.SAMPLER_KEEP || samplingPriority == PrioritySampling.SAMPLER_DROP } protected hasAppsecPropagationTag(DecodedTrace trace) { diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy index 56668fc4e5f..b2744605c6f 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy @@ -5,40 +5,40 @@ import okhttp3.Request class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { - static final String standAloneBillingServiceName = "asm-standalone-billing-matrix-smoketest-app" - static final String standAloneBillingServiceName2 = "asm-standalone-billing-matrix-smoketest-app2" - static final String apmEnabledServiceName = "apm-enabled-matrix-smoketest-app" - static final String asmEnabledServiceName = "asm-enabled-matrix-smoketest-app" + static final String STANDALONE_BILLING_SERVICE_NAME = "asm-standalone-billing-matrix-smoketest-app" + static final String STANDALONE_BILLING_SERVICE_NAME_2 = "asm-standalone-billing-matrix-smoketest-app2" + static final String APM_ENABLED_SERVICE_NAME = "apm-enabled-matrix-smoketest-app" + static final String ASM_ENABLED_SERVICE_NAME = "asm-enabled-matrix-smoketest-app" - static final String[] standAloneBillingProperties = [ + static final String[] STANDALONE_BILLING_PROPERTIES = [ "-Ddd.experimental.appsec.standalone.enabled=true", "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", //"-Ddd.appsec.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", - "-Ddd.service.name=${standAloneBillingServiceName}", + "-Ddd.service.name=${STANDALONE_BILLING_SERVICE_NAME}", ] - static final String[] standAloneBillingProperties2 = [ + static final String[] STANDALONE_BILLING_PROPERTIES_2 = [ "-Ddd.experimental.appsec.standalone.enabled=true", "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", //"-Ddd.appsec.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", - "-Ddd.service.name=${standAloneBillingServiceName2}", + "-Ddd.service.name=${STANDALONE_BILLING_SERVICE_NAME_2}", ] - static final String[] apmEnabledProperties = ["-Ddd.service.name=${apmEnabledServiceName}", "-Ddd.trace.tracer.metrics.enabled=true",] + static final String[] APM_ENABLED_PROPERTIES = ["-Ddd.service.name=${APM_ENABLED_SERVICE_NAME}", "-Ddd.trace.tracer.metrics.enabled=true",] - static final String[] asmEnabledProperties = [ + static final String[] ASM_ENABLED_PROPERTIES = [ "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", //"-Ddd.appsec.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", - "-Ddd.service.name=${asmEnabledServiceName}", + "-Ddd.service.name=${ASM_ENABLED_SERVICE_NAME}", ] protected int numberOfProcesses() { @@ -49,13 +49,13 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm ProcessBuilder createProcessBuilder(int processIndex) { switch (processIndex) { case 0: - return createProcess(processIndex, standAloneBillingProperties) + return createProcess(processIndex, STANDALONE_BILLING_PROPERTIES) case 1: - return createProcess(processIndex, apmEnabledProperties) + return createProcess(processIndex, APM_ENABLED_PROPERTIES) case 2: - return createProcess(processIndex, asmEnabledProperties) + return createProcess(processIndex, ASM_ENABLED_PROPERTIES) case 3: - return createProcess(processIndex, standAloneBillingProperties2) + return createProcess(processIndex, STANDALONE_BILLING_PROPERTIES_2) default: throw new IllegalArgumentException("Invalid process index: ${processIndex}") } @@ -79,19 +79,19 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm waitForTraceCount(3) and: "No traced upstream service, resulting in no propagated sampling decision" - def upstreamTrace = getServiceTrace(apmEnabledServiceName) + def upstreamTrace = getServiceTrace(APM_ENABLED_SERVICE_NAME) isSampledBySampler(upstreamTrace) !hasAppsecPropagationTag (upstreamTrace) !hasApmDisabledTag (upstreamTrace) and:"No ASM events, resulting in the local sampling decision" - def standAloneBillingTrace = getServiceTrace(standAloneBillingServiceName) + def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) isSampledBySampler(standAloneBillingTrace) !hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) and:"No assumption can be done about the setup of the downstream services, so the default APM tracing behavior must be kept" - def downstreamTrace = getServiceTrace(asmEnabledServiceName) + def downstreamTrace = getServiceTrace(ASM_ENABLED_SERVICE_NAME) isSampledBySampler(downstreamTrace) !hasAppsecPropagationTag (downstreamTrace) !hasApmDisabledTag (downstreamTrace) @@ -115,19 +115,19 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm waitForTraceCount(3) and: "No traced upstream service, resulting in no propagated sampling decision" - def upstreamTrace = getServiceTrace(apmEnabledServiceName) + def upstreamTrace = getServiceTrace(APM_ENABLED_SERVICE_NAME) isSampledBySampler(upstreamTrace) !hasAppsecPropagationTag (upstreamTrace) !hasApmDisabledTag (upstreamTrace) and:"ASM events" - def standAloneBillingTrace = getServiceTrace(standAloneBillingServiceName) + def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) and:"ASM requires the distributed trace" - def downstreamTrace = getServiceTrace(asmEnabledServiceName) + def downstreamTrace = getServiceTrace(ASM_ENABLED_SERVICE_NAME) checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) hasAppsecPropagationTag (standAloneBillingTrace) !hasApmDisabledTag (downstreamTrace) @@ -151,20 +151,20 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm waitForTraceCount(3) and: "Upstream APM service propagating the force keep" - def upstreamTrace = getServiceTrace(apmEnabledServiceName) + def upstreamTrace = getServiceTrace(APM_ENABLED_SERVICE_NAME) checkRootSpanPrioritySampling(upstreamTrace, PrioritySampling.USER_KEEP) !hasAppsecPropagationTag (upstreamTrace) !hasApmDisabledTag (upstreamTrace) and:"No ASM events, resulting in the local sampling decision" - def standAloneBillingTrace = getServiceTrace(standAloneBillingServiceName) + def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) //TODO Check RFC as it said isSampledBySampler(standAloneBillingTrace) but sampling priority it's set in the upstream service and locked for all downstream processes checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) !hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) and:"Only the local trace is expected to be “muted” and no assumption must be done about the downstream one, so the sampling decision propagated by upstream services must be honored" - def downstreamTrace = getServiceTrace(asmEnabledServiceName) + def downstreamTrace = getServiceTrace(ASM_ENABLED_SERVICE_NAME) checkRootSpanPrioritySampling(downstreamTrace, PrioritySampling.USER_KEEP) !hasAppsecPropagationTag (downstreamTrace) !hasApmDisabledTag (downstreamTrace) @@ -188,20 +188,20 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm waitForTraceCount(3) and: "Upstream standalone ASM service having ASM events result in force keep and propagation of the tag" - def upstreamTrace = getServiceTrace(standAloneBillingServiceName2) + def upstreamTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME_2) checkRootSpanPrioritySampling(upstreamTrace, PrioritySampling.USER_KEEP) hasAppsecPropagationTag (upstreamTrace) hasApmDisabledTag (upstreamTrace) and:"standalone service must keep the local trace with the local sampling priority" - def standAloneBillingTrace = getServiceTrace(standAloneBillingServiceName) + def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) //TODO Check RFC as it said !hasAppsecPropagationTag (standAloneBillingTrace) but the tag is propagated from the upstream service hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) and:"Only the local trace is expected to be “muted” and no assumption must be done about the downstream one, so the sampling decision propagated by upstream services must be honored" - def downstreamTrace = getServiceTrace(asmEnabledServiceName) + def downstreamTrace = getServiceTrace(ASM_ENABLED_SERVICE_NAME) checkRootSpanPrioritySampling(downstreamTrace, PrioritySampling.USER_KEEP) hasAppsecPropagationTag (downstreamTrace) !hasApmDisabledTag (downstreamTrace) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy index 797d91bfb09..f5a1ec9757a 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy @@ -4,20 +4,20 @@ import okhttp3.Request class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { - private static final String standAloneBillingServiceName = "asm-standalone-billing-smoketest-app" - private static final String apmEnabledServiceName = "apm-enabled-smoketest-app" + private static final String STANDALONE_BILLING_SERVICE_NAME = "asm-standalone-billing-smoketest-app" + private static final String APM_ENABLED_SERVICE_NAME = "apm-enabled-smoketest-app" - static final String[] standAloneBillingProperties = [ + static final String[] STANDALONE_BILLING_PROPERTIES = [ "-Ddd.experimental.appsec.standalone.enabled=true", "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", "-Ddd.appsec.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", - "-Ddd.service.name=${standAloneBillingServiceName}", + "-Ddd.service.name=${STANDALONE_BILLING_SERVICE_NAME}", ] - static final String[] apmEnabledProperties = ["-Ddd.service.name=${apmEnabledServiceName}", "-Ddd.trace.tracer.metrics.enabled=true",] + static final String[] APM_ENABLED_PROPERTIES = ["-Ddd.service.name=${APM_ENABLED_SERVICE_NAME}", "-Ddd.trace.tracer.metrics.enabled=true",] protected int numberOfProcesses() { return 2 @@ -26,10 +26,9 @@ class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTes @Override ProcessBuilder createProcessBuilder(int processIndex) { if(processIndex == 0){ - return createProcess(processIndex, standAloneBillingProperties) - } else { - return createProcess(processIndex, apmEnabledProperties) + return createProcess(processIndex, STANDALONE_BILLING_PROPERTIES) } + return createProcess(processIndex, APM_ENABLED_PROPERTIES) } void 'When APM is disabled, numeric tag _dd.apm.enabled:0 must be added to the metrics map of the service entry spans.'() { @@ -47,8 +46,8 @@ class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTes response1.successful response2.successful waitForTraceCount(2) - hasApmDisabledTag(getServiceTrace(standAloneBillingServiceName)) - !hasApmDisabledTag(getServiceTrace(apmEnabledServiceName)) + hasApmDisabledTag(getServiceTrace(STANDALONE_BILLING_SERVICE_NAME)) + !hasApmDisabledTag(getServiceTrace(APM_ENABLED_SERVICE_NAME)) } void 'When APM is disabled, libraries must completely disable the generation of APM trace metrics'(){ @@ -66,10 +65,7 @@ class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTes then:'metrics should be disabled' checkLogPostExit { log -> - if (log.contains('datadog.trace.agent.common.metrics.MetricsAggregatorFactory - tracer metrics disabled')) { - return true - } - return false + return log.contains('datadog.trace.agent.common.metrics.MetricsAggregatorFactory - tracer metrics disabled') } } From 38978deb12da4f255320dfd083da5f3bb142afe9 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 3 Jun 2024 10:08:57 +0200 Subject: [PATCH 29/62] Fix test --- .../src/test/groovy/com/datadog/iast/ReporterTest.groovy | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy index 7db2f108636..1ea2ceb5559 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy @@ -80,6 +80,7 @@ class ReporterTest extends DDSpecification { ] }''', batch.toString(), true) 1 * traceSegment.setTagTop('asm.keep', true) + 1 * traceSegment.setTagTop('p.appsec', true) 0 * _ } @@ -146,6 +147,7 @@ class ReporterTest extends DDSpecification { ] }''', batch.toString(), true) 1 * traceSegment.setTagTop('asm.keep', true) + 1 * traceSegment.setTagTop('p.appsec', true) 0 * _ } @@ -269,6 +271,7 @@ class ReporterTest extends DDSpecification { 1 * traceSegment.getDataTop('iast') >> null 1 * traceSegment.setDataTop('iast', _ as VulnerabilityBatch) 1 * traceSegment.setTagTop('asm.keep', true) + 1 * traceSegment.setTagTop('p.appsec', true) 1 * traceSegment.setTagTop('_dd.iast.enabled', 1) 0 * _ } From 3c3b9f5a338b8e82cd25a2364913e93480f5b925 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 4 Jun 2024 12:14:10 +0200 Subject: [PATCH 30/62] Add appsec propagation tag to other codecs --- .../propagation/ptags/DatadogPTagsCodec.java | 6 +++- .../core/propagation/ptags/PTagsCodec.java | 6 ++++ .../core/propagation/ptags/PTagsFactory.java | 24 +++++++++++---- .../core/propagation/ptags/W3CPTagsCodec.java | 10 +++++-- .../DatadogPropagationTagsTest.groovy | 25 ++++++++++++++++ .../propagation/W3CPropagationTagsTest.groovy | 29 +++++++++++++++++++ 6 files changed, 91 insertions(+), 9 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java index 12bd59738af..918142bcfaa 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java @@ -61,6 +61,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { int tagPos = 0; TagValue decisionMakerTagValue = null; TagValue traceIdTagValue = null; + boolean appsecPropagationEnabled = false; while (tagPos < len) { int tagKeyEndsAt = validateCharsUntilSeparatorOrEnd( @@ -95,6 +96,8 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { decisionMakerTagValue = tagValue; } else if (tagKey.equals(TRACE_ID_TAG)) { traceIdTagValue = tagValue; + } else if (tagKey.equals(APPSEC_TAG)) { + appsecPropagationEnabled = true; } else { if (tagPairs == null) { // This is roughly the size of a two element linked list but can hold six @@ -107,7 +110,8 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { } tagPos = tagValueEndsAt + 1; } - return tagsFactory.createValid(tagPairs, decisionMakerTagValue, traceIdTagValue); + return tagsFactory.createValid( + tagPairs, decisionMakerTagValue, traceIdTagValue, appsecPropagationEnabled); } @Override diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java index b7aa7486dbb..c865305bae8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java @@ -149,6 +149,8 @@ protected static boolean validateTagValue(TagKey tagKey, TagValue tagValue) { return false; } else if (tagKey.equals(TRACE_ID_TAG) && !validateTraceId(tagValue)) { return false; + } else if (tagKey.equals(APPSEC_TAG) && !validateAppsecTagValue(tagValue)) { + return false; } return true; } @@ -211,6 +213,10 @@ private static boolean validateTraceId(TagValue value) { return true; } + private static boolean validateAppsecTagValue(TagValue value) { + return value.length() == 1 && value.charAt(0) == '1'; + } + protected static boolean isDigit(char c) { return c >= '0' && c <= '9'; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index 8ac6e49506a..fffd8705904 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -2,6 +2,7 @@ import static datadog.trace.core.propagation.PropagationTags.HeaderType.DATADOG; import static datadog.trace.core.propagation.PropagationTags.HeaderType.W3C; +import static datadog.trace.core.propagation.ptags.PTagsCodec.APPSEC_TAG; import static datadog.trace.core.propagation.ptags.PTagsCodec.DECISION_MAKER_TAG; import static datadog.trace.core.propagation.ptags.PTagsCodec.TRACE_ID_TAG; @@ -45,7 +46,7 @@ PTagsCodec getDecoderEncoder(@Nonnull HeaderType headerType) { @Override public final PropagationTags empty() { - return createValid(null, null, null); + return createValid(null, null, null, false); } @Override @@ -54,8 +55,12 @@ public final PropagationTags fromHeaderValue(@Nonnull HeaderType headerType, Str } PropagationTags createValid( - List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue) { - return new PTags(this, tagPairs, decisionMakerTagValue, traceIdTagValue); + List tagPairs, + TagValue decisionMakerTagValue, + TagValue traceIdTagValue, + boolean appsecPropagationEnabled) { + return new PTags( + this, tagPairs, decisionMakerTagValue, traceIdTagValue, appsecPropagationEnabled); } PropagationTags createInvalid(String error) { @@ -107,14 +112,16 @@ public PTags( PTagsFactory factory, List tagPairs, TagValue decisionMakerTagValue, - TagValue traceIdTagValue) { + TagValue traceIdTagValue, + boolean appsecPropagationEnabled) { this( factory, tagPairs, decisionMakerTagValue, traceIdTagValue, + appsecPropagationEnabled, PrioritySampling.UNSET, - null, + null, null); } @@ -123,6 +130,7 @@ public PTags( List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue, + boolean appsecPropagationEnabled, int samplingPriority, CharSequence origin, CharSequence lastParentId) { @@ -131,6 +139,7 @@ public PTags( this.tagPairs = tagPairs; this.canChangeDecisionMaker = decisionMakerTagValue == null; this.decisionMakerTagValue = decisionMakerTagValue; + this.appsecPropagationEnabled = appsecPropagationEnabled; this.samplingPriority = samplingPriority; this.origin = origin; this.lastParentId = lastParentId; @@ -145,7 +154,7 @@ public PTags( } static PTags withError(PTagsFactory factory, String error) { - PTags pTags = new PTags(factory, null, null, null, PrioritySampling.UNSET, null, null); + PTags pTags = new PTags(factory, null, null, null, false, PrioritySampling.UNSET, null, null); pTags.error = error; return pTags; } @@ -329,6 +338,9 @@ int getXDatadogTagsSize() { size = PTagsCodec.calcXDatadogTagsSize(getTagPairs()); size = PTagsCodec.calcXDatadogTagsSize(size, DECISION_MAKER_TAG, decisionMakerTagValue); size = PTagsCodec.calcXDatadogTagsSize(size, TRACE_ID_TAG, traceIdHighOrderBitsHexTagValue); + if (appsecPropagationEnabled) { + size = PTagsCodec.calcXDatadogTagsSize(size, APPSEC_TAG, TagValue.from("1")); + } xDatadogTagsSize = size; } return size; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index 86837529553..89c66c46a74 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -94,6 +94,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { CharSequence origin = null; TagValue decisionMakerTagValue = null; TagValue traceIdTagValue = null; + boolean appsecPropagationEnabled = false; int maxUnknownSize = 0; CharSequence lastParentId = LAST_PARENT_ZERO; while (tagPos < ddMemberValueEnd) { @@ -147,6 +148,8 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { decisionMakerTagValue = tagValue; } else if (tagKey.equals(TRACE_ID_TAG)) { traceIdTagValue = tagValue; + } else if (tagKey.equals(APPSEC_TAG)) { + appsecPropagationEnabled = true; } else { if (tagPairs == null) { // This is roughly the size of a two element linked list but can hold six @@ -180,6 +183,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { tagPairs, decisionMakerTagValue, traceIdTagValue, + appsecPropagationEnabled, samplingPriority, origin, value, @@ -704,6 +708,7 @@ private static W3CPTags empty( null, null, null, + false, PrioritySampling.UNSET, null, original, @@ -735,6 +740,7 @@ public W3CPTags( List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue, + boolean appsecPropagationEnabled, int samplingPriority, CharSequence origin, String original, @@ -742,12 +748,12 @@ public W3CPTags( int ddMemberStart, int ddMemberValueEnd, int maxUnknownSize, - CharSequence lastParentId) { - super( + CharSequence lastParentId) {super( factory, tagPairs, decisionMakerTagValue, traceIdTagValue, + appsecPropagationEnabled, samplingPriority, origin, lastParentId); diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy index 8ec0f4da1af..a9fa2e299ef 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy @@ -72,6 +72,7 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { "_dd.p.tid=123456789ABCDEF0" | null | ["_dd.propagation_error": "malformed_tid 123456789ABCDEF0"] // invalid tid tag value: upper-case characters "_dd.p.tid=123456789abcdefg" | null | ["_dd.propagation_error": "malformed_tid 123456789abcdefg"] // invalid tid tag value: non-hexadecimal characters "_dd.p.tid=-123456789abcdef" | null | ["_dd.propagation_error": "malformed_tid -123456789abcdef"] // invalid tid tag value: non-hexadecimal characters + "_dd.p.appsec=1" | "_dd.p.appsec=1" | ["_dd.p.appsec": "1"] } def "datadog propagation tags should translate to w3c tags #headerValue"() { @@ -91,6 +92,7 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { headerValue | expectedHeaderValue | tags '_dd.p.dm=934086a686-4' | 'dd=t.dm:934086a686-4' | ['_dd.p.dm': '934086a686-4'] '_dd.p.dm=934086a686-4,_dd.p.f=w00t==' | 'dd=t.dm:934086a686-4;t.f:w00t~~' | ['_dd.p.dm': '934086a686-4', '_dd.p.f': 'w00t=='] + '_dd.p.appsec=1' | 'dd=t.appsec:1' | ['_dd.p.appsec': '1'] } def "update propagation tags sampling mechanism #originalTagSet"() { @@ -134,6 +136,29 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { ",_dd.p.dm=Value" | SAMPLER_KEEP | AGENT_RATE | "_dd.p.dm=-1" | ["_dd.propagation_error": "decoding_error", "_dd.p.dm": "-1"] } + def "update propagation tags appsec propagation #originalTagSet"() { + setup: + def config = Mock(Config) + config.getxDatadogTagsMaxLength() >> 512 + def propagationTagsFactory = PropagationTags.factory(config) + def propagationTags = propagationTagsFactory.fromHeaderValue(PropagationTags.HeaderType.DATADOG, originalTagSet) + + when: + propagationTags.updateAppsecPropagation(enabled) + + then: + propagationTags.headerValue(PropagationTags.HeaderType.DATADOG) == expectedHeaderValue + propagationTags.createTagMap() == tags + + where: + originalTagSet | enabled | expectedHeaderValue | tags + // keep the existing dm tag as is + "_dd.p.appsec=1" | true | "_dd.p.appsec=1" | ["_dd.p.appsec": "1"] + "" | false | null | [:] + //Invalid input + "_dd.p.appsec=0" | false | null | ["_dd.propagation_error": "decoding_error"] + } + def extractionLimitExceeded() { setup: def tags = "_dd.p.anytag=value" diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy index 3a89cdeb256..837334558a6 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy @@ -237,6 +237,8 @@ class W3CPropagationTagsTest extends DDCoreSpecification { null | null | [:] '' | null | [:] 'dd=s:0;t.dm:934086a686-4' | 'dd=s:0;t.dm:934086a686-4' | ['_dd.p.dm': '934086a686-4'] + 'dd=s:0;t.appsec:1' | 'dd=s:0;t.appsec:1' | ['_dd.p.appsec': '1'] + 'dd=s:0;t.dm:934086a686-4;t.appsec:1' | 'dd=s:0;t.dm:934086a686-4;t.appsec:1' | ['_dd.p.dm': '934086a686-4', '_dd.p.appsec': '1'] 'other=whatever,dd=s:0;t.dm:934086a686-4' | 'dd=s:0;t.dm:934086a686-4,other=whatever' | ['_dd.p.dm': '934086a686-4'] 'dd=s:0;t.dm:934086a687-3,other=whatever' | 'dd=s:0;t.dm:934086a687-3,other=whatever' | ['_dd.p.dm': '934086a687-3'] 'some=thing,dd=s:0;t.dm:934086a687-3,other=whatever' | 'dd=s:0;t.dm:934086a687-3,some=thing,other=whatever' | ['_dd.p.dm': '934086a687-3'] @@ -278,6 +280,8 @@ class W3CPropagationTagsTest extends DDCoreSpecification { headerValue | expectedHeaderValue | tags 'dd=s:0;t.dm:934086a686-4' | '_dd.p.dm=934086a686-4' | ['_dd.p.dm': '934086a686-4'] 'other=whatever,dd=s:0;t.dm:934086a686-4;t.f:w00t~~' | '_dd.p.dm=934086a686-4,_dd.p.f=w00t==' | ['_dd.p.dm': '934086a686-4', '_dd.p.f': 'w00t=='] + 'dd=s:0;t.appsec:1' | '_dd.p.appsec=1' | ['_dd.p.appsec': '1'] + 'other=whatever,dd=s:0;t.dm:934086a686-4;t.f:w00t~~;t.appsec:1' | '_dd.p.dm=934086a686-4,_dd.p.appsec=1,_dd.p.f=w00t==' | ['_dd.p.dm': '934086a686-4', '_dd.p.f': 'w00t==', '_dd.p.appsec': '1'] 'some=thing,other=whatever' | null | [:] } @@ -310,6 +314,31 @@ class W3CPropagationTagsTest extends DDCoreSpecification { 'dd=s:1;o:some;t.dm:934086a686-4' | PrioritySampling.SAMPLER_DROP | SamplingMechanism.EXTERNAL_OVERRIDE | "other" | 'dd=s:0;o:other' | [:] } + def "propagation tags should be updated by appsec propagation #appsec"() { + setup: + def config = Mock(Config) + config.getxDatadogTagsMaxLength() >> 512 + def propagationTagsFactory = PropagationTags.factory(config) + + when: + def propagationTags = propagationTagsFactory.fromHeaderValue(HeaderType.W3C, headerValue) + + then: + propagationTags.headerValue(HeaderType.W3C) != expectedHeaderValue + + when: + propagationTags.updateAppsecPropagation(appsec) + + then: + propagationTags.headerValue(HeaderType.W3C) == expectedHeaderValue + propagationTags.createTagMap() == tags + + where: + headerValue | appsec | expectedHeaderValue | tags + 'dd=t.appsec:1;x:unknown' | false | 'dd=x:unknown' | [:] + 'dd=x:unknown' | true | 'dd=t.appsec:1;x:unknown' | ['_dd.p.appsec': '1'] + } + static private String toLcAlpha(String cs) { // Argh groovy and characters char c = cs From 340da3c6f3a4a3e910d426ddd0c18c7eaf467dd1 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 6 Jun 2024 17:03:13 +0200 Subject: [PATCH 31/62] Rename sampler and modify context to be able to modify the sampling priority for standalone billing and APPSEC sampling mechanism - More smoke tests --- ...stractAsmStandaloneBillingSmokeTest.groovy | 4 + ...tandaloneBillingConerCasesSmokeTest.groovy | 95 +++++++++++++++++++ ...AsmStandaloneBillingMatrixSmokeTest.groovy | 5 +- ...mStandaloneBillingSamplingSmokeTest.groovy | 4 +- ...Sampler.java => AsmStandaloneSampler.java} | 10 +- .../trace/common/sampling/Sampler.java | 2 +- .../datadog/trace/core/DDSpanContext.java | 7 ++ ...groovy => AsmStandaloneSamplerTest.groovy} | 4 +- .../trace/common/sampling/SamplerTest.groovy | 4 +- .../trace/api/sampling/SamplingMechanism.java | 4 + 10 files changed, 124 insertions(+), 15 deletions(-) create mode 100644 dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy rename dd-trace-core/src/main/java/datadog/trace/common/sampling/{TimeSampler.java => AsmStandaloneSampler.java} (84%) rename dd-trace-core/src/test/groovy/datadog/trace/common/sampling/{TimeSamplerTest.groovy => AsmStandaloneSamplerTest.groovy} (90%) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy index ad02462a28f..8065e1cc3f5 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy @@ -65,4 +65,8 @@ abstract class AbstractAsmStandaloneBillingSmokeTest extends AbstractServerSmoke protected hasApmDisabledTag(DecodedTrace trace) { return trace.spans[0].metrics['_dd.apm.enabled'] == 0 } + + protected hasASMEvents(DecodedTrace trace){ + return trace.spans[0].meta['_dd.iast.json'] != null || trace.spans[0].meta['_dd.appsec.json'] != null + } } diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy new file mode 100644 index 00000000000..a676b7718ce --- /dev/null +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy @@ -0,0 +1,95 @@ +package datadog.smoketest.asmstandalonebilling + +import datadog.trace.api.sampling.PrioritySampling +import okhttp3.Request + +class AsmStandaloneBillingConerCasesSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { + + private static final String STANDALONE_BILLING_SERVICE_NAME = "asm-standalone-billing-corner-cases-smoketest-app" + private static final String ASM_ENABLED_SERVICE_NAME = "asm-enabled-corner-cases-smoketest-app" + + static final String[] STANDALONE_BILLING_PROPERTIES = [ + "-Ddd.experimental.appsec.standalone.enabled=true", + "-Ddd.iast.enabled=true", + "-Ddd.iast.detection.mode=FULL", + "-Ddd.iast.debug.enabled=true", + //"-Ddd.appsec.enabled=true", + "-Ddd.trace.tracer.metrics.enabled=true", + "-Ddd.service.name=${STANDALONE_BILLING_SERVICE_NAME}", + ] + + static final String[] ASM_ENABLED_PROPERTIES = [ + "-Ddd.iast.enabled=true", + "-Ddd.iast.detection.mode=FULL", + "-Ddd.iast.debug.enabled=true", + //"-Ddd.appsec.enabled=true", + "-Ddd.trace.tracer.metrics.enabled=true", + "-Ddd.service.name=${ASM_ENABLED_SERVICE_NAME}", + ] + + protected int numberOfProcesses() { + return 2 + } + + @Override + ProcessBuilder createProcessBuilder(int processIndex) { + if(processIndex == 0){ + return createProcess(processIndex, STANDALONE_BILLING_PROPERTIES) + } + return createProcess(processIndex, ASM_ENABLED_PROPERTIES) + } + + void 'test'() { + setup: + final vulnerableUrl = "http://localhost:${httpPorts[1]}/rest-api/iast?injection=xss" + final url1 = "http://localhost:${httpPorts[0]}/rest-api/greetings" + final request1 = new Request.Builder().url(url1).get().build() + final upstreamUrl = "http://localhost:${httpPorts[0]}/rest-api/greetings?url=${vulnerableUrl}" + final upstreamRequest = new Request.Builder().url(upstreamUrl).get().build() + + when: "firs request upstream service" + final response1 = client.newCall(request1).execute() + + then: "First trace should be sampled with sampler keep as it's the first span checked in a minute" + response1.successful + waitForTraceCount(1) + checkRootSpanPrioritySampling(traces[0], PrioritySampling.SAMPLER_KEEP) + traces.clear() + + when: "Second request upstream service" + final upstreamResponse = client.newCall(upstreamRequest).execute() + + then: "This trace should enter into the sampling mechanism and have a root span with SAMPLER_DROP sampling priority as it's not the first span checked in a minute" + upstreamResponse.successful + waitForTraceCount(2) + checkRootSpanPrioritySampling(getServiceTrace(STANDALONE_BILLING_SERVICE_NAME), PrioritySampling.SAMPLER_DROP) + + and: "Upstream sampler drop ca be overwritten with USER_KEEP for ASM Events" + def asmTrace = getServiceTrace(ASM_ENABLED_SERVICE_NAME) + checkRootSpanPrioritySampling(asmTrace, PrioritySampling.USER_KEEP) + hasASMEvents(asmTrace) + + } + + void "Check that a malicious x-datadog-sampling-priority header can't drop traces with ASM events"() { + setup: + final vulnerableUrl = "http://localhost:${httpPorts[1]}/rest-api/iast?injection=xss" + final request = new Request.Builder().url(vulnerableUrl) + .header("x-datadog-sampling-priority", "-1") + .header("x-datadog-trace-id", "666") + .header("x-datadog-parent-id", "777") + .header("X-Forwarded-For", "8.8.8.8") + .get().build() + + when: + final response1 = client.newCall(request).execute() + + then: "First trace should be sampled with sampler keep as it's the first span checked in a minute" + response1.successful + waitForTraceCount(1) + checkRootSpanPrioritySampling(traces[0], PrioritySampling.USER_KEEP) + + } + + +} diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy index b2744605c6f..ff930409ea1 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy @@ -159,13 +159,13 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm and:"No ASM events, resulting in the local sampling decision" def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) //TODO Check RFC as it said isSampledBySampler(standAloneBillingTrace) but sampling priority it's set in the upstream service and locked for all downstream processes - checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) + isSampledBySampler(standAloneBillingTrace) !hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) and:"Only the local trace is expected to be “muted” and no assumption must be done about the downstream one, so the sampling decision propagated by upstream services must be honored" def downstreamTrace = getServiceTrace(ASM_ENABLED_SERVICE_NAME) - checkRootSpanPrioritySampling(downstreamTrace, PrioritySampling.USER_KEEP) + isSampledBySampler(downstreamTrace) !hasAppsecPropagationTag (downstreamTrace) !hasApmDisabledTag (downstreamTrace) } @@ -196,7 +196,6 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm and:"standalone service must keep the local trace with the local sampling priority" def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) - //TODO Check RFC as it said !hasAppsecPropagationTag (standAloneBillingTrace) but the tag is propagated from the upstream service hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy index 981624a7bda..a9d5e396bd2 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy @@ -49,11 +49,11 @@ class AsmStandaloneBillingSamplingSmokeTest extends AbstractAsmStandaloneBilling when: "Request without ASM events and force kept span" final forceKeepResponse = client.newCall(forceKeepRequest).execute() - then: "This trace should have a root span with USER_KEEP sampling priority as although it's not the first span checked in a minute, it's force kept'" + then: "This trace should have a root span with SAMPLER_DROP sampling priority although it's force kept, as it's not the first span checked in a minute and it has not ASM events" forceKeepResponse.successful waitForTraceCount(3) assert traces.size() == 3 - checkRootSpanPrioritySampling(traces[2], PrioritySampling.USER_KEEP) + checkRootSpanPrioritySampling(traces[2], PrioritySampling.SAMPLER_DROP) !hasAppsecPropagationTag(traces[2]) when: "Second request without ASM events and no force kept span" diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/TimeSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java similarity index 84% rename from dd-trace-core/src/main/java/datadog/trace/common/sampling/TimeSampler.java rename to dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java index eb6a09e1a99..09e74455197 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/TimeSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java @@ -7,14 +7,14 @@ import org.slf4j.LoggerFactory; /** Sampler that samples traces based on a fixed time rate in milliseconds. */ -public class TimeSampler implements Sampler, PrioritySampler { +public class AsmStandaloneSampler implements Sampler, PrioritySampler { - private static final Logger log = LoggerFactory.getLogger(TimeSampler.class); + private static final Logger log = LoggerFactory.getLogger(AsmStandaloneSampler.class); private final AtomicLong lastSampleTime; private final int rateInMilliseconds; - public TimeSampler(int rateInMilliseconds) { + public AsmStandaloneSampler(int rateInMilliseconds) { this.rateInMilliseconds = rateInMilliseconds; this.lastSampleTime = new AtomicLong(-1); } @@ -31,10 +31,10 @@ public > void setSamplingPriority(final T span) { if (shouldSample()) { log.debug("Set SAMPLER_KEEP for span {}", span.getSpanId()); - span.setSamplingPriority(PrioritySampling.SAMPLER_KEEP, SamplingMechanism.DEFAULT); + span.setSamplingPriority(PrioritySampling.SAMPLER_KEEP, SamplingMechanism.APPSEC); } else { log.debug("Set SAMPLER_DROP for span {}", span.getSpanId()); - span.setSamplingPriority(PrioritySampling.SAMPLER_DROP, SamplingMechanism.DEFAULT); + span.setSamplingPriority(PrioritySampling.SAMPLER_DROP, SamplingMechanism.APPSEC); } } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java index 134f3d5d6fc..ef3755f662a 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java @@ -36,7 +36,7 @@ public static Sampler forConfig(final Config config, final TraceConfig traceConf if (config != null) { if (config.isExperimentalAppSecStandaloneEnabled()) { log.debug("APM is disabled. Only 1 trace per minute will be sent."); - return new TimeSampler(60000); // 1 trace per minute + return new AsmStandaloneSampler(60000); // 1 trace per minute } final Map serviceRules = config.getTraceSamplingServiceRules(); final Map operationRules = config.getTraceSamplingOperationRules(); 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 1b2f3252c3b..f59b151c7f8 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 @@ -4,6 +4,7 @@ import static datadog.trace.api.cache.RadixTreeCache.HTTP_STATUSES; import static datadog.trace.bootstrap.instrumentation.api.ErrorPriorities.UNSET; +import datadog.trace.api.Config; import datadog.trace.api.DDTags; import datadog.trace.api.DDTraceId; import datadog.trace.api.Functions; @@ -541,6 +542,12 @@ private boolean setThisSpanSamplingPriority(final int newPriority, final int new if (!validateSamplingPriority(newPriority, newMechanism)) { return false; } + if (Config.get().isExperimentalAppSecStandaloneEnabled() + && newMechanism == SamplingMechanism.APPSEC) { + SAMPLING_PRIORITY_UPDATER.set(this, newPriority); + propagationTags.updateTraceSamplingPriority(newPriority, SamplingMechanism.APPSEC); + return true; + } if (!SAMPLING_PRIORITY_UPDATER.compareAndSet(this, PrioritySampling.UNSET, newPriority)) { if (log.isDebugEnabled()) { log.debug( diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/TimeSamplerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy similarity index 90% rename from dd-trace-core/src/test/groovy/datadog/trace/common/sampling/TimeSamplerTest.groovy rename to dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy index e5724e35146..f6623530bf9 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/TimeSamplerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy @@ -4,14 +4,14 @@ import datadog.trace.common.writer.ListWriter import datadog.trace.core.test.DDCoreSpecification import datadog.trace.api.sampling.PrioritySampling -class TimeSamplerTest extends DDCoreSpecification{ +class AsmStandaloneSamplerTest extends DDCoreSpecification{ def writer = new ListWriter() void "test setSamplingPriority"(){ setup: final rate = 1000 // 1 trace per second - def sampler = new TimeSampler(rate) + def sampler = new AsmStandaloneSampler(rate) def tracer = tracerBuilder().writer(writer).sampler(sampler).build() when: diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy index fa4b21c2e11..cde5087cfcb 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy @@ -14,7 +14,7 @@ class SamplerTest extends DDSpecification{ Sampler sampler = Sampler.Builder.forConfig(config, null) then: - sampler instanceof TimeSampler - ((TimeSampler)sampler).rateInMilliseconds == 60000 //1 minute + sampler instanceof AsmStandaloneSampler + ((AsmStandaloneSampler)sampler).rateInMilliseconds == 60000 //1 minute } } diff --git a/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java b/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java index 3886d07392b..e172d05d713 100644 --- a/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java +++ b/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java @@ -46,6 +46,10 @@ public static boolean validateWithSamplingPriority(int mechanism, int priority) return priority == USER_DROP || priority == USER_KEEP; case APPSEC: + return priority == PrioritySampling.USER_KEEP + || priority == SAMPLER_DROP + || priority == SAMPLER_KEEP; // Necessary for ASM standalone billing + case DATA_JOBS: return priority == PrioritySampling.USER_KEEP; From 768de96a8d8e15eac399b819a1ce4cb98a615d02 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 7 Jun 2024 09:53:45 +0200 Subject: [PATCH 32/62] BE able to stop propagation when standalone is enabled and no appsec propagation tag available --- ...AsmStandaloneBillingMatrixSmokeTest.groovy | 114 ++++++++++++++++++ .../core/propagation/CorePropagation.java | 10 ++ 2 files changed, 124 insertions(+) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy index ff930409ea1..d8d3cfc76f6 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy @@ -206,6 +206,120 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm !hasApmDisabledTag (downstreamTrace) } + void 'Behave as a non APM-instrumented service: Anything but upstream ASM events - determined by the absence of _dd.p.appsec:1 AND No local ASM security events '() { + setup: + final downstreamUrl = "http://localhost:${httpPorts[2]}/rest-api/greetings" + final standAloneBillingUrl = "http://localhost:${httpPorts[0]}/rest-api/greetings?url=${downstreamUrl}" + final upstreamUrl = "http://localhost:${httpPorts[1]}/rest-api/greetings?forceKeep=true&url=${standAloneBillingUrl}" + final request = new Request.Builder().url(upstreamUrl).get().build() + + when: + final response = client.newCall(request).execute() + + then: + response.successful + waitForTraceCount(3) + + and: 'No upstream ASM events but force keep span' + def upstreamTrace = getServiceTrace(APM_ENABLED_SERVICE_NAME) + checkRootSpanPrioritySampling(upstreamTrace, PrioritySampling.USER_KEEP) + !hasAppsecPropagationTag (upstreamTrace) + !hasApmDisabledTag (upstreamTrace) + def upstreamTraceId = getServiceTrace(APM_ENABLED_SERVICE_NAME).spans[0].traceId + + and: 'No ASM events, resulting in the local sampling decision' + def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) + isSampledBySampler(standAloneBillingTrace) + !hasAppsecPropagationTag (standAloneBillingTrace) + hasApmDisabledTag (standAloneBillingTrace) + def standAloneBillingTraceId = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME).spans[0].traceId + upstreamTraceId == standAloneBillingTraceId //There is propagation + + and: 'Propagation is stopped' + def downstreamTrace = getServiceTrace(ASM_ENABLED_SERVICE_NAME) + isSampledBySampler(downstreamTrace) + !hasAppsecPropagationTag (downstreamTrace) + !hasApmDisabledTag (downstreamTrace) + def downstreamTraceId = getServiceTrace(ASM_ENABLED_SERVICE_NAME).spans[0].traceId + standAloneBillingTraceId != downstreamTraceId //There is no propagation + } + + void 'Behave as an APM-instrumented service: Upstream sampling decision involving ASM with the presence of _dd.p.appsec:1 along with Force Keep (2)'(){ + setup: + final downstreamUrl = "http://localhost:${httpPorts[2]}/rest-api/greetings" + final standAloneBillingUrl = "http://localhost:${httpPorts[0]}/rest-api/greetings?url=${downstreamUrl}" + final upstreamUrl = "http://localhost:${httpPorts[3]}/rest-api/iast?injection=vulnerable&url=${standAloneBillingUrl}" + final request = new Request.Builder().url(upstreamUrl).get().build() + + when: + final response = client.newCall(request).execute() + + then: + response.successful + waitForTraceCount(3) + + and: 'Upstream ASM events' + def upstreamTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME_2) + checkRootSpanPrioritySampling(upstreamTrace, PrioritySampling.USER_KEEP) + hasAppsecPropagationTag (upstreamTrace) + hasApmDisabledTag (upstreamTrace) + def upstreamTraceId = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME_2).spans[0].traceId + + and: 'No ASM events, resulting in the local sampling decision' + def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) + checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) + hasAppsecPropagationTag (standAloneBillingTrace) + hasApmDisabledTag (standAloneBillingTrace) + def standAloneBillingTraceId = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME).spans[0].traceId + upstreamTraceId == standAloneBillingTraceId //There is propagation + + and: 'Default APM distributed tracing behavior with' + def downstreamTrace = getServiceTrace(ASM_ENABLED_SERVICE_NAME) + checkRootSpanPrioritySampling(downstreamTrace, PrioritySampling.USER_KEEP) + hasAppsecPropagationTag (downstreamTrace) + !hasApmDisabledTag (downstreamTrace) + def downstreamTraceId = getServiceTrace(ASM_ENABLED_SERVICE_NAME).spans[0].traceId + standAloneBillingTraceId == downstreamTraceId //There is propagation + } + + void 'Behave as an APM-instrumented service: Upstream sampling decision involving ASM with the presence of _dd.p.appsec:1 along with Force Keep (2)'(){ + setup: + final downstreamUrl = "http://localhost:${httpPorts[2]}/rest-api/greetings" + final standAloneBillingUrl = "http://localhost:${httpPorts[0]}/rest-api/iast?injection=vulnerable%26url=${downstreamUrl}" + final upstreamUrl = "http://localhost:${httpPorts[1]}/rest-api/greetings?&url=${standAloneBillingUrl}" + final request = new Request.Builder().url(upstreamUrl).get().build() + + when: + final response = client.newCall(request).execute() + + then: + response.successful + waitForTraceCount(3) + + and: 'No upstream ASM events' + def upstreamTrace = getServiceTrace(APM_ENABLED_SERVICE_NAME) + isSampledBySampler (upstreamTrace) + !hasAppsecPropagationTag (upstreamTrace) + !hasApmDisabledTag (upstreamTrace) + def upstreamTraceId = getServiceTrace(APM_ENABLED_SERVICE_NAME).spans[0].traceId + + and: 'ASM events, resulting in force keep and appsec propagation' + def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) + checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) + hasAppsecPropagationTag (standAloneBillingTrace) + hasApmDisabledTag (standAloneBillingTrace) + def standAloneBillingTraceId = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME).spans[0].traceId + upstreamTraceId == standAloneBillingTraceId //There is propagation + + and: 'Default APM distributed tracing behavior with' + def downstreamTrace = getServiceTrace(ASM_ENABLED_SERVICE_NAME) + checkRootSpanPrioritySampling(downstreamTrace, PrioritySampling.USER_KEEP) + hasAppsecPropagationTag (downstreamTrace) + !hasApmDisabledTag (downstreamTrace) + def downstreamTraceId = getServiceTrace(ASM_ENABLED_SERVICE_NAME).spans[0].traceId + standAloneBillingTraceId == downstreamTraceId //There is propagation + } + } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/CorePropagation.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/CorePropagation.java index 135fff03275..5b95847c8b8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/CorePropagation.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/CorePropagation.java @@ -1,5 +1,6 @@ package datadog.trace.core.propagation; +import datadog.trace.api.Config; import datadog.trace.api.TracePropagationStyle; import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; @@ -58,6 +59,15 @@ private void inject( final DDSpanContext ddSpanContext = (DDSpanContext) context; ddSpanContext.getTrace().setSamplingPriorityIfNecessary(); + /** + * If the experimental appsec standalone feature is enabled and appsec propagation is disabled + * (no ASM events), stop propagation + */ + if (Config.get().isExperimentalAppSecStandaloneEnabled() + && !ddSpanContext.getPropagationTags().isAppsecPropagationEnabled()) { + return; + } + if (null == style) { injector.inject(ddSpanContext, carrier, setter); } else { From 34f21127df7032627bbada4f68e44aab63c57105 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 7 Jun 2024 10:27:24 +0200 Subject: [PATCH 33/62] fix test --- .../datadog/trace/api/sampling/SamplingMechanismTest.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy index 19d5cc6ba63..3d1669cd0c6 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy @@ -72,8 +72,8 @@ class SamplingMechanismTest extends Specification { REMOTE_USER_RATE | userKeepX | false APPSEC | UNSET | false - APPSEC | SAMPLER_DROP | false - APPSEC | SAMPLER_KEEP | false + APPSEC | SAMPLER_DROP | true + APPSEC | SAMPLER_KEEP | true APPSEC | USER_DROP | false APPSEC | USER_KEEP | true APPSEC | userDropX | false From a7c02ddcd7eaf3470a9258e4ce73dde3825e659e Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 7 Jun 2024 13:48:19 +0200 Subject: [PATCH 34/62] fix rebase --- internal-api/src/main/java/datadog/trace/api/Config.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index f3744b07b7d..be7064546d1 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -2262,10 +2262,6 @@ public boolean isTraceEnabled() { return instrumenterConfig.isTraceEnabled(); } - public boolean areTracingDependantProductsEnabled() { - return instrumenterConfig.areTracingDependantProductsEnabled(); - } - public boolean isLongRunningTraceEnabled() { return longRunningTraceEnabled; } @@ -3606,7 +3602,7 @@ public Map getLocalRootSpanTags() { result.put(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE); result.put(SCHEMA_VERSION_TAG_KEY, SpanNaming.instance().version()); result.put(PROFILING_ENABLED, isProfilingEnabled() ? 1 : 0); - if (areTracingDependantProductsEnabled() || isExperimentalAppSecStandaloneEnabled()) { + if (isExperimentalAppSecStandaloneEnabled()) { result.put(APM_ENABLED, 0); } From c2531663c1d26d4656e19414484346707f1c2a1f Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 7 Jun 2024 14:03:29 +0200 Subject: [PATCH 35/62] Add stop propagation test --- .../propagation/CorePropagationTest.groovy | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/CorePropagationTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/CorePropagationTest.groovy index 32d6b3c4774..aa8bde29e1b 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/CorePropagationTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/CorePropagationTest.groovy @@ -204,4 +204,26 @@ class CorePropagationTest extends DDCoreSpecification { root.finish() tracer.close() } + + def 'test ASM standalone billing stop propagation'() { + setup: + injectSysConfig("experimental.appsec.standalone.enabled", "true") + def tracer = tracerBuilder().build() + def span = tracer.buildSpan('test', 'operation').start() + def setter = Mock(AgentPropagation.Setter) + def carrier = new Object() + + when: + propagation.inject(span, carrier, setter) + + then: + 0 * datadogInjector.inject(_, carrier, setter) + 0 * b3Injector.inject(_, carrier, setter) + 0 * traceContextInjector.inject(_, carrier, setter) + 0 * dataStreamContextInjector.injectPathwayContext(_, carrier, setter, _) + + cleanup: + span.finish() + tracer.close() + } } From 86554b80dbbc49c2811c8231dff6702d49d901be Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Jun 2024 08:53:17 +0200 Subject: [PATCH 36/62] spotless --- .../datadog/trace/core/propagation/ptags/PTagsFactory.java | 2 +- .../datadog/trace/core/propagation/ptags/W3CPTagsCodec.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index fffd8705904..1ade2775487 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -121,7 +121,7 @@ public PTags( traceIdTagValue, appsecPropagationEnabled, PrioritySampling.UNSET, - null, + null, null); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index 89c66c46a74..03f724f4427 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -748,7 +748,8 @@ public W3CPTags( int ddMemberStart, int ddMemberValueEnd, int maxUnknownSize, - CharSequence lastParentId) {super( + CharSequence lastParentId) { + super( factory, tagPairs, decisionMakerTagValue, From af0fb3c4cf4caf6be78e041600aba4b4ce41c6b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez=20Garc=C3=ADa?= Date: Mon, 10 Jun 2024 12:22:49 +0200 Subject: [PATCH 37/62] Update dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy Co-authored-by: Santiago M. Mola --- .../AsmStandaloneBillingConerCasesSmokeTest.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy index a676b7718ce..dc4178a90e8 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy @@ -88,7 +88,6 @@ class AsmStandaloneBillingConerCasesSmokeTest extends AbstractAsmStandaloneBilli response1.successful waitForTraceCount(1) checkRootSpanPrioritySampling(traces[0], PrioritySampling.USER_KEEP) - } From 36cafeadb290bf795451f0260265a6ba9e3ce061 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez=20Garc=C3=ADa?= Date: Mon, 10 Jun 2024 12:22:55 +0200 Subject: [PATCH 38/62] Update dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy Co-authored-by: Santiago M. Mola --- .../AsmStandaloneBillingConerCasesSmokeTest.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy index dc4178a90e8..7c90bbbc166 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy @@ -90,5 +90,4 @@ class AsmStandaloneBillingConerCasesSmokeTest extends AbstractAsmStandaloneBilli checkRootSpanPrioritySampling(traces[0], PrioritySampling.USER_KEEP) } - } From 87a6c85013eeba438013942011ff3190d4e32141 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Jun 2024 12:45:49 +0200 Subject: [PATCH 39/62] refactor experimentalAppSecStandaloneEnabled --- .../java/datadog/trace/api/config/AppSecConfig.java | 3 +-- .../common/metrics/MetricsAggregatorFactory.java | 2 +- .../java/datadog/trace/common/sampling/Sampler.java | 2 +- .../trace/common/writer/ddagent/DDAgentApi.java | 2 +- .../main/java/datadog/trace/core/DDSpanContext.java | 3 +-- .../main/java/datadog/trace/core/PendingTrace.java | 2 +- .../trace/core/propagation/CorePropagation.java | 2 +- .../src/main/java/datadog/trace/api/Config.java | 13 ++++++------- 8 files changed, 13 insertions(+), 16 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java index a842b68503c..ba3af7c181e 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java @@ -32,8 +32,7 @@ public final class AppSecConfig { public static final String APPSEC_MAX_STACK_TRACES = "appsec.max.stacktraces"; public static final String APPSEC_MAX_STACK_TRACE_DEPTH = "appsec.max.stacktrace.depth"; - public static final String EXPERIMENTAL_APPSEC_STANDALONE_ENABLED = - "experimental.appsec.standalone.enabled"; + public static final String APPSEC_STANDALONE_ENABLED = "experimental.appsec.standalone.enabled"; private AppSecConfig() {} } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java index 5f27548814e..304f9a4ca9c 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java @@ -11,7 +11,7 @@ public class MetricsAggregatorFactory { public static MetricsAggregator createMetricsAggregator( Config config, SharedCommunicationObjects sharedCommunicationObjects) { if (config.isTracerMetricsEnabled() - && !config.isExperimentalAppSecStandaloneEnabled() // When APM is disabled, libraries must + && !config.isAppSecStandaloneEnabled() // When APM is disabled, libraries must // completely disable the generation of APM // trace metrics ) { diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java index ef3755f662a..dfe2bfb9900 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java @@ -34,7 +34,7 @@ final class Builder { public static Sampler forConfig(final Config config, final TraceConfig traceConfig) { Sampler sampler; if (config != null) { - if (config.isExperimentalAppSecStandaloneEnabled()) { + if (config.isAppSecStandaloneEnabled()) { log.debug("APM is disabled. Only 1 trace per minute will be sent."); return new AsmStandaloneSampler(60000); // 1 trace per minute } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java index 14f124748af..e2649b0929c 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java @@ -110,7 +110,7 @@ public Response sendSerializedTraces(final Payload payload) { .addHeader( DATADOG_CLIENT_COMPUTED_STATS, (metricsEnabled && featuresDiscovery.supportsMetrics()) - || Config.get().isExperimentalAppSecStandaloneEnabled() + || Config.get().isAppSecStandaloneEnabled() ? "true" : "") .put(payload.toRequest()) 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 f59b151c7f8..6b153a23d19 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 @@ -542,8 +542,7 @@ private boolean setThisSpanSamplingPriority(final int newPriority, final int new if (!validateSamplingPriority(newPriority, newMechanism)) { return false; } - if (Config.get().isExperimentalAppSecStandaloneEnabled() - && newMechanism == SamplingMechanism.APPSEC) { + if (Config.get().isAppSecStandaloneEnabled() && newMechanism == SamplingMechanism.APPSEC) { SAMPLING_PRIORITY_UPDATER.set(this, newPriority); propagationTags.updateTraceSamplingPriority(newPriority, SamplingMechanism.APPSEC); return true; 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 c77a3da9339..315f6ea44eb 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 @@ -491,7 +491,7 @@ public void setSamplingPriorityIfNecessary() { // Locks inside DDSpanContext ensure the correct behavior in the race case if (traceConfig.sampler instanceof PrioritySampler && rootSpan != null) { // Ignore the force-keep priority in the absence of propagated _dd.p.appsec span tag. - if (Config.get().isExperimentalAppSecStandaloneEnabled() + if (Config.get().isAppSecStandaloneEnabled() && !rootSpan.context().getPropagationTags().isAppsecPropagationEnabled()) { ((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan); return; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/CorePropagation.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/CorePropagation.java index 5b95847c8b8..960007d091a 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/CorePropagation.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/CorePropagation.java @@ -63,7 +63,7 @@ private void inject( * If the experimental appsec standalone feature is enabled and appsec propagation is disabled * (no ASM events), stop propagation */ - if (Config.get().isExperimentalAppSecStandaloneEnabled() + if (Config.get().isAppSecStandaloneEnabled() && !ddSpanContext.getPropagationTags().isAppsecPropagationEnabled()) { return; } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index be7064546d1..0e853aae540 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -160,10 +160,10 @@ import static datadog.trace.api.config.AppSecConfig.APPSEC_RULES_FILE; import static datadog.trace.api.config.AppSecConfig.APPSEC_SCA_ENABLED; import static datadog.trace.api.config.AppSecConfig.APPSEC_STACK_TRACE_ENABLED; +import static datadog.trace.api.config.AppSecConfig.APPSEC_STANDALONE_ENABLED; import static datadog.trace.api.config.AppSecConfig.APPSEC_TRACE_RATE_LIMIT; import static datadog.trace.api.config.AppSecConfig.APPSEC_WAF_METRICS; import static datadog.trace.api.config.AppSecConfig.APPSEC_WAF_TIMEOUT; -import static datadog.trace.api.config.AppSecConfig.EXPERIMENTAL_APPSEC_STANDALONE_ENABLED; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_ADDITIONAL_CHILD_PROCESS_JVM_ARGS; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_AGENTLESS_ENABLED; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_AGENTLESS_URL; @@ -758,7 +758,7 @@ static class HostNameHolder { private final boolean appSecStackTraceEnabled; private final int appSecMaxStackTraces; private final int appSecMaxStackTraceDepth; - private final boolean experimentalAppSecStandaloneEnabled; + private final boolean appSecStandaloneEnabled; private final boolean apiSecurityEnabled; private final float apiSecurityRequestSampleRate; @@ -1672,8 +1672,7 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) configProvider.getStringNotEmpty( APPSEC_AUTOMATED_USER_EVENTS_TRACKING, SAFE.toString())); appSecScaEnabled = configProvider.getBoolean(APPSEC_SCA_ENABLED); - experimentalAppSecStandaloneEnabled = - configProvider.getBoolean(EXPERIMENTAL_APPSEC_STANDALONE_ENABLED, false); + appSecStandaloneEnabled = configProvider.getBoolean(APPSEC_STANDALONE_ENABLED, false); appSecRaspEnabled = configProvider.getBoolean(APPSEC_RASP_ENABLED, DEFAULT_APPSEC_RASP_ENABLED); appSecStackTraceEnabled = configProvider.getBoolean(APPSEC_STACK_TRACE_ENABLED, DEFAULT_APPSEC_STACK_TRACE_ENABLED); @@ -3602,7 +3601,7 @@ public Map getLocalRootSpanTags() { result.put(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE); result.put(SCHEMA_VERSION_TAG_KEY, SpanNaming.instance().version()); result.put(PROFILING_ENABLED, isProfilingEnabled() ? 1 : 0); - if (isExperimentalAppSecStandaloneEnabled()) { + if (isAppSecStandaloneEnabled()) { result.put(APM_ENABLED, 0); } @@ -4086,8 +4085,8 @@ public Boolean getAppSecScaEnabled() { return appSecScaEnabled; } - public boolean isExperimentalAppSecStandaloneEnabled() { - return experimentalAppSecStandaloneEnabled; + public boolean isAppSecStandaloneEnabled() { + return appSecStandaloneEnabled; } public boolean isAppSecRaspEnabled() { From 1cc7f6ee400a5605463cd81ec922fe9d149e4d06 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Jun 2024 12:47:01 +0200 Subject: [PATCH 40/62] clean comments --- .../AsmStandaloneBillingConerCasesSmokeTest.groovy | 2 -- .../AsmStandaloneBillingMatrixSmokeTest.groovy | 2 -- 2 files changed, 4 deletions(-) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy index 7c90bbbc166..c81ade37501 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy @@ -13,7 +13,6 @@ class AsmStandaloneBillingConerCasesSmokeTest extends AbstractAsmStandaloneBilli "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", - //"-Ddd.appsec.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", "-Ddd.service.name=${STANDALONE_BILLING_SERVICE_NAME}", ] @@ -22,7 +21,6 @@ class AsmStandaloneBillingConerCasesSmokeTest extends AbstractAsmStandaloneBilli "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", - //"-Ddd.appsec.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", "-Ddd.service.name=${ASM_ENABLED_SERVICE_NAME}", ] diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy index d8d3cfc76f6..00f6cec1bed 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy @@ -15,7 +15,6 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", - //"-Ddd.appsec.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", "-Ddd.service.name=${STANDALONE_BILLING_SERVICE_NAME}", ] @@ -25,7 +24,6 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", - //"-Ddd.appsec.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", "-Ddd.service.name=${STANDALONE_BILLING_SERVICE_NAME_2}", ] From 5b9a04dcdd3272867c237d7e3d6df6500d9b938c Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Jun 2024 13:59:16 +0200 Subject: [PATCH 41/62] change valid values for _dd.p.appsec --- .../java/datadog/trace/core/propagation/ptags/PTagsCodec.java | 2 +- .../trace/core/propagation/DatadogPropagationTagsTest.groovy | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java index c865305bae8..3ef1780a6d9 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java @@ -214,7 +214,7 @@ private static boolean validateTraceId(TagValue value) { } private static boolean validateAppsecTagValue(TagValue value) { - return value.length() == 1 && value.charAt(0) == '1'; + return value.length() == 1 && (value.charAt(0) == '1' || value.charAt(0) == '0'); } protected static boolean isDigit(char c) { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy index a9fa2e299ef..d2be877c0ba 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy @@ -154,9 +154,10 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { originalTagSet | enabled | expectedHeaderValue | tags // keep the existing dm tag as is "_dd.p.appsec=1" | true | "_dd.p.appsec=1" | ["_dd.p.appsec": "1"] + "_dd.p.appsec=0" | false | null | [:] "" | false | null | [:] //Invalid input - "_dd.p.appsec=0" | false | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.appsec=foo" | false | null | ["_dd.propagation_error": "decoding_error"] } def extractionLimitExceeded() { From 4b075715010cd8cb32be7fcb0be57f651d3060a2 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Jun 2024 14:07:44 +0200 Subject: [PATCH 42/62] add new test case --- .../trace/core/propagation/DatadogPropagationTagsTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy index d2be877c0ba..bf98320a944 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy @@ -92,7 +92,7 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { headerValue | expectedHeaderValue | tags '_dd.p.dm=934086a686-4' | 'dd=t.dm:934086a686-4' | ['_dd.p.dm': '934086a686-4'] '_dd.p.dm=934086a686-4,_dd.p.f=w00t==' | 'dd=t.dm:934086a686-4;t.f:w00t~~' | ['_dd.p.dm': '934086a686-4', '_dd.p.f': 'w00t=='] - '_dd.p.appsec=1' | 'dd=t.appsec:1' | ['_dd.p.appsec': '1'] + '_dd.p.dm=934086a686-4,_dd.p.appsec=1' | 'dd=t.dm:934086a686-4;t.appsec:1' | ['_dd.p.dm': '934086a686-4', '_dd.p.appsec': '1'] } def "update propagation tags sampling mechanism #originalTagSet"() { From 2e2ee212ed3eb37a2f04e629b171d1e6710199e8 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Jun 2024 14:19:22 +0200 Subject: [PATCH 43/62] add _dd. to asux propagation tag --- .../src/test/groovy/com/datadog/iast/ReporterTest.groovy | 6 +++--- .../datadog/trace/bootstrap/instrumentation/api/Tags.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy index 1ea2ceb5559..96e2cf37f07 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy @@ -80,7 +80,7 @@ class ReporterTest extends DDSpecification { ] }''', batch.toString(), true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('p.appsec', true) + 1 * traceSegment.setTagTop('_dd.p.appsec', true) 0 * _ } @@ -147,7 +147,7 @@ class ReporterTest extends DDSpecification { ] }''', batch.toString(), true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('p.appsec', true) + 1 * traceSegment.setTagTop('_dd.p.appsec', true) 0 * _ } @@ -271,7 +271,7 @@ class ReporterTest extends DDSpecification { 1 * traceSegment.getDataTop('iast') >> null 1 * traceSegment.setDataTop('iast', _ as VulnerabilityBatch) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('p.appsec', true) + 1 * traceSegment.setTagTop('_dd.p.appsec', true) 1 * traceSegment.setTagTop('_dd.iast.enabled', 1) 0 * _ } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java index beda142f321..1cef35a54a0 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java @@ -130,5 +130,5 @@ public class Tags { /** ASM force tracer to keep the trace */ public static final String ASM_KEEP = "asm.keep"; - public static final String PROPAGATED_APPSEC = "p.appsec"; + public static final String PROPAGATED_APPSEC = "_dd.p.appsec"; } From 028bedd8dda173af961f45683faa2c7356ccfc2b Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Jun 2024 14:57:43 +0200 Subject: [PATCH 44/62] move propagation tag set to PowerWaf --- .../main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java | 4 ++++ .../AsmStandaloneBillingConerCasesSmokeTest.groovy | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java index 9994532dfcd..10c08e900c8 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java @@ -481,6 +481,10 @@ public void onDataAvailable( // Keep event related span, because it could be ignored in case of // reduced datadog sampling rate. activeSpan.getLocalRootSpan().setTag(Tags.ASM_KEEP, true); + // If APM is disabled, inform downstream services that the current + // distributed trace contains at least one ASM event and must inherit + // the given force-keep priority + activeSpan.getLocalRootSpan().setTag(Tags.PROPAGATED_APPSEC, true); } else { // If active span is not available the ASK_KEEP tag will be set in the GatewayBridge // when the request ends diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy index c81ade37501..7faf8429507 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy @@ -66,7 +66,6 @@ class AsmStandaloneBillingConerCasesSmokeTest extends AbstractAsmStandaloneBilli def asmTrace = getServiceTrace(ASM_ENABLED_SERVICE_NAME) checkRootSpanPrioritySampling(asmTrace, PrioritySampling.USER_KEEP) hasASMEvents(asmTrace) - } void "Check that a malicious x-datadog-sampling-priority header can't drop traces with ASM events"() { @@ -87,5 +86,4 @@ class AsmStandaloneBillingConerCasesSmokeTest extends AbstractAsmStandaloneBilli waitForTraceCount(1) checkRootSpanPrioritySampling(traces[0], PrioritySampling.USER_KEEP) } - } From b87040b18c7321363f0c9e7e58db9d4fcc32e725 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Jun 2024 15:27:18 +0200 Subject: [PATCH 45/62] improve code --- .../src/main/java/datadog/trace/core/PendingTrace.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) 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 315f6ea44eb..0a3313f4314 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 @@ -491,12 +491,9 @@ public void setSamplingPriorityIfNecessary() { // Locks inside DDSpanContext ensure the correct behavior in the race case if (traceConfig.sampler instanceof PrioritySampler && rootSpan != null) { // Ignore the force-keep priority in the absence of propagated _dd.p.appsec span tag. - if (Config.get().isAppSecStandaloneEnabled() - && !rootSpan.context().getPropagationTags().isAppsecPropagationEnabled()) { - ((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan); - return; - } - if (rootSpan.context().getSamplingPriority() == PrioritySampling.UNSET) { + if ((Config.get().isAppSecStandaloneEnabled() + && !rootSpan.context().getPropagationTags().isAppsecPropagationEnabled()) + || rootSpan.context().getSamplingPriority() == PrioritySampling.UNSET) { ((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan); } } From abce9462205e3cb4c3038b85e90549f38ec077d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez=20Garc=C3=ADa?= Date: Mon, 10 Jun 2024 19:22:48 +0200 Subject: [PATCH 46/62] Update dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel Álvarez Álvarez --- .../datadog/trace/common/sampling/AsmStandaloneSampler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java index 09e74455197..022cd49cd5e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java @@ -16,7 +16,7 @@ public class AsmStandaloneSampler implements Sampler, PrioritySampler { public AsmStandaloneSampler(int rateInMilliseconds) { this.rateInMilliseconds = rateInMilliseconds; - this.lastSampleTime = new AtomicLong(-1); + this.lastSampleTime = new AtomicLong(System.currentTimeMillis() - rateInMilliseconds); } @Override From efe6140b5772553561534457c6d9b839d18874a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez=20Garc=C3=ADa?= Date: Mon, 10 Jun 2024 19:22:58 +0200 Subject: [PATCH 47/62] Update dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel Álvarez Álvarez --- .../trace/common/sampling/AsmStandaloneSampler.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java index 022cd49cd5e..0437fb501d6 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java @@ -40,13 +40,6 @@ public > void setSamplingPriority(final T span) { private boolean shouldSample() { long now = System.currentTimeMillis(); - long lastTime = lastSampleTime.get(); - - if (lastTime == -1 || now - lastTime >= rateInMilliseconds) { - if (lastSampleTime.compareAndSet(lastTime, now)) { - return true; - } - } - return false; + return lastSampleTime.updateAndGet(lastTime -> now - lastTime >= rateInMilliseconds ? now : lastTime) == now; } } From 43f5c2383f1be81b8f22ba799544d88d7428a8b0 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Jun 2024 19:32:36 +0200 Subject: [PATCH 48/62] fix spotless --- .../datadog/trace/common/sampling/AsmStandaloneSampler.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java index 0437fb501d6..bf1fac9ef51 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java @@ -40,6 +40,8 @@ public > void setSamplingPriority(final T span) { private boolean shouldSample() { long now = System.currentTimeMillis(); - return lastSampleTime.updateAndGet(lastTime -> now - lastTime >= rateInMilliseconds ? now : lastTime) == now; + return lastSampleTime.updateAndGet( + lastTime -> now - lastTime >= rateInMilliseconds ? now : lastTime) + == now; } } From 1240d440e9624c270c4197102998baf8bfe8253a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Jun 2024 08:42:23 +0200 Subject: [PATCH 49/62] Remove not necessary tests --- ...tandaloneBillingConerCasesSmokeTest.groovy | 89 ------------------- 1 file changed, 89 deletions(-) delete mode 100644 dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy deleted file mode 100644 index 7faf8429507..00000000000 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingConerCasesSmokeTest.groovy +++ /dev/null @@ -1,89 +0,0 @@ -package datadog.smoketest.asmstandalonebilling - -import datadog.trace.api.sampling.PrioritySampling -import okhttp3.Request - -class AsmStandaloneBillingConerCasesSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { - - private static final String STANDALONE_BILLING_SERVICE_NAME = "asm-standalone-billing-corner-cases-smoketest-app" - private static final String ASM_ENABLED_SERVICE_NAME = "asm-enabled-corner-cases-smoketest-app" - - static final String[] STANDALONE_BILLING_PROPERTIES = [ - "-Ddd.experimental.appsec.standalone.enabled=true", - "-Ddd.iast.enabled=true", - "-Ddd.iast.detection.mode=FULL", - "-Ddd.iast.debug.enabled=true", - "-Ddd.trace.tracer.metrics.enabled=true", - "-Ddd.service.name=${STANDALONE_BILLING_SERVICE_NAME}", - ] - - static final String[] ASM_ENABLED_PROPERTIES = [ - "-Ddd.iast.enabled=true", - "-Ddd.iast.detection.mode=FULL", - "-Ddd.iast.debug.enabled=true", - "-Ddd.trace.tracer.metrics.enabled=true", - "-Ddd.service.name=${ASM_ENABLED_SERVICE_NAME}", - ] - - protected int numberOfProcesses() { - return 2 - } - - @Override - ProcessBuilder createProcessBuilder(int processIndex) { - if(processIndex == 0){ - return createProcess(processIndex, STANDALONE_BILLING_PROPERTIES) - } - return createProcess(processIndex, ASM_ENABLED_PROPERTIES) - } - - void 'test'() { - setup: - final vulnerableUrl = "http://localhost:${httpPorts[1]}/rest-api/iast?injection=xss" - final url1 = "http://localhost:${httpPorts[0]}/rest-api/greetings" - final request1 = new Request.Builder().url(url1).get().build() - final upstreamUrl = "http://localhost:${httpPorts[0]}/rest-api/greetings?url=${vulnerableUrl}" - final upstreamRequest = new Request.Builder().url(upstreamUrl).get().build() - - when: "firs request upstream service" - final response1 = client.newCall(request1).execute() - - then: "First trace should be sampled with sampler keep as it's the first span checked in a minute" - response1.successful - waitForTraceCount(1) - checkRootSpanPrioritySampling(traces[0], PrioritySampling.SAMPLER_KEEP) - traces.clear() - - when: "Second request upstream service" - final upstreamResponse = client.newCall(upstreamRequest).execute() - - then: "This trace should enter into the sampling mechanism and have a root span with SAMPLER_DROP sampling priority as it's not the first span checked in a minute" - upstreamResponse.successful - waitForTraceCount(2) - checkRootSpanPrioritySampling(getServiceTrace(STANDALONE_BILLING_SERVICE_NAME), PrioritySampling.SAMPLER_DROP) - - and: "Upstream sampler drop ca be overwritten with USER_KEEP for ASM Events" - def asmTrace = getServiceTrace(ASM_ENABLED_SERVICE_NAME) - checkRootSpanPrioritySampling(asmTrace, PrioritySampling.USER_KEEP) - hasASMEvents(asmTrace) - } - - void "Check that a malicious x-datadog-sampling-priority header can't drop traces with ASM events"() { - setup: - final vulnerableUrl = "http://localhost:${httpPorts[1]}/rest-api/iast?injection=xss" - final request = new Request.Builder().url(vulnerableUrl) - .header("x-datadog-sampling-priority", "-1") - .header("x-datadog-trace-id", "666") - .header("x-datadog-parent-id", "777") - .header("X-Forwarded-For", "8.8.8.8") - .get().build() - - when: - final response1 = client.newCall(request).execute() - - then: "First trace should be sampled with sampler keep as it's the first span checked in a minute" - response1.successful - waitForTraceCount(1) - checkRootSpanPrioritySampling(traces[0], PrioritySampling.USER_KEEP) - } -} From 91b9b126c97d200b53ff92e5fd787e835569d3ff Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 17 Jun 2024 07:55:27 +0200 Subject: [PATCH 50/62] add tests to validate system tests --- .../asmstandalonebilling/Controller.java | 15 +++-- ...stractAsmStandaloneBillingSmokeTest.groovy | 8 +++ ...mStandaloneBillingSamplingSmokeTest.groovy | 66 +++++++++++++++++++ 3 files changed, 84 insertions(+), 5 deletions(-) diff --git a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java index 38862ad2f26..ebeecb88642 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java +++ b/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java @@ -4,12 +4,11 @@ import io.opentracing.Span; import io.opentracing.util.GlobalTracer; import java.io.IOException; +import java.util.Map; import javax.servlet.http.HttpServletResponse; -import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.PathVariable; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RequestParam; -import org.springframework.web.bind.annotation.RestController; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.*; import org.springframework.web.client.RestTemplate; @RestController @@ -30,6 +29,12 @@ public String greetings( return "Hello I'm service " + System.getProperty("dd.service.name"); } + @GetMapping(value = "/returnheaders", produces = MediaType.APPLICATION_JSON_VALUE) + public ResponseEntity> returnheaders( + @RequestHeader Map headers) { + return ResponseEntity.ok(headers); + } + @GetMapping("/appsec/{id}") public String pathParam( @PathVariable("id") String id, diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy index 8065e1cc3f5..8c8663fa946 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy @@ -49,6 +49,14 @@ abstract class AbstractAsmStandaloneBillingSmokeTest extends AbstractServerSmoke } } + protected DecodedTrace getServiceTraceFromUrl(String url) { + return traces.find { trace -> + trace.spans.find { span -> + span.meta["http.url"] == url + } + } + } + protected checkRootSpanPrioritySampling(DecodedTrace trace, byte priority) { return trace.spans[0].metrics['_sampling_priority_v1'] == priority } diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy index a9d5e396bd2..170519f7bce 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy @@ -1,6 +1,7 @@ package datadog.smoketest.asmstandalonebilling import datadog.trace.api.sampling.PrioritySampling +import groovy.json.JsonSlurper import okhttp3.Request class AsmStandaloneBillingSamplingSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { @@ -10,6 +11,7 @@ class AsmStandaloneBillingSamplingSmokeTest extends AbstractAsmStandaloneBilling final String[] processProperties = [ "-Ddd.experimental.appsec.standalone.enabled=true", "-Ddd.iast.enabled=true", + "-Ddd.appsec.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", "-Ddd.service.name=asm-standalone-billing-sampling-spring-smoketest-app", @@ -66,4 +68,68 @@ class AsmStandaloneBillingSamplingSmokeTest extends AbstractAsmStandaloneBilling checkRootSpanPrioritySampling(traces[3], PrioritySampling.SAMPLER_DROP) !hasAppsecPropagationTag(traces[3]) } + + void 'test propagation in single process'(){ + setup: + final downstreamUrl = "http://localhost:${httpPorts[0]}/rest-api/greetings" + final standAloneBillingUrl = "http://localhost:${httpPorts[0]}/rest-api/iast?injection=vulnerable%26url=${downstreamUrl}" + final upstreamUrl = "http://localhost:${httpPorts[0]}/rest-api/greetings?&url=${standAloneBillingUrl}" + final request = new Request.Builder().url(upstreamUrl).get().build() + + when: + final response = client.newCall(request).execute() + + then: + response.successful + waitForTraceCount(3) + + and: 'No upstream ASM events' + def upstreamTrace = getServiceTraceFromUrl(upstreamUrl) + isSampledBySampler (upstreamTrace) + !hasAppsecPropagationTag (upstreamTrace) + hasApmDisabledTag (upstreamTrace) + def upstreamTraceId = upstreamTrace.spans[0].traceId + + and: 'ASM events, resulting in force keep and appsec propagation' + def standAloneBillingTrace = getServiceTraceFromUrl("http://localhost:${httpPorts[0]}/rest-api/iast?injection=vulnerable&url=http://localhost:${httpPorts[0]}/rest-api/greetings") + checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) + hasAppsecPropagationTag (standAloneBillingTrace) + hasApmDisabledTag (standAloneBillingTrace) + def standAloneBillingTraceId = standAloneBillingTrace.spans[0].traceId + upstreamTraceId != standAloneBillingTraceId //There is no propagation!!!! + + and: 'Default APM distributed tracing behavior with' + def downstreamTrace = getServiceTraceFromUrl(downstreamUrl) + checkRootSpanPrioritySampling(downstreamTrace, PrioritySampling.USER_KEEP) + hasAppsecPropagationTag (downstreamTrace) + hasApmDisabledTag (downstreamTrace) + def downstreamTraceId = downstreamTrace.spans[0].traceId + standAloneBillingTraceId == downstreamTraceId //There is propagation + } + + void 'test propagation simulating 3 process'(){ + setup: + def jsonSlurper = new JsonSlurper() + final trace_id = "1212121212121212121" + final parent_id = "34343434" + final url = "http://localhost:${httpPorts[0]}/rest-api/appsec/appscan_fingerprint?url=http://localhost:${httpPorts[0]}/rest-api/returnheaders" + final request = new Request.Builder() + .url(url) + .header("x-datadog-trace-id", trace_id) + .header("x-datadog-parent-id", parent_id) + .header("x-datadog-origin", "rum") + .header("x-datadog-sampling-priority", "1") + .get().build() + + when: + final response = client.newCall(request).execute() + + then: + response.successful + waitForTraceCount(2) + def downstreamTrace = getServiceTraceFromUrl("http://localhost:${httpPorts[0]}/rest-api/returnheaders") + checkRootSpanPrioritySampling(downstreamTrace, PrioritySampling.USER_KEEP) + def downstreamHeaders = jsonSlurper.parseText(response.body().string()) + downstreamHeaders["x-datadog-sampling-priority"] == "2" + } } From 49c5b75a40d7f30a50aca9163be0c6ef9e130cb7 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 17 Jun 2024 08:03:16 +0200 Subject: [PATCH 51/62] fix merge --- .../appsec/gateway/AppSecRequestContext.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java index 7d4c0a0f31e..f01cdf7009b 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java @@ -6,6 +6,7 @@ import com.datadog.appsec.stack_trace.StackTraceCollection; import com.datadog.appsec.stack_trace.StackTraceEvent; import com.datadog.appsec.util.StandardizedLogging; +import datadog.trace.api.Config; import datadog.trace.api.http.StoredBodySupplier; import datadog.trace.api.internal.TraceSegment; import io.sqreen.powerwaf.Additive; @@ -417,14 +418,11 @@ public void reportEvents(Collection appSecEvents) { for (AppSecEvent event : appSecEvents) { StandardizedLogging.attackDetected(log, event); } - synchronized (this) { - if (this.collectedEvents == null) { - this.collectedEvents = new ArrayList<>(); - } - try { - this.collectedEvents.addAll(events); - } catch (UnsupportedOperationException e) { - throw new IllegalStateException("Events cannot be added anymore"); + if (this.appSecEvents == null) { + synchronized (this) { + if (this.appSecEvents == null) { + this.appSecEvents = new ConcurrentLinkedQueue<>(); + } } } this.appSecEvents.addAll(appSecEvents); From e5afce06fc6fcb7849dda56ba02453716b42c645 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 17 Jun 2024 12:27:51 +0200 Subject: [PATCH 52/62] fix codenarc --- .../AsmStandaloneBillingSamplingSmokeTest.groovy | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy index 170519f7bce..b9e8b988483 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy @@ -110,13 +110,13 @@ class AsmStandaloneBillingSamplingSmokeTest extends AbstractAsmStandaloneBilling void 'test propagation simulating 3 process'(){ setup: def jsonSlurper = new JsonSlurper() - final trace_id = "1212121212121212121" - final parent_id = "34343434" + final traceId = "1212121212121212121" + final parentId = "34343434" final url = "http://localhost:${httpPorts[0]}/rest-api/appsec/appscan_fingerprint?url=http://localhost:${httpPorts[0]}/rest-api/returnheaders" final request = new Request.Builder() .url(url) - .header("x-datadog-trace-id", trace_id) - .header("x-datadog-parent-id", parent_id) + .header("x-datadog-trace-id", traceId) + .header("x-datadog-parent-id", parentId) .header("x-datadog-origin", "rum") .header("x-datadog-sampling-priority", "1") .get().build() From 35b6ef8affd2bf2ba3ec83014ae2d61f426aaa99 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 21 Jun 2024 14:20:20 +0200 Subject: [PATCH 53/62] remove leftovers --- .../AsmStandaloneBillingMatrixSmokeTest.groovy | 2 -- 1 file changed, 2 deletions(-) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy index 00f6cec1bed..a436a6dc4d4 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy @@ -34,7 +34,6 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", - //"-Ddd.appsec.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", "-Ddd.service.name=${ASM_ENABLED_SERVICE_NAME}", ] @@ -156,7 +155,6 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm and:"No ASM events, resulting in the local sampling decision" def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) - //TODO Check RFC as it said isSampledBySampler(standAloneBillingTrace) but sampling priority it's set in the upstream service and locked for all downstream processes isSampledBySampler(standAloneBillingTrace) !hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) From ef4d8458871ba9ef7c13effc25801388a4df75ab Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 21 Jun 2024 14:23:27 +0200 Subject: [PATCH 54/62] Add appSecStandaloneEnabled to toString --- .../AsmStandaloneBillingMatrixSmokeTest.groovy | 3 --- internal-api/src/main/java/datadog/trace/api/Config.java | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy index a436a6dc4d4..4cd1731206d 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy +++ b/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy @@ -315,7 +315,4 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm def downstreamTraceId = getServiceTrace(ASM_ENABLED_SERVICE_NAME).spans[0].traceId standAloneBillingTraceId == downstreamTraceId //There is propagation } - - - } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 0e853aae540..32674c22fb8 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -4747,6 +4747,8 @@ public String toString() { + appSecRaspEnabled + ", dataJobsEnabled=" + dataJobsEnabled + + ", appSecStandaloneEnabled=" + + appSecStandaloneEnabled + '}'; } } From 2ea9ff2c1a1e9d15bb4cd07262ba64abfe64b483 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Jun 2024 09:08:36 +0200 Subject: [PATCH 55/62] minor improvements in PTags codecs and add more tests --- .../trace/core/propagation/ptags/DatadogPTagsCodec.java | 2 +- .../datadog/trace/core/propagation/ptags/W3CPTagsCodec.java | 2 +- .../trace/core/propagation/DatadogPropagationTagsTest.groovy | 2 ++ .../trace/core/propagation/W3CPropagationTagsTest.groovy | 4 ++++ 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java index 918142bcfaa..e5731e44a89 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java @@ -97,7 +97,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { } else if (tagKey.equals(TRACE_ID_TAG)) { traceIdTagValue = tagValue; } else if (tagKey.equals(APPSEC_TAG)) { - appsecPropagationEnabled = true; + appsecPropagationEnabled = tagValue.charAt(0) == '1'; } else { if (tagPairs == null) { // This is roughly the size of a two element linked list but can hold six diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index 03f724f4427..69d9d0277d4 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -149,7 +149,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { } else if (tagKey.equals(TRACE_ID_TAG)) { traceIdTagValue = tagValue; } else if (tagKey.equals(APPSEC_TAG)) { - appsecPropagationEnabled = true; + appsecPropagationEnabled = tagValue.charAt(0) == '1'; } else { if (tagPairs == null) { // This is roughly the size of a two element linked list but can hold six diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy index bf98320a944..caccaf98b4f 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy @@ -73,6 +73,8 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { "_dd.p.tid=123456789abcdefg" | null | ["_dd.propagation_error": "malformed_tid 123456789abcdefg"] // invalid tid tag value: non-hexadecimal characters "_dd.p.tid=-123456789abcdef" | null | ["_dd.propagation_error": "malformed_tid -123456789abcdef"] // invalid tid tag value: non-hexadecimal characters "_dd.p.appsec=1" | "_dd.p.appsec=1" | ["_dd.p.appsec": "1"] + "_dd.p.appsec=0" | null | [:] + "_dd.p.appsec=foo" | null | ["_dd.propagation_error":"decoding_error"] } def "datadog propagation tags should translate to w3c tags #headerValue"() { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy index 837334558a6..3bd0ddfe7b1 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy @@ -238,6 +238,7 @@ class W3CPropagationTagsTest extends DDCoreSpecification { '' | null | [:] 'dd=s:0;t.dm:934086a686-4' | 'dd=s:0;t.dm:934086a686-4' | ['_dd.p.dm': '934086a686-4'] 'dd=s:0;t.appsec:1' | 'dd=s:0;t.appsec:1' | ['_dd.p.appsec': '1'] + 'dd=s:0;t.appsec:0' | 'dd=s:0' | [:] 'dd=s:0;t.dm:934086a686-4;t.appsec:1' | 'dd=s:0;t.dm:934086a686-4;t.appsec:1' | ['_dd.p.dm': '934086a686-4', '_dd.p.appsec': '1'] 'other=whatever,dd=s:0;t.dm:934086a686-4' | 'dd=s:0;t.dm:934086a686-4,other=whatever' | ['_dd.p.dm': '934086a686-4'] 'dd=s:0;t.dm:934086a687-3,other=whatever' | 'dd=s:0;t.dm:934086a687-3,other=whatever' | ['_dd.p.dm': '934086a687-3'] @@ -281,6 +282,8 @@ class W3CPropagationTagsTest extends DDCoreSpecification { 'dd=s:0;t.dm:934086a686-4' | '_dd.p.dm=934086a686-4' | ['_dd.p.dm': '934086a686-4'] 'other=whatever,dd=s:0;t.dm:934086a686-4;t.f:w00t~~' | '_dd.p.dm=934086a686-4,_dd.p.f=w00t==' | ['_dd.p.dm': '934086a686-4', '_dd.p.f': 'w00t=='] 'dd=s:0;t.appsec:1' | '_dd.p.appsec=1' | ['_dd.p.appsec': '1'] + 'dd=s:0;t.appsec:0' | null | [:] + 'dd=s:0;t.appsec:invalid' | null | [:] 'other=whatever,dd=s:0;t.dm:934086a686-4;t.f:w00t~~;t.appsec:1' | '_dd.p.dm=934086a686-4,_dd.p.appsec=1,_dd.p.f=w00t==' | ['_dd.p.dm': '934086a686-4', '_dd.p.f': 'w00t==', '_dd.p.appsec': '1'] 'some=thing,other=whatever' | null | [:] } @@ -337,6 +340,7 @@ class W3CPropagationTagsTest extends DDCoreSpecification { headerValue | appsec | expectedHeaderValue | tags 'dd=t.appsec:1;x:unknown' | false | 'dd=x:unknown' | [:] 'dd=x:unknown' | true | 'dd=t.appsec:1;x:unknown' | ['_dd.p.appsec': '1'] + 'dd=t.appsec:0;x:unknown' | true | 'dd=t.appsec:1;x:unknown' | ['_dd.p.appsec': '1'] } static private String toLcAlpha(String cs) { From 4195bf4f76962d983325f6014b117eb2b2b49c0b Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Jun 2024 09:40:43 +0200 Subject: [PATCH 56/62] Add documented constant to 1 minute value for asm stand alone sampler --- .../trace/common/sampling/AsmStandaloneSampler.java | 6 +++++- .../java/datadog/trace/common/sampling/Sampler.java | 11 ++++++++++- .../datadog/trace/common/sampling/SamplerTest.groovy | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java index bf1fac9ef51..e45f453462e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java @@ -6,7 +6,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** Sampler that samples traces based on a fixed time rate in milliseconds. */ +/** + * This class is designed to sample traces based on a fixed time rate in milliseconds. However, it's + * important to note that the precision of this class is not in the millisecond range due to the use + * of System.currentTimeMillis(). + */ public class AsmStandaloneSampler implements Sampler, PrioritySampler { private static final Logger log = LoggerFactory.getLogger(AsmStandaloneSampler.class); diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java index dfe2bfb9900..18805a321be 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java @@ -20,6 +20,15 @@ /** Main interface to sample a collection of traces. */ public interface Sampler { + /** + * To only allow 1 APM trace per minute as standalone ASM is only interested in the traces + * containing ASM events. But the service catalog and the billing need a continuous ingestion of + * at least at 1 trace per minute to consider a service as being live and billable. In the absence + * of ASM events, no APM traces must be sent, so we need to let some regular APM traces go + * through, even in the absence of ASM events. + */ + int ASM_STANDALONE_BILLING_SAMPLER_RATE = 60000; // 1 minute + /** * Sample a collection of traces based on the parent span * @@ -36,7 +45,7 @@ public static Sampler forConfig(final Config config, final TraceConfig traceConf if (config != null) { if (config.isAppSecStandaloneEnabled()) { log.debug("APM is disabled. Only 1 trace per minute will be sent."); - return new AsmStandaloneSampler(60000); // 1 trace per minute + return new AsmStandaloneSampler(ASM_STANDALONE_BILLING_SAMPLER_RATE); } final Map serviceRules = config.getTraceSamplingServiceRules(); final Map operationRules = config.getTraceSamplingOperationRules(); diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy index cde5087cfcb..87e6bca6c3a 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy @@ -15,6 +15,6 @@ class SamplerTest extends DDSpecification{ then: sampler instanceof AsmStandaloneSampler - ((AsmStandaloneSampler)sampler).rateInMilliseconds == 60000 //1 minute + ((AsmStandaloneSampler)sampler).rateInMilliseconds == Sampler.ASM_STANDALONE_BILLING_SAMPLER_RATE } } From eb471eb1a00fc9c3bcf0aac896f4b0cf477bb90b Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Jun 2024 09:46:37 +0200 Subject: [PATCH 57/62] cache the TagValue.from("1") value --- .../datadog/trace/core/propagation/ptags/PTagsCodec.java | 5 +++-- .../datadog/trace/core/propagation/ptags/PTagsFactory.java | 6 ++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java index 3ef1780a6d9..47bc424fd37 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java @@ -19,6 +19,7 @@ abstract class PTagsCodec { protected static final String PROPAGATION_ERROR_MALFORMED_TID = "malformed_tid "; protected static final String PROPAGATION_ERROR_INCONSISTENT_TID = "inconsistent_tid "; protected static final TagKey UPSTREAM_SERVICES_DEPRECATED_TAG = TagKey.from("upstream_services"); + protected static final TagValue APPSEC_ENABLED_TAG_VALUE = TagValue.from("1"); static String headerValue(PTagsCodec codec, PTags ptags) { int estimate = codec.estimateHeaderSize(ptags); @@ -37,7 +38,7 @@ static String headerValue(PTagsCodec codec, PTags ptags) { size = codec.appendTag(sb, TRACE_ID_TAG, ptags.getTraceIdHighOrderBitsHexTagValue(), size); } if (ptags.isAppsecPropagationEnabled()) { - size = codec.appendTag(sb, APPSEC_TAG, TagValue.from("1"), size); + size = codec.appendTag(sb, APPSEC_TAG, APPSEC_ENABLED_TAG_VALUE, size); } Iterator it = ptags.getTagPairs().iterator(); while (it.hasNext() && !codec.isTooLarge(sb, size)) { @@ -85,7 +86,7 @@ static void fillTagMap(PTags propagationTags, Map tagMap) { if (propagationTags.isAppsecPropagationEnabled()) { tagMap.put( APPSEC_TAG.forType(Encoding.DATADOG).toString(), - TagValue.from("1").forType(Encoding.DATADOG).toString()); + APPSEC_ENABLED_TAG_VALUE.forType(Encoding.DATADOG).toString()); } if (propagationTags.getTraceIdHighOrderBitsHexTagValue() != null) { tagMap.put( diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index 1ade2775487..7351ca47762 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -2,9 +2,7 @@ import static datadog.trace.core.propagation.PropagationTags.HeaderType.DATADOG; import static datadog.trace.core.propagation.PropagationTags.HeaderType.W3C; -import static datadog.trace.core.propagation.ptags.PTagsCodec.APPSEC_TAG; -import static datadog.trace.core.propagation.ptags.PTagsCodec.DECISION_MAKER_TAG; -import static datadog.trace.core.propagation.ptags.PTagsCodec.TRACE_ID_TAG; +import static datadog.trace.core.propagation.ptags.PTagsCodec.*; import datadog.trace.api.internal.util.LongStringUtils; import datadog.trace.api.sampling.PrioritySampling; @@ -339,7 +337,7 @@ int getXDatadogTagsSize() { size = PTagsCodec.calcXDatadogTagsSize(size, DECISION_MAKER_TAG, decisionMakerTagValue); size = PTagsCodec.calcXDatadogTagsSize(size, TRACE_ID_TAG, traceIdHighOrderBitsHexTagValue); if (appsecPropagationEnabled) { - size = PTagsCodec.calcXDatadogTagsSize(size, APPSEC_TAG, TagValue.from("1")); + size = PTagsCodec.calcXDatadogTagsSize(size, APPSEC_TAG, APPSEC_ENABLED_TAG_VALUE); } xDatadogTagsSize = size; } From 67c66b6fb7e3b7e96e0733b78901db21a1489b19 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Jun 2024 11:18:24 +0200 Subject: [PATCH 58/62] move Appsec references from DDSpanContext --- .../main/java/datadog/trace/core/DDSpanContext.java | 5 ++--- .../trace/api/sampling/SamplingMechanism.java | 13 +++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) 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 6b153a23d19..32089d1cd7f 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 @@ -4,7 +4,6 @@ import static datadog.trace.api.cache.RadixTreeCache.HTTP_STATUSES; import static datadog.trace.bootstrap.instrumentation.api.ErrorPriorities.UNSET; -import datadog.trace.api.Config; import datadog.trace.api.DDTags; import datadog.trace.api.DDTraceId; import datadog.trace.api.Functions; @@ -542,9 +541,9 @@ private boolean setThisSpanSamplingPriority(final int newPriority, final int new if (!validateSamplingPriority(newPriority, newMechanism)) { return false; } - if (Config.get().isAppSecStandaloneEnabled() && newMechanism == SamplingMechanism.APPSEC) { + if (SamplingMechanism.canAvoidSamplingPriorityLock(newPriority, newMechanism)) { SAMPLING_PRIORITY_UPDATER.set(this, newPriority); - propagationTags.updateTraceSamplingPriority(newPriority, SamplingMechanism.APPSEC); + propagationTags.updateTraceSamplingPriority(newPriority, newMechanism); return true; } if (!SAMPLING_PRIORITY_UPDATER.compareAndSet(this, PrioritySampling.UNSET, newPriority)) { diff --git a/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java b/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java index e172d05d713..0c71c9968ab 100644 --- a/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java +++ b/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java @@ -2,6 +2,8 @@ import static datadog.trace.api.sampling.PrioritySampling.*; +import datadog.trace.api.Config; + public class SamplingMechanism { /** Not encouraged to use */ public static final byte UNKNOWN = -1; @@ -59,5 +61,16 @@ public static boolean validateWithSamplingPriority(int mechanism, int priority) return true; } + /** + * Returns true if sampling priority lock can be avoided for the given mechanism and priority + * + * @param mechanism + * @param priority + * @return + */ + public static boolean canAvoidSamplingPriorityLock(int priority, int mechanism) { + return Config.get().isAppSecStandaloneEnabled() && mechanism == SamplingMechanism.APPSEC; + } + private SamplingMechanism() {} } From 3cfef896404294ea2839ba441b3b4cca29d2b67c Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Jun 2024 13:15:03 +0200 Subject: [PATCH 59/62] add test to canAvoidSamplingPriorityLock --- .../api/sampling/SamplingMechanismTest.groovy | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy index 3d1669cd0c6..4d98905fc2a 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy @@ -95,4 +95,26 @@ class SamplingMechanismTest extends Specification { EXTERNAL_OVERRIDE | userDropX | false EXTERNAL_OVERRIDE | userKeepX | false } + + void 'Test canAvoidSamplingPriorityLock'(){ + setup: + System.setProperty("dd.experimental.appsec.standalone.enabled", "true") + + expect: + canAvoidSamplingPriorityLock(priority, mechanism) == valid + + where: + mechanism | priority | valid + APPSEC | UNSET | true + APPSEC | SAMPLER_KEEP | true + UNKNOWN | SAMPLER_KEEP | false + DEFAULT | SAMPLER_KEEP | false + AGENT_RATE | SAMPLER_KEEP | false + REMOTE_AUTO_RATE | SAMPLER_KEEP | false + LOCAL_USER_RULE | SAMPLER_KEEP | false + MANUAL | SAMPLER_KEEP | false + REMOTE_USER_RATE | SAMPLER_KEEP | false + DATA_JOBS | SAMPLER_KEEP | false + EXTERNAL_OVERRIDE | SAMPLER_KEEP | false + } } From 26c7ee4f76d934c5f3411feab62628aeec7a3ff5 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 25 Jun 2024 08:03:07 +0200 Subject: [PATCH 60/62] update the config to disable the metric enable flag according standalone appsec --- .../trace/common/metrics/MetricsAggregatorFactory.java | 6 +----- .../datadog/trace/common/writer/ddagent/DDAgentApi.java | 2 ++ internal-api/src/main/java/datadog/trace/api/Config.java | 3 ++- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java index 304f9a4ca9c..491cc12b1c0 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricsAggregatorFactory.java @@ -10,11 +10,7 @@ public class MetricsAggregatorFactory { public static MetricsAggregator createMetricsAggregator( Config config, SharedCommunicationObjects sharedCommunicationObjects) { - if (config.isTracerMetricsEnabled() - && !config.isAppSecStandaloneEnabled() // When APM is disabled, libraries must - // completely disable the generation of APM - // trace metrics - ) { + if (config.isTracerMetricsEnabled()) { log.debug("tracer metrics enabled"); return new ConflatingMetricsAggregator(config, sharedCommunicationObjects); } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java index e2649b0929c..92af3027b91 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java @@ -110,6 +110,8 @@ public Response sendSerializedTraces(final Payload payload) { .addHeader( DATADOG_CLIENT_COMPUTED_STATS, (metricsEnabled && featuresDiscovery.supportsMetrics()) + // Disabling the computation agent-side of the APM trace metrics by + // pretending it was already done by the library || Config.get().isAppSecStandaloneEnabled() ? "true" : "") diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 32674c22fb8..d93b062e7bc 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -2600,7 +2600,8 @@ public boolean isPerfMetricsEnabled() { } public boolean isTracerMetricsEnabled() { - return tracerMetricsEnabled; + // When ASM Standalone Billing is enabled metrics should be disabled + return tracerMetricsEnabled && !isAppSecStandaloneEnabled(); } public boolean isTracerMetricsBufferingEnabled() { From 7a8c016106e313f2ce25cb7f2e620168d8a29575 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 25 Jun 2024 08:59:14 +0200 Subject: [PATCH 61/62] fix test --- .../datadog/trace/api/sampling/SamplingMechanismTest.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy index 4d98905fc2a..60c3634d0e6 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy @@ -1,10 +1,10 @@ package datadog.trace.api.sampling -import spock.lang.Specification +import datadog.trace.test.util.DDSpecification import static datadog.trace.api.sampling.PrioritySampling.* import static datadog.trace.api.sampling.SamplingMechanism.* -class SamplingMechanismTest extends Specification { +class SamplingMechanismTest extends DDSpecification { static userDropX = USER_DROP - 1 static userKeepX = USER_KEEP + 1 @@ -98,7 +98,7 @@ class SamplingMechanismTest extends Specification { void 'Test canAvoidSamplingPriorityLock'(){ setup: - System.setProperty("dd.experimental.appsec.standalone.enabled", "true") + injectSysConfig("dd.experimental.appsec.standalone.enabled", "true") expect: canAvoidSamplingPriorityLock(priority, mechanism) == valid From a88ab8a9db64de6ec4e4011475cffeaf3bbd1795 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 26 Jun 2024 08:42:49 +0200 Subject: [PATCH 62/62] increase rate to avoid test flakiness --- .../trace/common/sampling/AsmStandaloneSamplerTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy index f6623530bf9..69e7856a3ff 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy @@ -10,7 +10,7 @@ class AsmStandaloneSamplerTest extends DDCoreSpecification{ void "test setSamplingPriority"(){ setup: - final rate = 1000 // 1 trace per second + final rate = 2000 // 1 trace each 2 seconds def sampler = new AsmStandaloneSampler(rate) def tracer = tracerBuilder().writer(writer).sampler(sampler).build()