Skip to content

Commit e57d2c4

Browse files
jandro996jandro996
committed
Fix appsec.rasp.error and appsec.waf.error telemetry metrics
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 Remove the hardcoded waf error codes (provisional enum is used until upgrade libddwaf lib) Replace ConcurrentHashMap with AtomicLongArray for raspErrorCodeCounter to improve performance and memory efficiency
1 parent f020e29 commit e57d2c4

File tree

5 files changed

+110
-197
lines changed

5 files changed

+110
-197
lines changed

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

Lines changed: 0 additions & 90 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);
@@ -125,12 +122,6 @@ public class AppSecRequestContext implements DataBundle, Closeable {
125122
private volatile boolean blocked;
126123
private volatile int wafTimeouts;
127124
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;
134125

135126
// keep a reference to the last published usr.id
136127
private volatile String userId;
@@ -150,29 +141,6 @@ public class AppSecRequestContext implements DataBundle, Closeable {
150141
private static final AtomicIntegerFieldUpdater<AppSecRequestContext> RASP_TIMEOUTS_UPDATER =
151142
AtomicIntegerFieldUpdater.newUpdater(AppSecRequestContext.class, "raspTimeouts");
152143

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-
176144
// to be called by the Event Dispatcher
177145
public void addAll(DataBundle newData) {
178146
for (Map.Entry<Address<?>, Object> entry : newData) {
@@ -222,38 +190,6 @@ public void increaseRaspTimeouts() {
222190
RASP_TIMEOUTS_UPDATER.incrementAndGet(this);
223191
}
224192

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-
}
239-
}
240-
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-
}
255-
}
256-
257193
public int getWafTimeouts() {
258194
return wafTimeouts;
259195
}
@@ -262,32 +198,6 @@ public int getRaspTimeouts() {
262198
return raspTimeouts;
263199
}
264200

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-
}
276-
}
277-
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-
}
289-
}
290-
291201
public Additive getOrCreateAdditive(PowerwafContext ctx, boolean createMetrics, boolean isRasp) {
292202

293203
if (createMetrics) {

‎dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -450,15 +450,11 @@ public void onDataAvailable(
450450
log.error("Error calling WAF", e);
451451
}
452452
WafMetricCollector.get().wafRequestError();
453+
// TODO: replace -127 for e.code when UnclassifiedPowerwafException code is fixed
454+
incrementErrorCodeMetric(gwCtx, -127);
453455
return;
454456
} catch (AbstractPowerwafException e) {
455-
if (gwCtx.isRasp) {
456-
reqCtx.increaseRaspErrorCode(e.code);
457-
WafMetricCollector.get().raspErrorCode(gwCtx.raspRuleType, e.code);
458-
} else {
459-
reqCtx.increaseWafErrorCode(e.code);
460-
WafMetricCollector.get().wafErrorCode(gwCtx.raspRuleType, e.code);
461-
}
457+
incrementErrorCodeMetric(gwCtx, e.code);
462458
return;
463459
} finally {
464460
if (log.isDebugEnabled()) {
@@ -654,6 +650,14 @@ private Powerwaf.ResultWithData runPowerwafAdditive(
654650
}
655651
}
656652

653+
private static void incrementErrorCodeMetric(GatewayContext gwCtx, int code) {
654+
if (gwCtx.isRasp) {
655+
WafMetricCollector.get().raspErrorCode(gwCtx.raspRuleType, code);
656+
} else {
657+
WafMetricCollector.get().wafErrorCode(code);
658+
}
659+
}
660+
657661
private Powerwaf.ResultWithData runPowerwafTransient(
658662
Additive additive, PowerwafMetrics metrics, DataBundle bundle, CtxAndAddresses ctxAndAddr)
659663
throws AbstractPowerwafException {

‎dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/AppSecRequestContextSpecification.groovy‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -290,32 +290,6 @@ class AppSecRequestContextSpecification extends DDSpecification {
290290
ctx.getRaspTimeouts() == 2
291291
}
292292

293-
def "test increase and get RaspErrors"() {
294-
when:
295-
ctx.increaseRaspErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR)
296-
ctx.increaseRaspErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR)
297-
ctx.increaseRaspErrorCode(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR)
298-
299-
then:
300-
ctx.getRaspError(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR) == 2
301-
ctx.getRaspError(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR) == 1
302-
ctx.getRaspError(AppSecRequestContext.DD_WAF_RUN_INVALID_ARGUMENT_ERROR) == 0
303-
ctx.getRaspError(0) == 0
304-
}
305-
306-
def "test increase and get WafErrors"() {
307-
when:
308-
ctx.increaseWafErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR)
309-
ctx.increaseWafErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR)
310-
ctx.increaseWafErrorCode(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR)
311-
312-
then:
313-
ctx.getWafError(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR) == 2
314-
ctx.getWafError(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR) == 1
315-
ctx.getWafError(AppSecRequestContext.DD_WAF_RUN_INVALID_ARGUMENT_ERROR) == 0
316-
ctx.getWafError(0) == 0
317-
}
318-
319293
void 'close logs if request end was not called'() {
320294
given:
321295
TestLogCollector.enable()

‎internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java‎

Lines changed: 75 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,19 @@
22

33
import java.util.Collection;
44
import java.util.Collections;
5+
import java.util.HashMap;
56
import java.util.LinkedList;
67
import java.util.List;
8+
import java.util.Map;
79
import java.util.concurrent.ArrayBlockingQueue;
810
import java.util.concurrent.BlockingQueue;
9-
import java.util.concurrent.ConcurrentMap;
10-
import java.util.concurrent.ConcurrentSkipListMap;
1111
import java.util.concurrent.atomic.AtomicInteger;
1212
import java.util.concurrent.atomic.AtomicLong;
1313
import java.util.concurrent.atomic.AtomicLongArray;
1414

1515
public class WafMetricCollector implements MetricCollector<WafMetricCollector.WafMetric> {
1616

1717
public static WafMetricCollector INSTANCE = new WafMetricCollector();
18-
private static final int ABSTRACT_POWERWAF_EXCEPTION_NUMBER =
19-
3; // only 3 error codes are possible for now in AbstractPowerwafException
2018

2119
public static WafMetricCollector get() {
2220
return WafMetricCollector.INSTANCE;
@@ -53,18 +51,10 @@ private WafMetricCollector() {
5351
new AtomicLongArray(RuleType.getNumValues());
5452
private static final AtomicLongArray raspTimeoutCounter =
5553
new AtomicLongArray(RuleType.getNumValues());
56-
private static final ConcurrentMap<Integer, AtomicLongArray> raspErrorCodeCounter =
57-
new ConcurrentSkipListMap<>();
58-
private static final ConcurrentMap<Integer, AtomicLongArray> wafErrorCodeCounter =
59-
new ConcurrentSkipListMap<>();
60-
61-
static {
62-
for (int i = -1 * ABSTRACT_POWERWAF_EXCEPTION_NUMBER; i < 0; i++) {
63-
raspErrorCodeCounter.put(i, new AtomicLongArray(RuleType.getNumValues()));
64-
wafErrorCodeCounter.put(i, new AtomicLongArray(RuleType.getNumValues()));
65-
}
66-
}
67-
54+
private static final AtomicLongArray raspErrorCodeCounter =
55+
new AtomicLongArray(WafErrorCode.values().length * RuleType.getNumValues());
56+
private static final AtomicLongArray wafErrorCodeCounter =
57+
new AtomicLongArray(WafErrorCode.values().length);
6858
private static final AtomicLongArray missingUserLoginQueue =
6959
new AtomicLongArray(LoginFramework.getNumValues() * LoginEvent.getNumValues());
7060
private static final AtomicLongArray missingUserIdQueue =
@@ -151,12 +141,23 @@ public void raspTimeout(final RuleType ruleType) {
151141
raspTimeoutCounter.incrementAndGet(ruleType.ordinal());
152142
}
153143

154-
public void raspErrorCode(final RuleType ruleType, final int ddwafRunErrorCode) {
155-
raspErrorCodeCounter.get(ddwafRunErrorCode).incrementAndGet(ruleType.ordinal());
144+
public void raspErrorCode(RuleType ruleType, final int errorCode) {
145+
WafErrorCode wafErrorCode = WafErrorCode.fromCode(errorCode);
146+
// Unsupported waf error code
147+
if (wafErrorCode == null) {
148+
return;
149+
}
150+
int index = wafErrorCode.ordinal() * RuleType.getNumValues() + ruleType.ordinal();
151+
raspErrorCodeCounter.incrementAndGet(index);
156152
}
157153

158-
public void wafErrorCode(final RuleType ruleType, final int ddwafRunErrorCode) {
159-
wafErrorCodeCounter.get(ddwafRunErrorCode).incrementAndGet(ruleType.ordinal());
154+
public void wafErrorCode(final int errorCode) {
155+
WafErrorCode wafErrorCode = WafErrorCode.fromCode(errorCode);
156+
// Unsupported waf error code
157+
if (wafErrorCode == null) {
158+
return;
159+
}
160+
wafErrorCodeCounter.incrementAndGet(wafErrorCode.ordinal());
160161
}
161162

162163
public void missingUserLogin(final LoginFramework framework, final LoginEvent eventType) {
@@ -321,16 +322,13 @@ public void prepareMetrics() {
321322
}
322323

323324
// RASP rule type for each possible error code
324-
for (int i = -1 * ABSTRACT_POWERWAF_EXCEPTION_NUMBER; i < 0; i++) {
325+
for (WafErrorCode errorCode : WafErrorCode.values()) {
325326
for (RuleType ruleType : RuleType.values()) {
326-
long counter = raspErrorCodeCounter.get(i).getAndSet(ruleType.ordinal(), 0);
327-
if (counter > 0) {
328-
if (!rawMetricsQueue.offer(
329-
new RaspError(counter, ruleType, WafMetricCollector.wafVersion, i))) {
330-
return;
331-
}
327+
int index = errorCode.ordinal() * RuleType.getNumValues() + ruleType.ordinal();
328+
long count = raspErrorCodeCounter.getAndSet(index, 0);
329+
if (count > 0) {
332330
if (!rawMetricsQueue.offer(
333-
new WafError(counter, ruleType, WafMetricCollector.wafVersion, i))) {
331+
new RaspError(count, ruleType, WafMetricCollector.wafVersion, errorCode.getCode()))) {
334332
return;
335333
}
336334
}
@@ -375,6 +373,17 @@ public void prepareMetrics() {
375373
}
376374
}
377375

376+
// WAF rule type for each possible error code
377+
for (WafErrorCode errorCode : WafErrorCode.values()) {
378+
long count = wafErrorCodeCounter.getAndSet(errorCode.ordinal(), 0);
379+
if (count > 0) {
380+
if (!rawMetricsQueue.offer(
381+
new WafError(count, WafMetricCollector.wafVersion, errorCode.getCode()))) {
382+
return;
383+
}
384+
}
385+
}
386+
378387
// RASP rule skipped per rule type for after-request reason
379388
for (RuleType ruleType : RuleType.values()) {
380389
long counter = raspRuleSkippedCounter.getAndSet(ruleType.ordinal(), 0);
@@ -569,27 +578,13 @@ public RaspError(
569578
}
570579

571580
public static class WafError extends WafMetric {
572-
public WafError(
573-
final long counter,
574-
final RuleType ruleType,
575-
final String wafVersion,
576-
final Integer ddwafRunError) {
581+
public WafError(final long counter, final String wafVersion, final Integer ddwafRunError) {
577582
super(
578583
"waf.error",
579584
counter,
580-
ruleType.variant != null
581-
? new String[] {
582-
"rule_type:" + ruleType.type,
583-
"rule_variant:" + ruleType.variant,
584-
"waf_version:" + wafVersion,
585-
"event_rules_version:" + rulesVersion,
586-
"waf_error:" + ddwafRunError
587-
}
588-
: new String[] {
589-
"rule_type:" + ruleType.type,
590-
"waf_version:" + wafVersion,
591-
"waf_error:" + ddwafRunError
592-
});
585+
"waf_version:" + wafVersion,
586+
"event_rules_version:" + rulesVersion,
587+
"waf_error:" + ddwafRunError);
593588
}
594589
}
595590

@@ -614,4 +609,38 @@ public final void increment() {
614609
atomicLong.incrementAndGet();
615610
}
616611
}
612+
613+
// TODO This enum should be in liddwaf, remove it when available
614+
public enum WafErrorCode {
615+
INVALID_ARGUMENT(-1),
616+
INVALID_OBJECT(-2),
617+
INTERNAL_ERROR(-3),
618+
BINDING_ERROR(
619+
-127); // This is a special error code that is not returned by the WAF, is used to signal a
620+
// binding error
621+
622+
private final int code;
623+
624+
private static final Map<Integer, WafErrorCode> CODE_MAP;
625+
626+
static {
627+
Map<Integer, WafErrorCode> map = new HashMap<>();
628+
for (WafErrorCode errorCode : values()) {
629+
map.put(errorCode.code, errorCode);
630+
}
631+
CODE_MAP = Collections.unmodifiableMap(map);
632+
}
633+
634+
WafErrorCode(int code) {
635+
this.code = code;
636+
}
637+
638+
public int getCode() {
639+
return code;
640+
}
641+
642+
public static WafErrorCode fromCode(int code) {
643+
return CODE_MAP.get(code);
644+
}
645+
}
617646
}

0 commit comments

Comments
 (0)