Skip to content

Commit bc5f9b3

Browse files
committed
iteration 2
1 parent cc5a6bc commit bc5f9b3

File tree

2 files changed

+11
-69
lines changed

2 files changed

+11
-69
lines changed

dd-java-agent/appsec/src/main/java/com/datadog/appsec/api/security/ApiSecuritySamplerImpl.java

Lines changed: 10 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -54,81 +54,49 @@ public ApiSecuritySamplerImpl(
5454
public boolean preSampleRequest(final @Nonnull AppSecRequestContext ctx) {
5555
final String route = ctx.getRoute();
5656
if (route == null) {
57-
log.info("[APPSEC-57815] preSampleRequest returning false - route is null");
5857
return false;
5958
}
6059
final String method = ctx.getMethod();
6160
if (method == null) {
62-
log.info("[APPSEC-57815] preSampleRequest returning false - method is null, route={}", route);
6361
return false;
6462
}
6563
final int statusCode = ctx.getResponseStatus();
6664
if (statusCode <= 0) {
67-
log.info(
68-
"[APPSEC-57815] preSampleRequest returning false - invalid statusCode={}, route={}, method={}",
69-
statusCode,
70-
route,
71-
method);
7265
return false;
7366
}
7467
long hash = computeApiHash(route, method, statusCode);
7568
ctx.setApiSecurityEndpointHash(hash);
7669

77-
boolean isExpired = isApiAccessExpired(hash);
78-
log.info(
79-
"[APPSEC-57815] preSampleRequest checking expiration - route={}, method={}, statusCode={}, hash={}, isExpired={}, expirationTimeMs={}",
80-
route,
81-
method,
82-
statusCode,
83-
hash,
84-
isExpired,
85-
expirationTimeInMs);
86-
87-
if (!isExpired) {
70+
if (!isApiAccessExpired(hash)) {
8871
return false;
8972
}
9073

91-
int availablePermits = counter.availablePermits();
92-
boolean acquired = counter.tryAcquire();
93-
log.info(
94-
"[APPSEC-57815] preSampleRequest semaphore check - availablePermits={}, acquired={}, route={}",
95-
availablePermits,
96-
acquired,
97-
route);
98-
99-
if (acquired) {
74+
if (counter.tryAcquire()) {
10075
log.debug("API security sampling is required for this request (presampled)");
10176
ctx.setKeepOpenForApiSecurityPostProcessing(true);
102-
// Update the map immediately to prevent race condition where multiple concurrent
103-
// requests see the same expired state before any of them updates the map
77+
// Update immediately to prevent concurrent requests from seeing the same expired state
10478
updateApiAccessIfExpired(hash);
105-
log.info(
106-
"[APPSEC-57815] preSampleRequest updated accessMap immediately - route={}, hash={}",
107-
route,
108-
hash);
10979
return true;
11080
}
11181
return false;
11282
}
11383

114-
/** Get the final sampling decision. This method is NOT thread-safe. */
84+
/**
85+
* Confirms the final sampling decision.
86+
*
87+
* <p>This method is called after the span completes. The actual sampling decision and map update
88+
* already happened in {@link #preSampleRequest(AppSecRequestContext)} to prevent race conditions.
89+
* This method only serves as a final confirmation gate before schema extraction.
90+
*/
11591
@Override
11692
public boolean sampleRequest(AppSecRequestContext ctx) {
11793
if (ctx == null) {
11894
return false;
11995
}
12096
final Long hash = ctx.getApiSecurityEndpointHash();
12197
if (hash == null) {
122-
// This should never happen, it should have been short-circuited before.
12398
return false;
12499
}
125-
// Note: With the race condition fix, the map was already updated by preSampleRequest()
126-
// when the semaphore was acquired. We just need to confirm the sampling decision.
127-
// No need to check expiration again since that would fail (we just updated it).
128-
log.info(
129-
"[APPSEC-57815] sampleRequest called - hash={}, route={}, returning true (map already updated in preSampleRequest)",
130-
hash,
131-
ctx.getRoute());
132100
return true;
133101
}
134102

dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -836,23 +836,9 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) {
836836
TraceSegment traceSeg = ctx_.getTraceSegment();
837837
Map<String, Object> tags = spanInfo.getTags();
838838

839-
// Log upstream propagated tags
840-
Object upstreamPropagatedTs = tags.get(Tags.PROPAGATED_TRACE_SOURCE);
841-
log.info(
842-
"[APPSEC-57815] Request ended - spanId={}, upstream _dd.p.ts={}",
843-
spanInfo.getSpanId(),
844-
upstreamPropagatedTs);
845-
846839
boolean sampledForApiSec = maybeSampleForApiSecurity(ctx, spanInfo, tags);
847840
boolean apmTracingEnabled = Config.get().isApmTracingEnabled();
848841

849-
log.info(
850-
"[APPSEC-57815] sampledForApiSec={}, apmTracingEnabled={}, traceSeg={}, isNoOp={}",
851-
sampledForApiSec,
852-
apmTracingEnabled,
853-
traceSeg,
854-
traceSeg == TraceSegment.NoOp.INSTANCE);
855-
856842
if (!sampledForApiSec) {
857843
ctx.closeWafContext();
858844
}
@@ -959,25 +945,13 @@ private boolean maybeSampleForApiSecurity(
959945

960946
// API Security sampling requires http.route tag.
961947
final Object route = tags.get(Tags.HTTP_ROUTE);
962-
final String existingRoute = ctx.getRoute();
963-
964-
log.info(
965-
"[APPSEC-57815] maybeSampleForApiSecurity called - spanId={}, route from tags={}, existingRoute={}, ctx.hashCode={}",
966-
spanInfo.getSpanId(),
967-
route,
968-
existingRoute,
969-
System.identityHashCode(ctx));
970948

971949
if (route != null) {
972950
ctx.setRoute(route.toString());
973951
}
974952

975953
ApiSecuritySampler requestSampler = requestSamplerSupplier.get();
976-
boolean sampled = requestSampler.preSampleRequest(ctx);
977-
978-
log.info(
979-
"[APPSEC-57815] API Security sampling decision - route={}, sampled={}", route, sampled);
980-
return sampled;
954+
return requestSampler.preSampleRequest(ctx);
981955
}
982956

983957
private Flow<Void> onRequestHeadersDone(RequestContext ctx_) {

0 commit comments

Comments
 (0)