Skip to content

Commit 5fe1015

Browse files
jandro996jandro996
committed
Fix appsec.rasp.error and appsec.waf.error telemetry metrics (#8624)
What Does This Do Remove counters from AppsecRequestContext as they are not used in the metrics Fix appsec.waf.error metric tags as they didn't match the RFC Fix that appsec.waf.error was created in the same loop using the rasp counter instead of using the waf counter Increment metrics if UnclassifiedPowerwafException is thrown Replace ConcurrentHashMap with AtomicLongArray for raspErrorCodeCounter to improve performance and memory efficiency Add test to cover WafModule implementation
1 parent 639a30d commit 5fe1015

File tree

7 files changed

+437
-474
lines changed

7 files changed

+437
-474
lines changed

‎dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java‎

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import datadog.trace.api.gateway.Flow;
4141
import datadog.trace.api.telemetry.LogCollector;
4242
import datadog.trace.api.telemetry.WafMetricCollector;
43-
import datadog.trace.api.telemetry.WafTruncatedType;
4443
import datadog.trace.api.time.SystemTimeSource;
4544
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
4645
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -440,24 +439,17 @@ public void onDataAvailable(
440439
WafMetricCollector.get().raspTimeout(gwCtx.raspRuleType);
441440
} else {
442441
reqCtx.increaseWafTimeouts();
443-
WafMetricCollector.get().wafRequestTimeout();
444442
log.debug(LogCollector.EXCLUDE_TELEMETRY, "Timeout calling the WAF", tpe);
445443
}
446444
return;
447445
} catch (UnclassifiedWafException e) {
448446
if (!reqCtx.isWafContextClosed()) {
449447
log.error("Error calling WAF", e);
450448
}
451-
WafMetricCollector.get().wafRequestError();
449+
incrementErrorCodeMetric(reqCtx, gwCtx, e.code);
452450
return;
453451
} catch (AbstractWafException e) {
454-
if (gwCtx.isRasp) {
455-
reqCtx.increaseRaspErrorCode(e.code);
456-
WafMetricCollector.get().raspErrorCode(gwCtx.raspRuleType, e.code);
457-
} else {
458-
reqCtx.increaseWafErrorCode(e.code);
459-
WafMetricCollector.get().wafErrorCode(gwCtx.raspRuleType, e.code);
460-
}
452+
incrementErrorCodeMetric(reqCtx, gwCtx, e.code);
461453
return;
462454
} finally {
463455
if (log.isDebugEnabled()) {
@@ -471,17 +463,8 @@ public void onDataAvailable(
471463
final long listMapTooLarge = wafMetrics.getTruncatedListMapTooLargeCount();
472464
final long objectTooDeep = wafMetrics.getTruncatedObjectTooDeepCount();
473465

474-
if (stringTooLong > 0) {
475-
WafMetricCollector.get()
476-
.wafInputTruncated(WafTruncatedType.STRING_TOO_LONG, stringTooLong);
477-
}
478-
if (listMapTooLarge > 0) {
479-
WafMetricCollector.get()
480-
.wafInputTruncated(WafTruncatedType.LIST_MAP_TOO_LARGE, listMapTooLarge);
481-
}
482-
if (objectTooDeep > 0) {
483-
WafMetricCollector.get()
484-
.wafInputTruncated(WafTruncatedType.OBJECT_TOO_DEEP, objectTooDeep);
466+
if (stringTooLong > 0 || listMapTooLarge > 0 || objectTooDeep > 0) {
467+
reqCtx.setWafTruncated();
485468
}
486469
}
487470
}
@@ -505,10 +488,12 @@ public void onDataAvailable(
505488
ActionInfo actionInfo = new ActionInfo(actionType, actionParams);
506489

507490
if ("block_request".equals(actionInfo.type)) {
508-
Flow.Action.RequestBlockingAction rba = createBlockRequestAction(actionInfo);
491+
Flow.Action.RequestBlockingAction rba =
492+
createBlockRequestAction(actionInfo, reqCtx, gwCtx.isRasp);
509493
flow.setAction(rba);
510494
} else if ("redirect_request".equals(actionInfo.type)) {
511-
Flow.Action.RequestBlockingAction rba = createRedirectRequestAction(actionInfo);
495+
Flow.Action.RequestBlockingAction rba =
496+
createRedirectRequestAction(actionInfo, reqCtx, gwCtx.isRasp);
512497
flow.setAction(rba);
513498
} else if ("generate_stack".equals(actionInfo.type)) {
514499
if (Config.get().isAppSecStackTraceEnabled()) {
@@ -520,7 +505,9 @@ public void onDataAvailable(
520505
}
521506
} else {
522507
log.info("Ignoring action with type {}", actionInfo.type);
523-
WafMetricCollector.get().wafRequestBlockFailure();
508+
if (!gwCtx.isRasp) {
509+
reqCtx.setWafRequestBlockFailure();
510+
}
524511
}
525512
}
526513
Collection<AppSecEvent> events = buildEvents(resultWithData);
@@ -547,12 +534,16 @@ public void onDataAvailable(
547534
reqCtx.reportEvents(events);
548535
} else {
549536
log.debug("Rate limited WAF events");
550-
WafMetricCollector.get().wafRequestRateLimited();
537+
if (!gwCtx.isRasp) {
538+
reqCtx.setWafRateLimited();
539+
}
551540
}
552541
}
553542

554543
if (flow.isBlocking()) {
555-
reqCtx.setBlocked();
544+
if (!gwCtx.isRasp) {
545+
reqCtx.setWafBlocked();
546+
}
556547
}
557548
}
558549

@@ -561,7 +552,8 @@ public void onDataAvailable(
561552
}
562553
}
563554

