Skip to content

Add block_failure telemetry for Tomcat blocking enforcement failures #12493

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
jandro996 wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Add block_failure telemetry for Tomcat blocking enforcement failures #12493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
803a700
Add block-outcome telemetry for Tomcat blocking enforcement failures
jandro996 Sep 15, 2026
b6b1546
Split TomcatBlockingHelper to fix muzzle failures in block-outcome te…
jandro996 Sep 15, 2026
cbff781
Merge branch 'master' into block-telemetry-2
jandro996 Sep 15, 2026
1fc84cc
Merge branch 'master' into block-telemetry-2
jandro996 Sep 15, 2026
e794405
Fix spring-webmvc test fixture clobbering Tomcat's real BlockResponse…
jandro996 Sep 15, 2026
f3cbd0a
Merge remote-tracking branch 'origin/block-telemetry-2' into block-te…
jandro996 Sep 15, 2026
f34f16f
Fix: do not report block_failure when GlassFishBlockingHelper has no …
jandro996 Sep 15, 2026
f468901
Fix: propagate commit exceptions instead of swallowing them in BlockF…
jandro996 Sep 15, 2026
1e718e1
Merge branch 'master' into block-telemetry-2
jandro996 Sep 15, 2026
6fc0248
Merge branch 'master' into block-telemetry-2
jandro996 Sep 15, 2026
ad61aa5
Update tomcat-common gradle.lockfile for new test dependencies
jandro996 Sep 15, 2026
d0b8dcf
Merge remote-tracking branch 'origin/block-telemetry-2' into block-te…
jandro996 Sep 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions ...ring/spring-webmvc/spring-webmvc-3.1/src/latestDepTest/groovy/test/SetupSpecHelper.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,34 @@ class SetupSpecHelper {
ss.registerCallback(uriEvent, new TriFunction<RequestContext, String, URIDataAdapter, Flow<Void>>() {
@Override
Flow<Void> apply(RequestContext requestContext, String s, URIDataAdapter uriDataAdapter) {
requestContext.setBlockResponseFunction(TestSpringBlockResponseFunction.INSTANCE)
BlockResponseFunction original = requestContext.getBlockResponseFunction()
requestContext.setBlockResponseFunction(new TestSpringBlockResponseFunction(original))
origUriCallback.apply(requestContext, s, uriDataAdapter)
}
})
}

enum TestSpringBlockResponseFunction implements BlockResponseFunction {
INSTANCE
/**
* Commits the blocking response through Spring's request attributes when they are available,
* falling back to the block response function the server instrumentation had already registered
* (e.g. Tomcat's) for block points that happen before Spring populates RequestContextHolder.
*/
static class TestSpringBlockResponseFunction implements BlockResponseFunction {
private final BlockResponseFunction original

TestSpringBlockResponseFunction(BlockResponseFunction original) {
this.original = original
}

@Override
boolean tryCommitBlockingResponse(TraceSegment segment, int statusCode, BlockingContentType templateType, Map<String, String> extraHeaders, String securityResponseId) {
ServletRequestAttributes attributes = RequestContextHolder.requestAttributes
if (attributes) {
ServletBlockingHelper
.commitBlockingResponse(segment, attributes.request, attributes.response, statusCode, templateType, extraHeaders, securityResponseId)
return true
}
true
original != null && original.tryCommitBlockingResponse(segment, statusCode, templateType, extraHeaders, securityResponseId)
}
}
}
19 changes: 15 additions & 4 deletions ...tation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/test/SetupSpecHelper.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,34 @@ class SetupSpecHelper {
ss.registerCallback(uriEvent, new TriFunction<RequestContext, String, URIDataAdapter, Flow<Void>>() {
@Override
Flow<Void> apply(RequestContext requestContext, String s, URIDataAdapter uriDataAdapter) {
requestContext.setBlockResponseFunction(TestSpringBlockResponseFunction.INSTANCE)
BlockResponseFunction original = requestContext.getBlockResponseFunction()
requestContext.setBlockResponseFunction(new TestSpringBlockResponseFunction(original))
origUriCallback.apply(requestContext, s, uriDataAdapter)
}
})
}

