Skip to content

Commit d354d38

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 Add tests
1 parent a1a175c commit d354d38

File tree

6 files changed

+217
-197
lines changed

6 files changed

+217
-197
lines changed

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

+11-7
Original file line numberDiff line numberDiff line change
@@ -448,16 +448,12 @@ public void onDataAvailable(
448448
if (!reqCtx.isWafContextClosed()) {
449449
log.error("Error calling WAF", e);
450450
}
451+
// TODO this is wrong and will be fixed in another PR
451452
WafMetricCollector.get().wafRequestError();
453+
incrementErrorCodeMetric(gwCtx, e.code);
452454
return;
453455
} 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-
}
456+
incrementErrorCodeMetric(gwCtx, e.code);
461457
return;
462458
} finally {
463459
if (log.isDebugEnabled()) {
@@ -653,6 +649,14 @@ private Waf.ResultWithData runWafContext(
653649
}
654650
}
655651

652+
private static void incrementErrorCodeMetric(GatewayContext gwCtx, int code) {
653+
if (gwCtx.isRasp) {
654+
WafMetricCollector.get().raspErrorCode(gwCtx.raspRuleType, code);
655+
} else {
656+
WafMetricCollector.get().wafErrorCode(code);
657+
}
658+
}
659+
656660
private Waf.ResultWithData runWafTransient(
657661
WafContext wafContext, WafMetrics metrics, DataBundle bundle, CtxAndAddresses ctxAndAddr)
658662
throws AbstractWafException {

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

-90
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 WafContext getOrCreateWafContext(WafHandle ctx, boolean createMetrics, boolean isRasp) {
292202

293203
if (createMetrics) {

Diff for: ‎dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy

+96
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package com.datadog.appsec.ddwaf
22

3+
import com.datadog.ddwaf.WafErrorCode as LibWafErrorCode
4+
import datadog.trace.api.telemetry.WafMetricCollector.WafErrorCode as InternalWafErrorCode
5+
36
import com.datadog.appsec.AppSecModule
47
import com.datadog.appsec.config.AppSecConfig
58
import com.datadog.appsec.config.AppSecData
@@ -17,6 +20,11 @@ import com.datadog.appsec.event.data.MapDataBundle
1720
import com.datadog.appsec.gateway.AppSecRequestContext
1821
import com.datadog.appsec.gateway.GatewayContext
1922
import com.datadog.appsec.report.AppSecEvent
23+
import com.datadog.ddwaf.exception.AbstractWafException
24+
import com.datadog.ddwaf.exception.InternalWafException
25+
import com.datadog.ddwaf.exception.InvalidArgumentWafException
26+
import com.datadog.ddwaf.exception.InvalidObjectWafException
27+
import com.datadog.ddwaf.exception.UnclassifiedWafException
2028
import datadog.trace.api.telemetry.RuleType
2129
import datadog.trace.util.stacktrace.StackTraceEvent
2230
import com.datadog.appsec.test.StubAppSecConfigService
@@ -1755,6 +1763,94 @@ class WAFModuleSpecification extends DDSpecification {
17551763
0 * _
17561764
}
17571765

1766+
void 'test raspErrorCode metric is increased when waf call throws #wafErrorCode '() {
1767+
setup:
1768+
ChangeableFlow flow = Mock()
1769+
GatewayContext gwCtxMock = new GatewayContext(false, RuleType.SQL_INJECTION)
1770+
WafContext wafContext = Mock()
1771+
1772+
when:
1773+
setupWithStubConfigService('rules_with_data_config.json')
1774+
dataListener = wafModule.dataSubscriptions.first()
1775+
1776+
def bundle = MapDataBundle.of(
1777+
KnownAddresses.USER_ID,
1778+
'legit-user'
1779+
)
1780+
dataListener.onDataAvailable(flow, ctx, bundle, gwCtxMock)
1781+
1782+
then:
1783+
(1..2) * ctx.isWafContextClosed() >> false // if UnclassifiedWafException it's called twice
1784+
1 * ctx.getOrCreateWafContext(_, true, true) >> wafContext
1785+
1 * wafMetricCollector.raspRuleEval(RuleType.SQL_INJECTION)
1786+
1 * wafContext.run(_, _, _) >> { throw createWafException(wafErrorCode) }
1787+
1 * wafMetricCollector.wafInit(Waf.LIB_VERSION, _, true)
1788+
1 * ctx.getRaspMetrics()
1789+
1 * ctx.getRaspMetricsCounter()
1790+
(0..1) * WafMetricCollector.get().wafRequestError() // TODO: remove this line when WAFModule is removed
1791+
1 * wafMetricCollector.raspErrorCode(RuleType.SQL_INJECTION, wafErrorCode.code)
1792+
0 * _
1793+
1794+
where:
1795+
wafErrorCode << LibWafErrorCode.values()
1796+
}
1797+
1798+
void 'test wafErrorCode metric is increased when waf call throws #wafErrorCode '() {
1799+
setup:
1800+
ChangeableFlow flow = Mock()
1801+
WafContext wafContext = Mock()
1802+
1803+
when:
1804+
setupWithStubConfigService('rules_with_data_config.json')
1805+
dataListener = wafModule.dataSubscriptions.first()
1806+
1807+
def bundle = MapDataBundle.of(
1808+
KnownAddresses.USER_ID,
1809+
'legit-user'
1810+
)
1811+
dataListener.onDataAvailable(flow, ctx, bundle, gwCtx)
1812+
1813+
then:
1814+
(1..2) * ctx.isWafContextClosed() >> false // if UnclassifiedWafException it's called twice
1815+
1 * ctx.getOrCreateWafContext(_, true, false) >> wafContext
1816+
1 * wafContext.run(_, _, _) >> { throw createWafException(wafErrorCode) }
1817+
1 * wafMetricCollector.wafInit(Waf.LIB_VERSION, _, true)
1818+
2 * ctx.getWafMetrics()
1819+
(0..1) * WafMetricCollector.get().wafRequestError() // TODO: remove this line when WAFModule is removed
1820+
1 * wafMetricCollector.wafErrorCode(wafErrorCode.code)
1821+
0 * _
1822+
1823+
where:
1824+
wafErrorCode << LibWafErrorCode.values()
1825+
}
1826+
1827+
def 'internal-api WafErrorCode enum matches libddwaf-java by name and code'() {
1828+
given:
1829+
def libddwaf = com.datadog.ddwaf.WafErrorCode.values().collectEntries { [it.name(), it.code] }
1830+
def internal = datadog.trace.api.telemetry.WafMetricCollector.WafErrorCode.values().collectEntries { [it.name(), it.code] }
1831+
1832+
expect:
1833+
internal == libddwaf
1834+
}
1835+
1836+
/**
1837+
* Helper to return a concrete Waf exception for each WafErrorCode
1838+
*/
1839+
static AbstractWafException createWafException(LibWafErrorCode code) {
1840+
switch (code) {
1841+
case LibWafErrorCode.INVALID_ARGUMENT:
1842+
return new InvalidArgumentWafException(code.code)
1843+
case LibWafErrorCode.INVALID_OBJECT:
1844+
return new InvalidObjectWafException(code.code)
1845+
case LibWafErrorCode.INTERNAL_ERROR:
1846+
return new InternalWafException(code.code)
1847+
case LibWafErrorCode.BINDING_ERROR:
1848+
return new UnclassifiedWafException(code.code)
1849+
default:
1850+
throw new IllegalStateException("Unhandled WafErrorCode: $code")
1851+
}
1852+
}
1853+
17581854
private Map<String, Object> getDefaultConfig() {
17591855
def service = new StubAppSecConfigService()
17601856
service.init()

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

-26
Original file line numberDiff line numberDiff line change
@@ -289,32 +289,6 @@ class AppSecRequestContextSpecification extends DDSpecification {
289289
ctx.getRaspTimeouts() == 2
290290
}
291291

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

0 commit comments

Comments
 (0)