From 54e8a5da38835b61fca46ba4ea866137d4524a8f Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 21 Nov 2025 10:47:33 +0100 Subject: [PATCH 01/14] fix --- .../api/security/AppSecSpanPostProcessor.java | 58 ++++++++----- .../AppSecSpanPostProcessorTest.groovy | 82 +++++++++++++++++++ 2 files changed, 118 insertions(+), 22 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/AppSecSpanPostProcessor.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/AppSecSpanPostProcessor.java index af97152609e..564acf79830 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/AppSecSpanPostProcessor.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/AppSecSpanPostProcessor.java @@ -31,20 +31,26 @@ public AppSecSpanPostProcessor(ApiSecuritySampler sampler, EventProducerService @Override public void process(@Nonnull AgentSpan span, @Nonnull BooleanSupplier timeoutCheck) { - final RequestContext ctx_ = span.getRequestContext(); - if (ctx_ == null) { - return; - } - final AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); - if (ctx == null) { - return; - } - - if (!ctx.isKeepOpenForApiSecurityPostProcessing()) { - return; - } + AppSecRequestContext ctx = null; + RequestContext ctx_ = null; + boolean needsRelease = false; try { + ctx_ = span.getRequestContext(); + if (ctx_ == null) { + return; + } + ctx = ctx_.getData(RequestContextSlot.APPSEC); + if (ctx == null) { + return; + } + + // Check if we acquired a permit for this request - must be inside try to ensure finally runs + needsRelease = ctx.isKeepOpenForApiSecurityPostProcessing(); + if (!needsRelease) { + return; + } + if (timeoutCheck.getAsBoolean()) { log.debug("Timeout detected, skipping API security post-processing"); return; @@ -56,17 +62,25 @@ public void process(@Nonnull AgentSpan span, @Nonnull BooleanSupplier timeoutChe log.debug("Request sampled, processing API security post-processing"); extractSchemas(ctx, ctx_.getTraceSegment()); } finally { - ctx.setKeepOpenForApiSecurityPostProcessing(false); - try { - // XXX: Close the additive first. This is not strictly needed, but it'll prevent getting it - // detected as a - // missed request-ended event. - ctx.closeWafContext(); - ctx.close(); - } catch (Exception e) { - log.debug("Error closing AppSecRequestContext", e); + // Always release the semaphore permit if we acquired one + if (needsRelease) { + if (ctx != null) { + ctx.setKeepOpenForApiSecurityPostProcessing(false); + // XXX: Close the additive first. This is not strictly needed, but it'll prevent getting + // it detected as a missed request-ended event. + try { + ctx.closeWafContext(); + } catch (Exception e) { + log.debug("Error closing WAF context", e); + } + try { + ctx.close(); + } catch (Exception e) { + log.debug("Error closing AppSecRequestContext", e); + } + } + sampler.releaseOne(); } - sampler.releaseOne(); } } diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/AppSecSpanPostProcessorTest.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/AppSecSpanPostProcessorTest.groovy index 321f3876d94..952707dc950 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/AppSecSpanPostProcessorTest.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/AppSecSpanPostProcessorTest.groovy @@ -248,4 +248,86 @@ class AppSecSpanPostProcessorTest extends DDSpecification { 1 * sampler.releaseOne() 0 * _ } + + void 'permit is released even if extractSchemas throws exception'() { + given: + def sampler = Mock(ApiSecuritySamplerImpl) + def producer = Mock(EventProducerService) + def span = Mock(AgentSpan) + def reqCtx = Mock(RequestContext) + def traceSegment = Mock(TraceSegment) + def ctx = Mock(AppSecRequestContext) + def processor = new AppSecSpanPostProcessor(sampler, producer) + + when: + processor.process(span, { false }) + + then: + def ex = thrown(RuntimeException) + ex.message == "Unexpected error" + 1 * span.getRequestContext() >> reqCtx + 1 * reqCtx.getData(_) >> ctx + 1 * ctx.isKeepOpenForApiSecurityPostProcessing() >> true + 1 * sampler.sampleRequest(_) >> true + 1 * reqCtx.getTraceSegment() >> { throw new RuntimeException("Unexpected error") } + 1 * ctx.setKeepOpenForApiSecurityPostProcessing(false) + 1 * ctx.closeWafContext() + 1 * ctx.close() + 1 * sampler.releaseOne() // Critical: permit is still released despite exception + 0 * _ + } + + void 'multiple requests do not exhaust semaphore permits'() { + given: + // Use real ApiSecuritySamplerImpl which has a semaphore with 4 permits + def realSampler = new ApiSecuritySamplerImpl() + def producer = Mock(EventProducerService) + def processor = new AppSecSpanPostProcessor(realSampler, producer) + + when: 'Process 5 consecutive requests that acquire permits' + 5.times { i -> + def span = Mock(AgentSpan) + def reqCtx = Mock(RequestContext) + def ctx = Mock(AppSecRequestContext) + + // Mock the interactions + span.getRequestContext() >> reqCtx + reqCtx.getData(_) >> ctx + ctx.isKeepOpenForApiSecurityPostProcessing() >> true + ctx.setKeepOpenForApiSecurityPostProcessing(false) + ctx.closeWafContext() + ctx.close() + + // Process should complete without issues, releasing permit each time + processor.process(span, { false }) + } + + then: 'All requests complete successfully without permit exhaustion' + noExceptionThrown() + } + + void 'permit is released when ctx cleanup operations fail'() { + given: + def sampler = Mock(ApiSecuritySamplerImpl) + def producer = Mock(EventProducerService) + def span = Mock(AgentSpan) + def reqCtx = Mock(RequestContext) + def ctx = Mock(AppSecRequestContext) + def processor = new AppSecSpanPostProcessor(sampler, producer) + + when: + processor.process(span, { false }) + + then: + noExceptionThrown() + 1 * span.getRequestContext() >> reqCtx + 1 * reqCtx.getData(_) >> ctx + 1 * ctx.isKeepOpenForApiSecurityPostProcessing() >> true + 1 * sampler.sampleRequest(_) >> false + 1 * ctx.setKeepOpenForApiSecurityPostProcessing(false) + 1 * ctx.closeWafContext() >> { throw new RuntimeException("WAF context close failed") } + 1 * ctx.close() >> { throw new RuntimeException("Context close failed") } + 1 * sampler.releaseOne() // Critical: permit is still released despite cleanup failures + 0 * _ + } } From ebcd198656dfb5cc61702edaa97d2da0237bbe08 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 21 Nov 2025 11:45:12 +0100 Subject: [PATCH 02/14] fix spotless --- .../appsec/api/security/AppSecSpanPostProcessorTest.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/AppSecSpanPostProcessorTest.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/AppSecSpanPostProcessorTest.groovy index 952707dc950..d83b5fa29e6 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/AppSecSpanPostProcessorTest.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/AppSecSpanPostProcessorTest.groovy @@ -255,7 +255,6 @@ class AppSecSpanPostProcessorTest extends DDSpecification { def producer = Mock(EventProducerService) def span = Mock(AgentSpan) def reqCtx = Mock(RequestContext) - def traceSegment = Mock(TraceSegment) def ctx = Mock(AppSecRequestContext) def processor = new AppSecSpanPostProcessor(sampler, producer) From a5adf0eee5cffaf7b09f1529ede1e082c3aad01b Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 21 Nov 2025 14:28:05 +0100 Subject: [PATCH 03/14] not sure --- .../main/java/com/datadog/appsec/gateway/GatewayBridge.java | 3 ++- .../datadog/appsec/gateway/GatewayBridgeSpecification.groovy | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 65ae99aa17c..109786432f7 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -839,7 +839,8 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { if (maybeSampleForApiSecurity(ctx, spanInfo, tags)) { if (!Config.get().isApmTracingEnabled()) { traceSeg.setTagTop(Tags.ASM_KEEP, true); - traceSeg.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); + // Note: _dd.p.ts (PROPAGATED_TRACE_SOURCE) is only set when there are actual AppSec events + // (see lines below where collectedEvents is checked), not just for API Security sampling } } else { ctx.closeWafContext(); diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index fd9e99762a0..e47ba855dbb 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -1315,7 +1315,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * spanInfo.getTags() >> TagMap.fromMap(['http.route': 'route']) 1 * requestSampler.preSampleRequest(_) >> true 1 * traceSegment.setTagTop(Tags.ASM_KEEP, true) - 1 * traceSegment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM) + 0 * traceSegment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM) } From 4f0dbb787875df2f5f12ea7dcad95b05c85b7c74 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 21 Nov 2025 14:55:25 +0100 Subject: [PATCH 04/14] add logs --- .../datadog/appsec/gateway/GatewayBridge.java | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 109786432f7..13ebe905f8b 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -836,8 +836,25 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { TraceSegment traceSeg = ctx_.getTraceSegment(); Map tags = spanInfo.getTags(); - if (maybeSampleForApiSecurity(ctx, spanInfo, tags)) { - if (!Config.get().isApmTracingEnabled()) { + // Log upstream propagated tags + Object upstreamPropagatedTs = tags.get(Tags.PROPAGATED_TRACE_SOURCE); + log.info( + "[APPSEC-57815] Request ended - spanId={}, upstream _dd.p.ts={}", + spanInfo.getSpanId(), + upstreamPropagatedTs); + + boolean sampledForApiSec = maybeSampleForApiSecurity(ctx, spanInfo, tags); + boolean apmTracingEnabled = Config.get().isApmTracingEnabled(); + + log.info( + "[APPSEC-57815] sampledForApiSec={}, apmTracingEnabled={}", + sampledForApiSec, + apmTracingEnabled); + + if (sampledForApiSec) { + if (!apmTracingEnabled) { + log.info( + "[APPSEC-57815] Setting ASM_KEEP=true (API Security sampled, APM tracing disabled)"); traceSeg.setTagTop(Tags.ASM_KEEP, true); // Note: _dd.p.ts (PROPAGATED_TRACE_SOURCE) is only set when there are actual AppSec events // (see lines below where collectedEvents is checked), not just for API Security sampling @@ -853,6 +870,8 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { Collection collectedEvents = ctx.transferCollectedEvents(); + log.info("[APPSEC-57815] Collected {} AppSec events", collectedEvents.size()); + for (TraceSegmentPostProcessor pp : this.traceSegmentPostProcessors) { pp.processTraceSegment(traceSeg, ctx, collectedEvents); } @@ -864,8 +883,12 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { // If detected any events - mark span at appsec.event if (!collectedEvents.isEmpty()) { - if (ctx.isManuallyKept()) { + boolean manuallyKept = ctx.isManuallyKept(); + log.info("[APPSEC-57815] AppSec events detected - manuallyKept={}", manuallyKept); + if (manuallyKept) { // Set asm keep in case that root span was not available when events are detected + log.info( + "[APPSEC-57815] Setting ASM_KEEP=true and _dd.p.ts=ASM (AppSec events + manually kept)"); traceSeg.setTagTop(Tags.ASM_KEEP, true); traceSeg.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); } @@ -929,6 +952,14 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { ); } + // Log final state of propagation tags + Object finalPropagatedTs = tags.get(Tags.PROPAGATED_TRACE_SOURCE); + Object finalAsmKeep = tags.get(Tags.ASM_KEEP); + log.info( + "[APPSEC-57815] Request ended - final state: _dd.p.ts={}, _dd.appsec.keep={}", + finalPropagatedTs, + finalAsmKeep); + ctx.close(); return NoopFlow.INSTANCE; } @@ -942,7 +973,10 @@ private boolean maybeSampleForApiSecurity( ctx.setRoute(route.toString()); } ApiSecuritySampler requestSampler = requestSamplerSupplier.get(); - return requestSampler.preSampleRequest(ctx); + boolean sampled = requestSampler.preSampleRequest(ctx); + log.info( + "[APPSEC-57815] API Security sampling decision - route={}, sampled={}", route, sampled); + return sampled; } private Flow onRequestHeadersDone(RequestContext ctx_) { From 71274424a7c3b26a66e3b841b3d4728c5f756da2 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 21 Nov 2025 16:12:01 +0100 Subject: [PATCH 05/14] fix logs --- .../com/datadog/appsec/gateway/GatewayBridge.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 13ebe905f8b..78f2a997f74 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -856,6 +856,9 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { log.info( "[APPSEC-57815] Setting ASM_KEEP=true (API Security sampled, APM tracing disabled)"); traceSeg.setTagTop(Tags.ASM_KEEP, true); + // Verify the tag was set and check sampling priority + Object asmKeepAfterSet = traceSeg.getTagTop(Tags.ASM_KEEP); + log.info("[APPSEC-57815] ASM_KEEP after setTagTop: {}", asmKeepAfterSet); // Note: _dd.p.ts (PROPAGATED_TRACE_SOURCE) is only set when there are actual AppSec events // (see lines below where collectedEvents is checked), not just for API Security sampling } @@ -952,11 +955,12 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { ); } - // Log final state of propagation tags - Object finalPropagatedTs = tags.get(Tags.PROPAGATED_TRACE_SOURCE); - Object finalAsmKeep = tags.get(Tags.ASM_KEEP); + // Log final state of propagation tags from TraceSegment (not from spanInfo.getTags() which is + // immutable) + Object finalPropagatedTs = traceSeg.getTagTop(Tags.PROPAGATED_TRACE_SOURCE); + Object finalAsmKeep = traceSeg.getTagTop(Tags.ASM_KEEP); log.info( - "[APPSEC-57815] Request ended - final state: _dd.p.ts={}, _dd.appsec.keep={}", + "[APPSEC-57815] Request ended - final state from TraceSegment: _dd.p.ts={}, _dd.appsec.keep={}", finalPropagatedTs, finalAsmKeep); From 3337e0a92463f965ccb7755f93cc8d1d895e9887 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 21 Nov 2025 16:38:29 +0100 Subject: [PATCH 06/14] fix PROPAGATED_TRACE_SOURCE --- .../java/com/datadog/appsec/gateway/GatewayBridge.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 78f2a997f74..33a8dfde6c0 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -856,11 +856,12 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { log.info( "[APPSEC-57815] Setting ASM_KEEP=true (API Security sampled, APM tracing disabled)"); traceSeg.setTagTop(Tags.ASM_KEEP, true); - // Verify the tag was set and check sampling priority + // Must set _dd.p.ts locally so TraceCollector respects force-keep in standalone mode + // (TraceCollector.java lines 67-74 ignore force-keep without _dd.p.ts when APM disabled) + traceSeg.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); + // Verify the tag was set Object asmKeepAfterSet = traceSeg.getTagTop(Tags.ASM_KEEP); log.info("[APPSEC-57815] ASM_KEEP after setTagTop: {}", asmKeepAfterSet); - // Note: _dd.p.ts (PROPAGATED_TRACE_SOURCE) is only set when there are actual AppSec events - // (see lines below where collectedEvents is checked), not just for API Security sampling } } else { ctx.closeWafContext(); From 2f3257121c28b8b5ccb4d7d9505c88d47ee4222b Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Nov 2025 09:00:01 +0100 Subject: [PATCH 07/14] fix PROPAGATED_TRACE_SOURCE and test --- .../datadog/appsec/gateway/GatewayBridgeSpecification.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index e47ba855dbb..fd9e99762a0 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -1315,7 +1315,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * spanInfo.getTags() >> TagMap.fromMap(['http.route': 'route']) 1 * requestSampler.preSampleRequest(_) >> true 1 * traceSegment.setTagTop(Tags.ASM_KEEP, true) - 0 * traceSegment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM) + 1 * traceSegment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM) } From 8f0abf6328d0c390c37482ab123e859ed37d5910 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Nov 2025 10:22:53 +0100 Subject: [PATCH 08/14] change logs --- .../datadog/appsec/gateway/GatewayBridge.java | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 33a8dfde6c0..8295b182723 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -847,12 +847,20 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { boolean apmTracingEnabled = Config.get().isApmTracingEnabled(); log.info( - "[APPSEC-57815] sampledForApiSec={}, apmTracingEnabled={}", + "[APPSEC-57815] sampledForApiSec={}, apmTracingEnabled={}, traceSeg={}, isNoOp={}", sampledForApiSec, - apmTracingEnabled); + apmTracingEnabled, + traceSeg, + traceSeg == TraceSegment.NoOp.INSTANCE); + + if (!sampledForApiSec) { + ctx.closeWafContext(); + } - if (sampledForApiSec) { - if (!apmTracingEnabled) { + // AppSec report metric and events for web span only + if (traceSeg != null) { + // Set API Security sampling tags if needed + if (sampledForApiSec && !apmTracingEnabled) { log.info( "[APPSEC-57815] Setting ASM_KEEP=true (API Security sampled, APM tracing disabled)"); traceSeg.setTagTop(Tags.ASM_KEEP, true); @@ -861,14 +869,13 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { traceSeg.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); // Verify the tag was set Object asmKeepAfterSet = traceSeg.getTagTop(Tags.ASM_KEEP); - log.info("[APPSEC-57815] ASM_KEEP after setTagTop: {}", asmKeepAfterSet); + Object propagatedTsAfterSet = traceSeg.getTagTop(Tags.PROPAGATED_TRACE_SOURCE); + log.info( + "[APPSEC-57815] After setTagTop - ASM_KEEP: {}, _dd.p.ts: {}", + asmKeepAfterSet, + propagatedTsAfterSet); } - } else { - ctx.closeWafContext(); - } - // AppSec report metric and events for web span only - if (traceSeg != null) { traceSeg.setTagTop("_dd.appsec.enabled", 1); traceSeg.setTagTop("_dd.runtime_family", "jvm"); From fe7549460172b3ddb740e9a46eef657378ea6cb3 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Nov 2025 11:33:42 +0100 Subject: [PATCH 09/14] change logs --- .../api/security/ApiSecuritySamplerImpl.java | 31 ++++++++++++++-- .../datadog/appsec/gateway/GatewayBridge.java | 35 +++++++------------ 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java index c51bd46ef44..303e09bc341 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java @@ -54,22 +54,49 @@ public ApiSecuritySamplerImpl( public boolean preSampleRequest(final @Nonnull AppSecRequestContext ctx) { final String route = ctx.getRoute(); if (route == null) { + log.info("[APPSEC-57815] preSampleRequest returning false - route is null"); return false; } final String method = ctx.getMethod(); if (method == null) { + log.info("[APPSEC-57815] preSampleRequest returning false - method is null, route={}", route); return false; } final int statusCode = ctx.getResponseStatus(); if (statusCode <= 0) { + log.info( + "[APPSEC-57815] preSampleRequest returning false - invalid statusCode={}, route={}, method={}", + statusCode, + route, + method); return false; } long hash = computeApiHash(route, method, statusCode); ctx.setApiSecurityEndpointHash(hash); - if (!isApiAccessExpired(hash)) { + + boolean isExpired = isApiAccessExpired(hash); + log.info( + "[APPSEC-57815] preSampleRequest checking expiration - route={}, method={}, statusCode={}, hash={}, isExpired={}, expirationTimeMs={}", + route, + method, + statusCode, + hash, + isExpired, + expirationTimeInMs); + + if (!isExpired) { return false; } - if (counter.tryAcquire()) { + + int availablePermits = counter.availablePermits(); + boolean acquired = counter.tryAcquire(); + log.info( + "[APPSEC-57815] preSampleRequest semaphore check - availablePermits={}, acquired={}, route={}", + availablePermits, + acquired, + route); + + if (acquired) { log.debug("API security sampling is required for this request (presampled)"); ctx.setKeepOpenForApiSecurityPostProcessing(true); return true; diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 8295b182723..0986b8828b2 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -861,19 +861,10 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { if (traceSeg != null) { // Set API Security sampling tags if needed if (sampledForApiSec && !apmTracingEnabled) { - log.info( - "[APPSEC-57815] Setting ASM_KEEP=true (API Security sampled, APM tracing disabled)"); traceSeg.setTagTop(Tags.ASM_KEEP, true); // Must set _dd.p.ts locally so TraceCollector respects force-keep in standalone mode // (TraceCollector.java lines 67-74 ignore force-keep without _dd.p.ts when APM disabled) traceSeg.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); - // Verify the tag was set - Object asmKeepAfterSet = traceSeg.getTagTop(Tags.ASM_KEEP); - Object propagatedTsAfterSet = traceSeg.getTagTop(Tags.PROPAGATED_TRACE_SOURCE); - log.info( - "[APPSEC-57815] After setTagTop - ASM_KEEP: {}, _dd.p.ts: {}", - asmKeepAfterSet, - propagatedTsAfterSet); } traceSeg.setTagTop("_dd.appsec.enabled", 1); @@ -881,8 +872,6 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { Collection collectedEvents = ctx.transferCollectedEvents(); - log.info("[APPSEC-57815] Collected {} AppSec events", collectedEvents.size()); - for (TraceSegmentPostProcessor pp : this.traceSegmentPostProcessors) { pp.processTraceSegment(traceSeg, ctx, collectedEvents); } @@ -895,11 +884,8 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { // If detected any events - mark span at appsec.event if (!collectedEvents.isEmpty()) { boolean manuallyKept = ctx.isManuallyKept(); - log.info("[APPSEC-57815] AppSec events detected - manuallyKept={}", manuallyKept); if (manuallyKept) { // Set asm keep in case that root span was not available when events are detected - log.info( - "[APPSEC-57815] Setting ASM_KEEP=true and _dd.p.ts=ASM (AppSec events + manually kept)"); traceSeg.setTagTop(Tags.ASM_KEEP, true); traceSeg.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); } @@ -963,15 +949,6 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { ); } - // Log final state of propagation tags from TraceSegment (not from spanInfo.getTags() which is - // immutable) - Object finalPropagatedTs = traceSeg.getTagTop(Tags.PROPAGATED_TRACE_SOURCE); - Object finalAsmKeep = traceSeg.getTagTop(Tags.ASM_KEEP); - log.info( - "[APPSEC-57815] Request ended - final state from TraceSegment: _dd.p.ts={}, _dd.appsec.keep={}", - finalPropagatedTs, - finalAsmKeep); - ctx.close(); return NoopFlow.INSTANCE; } @@ -979,13 +956,25 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { private boolean maybeSampleForApiSecurity( AppSecRequestContext ctx, IGSpanInfo spanInfo, Map tags) { log.debug("Checking API Security for end of request handler on span: {}", spanInfo.getSpanId()); + // API Security sampling requires http.route tag. final Object route = tags.get(Tags.HTTP_ROUTE); + final String existingRoute = ctx.getRoute(); + + log.info( + "[APPSEC-57815] maybeSampleForApiSecurity called - spanId={}, route from tags={}, existingRoute={}, ctx.hashCode={}", + spanInfo.getSpanId(), + route, + existingRoute, + System.identityHashCode(ctx)); + if (route != null) { ctx.setRoute(route.toString()); } + ApiSecuritySampler requestSampler = requestSamplerSupplier.get(); boolean sampled = requestSampler.preSampleRequest(ctx); + log.info( "[APPSEC-57815] API Security sampling decision - route={}, sampled={}", route, sampled); return sampled; From cc5a6bc0c3fced012035ebc6cbddd96f9c53051f Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Nov 2025 12:06:16 +0100 Subject: [PATCH 10/14] iteration 1 --- .../api/security/ApiSecuritySamplerImpl.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java index 303e09bc341..fa3badf41e4 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java @@ -99,6 +99,13 @@ public boolean preSampleRequest(final @Nonnull AppSecRequestContext ctx) { if (acquired) { log.debug("API security sampling is required for this request (presampled)"); ctx.setKeepOpenForApiSecurityPostProcessing(true); + // Update the map immediately to prevent race condition where multiple concurrent + // requests see the same expired state before any of them updates the map + updateApiAccessIfExpired(hash); + log.info( + "[APPSEC-57815] preSampleRequest updated accessMap immediately - route={}, hash={}", + route, + hash); return true; } return false; @@ -115,7 +122,14 @@ public boolean sampleRequest(AppSecRequestContext ctx) { // This should never happen, it should have been short-circuited before. return false; } - return updateApiAccessIfExpired(hash); + // Note: With the race condition fix, the map was already updated by preSampleRequest() + // when the semaphore was acquired. We just need to confirm the sampling decision. + // No need to check expiration again since that would fail (we just updated it). + log.info( + "[APPSEC-57815] sampleRequest called - hash={}, route={}, returning true (map already updated in preSampleRequest)", + hash, + ctx.getRoute()); + return true; } @Override From bc5f9b30922a29bcc6c10bfaaa31765c01040990 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Nov 2025 12:52:46 +0100 Subject: [PATCH 11/14] iteration 2 --- .../api/security/ApiSecuritySamplerImpl.java | 52 ++++--------------- .../datadog/appsec/gateway/GatewayBridge.java | 28 +--------- 2 files changed, 11 insertions(+), 69 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java index fa3badf41e4..15185ad0096 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java @@ -54,64 +54,40 @@ public ApiSecuritySamplerImpl( public boolean preSampleRequest(final @Nonnull AppSecRequestContext ctx) { final String route = ctx.getRoute(); if (route == null) { - log.info("[APPSEC-57815] preSampleRequest returning false - route is null"); return false; } final String method = ctx.getMethod(); if (method == null) { - log.info("[APPSEC-57815] preSampleRequest returning false - method is null, route={}", route); return false; } final int statusCode = ctx.getResponseStatus(); if (statusCode <= 0) { - log.info( - "[APPSEC-57815] preSampleRequest returning false - invalid statusCode={}, route={}, method={}", - statusCode, - route, - method); return false; } long hash = computeApiHash(route, method, statusCode); ctx.setApiSecurityEndpointHash(hash); - boolean isExpired = isApiAccessExpired(hash); - log.info( - "[APPSEC-57815] preSampleRequest checking expiration - route={}, method={}, statusCode={}, hash={}, isExpired={}, expirationTimeMs={}", - route, - method, - statusCode, - hash, - isExpired, - expirationTimeInMs); - - if (!isExpired) { + if (!isApiAccessExpired(hash)) { return false; } - int availablePermits = counter.availablePermits(); - boolean acquired = counter.tryAcquire(); - log.info( - "[APPSEC-57815] preSampleRequest semaphore check - availablePermits={}, acquired={}, route={}", - availablePermits, - acquired, - route); - - if (acquired) { + if (counter.tryAcquire()) { log.debug("API security sampling is required for this request (presampled)"); ctx.setKeepOpenForApiSecurityPostProcessing(true); - // Update the map immediately to prevent race condition where multiple concurrent - // requests see the same expired state before any of them updates the map + // Update immediately to prevent concurrent requests from seeing the same expired state updateApiAccessIfExpired(hash); - log.info( - "[APPSEC-57815] preSampleRequest updated accessMap immediately - route={}, hash={}", - route, - hash); return true; } return false; } - /** Get the final sampling decision. This method is NOT thread-safe. */ + /** + * Confirms the final sampling decision. + * + *

This method is called after the span completes. The actual sampling decision and map update + * already happened in {@link #preSampleRequest(AppSecRequestContext)} to prevent race conditions. + * This method only serves as a final confirmation gate before schema extraction. + */ @Override public boolean sampleRequest(AppSecRequestContext ctx) { if (ctx == null) { @@ -119,16 +95,8 @@ public boolean sampleRequest(AppSecRequestContext ctx) { } final Long hash = ctx.getApiSecurityEndpointHash(); if (hash == null) { - // This should never happen, it should have been short-circuited before. return false; } - // Note: With the race condition fix, the map was already updated by preSampleRequest() - // when the semaphore was acquired. We just need to confirm the sampling decision. - // No need to check expiration again since that would fail (we just updated it). - log.info( - "[APPSEC-57815] sampleRequest called - hash={}, route={}, returning true (map already updated in preSampleRequest)", - hash, - ctx.getRoute()); return true; } diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 0986b8828b2..50981dd90c2 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -836,23 +836,9 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { TraceSegment traceSeg = ctx_.getTraceSegment(); Map tags = spanInfo.getTags(); - // Log upstream propagated tags - Object upstreamPropagatedTs = tags.get(Tags.PROPAGATED_TRACE_SOURCE); - log.info( - "[APPSEC-57815] Request ended - spanId={}, upstream _dd.p.ts={}", - spanInfo.getSpanId(), - upstreamPropagatedTs); - boolean sampledForApiSec = maybeSampleForApiSecurity(ctx, spanInfo, tags); boolean apmTracingEnabled = Config.get().isApmTracingEnabled(); - log.info( - "[APPSEC-57815] sampledForApiSec={}, apmTracingEnabled={}, traceSeg={}, isNoOp={}", - sampledForApiSec, - apmTracingEnabled, - traceSeg, - traceSeg == TraceSegment.NoOp.INSTANCE); - if (!sampledForApiSec) { ctx.closeWafContext(); } @@ -959,25 +945,13 @@ private boolean maybeSampleForApiSecurity( // API Security sampling requires http.route tag. final Object route = tags.get(Tags.HTTP_ROUTE); - final String existingRoute = ctx.getRoute(); - - log.info( - "[APPSEC-57815] maybeSampleForApiSecurity called - spanId={}, route from tags={}, existingRoute={}, ctx.hashCode={}", - spanInfo.getSpanId(), - route, - existingRoute, - System.identityHashCode(ctx)); if (route != null) { ctx.setRoute(route.toString()); } ApiSecuritySampler requestSampler = requestSamplerSupplier.get(); - boolean sampled = requestSampler.preSampleRequest(ctx); - - log.info( - "[APPSEC-57815] API Security sampling decision - route={}, sampled={}", route, sampled); - return sampled; + return requestSampler.preSampleRequest(ctx); } private Flow onRequestHeadersDone(RequestContext ctx_) { From 7d7782a1ae532382acb8d5c62492e6b835252ba2 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Nov 2025 13:51:19 +0100 Subject: [PATCH 12/14] fix tests --- .../security/ApiSecuritySamplerTest.groovy | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/ApiSecuritySamplerTest.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/ApiSecuritySamplerTest.groovy index a4ef9984786..4d60e7a8527 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/ApiSecuritySamplerTest.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/api/security/ApiSecuritySamplerTest.groovy @@ -138,35 +138,40 @@ class ApiSecuritySamplerTest extends DDSpecification { !sampled } - void 'sampleRequest honors expiration'() { + void 'preSampleRequest honors expiration'() { given: - def ctx = createContext('route1', 'GET', 200) - ctx.setApiSecurityEndpointHash(42L) - ctx.setKeepOpenForApiSecurityPostProcessing(true) + def ctx1 = createContext('route1', 'GET', 200) + def ctx2 = createContext('route1', 'GET', 200) + def ctx3 = createContext('route1', 'GET', 200) final timeSource = new ControllableTimeSource() timeSource.set(0) final long expirationTimeInMs = 10L final long expirationTimeInNs = expirationTimeInMs * 1_000_000 def sampler = new ApiSecuritySamplerImpl(10, expirationTimeInMs, timeSource) - when: - def sampled = sampler.sampleRequest(ctx) + when: 'first request samples' + def preSampled1 = sampler.preSampleRequest(ctx1) + def sampled1 = sampler.sampleRequest(ctx1) then: - sampled + preSampled1 + sampled1 - when: - sampled = sampler.sampleRequest(ctx) + when: 'second request to same endpoint before expiration' + def preSampled2 = sampler.preSampleRequest(ctx2) then: 'second request is not sampled' - !sampled + !preSampled2 when: 'expiration time has passed' + sampler.releaseOne() timeSource.advance(expirationTimeInNs) - sampled = sampler.sampleRequest(ctx) + def preSampled3 = sampler.preSampleRequest(ctx3) + def sampled3 = sampler.sampleRequest(ctx3) then: 'request is sampled again' - sampled + preSampled3 + sampled3 } void 'internal accessMap never goes beyond capacity'() { @@ -198,10 +203,13 @@ class ApiSecuritySamplerTest extends DDSpecification { expect: for (int i = 0; i < maxCapacity * 10; i++) { - final ctx = createContext('route1', 'GET', 200 + 1) - ctx.setApiSecurityEndpointHash(i as long) - ctx.setKeepOpenForApiSecurityPostProcessing(true) - assert sampler.sampleRequest(ctx) + final ctx = createContext('route1', 'GET', 200 + i) + def preSampled = sampler.preSampleRequest(ctx) + // First request always samples, then we advance time so each subsequent request expires + assert preSampled + def sampled = sampler.sampleRequest(ctx) + assert sampled + sampler.releaseOne() assert sampler.accessMap.size() <= 2 if (i % 2) { timeSource.advance(expirationTimeInMs * 1_000_000) From 4d1ba307c69bcbc6e150db29844dd568754d9f0a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 24 Nov 2025 15:15:00 +0100 Subject: [PATCH 13/14] self review --- .../appsec/api/security/ApiSecuritySamplerImpl.java | 3 +-- .../com/datadog/appsec/gateway/GatewayBridge.java | 12 ++---------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java index 15185ad0096..5be83b3e702 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java @@ -66,11 +66,9 @@ public boolean preSampleRequest(final @Nonnull AppSecRequestContext ctx) { } long hash = computeApiHash(route, method, statusCode); ctx.setApiSecurityEndpointHash(hash); - if (!isApiAccessExpired(hash)) { return false; } - if (counter.tryAcquire()) { log.debug("API security sampling is required for this request (presampled)"); ctx.setKeepOpenForApiSecurityPostProcessing(true); @@ -95,6 +93,7 @@ public boolean sampleRequest(AppSecRequestContext ctx) { } final Long hash = ctx.getApiSecurityEndpointHash(); if (hash == null) { + // This should never happen, it should have been short-circuited before. return false; } return true; diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 50981dd90c2..cb4d16c831e 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -837,7 +837,6 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { Map tags = spanInfo.getTags(); boolean sampledForApiSec = maybeSampleForApiSecurity(ctx, spanInfo, tags); - boolean apmTracingEnabled = Config.get().isApmTracingEnabled(); if (!sampledForApiSec) { ctx.closeWafContext(); @@ -845,11 +844,8 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { // AppSec report metric and events for web span only if (traceSeg != null) { - // Set API Security sampling tags if needed - if (sampledForApiSec && !apmTracingEnabled) { + if (sampledForApiSec && !Config.get().isApmTracingEnabled()) { traceSeg.setTagTop(Tags.ASM_KEEP, true); - // Must set _dd.p.ts locally so TraceCollector respects force-keep in standalone mode - // (TraceCollector.java lines 67-74 ignore force-keep without _dd.p.ts when APM disabled) traceSeg.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); } @@ -869,8 +865,7 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { // If detected any events - mark span at appsec.event if (!collectedEvents.isEmpty()) { - boolean manuallyKept = ctx.isManuallyKept(); - if (manuallyKept) { + if (ctx.isManuallyKept()) { // Set asm keep in case that root span was not available when events are detected traceSeg.setTagTop(Tags.ASM_KEEP, true); traceSeg.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); @@ -942,14 +937,11 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { private boolean maybeSampleForApiSecurity( AppSecRequestContext ctx, IGSpanInfo spanInfo, Map tags) { log.debug("Checking API Security for end of request handler on span: {}", spanInfo.getSpanId()); - // API Security sampling requires http.route tag. final Object route = tags.get(Tags.HTTP_ROUTE); - if (route != null) { ctx.setRoute(route.toString()); } - ApiSecuritySampler requestSampler = requestSamplerSupplier.get(); return requestSampler.preSampleRequest(ctx); } From a49f1b2289a8380cc3a8a2105430c68b8fb8805f Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 28 Nov 2025 10:02:13 +0100 Subject: [PATCH 14/14] remove unnecessary changes --- .../api/security/AppSecSpanPostProcessor.java | 62 ++++++++----------- 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/AppSecSpanPostProcessor.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/AppSecSpanPostProcessor.java index 564acf79830..d7ca7f5da11 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/AppSecSpanPostProcessor.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/AppSecSpanPostProcessor.java @@ -31,26 +31,20 @@ public AppSecSpanPostProcessor(ApiSecuritySampler sampler, EventProducerService @Override public void process(@Nonnull AgentSpan span, @Nonnull BooleanSupplier timeoutCheck) { - AppSecRequestContext ctx = null; - RequestContext ctx_ = null; - boolean needsRelease = false; - - try { - ctx_ = span.getRequestContext(); - if (ctx_ == null) { - return; - } - ctx = ctx_.getData(RequestContextSlot.APPSEC); - if (ctx == null) { - return; - } + final RequestContext ctx_ = span.getRequestContext(); + if (ctx_ == null) { + return; + } + final AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); + if (ctx == null) { + return; + } - // Check if we acquired a permit for this request - must be inside try to ensure finally runs - needsRelease = ctx.isKeepOpenForApiSecurityPostProcessing(); - if (!needsRelease) { - return; - } + if (!ctx.isKeepOpenForApiSecurityPostProcessing()) { + return; + } + try { if (timeoutCheck.getAsBoolean()) { log.debug("Timeout detected, skipping API security post-processing"); return; @@ -62,25 +56,21 @@ public void process(@Nonnull AgentSpan span, @Nonnull BooleanSupplier timeoutChe log.debug("Request sampled, processing API security post-processing"); extractSchemas(ctx, ctx_.getTraceSegment()); } finally { - // Always release the semaphore permit if we acquired one - if (needsRelease) { - if (ctx != null) { - ctx.setKeepOpenForApiSecurityPostProcessing(false); - // XXX: Close the additive first. This is not strictly needed, but it'll prevent getting - // it detected as a missed request-ended event. - try { - ctx.closeWafContext(); - } catch (Exception e) { - log.debug("Error closing WAF context", e); - } - try { - ctx.close(); - } catch (Exception e) { - log.debug("Error closing AppSecRequestContext", e); - } - } - sampler.releaseOne(); + ctx.setKeepOpenForApiSecurityPostProcessing(false); + // XXX: Close the additive first. This is not strictly needed, but it'll prevent getting it + // detected as a + // missed request-ended event. + try { + ctx.closeWafContext(); + } catch (Exception e) { + log.debug("Error closing WAF context", e); + } + try { + ctx.close(); + } catch (Exception e) { + log.debug("Error closing AppSecRequestContext", e); } + sampler.releaseOne(); } }