enum TestSpringBlockResponseFunction implements BlockResponseFunction {
INSTANCE
/**
* Commits the blocking response through Spring's request attributes when they are available,
* falling back to the block response function the server instrumentation had already registered
* (e.g. Tomcat's) for block points that happen before Spring populates RequestContextHolder.
*/
static class TestSpringBlockResponseFunction implements BlockResponseFunction {
private final BlockResponseFunction original

TestSpringBlockResponseFunction(BlockResponseFunction original) {
this.original = original
}

@Override
boolean tryCommitBlockingResponse(TraceSegment segment, int statusCode, BlockingContentType templateType, Map<String, String> extraHeaders, String securityResponseId) {
ServletRequestAttributes attributes = RequestContextHolder.requestAttributes
if (attributes) {
ServletBlockingHelper
.commitBlockingResponse(segment, attributes.request, attributes.response, statusCode, templateType, extraHeaders, securityResponseId)
return true
}
true
original != null && original.tryCommitBlockingResponse(segment, statusCode, templateType, extraHeaders, securityResponseId)
}
}
}
19 changes: 15 additions & 4 deletions ...ebmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SetupSpecHelper.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,34 @@ class SetupSpecHelper {
ss.registerCallback(uriEvent, new TriFunction<RequestContext, String, URIDataAdapter, Flow<Void>>() {
@Override
Flow<Void> apply(RequestContext requestContext, String s, URIDataAdapter uriDataAdapter) {
requestContext.setBlockResponseFunction(TestSpringBlockResponseFunction.INSTANCE)
BlockResponseFunction original = requestContext.getBlockResponseFunction()
requestContext.setBlockResponseFunction(new TestSpringBlockResponseFunction(original))
origUriCallback.apply(requestContext, s, uriDataAdapter)
}
})
}

enum TestSpringBlockResponseFunction implements BlockResponseFunction {
INSTANCE
/**
* Commits the blocking response through Spring's request attributes when they are available,
* falling back to the block response function the server instrumentation had already registered
* (e.g. Tomcat's) for block points that happen before Spring populates RequestContextHolder.
*/
static class TestSpringBlockResponseFunction implements BlockResponseFunction {
private final BlockResponseFunction original

TestSpringBlockResponseFunction(BlockResponseFunction original) {
this.original = original
}

@Override
boolean tryCommitBlockingResponse(TraceSegment segment, int statusCode, BlockingContentType templateType, Map<String, String> extraHeaders, String securityResponseId) {
ServletRequestAttributes attributes = RequestContextHolder.requestAttributes
if (attributes) {
JakartaServletBlockingHelper
.commitBlockingResponse(segment, attributes.request, attributes.response, statusCode, templateType, extraHeaders, securityResponseId)
return true
}
true
original != null && original.tryCommitBlockingResponse(segment, statusCode, templateType, extraHeaders, securityResponseId)
}
}
}
1 change: 1 addition & 0 deletions ...tomcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/RequestInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public String[] helperClassNames() {
packageName + ".TomcatDecorator",
packageName + ".TomcatDecorator$TomcatBlockResponseFunction",
packageName + ".TomcatBlockingHelper",
packageName + ".BlockFailureReporter",
packageName + ".RequestURIDataAdapter",
};
}
Expand Down
1 change: 1 addition & 0 deletions ...omcat-5.5/src/main/java/datadog/trace/instrumentation/tomcat/ResponseInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public String[] helperClassNames() {
packageName + ".TomcatDecorator",
packageName + ".TomcatDecorator$TomcatBlockResponseFunction",
packageName + ".TomcatBlockingHelper",
packageName + ".BlockFailureReporter",
packageName + ".RequestURIDataAdapter",
};
}
Expand Down
8 changes: 3 additions & 5 deletions ...t-5.5/src/main/java/datadog/trace/instrumentation/tomcat/TomcatServerInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public String[] helperClassNames() {
packageName + ".TomcatDecorator$TomcatBlockResponseFunction",
packageName + ".RequestURIDataAdapter",
packageName + ".TomcatBlockingHelper",
packageName + ".BlockFailureReporter",
};
}

