From d354d384c4b480e4e2c8e6f2bf1fcef7e716ea0b Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 25 Mar 2025 13:49:36 +0100 Subject: [PATCH 1/2] 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 --- .../com/datadog/appsec/ddwaf/WAFModule.java | 18 ++- .../appsec/gateway/AppSecRequestContext.java | 90 ------------ .../ddwaf/WAFModuleSpecification.groovy | 96 +++++++++++++ .../AppSecRequestContextSpecification.groovy | 26 ---- .../api/telemetry/WafMetricCollector.java | 132 ++++++++++++------ .../telemetry/WafMetricCollectorTest.groovy | 52 ++++--- 6 files changed, 217 insertions(+), 197 deletions(-) diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java index 77179cc1ea7..d21293e1dd0 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java @@ -448,16 +448,12 @@ public void onDataAvailable( if (!reqCtx.isWafContextClosed()) { log.error("Error calling WAF", e); } + // TODO this is wrong and will be fixed in another PR WafMetricCollector.get().wafRequestError(); + incrementErrorCodeMetric(gwCtx, e.code); return; } catch (AbstractWafException e) { - if (gwCtx.isRasp) { - reqCtx.increaseRaspErrorCode(e.code); - WafMetricCollector.get().raspErrorCode(gwCtx.raspRuleType, e.code); - } else { - reqCtx.increaseWafErrorCode(e.code); - WafMetricCollector.get().wafErrorCode(gwCtx.raspRuleType, e.code); - } + incrementErrorCodeMetric(gwCtx, e.code); return; } finally { if (log.isDebugEnabled()) { @@ -653,6 +649,14 @@ private Waf.ResultWithData runWafContext( } } + private static void incrementErrorCodeMetric(GatewayContext gwCtx, int code) { + if (gwCtx.isRasp) { + WafMetricCollector.get().raspErrorCode(gwCtx.raspRuleType, code); + } else { + WafMetricCollector.get().wafErrorCode(code); + } + } + private Waf.ResultWithData runWafTransient( WafContext wafContext, WafMetrics metrics, DataBundle bundle, CtxAndAddresses ctxAndAddr) throws AbstractWafException { diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java index ce5db92910d..fb3138f35dd 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java @@ -74,9 +74,6 @@ public class AppSecRequestContext implements DataBundle, Closeable { public static final Set RESPONSE_HEADERS_ALLOW_LIST = new TreeSet<>( Arrays.asList("content-length", "content-type", "content-encoding", "content-language")); - public static final int DD_WAF_RUN_INTERNAL_ERROR = -3; - public static final int DD_WAF_RUN_INVALID_OBJECT_ERROR = -2; - public static final int DD_WAF_RUN_INVALID_ARGUMENT_ERROR = -1; static { REQUEST_HEADERS_ALLOW_LIST.addAll(DEFAULT_REQUEST_HEADERS_ALLOW_LIST); @@ -125,12 +122,6 @@ public class AppSecRequestContext implements DataBundle, Closeable { private volatile boolean blocked; private volatile int wafTimeouts; private volatile int raspTimeouts; - private volatile int raspInternalErrors; - private volatile int raspInvalidObjectErrors; - private volatile int raspInvalidArgumentErrors; - private volatile int wafInternalErrors; - private volatile int wafInvalidObjectErrors; - private volatile int wafInvalidArgumentErrors; // keep a reference to the last published usr.id private volatile String userId; @@ -150,29 +141,6 @@ public class AppSecRequestContext implements DataBundle, Closeable { private static final AtomicIntegerFieldUpdater RASP_TIMEOUTS_UPDATER = AtomicIntegerFieldUpdater.newUpdater(AppSecRequestContext.class, "raspTimeouts"); - private static final AtomicIntegerFieldUpdater - RASP_INTERNAL_ERRORS_UPDATER = - AtomicIntegerFieldUpdater.newUpdater(AppSecRequestContext.class, "raspInternalErrors"); - private static final AtomicIntegerFieldUpdater - RASP_INVALID_OBJECT_ERRORS_UPDATER = - AtomicIntegerFieldUpdater.newUpdater( - AppSecRequestContext.class, "raspInvalidObjectErrors"); - private static final AtomicIntegerFieldUpdater - RASP_INVALID_ARGUMENT_ERRORS_UPDATER = - AtomicIntegerFieldUpdater.newUpdater( - AppSecRequestContext.class, "raspInvalidArgumentErrors"); - - private static final AtomicIntegerFieldUpdater WAF_INTERNAL_ERRORS_UPDATER = - AtomicIntegerFieldUpdater.newUpdater(AppSecRequestContext.class, "wafInternalErrors"); - private static final AtomicIntegerFieldUpdater - WAF_INVALID_OBJECT_ERRORS_UPDATER = - AtomicIntegerFieldUpdater.newUpdater( - AppSecRequestContext.class, "wafInvalidObjectErrors"); - private static final AtomicIntegerFieldUpdater - WAF_INVALID_ARGUMENT_ERRORS_UPDATER = - AtomicIntegerFieldUpdater.newUpdater( - AppSecRequestContext.class, "wafInvalidArgumentErrors"); - // to be called by the Event Dispatcher public void addAll(DataBundle newData) { for (Map.Entry, Object> entry : newData) { @@ -222,38 +190,6 @@ public void increaseRaspTimeouts() { RASP_TIMEOUTS_UPDATER.incrementAndGet(this); } - public void increaseRaspErrorCode(int code) { - switch (code) { - case DD_WAF_RUN_INTERNAL_ERROR: - RASP_INTERNAL_ERRORS_UPDATER.incrementAndGet(this); - break; - case DD_WAF_RUN_INVALID_OBJECT_ERROR: - RASP_INVALID_OBJECT_ERRORS_UPDATER.incrementAndGet(this); - break; - case DD_WAF_RUN_INVALID_ARGUMENT_ERROR: - RASP_INVALID_ARGUMENT_ERRORS_UPDATER.incrementAndGet(this); - break; - default: - break; - } - } - - public void increaseWafErrorCode(int code) { - switch (code) { - case DD_WAF_RUN_INTERNAL_ERROR: - WAF_INTERNAL_ERRORS_UPDATER.incrementAndGet(this); - break; - case DD_WAF_RUN_INVALID_OBJECT_ERROR: - WAF_INVALID_OBJECT_ERRORS_UPDATER.incrementAndGet(this); - break; - case DD_WAF_RUN_INVALID_ARGUMENT_ERROR: - WAF_INVALID_ARGUMENT_ERRORS_UPDATER.incrementAndGet(this); - break; - default: - break; - } - } - public int getWafTimeouts() { return wafTimeouts; } @@ -262,32 +198,6 @@ public int getRaspTimeouts() { return raspTimeouts; } - public int getRaspError(int code) { - switch (code) { - case DD_WAF_RUN_INTERNAL_ERROR: - return raspInternalErrors; - case DD_WAF_RUN_INVALID_OBJECT_ERROR: - return raspInvalidObjectErrors; - case DD_WAF_RUN_INVALID_ARGUMENT_ERROR: - return raspInvalidArgumentErrors; - default: - return 0; - } - } - - public int getWafError(int code) { - switch (code) { - case DD_WAF_RUN_INTERNAL_ERROR: - return wafInternalErrors; - case DD_WAF_RUN_INVALID_OBJECT_ERROR: - return wafInvalidObjectErrors; - case DD_WAF_RUN_INVALID_ARGUMENT_ERROR: - return wafInvalidArgumentErrors; - default: - return 0; - } - } - public WafContext getOrCreateWafContext(WafHandle ctx, boolean createMetrics, boolean isRasp) { if (createMetrics) { diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy index 5e7fd544788..c326b8f2089 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy @@ -1,5 +1,8 @@ package com.datadog.appsec.ddwaf +import com.datadog.ddwaf.WafErrorCode as LibWafErrorCode +import datadog.trace.api.telemetry.WafMetricCollector.WafErrorCode as InternalWafErrorCode + import com.datadog.appsec.AppSecModule import com.datadog.appsec.config.AppSecConfig import com.datadog.appsec.config.AppSecData @@ -17,6 +20,11 @@ import com.datadog.appsec.event.data.MapDataBundle import com.datadog.appsec.gateway.AppSecRequestContext import com.datadog.appsec.gateway.GatewayContext import com.datadog.appsec.report.AppSecEvent +import com.datadog.ddwaf.exception.AbstractWafException +import com.datadog.ddwaf.exception.InternalWafException +import com.datadog.ddwaf.exception.InvalidArgumentWafException +import com.datadog.ddwaf.exception.InvalidObjectWafException +import com.datadog.ddwaf.exception.UnclassifiedWafException import datadog.trace.api.telemetry.RuleType import datadog.trace.util.stacktrace.StackTraceEvent import com.datadog.appsec.test.StubAppSecConfigService @@ -1755,6 +1763,94 @@ class WAFModuleSpecification extends DDSpecification { 0 * _ } + void 'test raspErrorCode metric is increased when waf call throws #wafErrorCode '() { + setup: + ChangeableFlow flow = Mock() + GatewayContext gwCtxMock = new GatewayContext(false, RuleType.SQL_INJECTION) + WafContext wafContext = Mock() + + when: + setupWithStubConfigService('rules_with_data_config.json') + dataListener = wafModule.dataSubscriptions.first() + + def bundle = MapDataBundle.of( + KnownAddresses.USER_ID, + 'legit-user' + ) + dataListener.onDataAvailable(flow, ctx, bundle, gwCtxMock) + + then: + (1..2) * ctx.isWafContextClosed() >> false // if UnclassifiedWafException it's called twice + 1 * ctx.getOrCreateWafContext(_, true, true) >> wafContext + 1 * wafMetricCollector.raspRuleEval(RuleType.SQL_INJECTION) + 1 * wafContext.run(_, _, _) >> { throw createWafException(wafErrorCode) } + 1 * wafMetricCollector.wafInit(Waf.LIB_VERSION, _, true) + 1 * ctx.getRaspMetrics() + 1 * ctx.getRaspMetricsCounter() + (0..1) * WafMetricCollector.get().wafRequestError() // TODO: remove this line when WAFModule is removed + 1 * wafMetricCollector.raspErrorCode(RuleType.SQL_INJECTION, wafErrorCode.code) + 0 * _ + + where: + wafErrorCode << LibWafErrorCode.values() + } + + void 'test wafErrorCode metric is increased when waf call throws #wafErrorCode '() { + setup: + ChangeableFlow flow = Mock() + WafContext wafContext = Mock() + + when: + setupWithStubConfigService('rules_with_data_config.json') + dataListener = wafModule.dataSubscriptions.first() + + def bundle = MapDataBundle.of( + KnownAddresses.USER_ID, + 'legit-user' + ) + dataListener.onDataAvailable(flow, ctx, bundle, gwCtx) + + then: + (1..2) * ctx.isWafContextClosed() >> false // if UnclassifiedWafException it's called twice + 1 * ctx.getOrCreateWafContext(_, true, false) >> wafContext + 1 * wafContext.run(_, _, _) >> { throw createWafException(wafErrorCode) } + 1 * wafMetricCollector.wafInit(Waf.LIB_VERSION, _, true) + 2 * ctx.getWafMetrics() + (0..1) * WafMetricCollector.get().wafRequestError() // TODO: remove this line when WAFModule is removed + 1 * wafMetricCollector.wafErrorCode(wafErrorCode.code) + 0 * _ + + where: + wafErrorCode << LibWafErrorCode.values() + } + + def 'internal-api WafErrorCode enum matches libddwaf-java by name and code'() { + given: + def libddwaf = com.datadog.ddwaf.WafErrorCode.values().collectEntries { [it.name(), it.code] } + def internal = datadog.trace.api.telemetry.WafMetricCollector.WafErrorCode.values().collectEntries { [it.name(), it.code] } + + expect: + internal == libddwaf + } + + /** + * Helper to return a concrete Waf exception for each WafErrorCode + */ + static AbstractWafException createWafException(LibWafErrorCode code) { + switch (code) { + case LibWafErrorCode.INVALID_ARGUMENT: + return new InvalidArgumentWafException(code.code) + case LibWafErrorCode.INVALID_OBJECT: + return new InvalidObjectWafException(code.code) + case LibWafErrorCode.INTERNAL_ERROR: + return new InternalWafException(code.code) + case LibWafErrorCode.BINDING_ERROR: + return new UnclassifiedWafException(code.code) + default: + throw new IllegalStateException("Unhandled WafErrorCode: $code") + } + } + private Map getDefaultConfig() { def service = new StubAppSecConfigService() service.init() diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/AppSecRequestContextSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/AppSecRequestContextSpecification.groovy index 6acbd77dfb8..c3604d812c1 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/AppSecRequestContextSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/AppSecRequestContextSpecification.groovy @@ -289,32 +289,6 @@ class AppSecRequestContextSpecification extends DDSpecification { ctx.getRaspTimeouts() == 2 } - def "test increase and get RaspErrors"() { - when: - ctx.increaseRaspErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR) - ctx.increaseRaspErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR) - ctx.increaseRaspErrorCode(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR) - - then: - ctx.getRaspError(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR) == 2 - ctx.getRaspError(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR) == 1 - ctx.getRaspError(AppSecRequestContext.DD_WAF_RUN_INVALID_ARGUMENT_ERROR) == 0 - ctx.getRaspError(0) == 0 - } - - def "test increase and get WafErrors"() { - when: - ctx.increaseWafErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR) - ctx.increaseWafErrorCode(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR) - ctx.increaseWafErrorCode(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR) - - then: - ctx.getWafError(AppSecRequestContext.DD_WAF_RUN_INTERNAL_ERROR) == 2 - ctx.getWafError(AppSecRequestContext.DD_WAF_RUN_INVALID_OBJECT_ERROR) == 1 - 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() diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java index 248fc256d64..0ea292c7e97 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java @@ -2,12 +2,12 @@ import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; -import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicLongArray; @@ -15,8 +15,6 @@ public class WafMetricCollector implements MetricCollector { public static WafMetricCollector INSTANCE = new WafMetricCollector(); - private static final int ABSTRACT_POWERWAF_EXCEPTION_NUMBER = - 3; // only 3 error codes are possible for now in AbstractWafException public static WafMetricCollector get() { return WafMetricCollector.INSTANCE; @@ -53,18 +51,10 @@ private WafMetricCollector() { new AtomicLongArray(RuleType.getNumValues()); private static final AtomicLongArray raspTimeoutCounter = new AtomicLongArray(RuleType.getNumValues()); - private static final ConcurrentMap raspErrorCodeCounter = - new ConcurrentSkipListMap<>(); - private static final ConcurrentMap wafErrorCodeCounter = - new ConcurrentSkipListMap<>(); - - static { - for (int i = -1 * ABSTRACT_POWERWAF_EXCEPTION_NUMBER; i < 0; i++) { - raspErrorCodeCounter.put(i, new AtomicLongArray(RuleType.getNumValues())); - wafErrorCodeCounter.put(i, new AtomicLongArray(RuleType.getNumValues())); - } - } - + private static final AtomicLongArray raspErrorCodeCounter = + new AtomicLongArray(WafErrorCode.values().length * RuleType.getNumValues()); + private static final AtomicLongArray wafErrorCodeCounter = + new AtomicLongArray(WafErrorCode.values().length); private static final AtomicLongArray missingUserLoginQueue = new AtomicLongArray(LoginFramework.getNumValues() * LoginEvent.getNumValues()); private static final AtomicLongArray missingUserIdQueue = @@ -151,12 +141,23 @@ public void raspTimeout(final RuleType ruleType) { raspTimeoutCounter.incrementAndGet(ruleType.ordinal()); } - public void raspErrorCode(final RuleType ruleType, final int ddwafRunErrorCode) { - raspErrorCodeCounter.get(ddwafRunErrorCode).incrementAndGet(ruleType.ordinal()); + public void raspErrorCode(RuleType ruleType, final int errorCode) { + WafErrorCode wafErrorCode = WafErrorCode.fromCode(errorCode); + // Unsupported waf error code + if (wafErrorCode == null) { + return; + } + int index = wafErrorCode.ordinal() * RuleType.getNumValues() + ruleType.ordinal(); + raspErrorCodeCounter.incrementAndGet(index); } - public void wafErrorCode(final RuleType ruleType, final int ddwafRunErrorCode) { - wafErrorCodeCounter.get(ddwafRunErrorCode).incrementAndGet(ruleType.ordinal()); + public void wafErrorCode(final int errorCode) { + WafErrorCode wafErrorCode = WafErrorCode.fromCode(errorCode); + // Unsupported waf error code + if (wafErrorCode == null) { + return; + } + wafErrorCodeCounter.incrementAndGet(wafErrorCode.ordinal()); } public void missingUserLogin(final LoginFramework framework, final LoginEvent eventType) { @@ -321,16 +322,13 @@ public void prepareMetrics() { } // RASP rule type for each possible error code - for (int i = -1 * ABSTRACT_POWERWAF_EXCEPTION_NUMBER; i < 0; i++) { + for (WafErrorCode errorCode : WafErrorCode.values()) { for (RuleType ruleType : RuleType.values()) { - long counter = raspErrorCodeCounter.get(i).getAndSet(ruleType.ordinal(), 0); - if (counter > 0) { - if (!rawMetricsQueue.offer( - new RaspError(counter, ruleType, WafMetricCollector.wafVersion, i))) { - return; - } + int index = errorCode.ordinal() * RuleType.getNumValues() + ruleType.ordinal(); + long count = raspErrorCodeCounter.getAndSet(index, 0); + if (count > 0) { if (!rawMetricsQueue.offer( - new WafError(counter, ruleType, WafMetricCollector.wafVersion, i))) { + new RaspError(count, ruleType, WafMetricCollector.wafVersion, errorCode.getCode()))) { return; } } @@ -375,6 +373,17 @@ public void prepareMetrics() { } } + // WAF rule type for each possible error code + for (WafErrorCode errorCode : WafErrorCode.values()) { + long count = wafErrorCodeCounter.getAndSet(errorCode.ordinal(), 0); + if (count > 0) { + if (!rawMetricsQueue.offer( + new WafError(count, WafMetricCollector.wafVersion, errorCode.getCode()))) { + return; + } + } + } + // RASP rule skipped per rule type for after-request reason for (RuleType ruleType : RuleType.values()) { long counter = raspRuleSkippedCounter.getAndSet(ruleType.ordinal(), 0); @@ -569,27 +578,13 @@ public RaspError( } public static class WafError extends WafMetric { - public WafError( - final long counter, - final RuleType ruleType, - final String wafVersion, - final Integer ddwafRunError) { + public WafError(final long counter, final String wafVersion, final Integer ddwafRunError) { super( "waf.error", counter, - ruleType.variant != null - ? new String[] { - "rule_type:" + ruleType.type, - "rule_variant:" + ruleType.variant, - "waf_version:" + wafVersion, - "event_rules_version:" + rulesVersion, - "waf_error:" + ddwafRunError - } - : new String[] { - "rule_type:" + ruleType.type, - "waf_version:" + wafVersion, - "waf_error:" + ddwafRunError - }); + "waf_version:" + wafVersion, + "event_rules_version:" + rulesVersion, + "waf_error:" + ddwafRunError); } } @@ -614,4 +609,49 @@ public final void increment() { atomicLong.incrementAndGet(); } } + + /** + * Mirror of the {@code WafErrorCode} enum defined in the {@code libddwaf-java} module. + * + *

This enum is duplicated here to avoid adding a dependency on the native bindings module + * (`libddwaf-java`) within the {@code internal-api} module. + * + *

IMPORTANT: If the {@code WafErrorCode} definition in {@code libddwaf-java} is updated, this + * enum must be kept in sync manually to ensure correct behavior and compatibility. + * + *

Each enum value represents a specific WAF error condition, typically returned when running a + * WAF rule evaluation. + */ + public enum WafErrorCode { + INVALID_ARGUMENT(-1), + INVALID_OBJECT(-2), + INTERNAL_ERROR(-3), + BINDING_ERROR( + -127); // This is a special error code that is not returned by the WAF, is used to signal a + // binding error + + private final int code; + + private static final Map CODE_MAP; + + static { + Map map = new HashMap<>(); + for (WafErrorCode errorCode : values()) { + map.put(errorCode.code, errorCode); + } + CODE_MAP = Collections.unmodifiableMap(map); + } + + WafErrorCode(int code) { + this.code = code; + } + + public int getCode() { + return code; + } + + public static WafErrorCode fromCode(int code) { + return CODE_MAP.get(code); + } + } } diff --git a/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy index 2125ba3ace5..cbf489a58e2 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy @@ -13,8 +13,8 @@ import static datadog.trace.api.telemetry.LoginVersion.V2 class WafMetricCollectorTest extends DDSpecification { - public static final int DD_WAF_RUN_INTERNAL_ERROR = -3 - public static final int DD_WAF_RUN_INVALID_OBJECT_ERROR = -2 + public static final int DD_WAF_RUN_INTERNAL_ERROR = WafMetricCollector.WafErrorCode.INTERNAL_ERROR.getCode() + public static final int DD_WAF_RUN_INVALID_OBJECT_ERROR = WafMetricCollector.WafErrorCode.INVALID_OBJECT.getCode() def "no metrics - drain empty list"() { when: @@ -45,9 +45,9 @@ class WafMetricCollectorTest extends DDSpecification { WafMetricCollector.get().raspRuleEval(RuleType.SQL_INJECTION) WafMetricCollector.get().raspTimeout(RuleType.SQL_INJECTION) WafMetricCollector.get().raspErrorCode(RuleType.SHELL_INJECTION, DD_WAF_RUN_INTERNAL_ERROR) - WafMetricCollector.get().wafErrorCode(RuleType.SHELL_INJECTION, DD_WAF_RUN_INTERNAL_ERROR) + WafMetricCollector.get().wafErrorCode(DD_WAF_RUN_INVALID_OBJECT_ERROR) WafMetricCollector.get().raspErrorCode(RuleType.SQL_INJECTION, DD_WAF_RUN_INVALID_OBJECT_ERROR) - WafMetricCollector.get().wafErrorCode(RuleType.SQL_INJECTION, DD_WAF_RUN_INVALID_OBJECT_ERROR) + WafMetricCollector.get().wafErrorCode(DD_WAF_RUN_INTERNAL_ERROR) WafMetricCollector.get().raspRuleSkipped(RuleType.SQL_INJECTION) WafMetricCollector.get().prepareMetrics() @@ -127,7 +127,7 @@ class WafMetricCollectorTest extends DDSpecification { 'input_truncated:true', ].toSet() - def requestTimeoutMetric = (WafMetricCollector.WafRequestsRawMetric) metrics[6] + def requestTimeoutMetric = (WafMetricCollector.WafRequestsRawMetric)metrics[6] requestTimeoutMetric.namespace == 'appsec' requestTimeoutMetric.metricName == 'waf.requests' requestTimeoutMetric.type == 'count' @@ -182,7 +182,19 @@ class WafMetricCollectorTest extends DDSpecification { raspTimeout.metricName == 'rasp.timeout' raspTimeout.tags.toSet() == ['rule_type:sql_injection', 'waf_version:waf_ver1'].toSet() - def raspInvalidCode = (WafMetricCollector.RaspError) metrics[11] + def raspInvalidObjectCode = (WafMetricCollector.RaspError)metrics[11] + raspInvalidObjectCode.type == 'count' + raspInvalidObjectCode.value == 1 + raspInvalidObjectCode.namespace == 'appsec' + raspInvalidObjectCode.metricName == 'rasp.error' + raspInvalidObjectCode.tags.toSet() == [ + 'rule_type:sql_injection', + 'waf_version:waf_ver1', + 'waf_error:' + DD_WAF_RUN_INVALID_OBJECT_ERROR + ] + .toSet() + + def raspInvalidCode = (WafMetricCollector.RaspError)metrics[12] raspInvalidCode.type == 'count' raspInvalidCode.value == 1 raspInvalidCode.namespace == 'appsec' @@ -195,40 +207,26 @@ class WafMetricCollectorTest extends DDSpecification { 'waf_error:' + DD_WAF_RUN_INTERNAL_ERROR ].toSet() - def wafInvalidCode = (WafMetricCollector.WafError) metrics[12] + def wafInvalidCode = (WafMetricCollector.WafError)metrics[13] wafInvalidCode.type == 'count' wafInvalidCode.value == 1 wafInvalidCode.namespace == 'appsec' wafInvalidCode.metricName == 'waf.error' wafInvalidCode.tags.toSet() == [ 'waf_version:waf_ver1', - 'rule_type:command_injection', - 'rule_variant:shell', 'event_rules_version:rules.3', - 'waf_error:' + DD_WAF_RUN_INTERNAL_ERROR + 'waf_error:' +DD_WAF_RUN_INVALID_OBJECT_ERROR ].toSet() - def raspInvalidObjectCode = (WafMetricCollector.RaspError) metrics[13] - raspInvalidObjectCode.type == 'count' - raspInvalidObjectCode.value == 1 - raspInvalidObjectCode.namespace == 'appsec' - raspInvalidObjectCode.metricName == 'rasp.error' - raspInvalidObjectCode.tags.toSet() == [ - 'rule_type:sql_injection', - 'waf_version:waf_ver1', - 'waf_error:' + DD_WAF_RUN_INVALID_OBJECT_ERROR - ] - .toSet() - - def wafInvalidObjectCode = (WafMetricCollector.WafError) metrics[14] + def wafInvalidObjectCode = (WafMetricCollector.WafError)metrics[14] wafInvalidObjectCode.type == 'count' wafInvalidObjectCode.value == 1 wafInvalidObjectCode.namespace == 'appsec' wafInvalidObjectCode.metricName == 'waf.error' wafInvalidObjectCode.tags.toSet() == [ - 'rule_type:sql_injection', 'waf_version:waf_ver1', - 'waf_error:' + DD_WAF_RUN_INVALID_OBJECT_ERROR + 'event_rules_version:rules.3', + 'waf_error:'+DD_WAF_RUN_INTERNAL_ERROR ].toSet() def raspRuleSkipped = (WafMetricCollector.AfterRequestRaspRuleSkipped) metrics[15] @@ -411,7 +409,7 @@ class WafMetricCollectorTest extends DDSpecification { WafMetricCollector.get().raspRuleEval(ruleType) WafMetricCollector.get().raspTimeout(ruleType) WafMetricCollector.get().raspErrorCode(ruleType, DD_WAF_RUN_INTERNAL_ERROR) - WafMetricCollector.get().wafErrorCode(ruleType, DD_WAF_RUN_INTERNAL_ERROR) + WafMetricCollector.get().wafErrorCode(DD_WAF_RUN_INTERNAL_ERROR) WafMetricCollector.get().raspRuleSkipped(ruleType) WafMetricCollector.get().prepareMetrics() @@ -474,8 +472,6 @@ class WafMetricCollectorTest extends DDSpecification { wafInvalidCode.metricName == 'waf.error' wafInvalidCode.tags.toSet() == [ 'waf_version:waf_ver1', - 'rule_type:command_injection', - 'rule_variant:' + ruleType.variant, 'event_rules_version:rules.1', 'waf_error:' + DD_WAF_RUN_INTERNAL_ERROR ].toSet() From 5f4cc624759b15f1b4bdfd4f7e4c5fe47622dc81 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 2 Apr 2025 13:21:21 +0200 Subject: [PATCH 2/2] fix --- .../com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy index c326b8f2089..6962e99093b 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/ddwaf/WAFModuleSpecification.groovy @@ -1826,8 +1826,8 @@ class WAFModuleSpecification extends DDSpecification { def 'internal-api WafErrorCode enum matches libddwaf-java by name and code'() { given: - def libddwaf = com.datadog.ddwaf.WafErrorCode.values().collectEntries { [it.name(), it.code] } - def internal = datadog.trace.api.telemetry.WafMetricCollector.WafErrorCode.values().collectEntries { [it.name(), it.code] } + def libddwaf = LibWafErrorCode.values().collectEntries { [it.name(), it.code] } + def internal = InternalWafErrorCode.values().collectEntries { [it.name(), it.code] } expect: internal == libddwaf