Skip to content

Improve detection of missing request end events #8510

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.

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

Merged
merged 1 commit into from
Mar 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dd-java-agent/appsec/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ dependencies {
testImplementation project(':utils:test-utils')
testImplementation group: 'org.hamcrest', name: 'hamcrest', version: '2.2'
testImplementation group: 'com.flipkart.zjsonpatch', name: 'zjsonpatch', version: '0.4.11'
testImplementation libs.logback.classic

testFixturesApi project(':dd-java-agent:testing')
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ public class AppSecRequestContext implements DataBundle, Closeable {
// keep a reference to the last published usr.session_id
private volatile String sessionId;

// Used to detect missing request-end event at close.
private volatile boolean requestEndCalled;

private static final AtomicIntegerFieldUpdater<AppSecRequestContext> WAF_TIMEOUTS_UPDATER =
AtomicIntegerFieldUpdater.newUpdater(AppSecRequestContext.class, "wafTimeouts");
private static final AtomicIntegerFieldUpdater<AppSecRequestContext> RASP_TIMEOUTS_UPDATER =
Expand Down Expand Up @@ -584,12 +587,15 @@ public void close() {
/* Should be accessible from the modules */

public void close(boolean requiresPostProcessing) {
if (additive != null || derivatives != null) {
if (!requestEndCalled) {
log.debug(SEND_TELEMETRY, "Request end event was not called before close");
}
if (additive != null) {
log.debug(
SEND_TELEMETRY, "WAF object had not been closed (probably missed request-end event)");
closeAdditive();
derivatives = null;
}
derivatives = null;

// check if we might need to further post process data related to the span in order to not free
// related data
Expand Down Expand Up @@ -699,4 +705,9 @@ public boolean isThrottled(RateLimiter rateLimiter) {
public boolean isAdditiveClosed() {
return additiveClosed;
}

/** Must be called during request end event processing. */
void setRequestEndCalled() {
requestEndCalled = true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) {
if (ctx == null) {
return NoopFlow.INSTANCE;
}
ctx.setRequestEndCalled();

maybeExtractSchemas(ctx);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import com.datadog.appsec.config.CurrentAppSecConfig
import com.datadog.appsec.event.data.KnownAddresses
import com.datadog.appsec.event.data.MapDataBundle
import com.datadog.appsec.report.AppSecEvent
import datadog.trace.api.telemetry.LogCollector
import datadog.trace.test.logging.TestLogCollector
import datadog.trace.util.stacktrace.StackTraceEvent
import com.datadog.appsec.test.StubAppSecConfigService
import datadog.trace.test.util.DDSpecification
Expand Down Expand Up @@ -316,4 +318,21 @@ class AppSecRequestContextSpecification extends DDSpecification {
ctx.getWafError(AppSecRequestContext.DD_WAF_RUN_INVALID_ARGUMENT_ERROR) == 0
ctx.getWafError(0) == 0
}

void 'close logs if request end was not called'() {
given:
TestLogCollector.enable()
def ctx = new AppSecRequestContext()

when:
ctx.close()

then:
def log = TestLogCollector.drainCapturedLogs().find { it.message.contains('Request end event was not called before close') }
log != null
log.marker == LogCollector.SEND_TELEMETRY

cleanup:
TestLogCollector.disable()
}
}
12 changes: 12 additions & 0 deletions dd-java-agent/appsec/src/test/resources/logback-test.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<configuration>
<appender name="TEST" class="datadog.trace.test.logging.TestLogbackAppender"/>
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<encoder>
<pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
</encoder>
</appender>
<root level="DEBUG">
<appender-ref ref="TEST"/>
<appender-ref ref="STDOUT"/>
</root>
</configuration>