Expand Down Expand Up @@ -186,9 +187,7 @@ public static class PostParseAdvice {

@Advice.OnMethodExit(suppress = Throwable.class)
public static void afterParse(
@Advice.Argument(1) Request req,
@Advice.Argument(3) Response resp,
@Advice.Return(readOnly = false) Boolean ret) {
@Advice.Argument(1) Request req, @Advice.Return(readOnly = false) Boolean ret) {
Object contextObj = req.getAttribute(DD_CONTEXT_ATTRIBUTE);
if (contextObj instanceof Context) {
Context context = (Context) contextObj;
Expand All @@ -202,8 +201,7 @@ public static void afterParse(
DECORATE.onRequest(span, req, req, parentContext);
Flow.Action.RequestBlockingAction rba = span.getRequestBlockingAction();
if (rba != null) {
TomcatBlockingHelper.commitBlockingResponse(
span.getRequestContext().getTraceSegment(), req, resp, rba);
BlockFailureReporter.tryCommitAndReport(span.getRequestContext(), rba);
ret = false; // skip pipeline
}
}
Expand Down
4 changes: 3 additions & 1 deletion ...5.5/src/main/java/datadog/trace/instrumentation/tomcat55/CommitActionInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator;
import datadog.trace.instrumentation.tomcat.BlockFailureReporter;
import datadog.trace.instrumentation.tomcat.ExtractAdapter;
import datadog.trace.instrumentation.tomcat.TomcatDecorator;
import net.bytebuddy.asm.Advice;
Expand Down Expand Up @@ -75,6 +76,7 @@ public String[] helperClassNames() {
pkg + ".TomcatDecorator",
pkg + ".TomcatDecorator$TomcatBlockResponseFunction",
pkg + ".TomcatBlockingHelper",
pkg + ".BlockFailureReporter",
pkg + ".RequestURIDataAdapter",
};
}
Expand Down Expand Up @@ -114,7 +116,7 @@ static class ProcessCommitActionAdvice {
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
BlockResponseFunction brf = requestContext.getBlockResponseFunction();
if (brf != null) {
brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba);
BlockFailureReporter.tryCommitAndReport(requestContext, rba);
thiz.action(ActionCode.ACTION_CLOSE, null);
return true;
}
Expand Down
8 changes: 7 additions & 1 deletion ...main/java/datadog/trace/instrumentation/tomcat55/ParsedBodyParametersInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import datadog.trace.api.gateway.RequestContextSlot;
import datadog.trace.bootstrap.CallDepthThreadLocalMap;
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import datadog.trace.instrumentation.tomcat.BlockFailureReporter;
import java.util.Hashtable;
import java.util.function.BiFunction;
import net.bytebuddy.asm.Advice;
Expand Down Expand Up @@ -65,6 +66,11 @@ public Reference[] additionalMuzzleReferences() {
return new Reference[] {PARAM_HASH_STRING_ARRAY_REFERENCE};
}

@Override
public String[] helperClassNames() {
return new String[] {"datadog.trace.instrumentation.tomcat.BlockFailureReporter"};
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(
Expand Down Expand Up @@ -143,7 +149,7 @@ static void after(
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
if (blockResponseFunction != null) {
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
BlockFailureReporter.tryCommitAndReport(reqCtx, rba);
if (t == null) {
t = new BlockingException("Blocked request (for processParameters)");
}
Expand Down
1 change: 1 addition & 0 deletions dd-java-agent/instrumentation/tomcat/tomcat-appsec/tomcat-appsec-6.0/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ muzzle {

dependencies {
compileOnly group: 'org.apache.tomcat', name: 'coyote', version: '6.0.53'
implementation project(':dd-java-agent:instrumentation:tomcat:tomcat-common')

@AlexeyKuznetsov-DD AlexeyKuznetsov-DD Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here, probably lock files should be regenerated.

@jandro996 jandro996 Sep 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Regenerated but no-op diff for this file, it's a project dependency

}

// testing happens in tomcat-5.5 module
18 changes: 8 additions & 10 deletions .../main/java/datadog/trace/instrumentation/tomcat6/ParsedBodyParametersInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.agent.tooling.muzzle.Reference;
import datadog.trace.api.gateway.BlockResponseFunction;
import datadog.trace.api.gateway.CallbackProvider;
import datadog.trace.api.gateway.Flow;
import datadog.trace.api.gateway.RequestContext;
import datadog.trace.api.gateway.RequestContextSlot;
import datadog.trace.bootstrap.CallDepthThreadLocalMap;
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import datadog.trace.instrumentation.tomcat.BlockFailureReporter;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -53,6 +53,11 @@ public Reference[] additionalMuzzleReferences() {
return new Reference[] {PARAM_HASH_VALUES_MAP_REFERENCE};
}

@Override
public String[] helperClassNames() {
return new String[] {"datadog.trace.instrumentation.tomcat.BlockFailureReporter"};
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(
Expand Down Expand Up @@ -129,15 +134,8 @@ static void after(
Flow.Action action = flow.getAction();
if (action instanceof Flow.Action.RequestBlockingAction) {
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
if (blockResponseFunction != null) {
boolean committedBlockingResponse =
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
if (committedBlockingResponse) {
if (t == null) {
t = new BlockingException("Blocked request (for processParameters)");
}
}
if (BlockFailureReporter.tryCommitAndReport(reqCtx, rba) && t == null) {
t = new BlockingException("Blocked request (for processParameters)");
}
}
} finally {
Expand Down
4 changes: 3 additions & 1 deletion ...-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/CommitActionInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import datadog.trace.api.gateway.RequestContext;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator;
import datadog.trace.instrumentation.tomcat.BlockFailureReporter;
import datadog.trace.instrumentation.tomcat.ExtractAdapter;
import datadog.trace.instrumentation.tomcat.TomcatDecorator;
import net.bytebuddy.asm.Advice;
Expand Down Expand Up @@ -76,6 +77,7 @@ public String[] helperClassNames() {
pkg + ".TomcatDecorator",
pkg + ".TomcatDecorator$TomcatBlockResponseFunction",
pkg + ".TomcatBlockingHelper",
pkg + ".BlockFailureReporter",
pkg + ".RequestURIDataAdapter",
};
}
Expand Down Expand Up @@ -121,7 +123,7 @@ static class ProcessCommitActionAdvice {
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
BlockResponseFunction brf = requestContext.getBlockResponseFunction();
if (brf != null) {
brf.tryCommitBlockingResponse(requestContext.getTraceSegment(), rba);
BlockFailureReporter.tryCommitAndReport(requestContext, rba);
thiz.action(ActionCode.CLOSE, null);
return true;
}
Expand Down
20 changes: 17 additions & 3 deletions ...psec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/GlassFishBlockingHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import datadog.trace.api.gateway.RequestContext;
import datadog.trace.api.http.MultipartContentDecoder;
import datadog.trace.bootstrap.blocking.BlockingActionHelper;
import datadog.trace.instrumentation.tomcat.BlockFailureReporter;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -16,9 +17,13 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.Part;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public final class GlassFishBlockingHelper {

private static final Logger log = LoggerFactory.getLogger(GlassFishBlockingHelper.class);

public static final int MAX_FILE_CONTENT_COUNT = Config.get().getAppSecMaxFileContentCount();
public static final int MAX_FILE_CONTENT_BYTES = Config.get().getAppSecMaxFileContentBytes();

Expand All @@ -38,12 +43,21 @@ public static boolean tryBlock(
try {
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
if (brf != null) {
brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
// tryCommitAndReport already reports the block failure when the commit fails
if (!BlockFailureReporter.tryCommitAndReport(reqCtx, rba)) {
return false;
}
} else if (!commitBlocking(fallbackReq, fallbackResp, rba)) {
if (fallbackResp != null) {
// a commit was genuinely attempted (there was a response to write to) and failed
BlockFailureReporter.reportBlockFailure(reqCtx);
}
return false;
}
} catch (Exception ignored) {
// commit failed — response not sent, cannot block this request
} catch (Exception e) {
// commit failed - response not sent, cannot block this request
log.debug("Error committing blocking response", e);
BlockFailureReporter.reportBlockFailure(reqCtx);
return false;
}
// Response was committed — mark as blocked on a best-effort basis.
Expand Down
2 changes: 2 additions & 0 deletions ...rc/main/java/datadog/trace/instrumentation/tomcat7/GlassFishMultipartInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public String instrumentedType() {
@Override
public String[] helperClassNames() {
return new String[] {
// referenced by GlassFishBlockingHelper.tryBlock() to commit the blocking response
"datadog.trace.instrumentation.tomcat.BlockFailureReporter",
"datadog.trace.instrumentation.tomcat7.GlassFishBlockingHelper",
};
}
Expand Down
9 changes: 6 additions & 3 deletions ...ec-7.0/src/main/java/datadog/trace/instrumentation/tomcat7/ParsePartsInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import datadog.trace.api.gateway.RequestContextSlot;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import datadog.trace.instrumentation.tomcat.BlockFailureReporter;
import java.util.List;
import java.util.function.BiFunction;
import net.bytebuddy.asm.Advice;
Expand Down Expand Up @@ -56,6 +57,8 @@ public String instrumentedType() {
@Override
public String[] helperClassNames() {
return new String[] {
// referenced by the inlined advice below to commit the blocking response
"datadog.trace.instrumentation.tomcat.BlockFailureReporter",
"datadog.trace.instrumentation.tomcat7.ParameterCollector",
"datadog.trace.instrumentation.tomcat7.ParameterCollector$ParameterCollectorNoop",
"datadog.trace.instrumentation.tomcat7.ParameterCollector$ParameterCollectorImpl",
Expand Down Expand Up @@ -122,7 +125,7 @@ static void after(
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
BlockResponseFunction blockResponseFunction = reqCtx.getBlockResponseFunction();
if (blockResponseFunction != null) {
blockResponseFunction.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
BlockFailureReporter.tryCommitAndReport(reqCtx, rba);
t = new BlockingException("Blocked request (for Request/parseParts)");
reqCtx.getTraceSegment().effectivelyBlocked();
}
Expand All @@ -142,7 +145,7 @@ static void after(
(Flow.Action.RequestBlockingAction) filenamesAction;
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
if (brf != null) {
brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
BlockFailureReporter.tryCommitAndReport(reqCtx, rba);
t = new BlockingException("Blocked request (multipart file upload)");
reqCtx.getTraceSegment().effectivelyBlocked();
}
Expand All @@ -163,7 +166,7 @@ static void after(
(Flow.Action.RequestBlockingAction) contentAction;
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
if (brf != null) {
brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
BlockFailureReporter.tryCommitAndReport(reqCtx, rba);
t = new BlockingException("Blocked request (multipart file upload content)");
reqCtx.getTraceSegment().effectivelyBlocked();
}
Expand Down
Loading