564-
private Flow.Action.RequestBlockingAction createBlockRequestAction(ActionInfo actionInfo) {
555+
private Flow.Action.RequestBlockingAction createBlockRequestAction(
556+
final ActionInfo actionInfo, final AppSecRequestContext reqCtx, final boolean isRasp) {
565557
try {
566558
int statusCode;
567559
Object statusCodeObj = actionInfo.parameters.get("status_code");
@@ -582,12 +574,15 @@ private Flow.Action.RequestBlockingAction createBlockRequestAction(ActionInfo ac
582574
return new Flow.Action.RequestBlockingAction(statusCode, blockingContentType);
583575
} catch (RuntimeException cce) {
584576
log.warn("Invalid blocking action data", cce);
585-
WafMetricCollector.get().wafRequestBlockFailure();
577+
if (!isRasp) {
578+
reqCtx.setWafRequestBlockFailure();
579+
}
586580
return null;
587581
}
588582
}
589583

590-
private Flow.Action.RequestBlockingAction createRedirectRequestAction(ActionInfo actionInfo) {
584+
private Flow.Action.RequestBlockingAction createRedirectRequestAction(
585+
final ActionInfo actionInfo, final AppSecRequestContext reqCtx, final boolean isRasp) {
591586
try {
592587
int statusCode;
593588
Object statusCodeObj = actionInfo.parameters.get("status_code");
@@ -608,7 +603,9 @@ private Flow.Action.RequestBlockingAction createRedirectRequestAction(ActionInfo
608603
return Flow.Action.RequestBlockingAction.forRedirect(statusCode, location);
609604
} catch (RuntimeException cce) {
610605
log.warn("Invalid blocking action data", cce);
611-
WafMetricCollector.get().wafRequestBlockFailure();
606+
if (!isRasp) {
607+
reqCtx.setWafRequestBlockFailure();
608+
}
612609
return null;
613610
}
614611
}
@@ -653,6 +650,16 @@ private Waf.ResultWithData runWafContext(
653650
}
654651
}
655652

653+
private static void incrementErrorCodeMetric(
654+
AppSecRequestContext reqCtx, GatewayContext gwCtx, int code) {
655+
if (gwCtx.isRasp) {
656+
WafMetricCollector.get().raspErrorCode(gwCtx.raspRuleType, code);
657+
} else {
658+
WafMetricCollector.get().wafErrorCode(code);
659+
reqCtx.setWafErrors();
660+
}
661+
}
662+
656663
private Waf.ResultWithData runWafTransient(
657664
WafContext wafContext, WafMetrics metrics, DataBundle bundle, CtxAndAddresses ctxAndAddr)
658665
throws AbstractWafException {

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

Lines changed: 43 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,6 @@ public class AppSecRequestContext implements DataBundle, Closeable {
7474
public static final Set<String> RESPONSE_HEADERS_ALLOW_LIST =
7575
new TreeSet<>(
7676
Arrays.asList("content-length", "content-type", "content-encoding", "content-language"));
77-
public static final int DD_WAF_RUN_INTERNAL_ERROR = -3;
78-
public static final int DD_WAF_RUN_INVALID_OBJECT_ERROR = -2;
79-
public static final int DD_WAF_RUN_INVALID_ARGUMENT_ERROR = -1;
8077

8178
static {
8279
REQUEST_HEADERS_ALLOW_LIST.addAll(DEFAULT_REQUEST_HEADERS_ALLOW_LIST);
@@ -122,15 +119,15 @@ public class AppSecRequestContext implements DataBundle, Closeable {
122119
private volatile WafMetrics wafMetrics;
123120
private volatile WafMetrics raspMetrics;
124121
private final AtomicInteger raspMetricsCounter = new AtomicInteger(0);
125-
private volatile boolean blocked;
122+
123+
private volatile boolean wafBlocked;
124+
private volatile boolean wafErrors;
125+
private volatile boolean wafTruncated;
126+
private volatile boolean wafRequestBlockFailure;
127+
private volatile boolean wafRateLimited;
128+
126129
private volatile int wafTimeouts;
127130
private volatile int raspTimeouts;
128-
private volatile int raspInternalErrors;
129-
private volatile int raspInvalidObjectErrors;
130-
private volatile int raspInvalidArgumentErrors;
131-
private volatile int wafInternalErrors;
132-
private volatile int wafInvalidObjectErrors;
133-
private volatile int wafInvalidArgumentErrors;
134131

135132
// keep a reference to the last published usr.id
136133
private volatile String userId;
@@ -150,29 +147,6 @@ public class AppSecRequestContext implements DataBundle, Closeable {
150147
private static final AtomicIntegerFieldUpdater<AppSecRequestContext> RASP_TIMEOUTS_UPDATER =
151148
AtomicIntegerFieldUpdater.newUpdater(AppSecRequestContext.class, "raspTimeouts");
152149

153-
private static final AtomicIntegerFieldUpdater<AppSecRequestContext>
154-
RASP_INTERNAL_ERRORS_UPDATER =
155-
AtomicIntegerFieldUpdater.newUpdater(AppSecRequestContext.class, "raspInternalErrors");
156-
private static final AtomicIntegerFieldUpdater<AppSecRequestContext>
157-
RASP_INVALID_OBJECT_ERRORS_UPDATER =
158-
AtomicIntegerFieldUpdater.newUpdater(
159-
AppSecRequestContext.class, "raspInvalidObjectErrors");
160-
private static final AtomicIntegerFieldUpdater<AppSecRequestContext>
161-
RASP_INVALID_ARGUMENT_ERRORS_UPDATER =
162-
AtomicIntegerFieldUpdater.newUpdater(
163-
AppSecRequestContext.class, "raspInvalidArgumentErrors");
164-
165-
private static final AtomicIntegerFieldUpdater<AppSecRequestContext> WAF_INTERNAL_ERRORS_UPDATER =
166-
AtomicIntegerFieldUpdater.newUpdater(AppSecRequestContext.class, "wafInternalErrors");
167-
private static final AtomicIntegerFieldUpdater<AppSecRequestContext>
168-
WAF_INVALID_OBJECT_ERRORS_UPDATER =
169-
AtomicIntegerFieldUpdater.newUpdater(
170-
AppSecRequestContext.class, "wafInvalidObjectErrors");
171-
private static final AtomicIntegerFieldUpdater<AppSecRequestContext>
172-
WAF_INVALID_ARGUMENT_ERRORS_UPDATER =
173-
AtomicIntegerFieldUpdater.newUpdater(
174-
AppSecRequestContext.class, "wafInvalidArgumentErrors");
175-
176150
// to be called by the Event Dispatcher
177151
public void addAll(DataBundle newData) {
178152
for (Map.Entry<Address<?>, Object> entry : newData) {
@@ -206,86 +180,60 @@ public AtomicInteger getRaspMetricsCounter() {
206180
return raspMetricsCounter;
207181
}
208182

209-
public void setBlocked() {
210-
this.blocked = true;
183+
public void setWafBlocked() {
184+
this.wafBlocked = true;
211185
}
212186

213-
public boolean isBlocked() {
214-
return blocked;
187+
public boolean isWafBlocked() {
188+
return wafBlocked;
215189
}
216190

217-
public void increaseWafTimeouts() {
218-
WAF_TIMEOUTS_UPDATER.incrementAndGet(this);
191+
public void setWafErrors() {
192+
this.wafErrors = true;
219193
}
220194

221-
public void increaseRaspTimeouts() {
222-
RASP_TIMEOUTS_UPDATER.incrementAndGet(this);
195+
public boolean hasWafErrors() {
196+
return wafErrors;
223197
}
224198

225-
public void increaseRaspErrorCode(int code) {
226-
switch (code) {
227-
case DD_WAF_RUN_INTERNAL_ERROR:
228-
RASP_INTERNAL_ERRORS_UPDATER.incrementAndGet(this);
229-
break;
230-
case DD_WAF_RUN_INVALID_OBJECT_ERROR:
231-
RASP_INVALID_OBJECT_ERRORS_UPDATER.incrementAndGet(this);
232-
break;
233-
case DD_WAF_RUN_INVALID_ARGUMENT_ERROR:
234-
RASP_INVALID_ARGUMENT_ERRORS_UPDATER.incrementAndGet(this);
235-
break;
236-
default:
237-
break;
238-
}
199+
public void setWafTruncated() {
200+
this.wafTruncated = true;
239201
}
240202

241-
public void increaseWafErrorCode(int code) {
242-
switch (code) {
243-
case DD_WAF_RUN_INTERNAL_ERROR:
244-
WAF_INTERNAL_ERRORS_UPDATER.incrementAndGet(this);
245-
break;
246-
case DD_WAF_RUN_INVALID_OBJECT_ERROR:
247-
WAF_INVALID_OBJECT_ERRORS_UPDATER.incrementAndGet(this);
248-
break;
249-
case DD_WAF_RUN_INVALID_ARGUMENT_ERROR:
250-
WAF_INVALID_ARGUMENT_ERRORS_UPDATER.incrementAndGet(this);
251-
break;
252-
default:
253-
break;
254-
}
203+
public boolean isWafTruncated() {
204+
return wafTruncated;
255205
}
256206

257-
public int getWafTimeouts() {
258-
return wafTimeouts;
207+
public void setWafRequestBlockFailure() {
208+
this.wafRequestBlockFailure = true;
259209
}
260210

261-
public int getRaspTimeouts() {
262-
return raspTimeouts;
211+
public boolean isWafRequestBlockFailure() {
212+
return wafRequestBlockFailure;
263213
}
264214

265-
public int getRaspError(int code) {
266-
switch (code) {
267-
case DD_WAF_RUN_INTERNAL_ERROR:
268-
return raspInternalErrors;
269-
case DD_WAF_RUN_INVALID_OBJECT_ERROR:
270-
return raspInvalidObjectErrors;
271-
case DD_WAF_RUN_INVALID_ARGUMENT_ERROR:
272-
return raspInvalidArgumentErrors;
273-
default:
274-
return 0;
275-
}
215+
public void setWafRateLimited() {
216+
this.wafRateLimited = true;
276217
}
277218

278-
public int getWafError(int code) {
279-
switch (code) {
280-
case DD_WAF_RUN_INTERNAL_ERROR:
281-
return wafInternalErrors;
282-
case DD_WAF_RUN_INVALID_OBJECT_ERROR:
283-
return wafInvalidObjectErrors;
284-
case DD_WAF_RUN_INVALID_ARGUMENT_ERROR:
285-
return wafInvalidArgumentErrors;
286-
default:
287-
return 0;
288-
}
219+
public boolean isWafRateLimited() {
220+
return wafRateLimited;
221+
}
222+
223+
public void increaseWafTimeouts() {
224+
WAF_TIMEOUTS_UPDATER.incrementAndGet(this);
225+
}
226+
227+
public void increaseRaspTimeouts() {
228+
RASP_TIMEOUTS_UPDATER.incrementAndGet(this);
229+
}
230+
231+
public int getWafTimeouts() {
232+
return wafTimeouts;
233+
}
234+
235+
public int getRaspTimeouts() {
236+
return raspTimeouts;
289237
}
290238

291239
public WafContext getOrCreateWafContext(WafHandle ctx, boolean createMetrics, boolean isRasp) {

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -724,13 +724,16 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) {
724724
log.debug("Unable to commit, derivatives will be skipped {}", ctx.getDerivativeKeys());
725725
}
726726

727-
if (ctx.isBlocked()) {
728-
WafMetricCollector.get().wafRequestBlocked();
729-
} else if (!collectedEvents.isEmpty()) {
730-
WafMetricCollector.get().wafRequestTriggered();
731-
} else {
732-
WafMetricCollector.get().wafRequest();
733-
}
727+
WafMetricCollector.get()
728+
.wafRequest(
729+
!collectedEvents.isEmpty(), // ruleTriggered
730+
ctx.isWafBlocked(), // requestBlocked
731+
ctx.hasWafErrors(), // wafError
732+
ctx.getWafTimeouts() > 0, // wafTimeout,
733+
ctx.isWafRequestBlockFailure(), // blockFailure,
734+
ctx.isWafRateLimited(), // rateLimited,
735+
ctx.isWafTruncated() // inputTruncated
736+
);
734737
}
735738

736739
ctx.close();

0 commit comments

Comments
 (0)