diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java index b57d6d2c2f9..849ebc65a6e 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java @@ -245,12 +245,6 @@ private void initializeNewWafCtx( currentRulesVersion = initReport.rulesetVersion; } - if (prevContextAndAddresses == null) { - WafMetricCollector.get().wafInit(Powerwaf.LIB_VERSION, currentRulesVersion); - } else { - WafMetricCollector.get().wafUpdates(currentRulesVersion); - } - if (initReport != null) { log.info( "Created {} WAF context with rules ({} OK, {} BAD), version {}", @@ -273,11 +267,13 @@ private void initializeNewWafCtx( } } catch (InvalidRuleSetException irse) { initReport = irse.ruleSetInfo; + sendWafMetrics(prevContextAndAddresses, false); throw new AppSecModuleActivationException("Error creating WAF rules", irse); } catch (RuntimeException | AbstractPowerwafException e) { if (newPwafCtx != null) { newPwafCtx.close(); } + sendWafMetrics(prevContextAndAddresses, false); throw new AppSecModuleActivationException("Error creating WAF rules", e); } finally { if (initReport != null) { @@ -287,9 +283,12 @@ private void initializeNewWafCtx( if (!this.ctxAndAddresses.compareAndSet(prevContextAndAddresses, newContextAndAddresses)) { newPwafCtx.close(); + sendWafMetrics(prevContextAndAddresses, false); throw new AppSecModuleActivationException("Concurrent update of WAF configuration"); } + sendWafMetrics(prevContextAndAddresses, true); + if (prevContextAndAddresses != null) { prevContextAndAddresses.ctx.close(); } @@ -297,6 +296,14 @@ private void initializeNewWafCtx( reconf.reloadSubscriptions(); } + private void sendWafMetrics(CtxAndAddresses prevContextAndAddresses, boolean success) { + if (prevContextAndAddresses == null) { + WafMetricCollector.get().wafInit(Powerwaf.LIB_VERSION, currentRulesVersion, success); + } else { + WafMetricCollector.get().wafUpdates(currentRulesVersion, success); + } + } + private Map calculateEffectiveActions( CtxAndAddresses prevContextAndAddresses, AppSecConfig ruleConfig) { Map actionInfoMap; diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/powerwaf/PowerWAFModuleSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/powerwaf/PowerWAFModuleSpecification.groovy index efe94056180..b5f5403bbd1 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/powerwaf/PowerWAFModuleSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/powerwaf/PowerWAFModuleSpecification.groovy @@ -70,7 +70,10 @@ class PowerWAFModuleSpecification extends DDSpecification { Additive pwafAdditive PowerwafMetrics metrics + WafMetricCollector wafMetricCollector = Mock(WafMetricCollector) + void setup() { + WafMetricCollector.INSTANCE = wafMetricCollector AgentTracer.forceRegister(tracer) } @@ -206,6 +209,8 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: + 1 * wafMetricCollector.wafInit(Powerwaf.LIB_VERSION, _, true) + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() 1 * flow.setAction({ Flow.Action.RequestBlockingAction rba -> rba.statusCode == 501 && @@ -241,6 +246,7 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: + 1 * wafMetricCollector.wafInit(Powerwaf.LIB_VERSION, _, true) 1 * flow.setAction({ Flow.Action.RequestBlockingAction rba -> rba.statusCode == 403 && rba.blockingContentType == BlockingContentType.AUTO @@ -282,6 +288,7 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() 1 * flow.setAction({ Flow.Action.RequestBlockingAction rba -> rba.statusCode == 403 && @@ -364,6 +371,7 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() 1 * flow.setAction({ Flow.Action.RequestBlockingAction rba -> rba.statusCode == 403 && @@ -440,7 +448,10 @@ class PowerWAFModuleSpecification extends DDSpecification { } then: + 1 * wafMetricCollector.wafInit(Powerwaf.LIB_VERSION, _, true) + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() + 0 * _ when: dataListener.onDataAvailable(flow, ctx, ATTACK_BUNDLE, gwCtx) @@ -522,7 +533,10 @@ class PowerWAFModuleSpecification extends DDSpecification { } then: + 1 * wafMetricCollector.wafInit(Powerwaf.LIB_VERSION, _, true) + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() + 0 * _ when: dataListener.onDataAvailable(flow, ctx, ATTACK_BUNDLE, gwCtx) @@ -597,7 +611,10 @@ class PowerWAFModuleSpecification extends DDSpecification { } then: + 1 * wafMetricCollector.wafInit(Powerwaf.LIB_VERSION, _, true) + 2 * wafMetricCollector.wafUpdates(_, true) 2 * reconf.reloadSubscriptions() + 0 * _ when: dataListener.onDataAvailable(flow, ctx, ATTACK_BUNDLE, gwCtx) @@ -972,9 +989,6 @@ class PowerWAFModuleSpecification extends DDSpecification { TraceSegment segment = Mock() TraceSegmentPostProcessor pp = service.traceSegmentPostProcessors.last() - def mockWafMetricCollector = Mock(WafMetricCollector) - WafMetricCollector.INSTANCE = mockWafMetricCollector - when: dataListener.onDataAvailable(flow, ctx, db, gwCtx) @@ -987,7 +1001,7 @@ class PowerWAFModuleSpecification extends DDSpecification { pwafAdditive = it[0].openAdditive() } 1 * ctx.getWafMetrics() 1 * ctx.increaseWafTimeouts() - 1 * mockWafMetricCollector.get().wafRequestTimeout() + 1 * wafMetricCollector.get().wafRequestTimeout() 0 * _ when: @@ -1014,9 +1028,6 @@ class PowerWAFModuleSpecification extends DDSpecification { TraceSegment segment = Mock() TraceSegmentPostProcessor pp = service.traceSegmentPostProcessors.last() - def mockWafMetricCollector = Mock(WafMetricCollector) - WafMetricCollector.INSTANCE = mockWafMetricCollector - gwCtx = new GatewayContext(false, RuleType.SQL_INJECTION) when: @@ -1032,8 +1043,8 @@ class PowerWAFModuleSpecification extends DDSpecification { 1 * ctx.getRaspMetrics() 1 * ctx.getRaspMetricsCounter() 1 * ctx.increaseRaspTimeouts() - 1 * mockWafMetricCollector.get().raspTimeout(gwCtx.raspRuleType) - 1 * mockWafMetricCollector.raspRuleEval(RuleType.SQL_INJECTION) + 1 * wafMetricCollector.get().raspTimeout(gwCtx.raspRuleType) + 1 * wafMetricCollector.raspRuleEval(RuleType.SQL_INJECTION) 0 * _ when: @@ -1059,6 +1070,7 @@ class PowerWAFModuleSpecification extends DDSpecification { then: thrown AppSecModule.AppSecModuleActivationException + 0 * _ when: cfgService.listeners['waf'].onNewSubconfig(defaultConfig['waf'], reconf) @@ -1070,7 +1082,14 @@ class PowerWAFModuleSpecification extends DDSpecification { 1 * ctx.getOrCreateAdditive(_, true, false) >> { pwafAdditive = it[0].openAdditive() } 1 * ctx.reportEvents(_ as Collection) + 1 * ctx.isAdditiveClosed() + 1 * ctx.getWafMetrics() + 1 * ctx.isThrottled(null) + 1 * ctx.closeAdditive() + 2 * tracer.activeSpan() + 1 * wafMetricCollector.wafInit(Powerwaf.LIB_VERSION, _, true) 1 * reconf.reloadSubscriptions() + 0 * _ } void 'rule data given through configuration'() { @@ -1102,6 +1121,7 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() 1 * ctx.getOrCreateAdditive(_, true, false) >> { pwafAdditive = it[0].openAdditive() } 2 * tracer.activeSpan() @@ -1159,6 +1179,7 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: 'no match; rule is disabled' + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() 1 * ctx.getOrCreateAdditive(_, true, false) >> { pwafAdditive = it[0].openAdditive() } @@ -1189,6 +1210,7 @@ class PowerWAFModuleSpecification extends DDSpecification { 1 * ctx.getWafMetrics() 1 * ctx.isAdditiveClosed() >> false 1 * ctx.closeAdditive() >> {pwafAdditive.close()} + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() _ * ctx.increaseWafTimeouts() _ * ctx.increaseRaspTimeouts() @@ -1206,6 +1228,7 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: 'now we have match' + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() 1 * ctx.getOrCreateAdditive(_, true, false) >> { pwafAdditive = it[0].openAdditive() } @@ -1235,6 +1258,7 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: 'nothing again; we disabled the rule' + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() 1 * ctx.getOrCreateAdditive(_, true, false) >> { pwafAdditive = it[0].openAdditive() } 1 * ctx.getWafMetrics() @@ -1265,6 +1289,7 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() // no attack 1 * ctx.getOrCreateAdditive(_, true, false) >> { pwafAdditive = it[0].openAdditive() } @@ -1289,6 +1314,7 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() // no attack 1 * ctx.getOrCreateAdditive(_, true, false) >> { @@ -1314,6 +1340,7 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() // attack found 1 * ctx.getOrCreateAdditive(_, true, false) >> { @@ -1342,6 +1369,7 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() // no attack 1 * ctx.getOrCreateAdditive(_, true, false) >> { @@ -1390,7 +1418,9 @@ class PowerWAFModuleSpecification extends DDSpecification { pwafModule.config(cfgService) then: + 1 * wafMetricCollector.wafInit(Powerwaf.LIB_VERSION, _, true) !pwafModule.dataSubscriptions.first().subscribedAddresses.contains(doesNotExistAddress) + 0 * _ } void 'bad initial configuration is given results in no subscriptions'() { @@ -1403,6 +1433,7 @@ class PowerWAFModuleSpecification extends DDSpecification { then: thrown AppSecModule.AppSecModuleActivationException pwafModule.dataSubscriptions.empty + 0 * _ } void 'rule data not a config'() { @@ -1414,9 +1445,8 @@ class PowerWAFModuleSpecification extends DDSpecification { then: thrown AppSecModule.AppSecModuleActivationException - - then: pwafModule.ctxAndAddresses.get() == null + 0 * _ } void 'bad ResultWithData - empty list'() { @@ -1554,7 +1584,18 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: + 1 * ctx.isAdditiveClosed() + 1 * ctx.getOrCreateAdditive(_ as PowerwafContext, true, false) >> { + pwafAdditive = it[0].openAdditive() + } + 1 * ctx.getWafMetrics() + 1 * ctx.isThrottled(null) + 1 * ctx.reportEvents(_ as Collection) + 1 * ctx.closeAdditive() + 2 * tracer.activeSpan() + 1 * flow.isBlocking() 0 * flow.setAction(_) + 0 * _ when: final ipData = new AppSecData(exclusion: [ @@ -1576,11 +1617,22 @@ class PowerWAFModuleSpecification extends DDSpecification { ctx.closeAdditive() then: + 1 * wafMetricCollector.wafUpdates(_, true) 1 * reconf.reloadSubscriptions() 1 * flow.setAction({ Flow.Action.RequestBlockingAction rba -> rba.statusCode == 402 && rba.blockingContentType == BlockingContentType.AUTO }) + 1 * flow.isBlocking() 1 * ctx.isAdditiveClosed() >> false + 1 * ctx.getOrCreateAdditive(_ as PowerwafContext, true, false) >> { + pwafAdditive = it[0].openAdditive() + } + 1 * ctx.getWafMetrics() + 1 * ctx.isThrottled(null) + 1 * ctx.reportEvents(_ as Collection) + 1 * ctx.closeAdditive() + 2 * tracer.activeSpan() + 0 * _ } void 'http endpoint fingerprint support'() { @@ -1668,6 +1720,7 @@ class PowerWAFModuleSpecification extends DDSpecification { then: 1 * ctx.closeAdditive() 1 * ctx.isAdditiveClosed() >> true + 1 * wafMetricCollector.wafInit(Powerwaf.LIB_VERSION, _, true) 0 * _ } 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 824412fbf24..2567cc6c200 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 @@ -55,16 +55,17 @@ private WafMetricCollector() { */ private static String rulesVersion = ""; - public void wafInit(final String wafVersion, final String rulesVersion) { + public void wafInit(final String wafVersion, final String rulesVersion, final boolean success) { WafMetricCollector.wafVersion = wafVersion; WafMetricCollector.rulesVersion = rulesVersion; rawMetricsQueue.offer( - new WafInitRawMetric(wafInitCounter.incrementAndGet(), wafVersion, rulesVersion)); + new WafInitRawMetric(wafInitCounter.incrementAndGet(), wafVersion, rulesVersion, success)); } - public void wafUpdates(final String rulesVersion) { + public void wafUpdates(final String rulesVersion, final boolean success) { rawMetricsQueue.offer( - new WafUpdatesRawMetric(wafUpdatesCounter.incrementAndGet(), wafVersion, rulesVersion)); + new WafUpdatesRawMetric( + wafUpdatesCounter.incrementAndGet(), wafVersion, rulesVersion, success)); // Flush request metrics to get the new version. if (rulesVersion != null @@ -249,20 +250,31 @@ public WafMetric(String metricName, long counter, String... tags) { public static class WafInitRawMetric extends WafMetric { public WafInitRawMetric( - final long counter, final String wafVersion, final String rulesVersion) { + final long counter, + final String wafVersion, + final String rulesVersion, + final boolean success) { super( - "waf.init", counter, "waf_version:" + wafVersion, "event_rules_version:" + rulesVersion); + "waf.init", + counter, + "waf_version:" + wafVersion, + "event_rules_version:" + rulesVersion, + "success:" + success); } } public static class WafUpdatesRawMetric extends WafMetric { public WafUpdatesRawMetric( - final long counter, final String wafVersion, final String rulesVersion) { + final long counter, + final String wafVersion, + final String rulesVersion, + final boolean success) { super( "waf.updates", counter, "waf_version:" + wafVersion, - "event_rules_version:" + rulesVersion); + "event_rules_version:" + rulesVersion, + "success:" + success); } } 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 e039b2598f8..1e1159dcfa7 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 @@ -18,9 +18,9 @@ class WafMetricCollectorTest extends DDSpecification { def "put-get waf/rasp metrics"() { when: - WafMetricCollector.get().wafInit('waf_ver1', 'rules.1') - WafMetricCollector.get().wafUpdates('rules.2') - WafMetricCollector.get().wafUpdates('rules.3') + WafMetricCollector.get().wafInit('waf_ver1', 'rules.1', true) + WafMetricCollector.get().wafUpdates('rules.2', true) + WafMetricCollector.get().wafUpdates('rules.3', false) WafMetricCollector.get().wafRequest() WafMetricCollector.get().wafRequest() WafMetricCollector.get().wafRequest() @@ -43,21 +43,21 @@ class WafMetricCollectorTest extends DDSpecification { initMetric.value == 1 initMetric.namespace == 'appsec' initMetric.metricName == 'waf.init' - initMetric.tags.toSet() == ['waf_version:waf_ver1', 'event_rules_version:rules.1'].toSet() + initMetric.tags.toSet() == ['waf_version:waf_ver1', 'event_rules_version:rules.1', 'success:true'].toSet() def updateMetric1 = (WafMetricCollector.WafUpdatesRawMetric)metrics[1] updateMetric1.type == 'count' updateMetric1.value == 1 updateMetric1.namespace == 'appsec' updateMetric1.metricName == 'waf.updates' - updateMetric1.tags.toSet() == ['waf_version:waf_ver1', 'event_rules_version:rules.2'].toSet() + updateMetric1.tags.toSet() == ['waf_version:waf_ver1', 'event_rules_version:rules.2', 'success:true'].toSet() def updateMetric2 = (WafMetricCollector.WafUpdatesRawMetric)metrics[2] updateMetric2.type == 'count' updateMetric2.value == 2 updateMetric2.namespace == 'appsec' updateMetric2.metricName == 'waf.updates' - updateMetric2.tags.toSet() == ['waf_version:waf_ver1', 'event_rules_version:rules.3'].toSet() + updateMetric2.tags.toSet() == ['waf_version:waf_ver1', 'event_rules_version:rules.3', 'success:false'].toSet() def requestMetric = (WafMetricCollector.WafRequestsRawMetric)metrics[3] requestMetric.namespace == 'appsec' @@ -140,7 +140,7 @@ class WafMetricCollectorTest extends DDSpecification { when: (0..limit*2).each { - collector.wafInit("foo", "bar") + collector.wafInit("foo", "bar", true) } then: @@ -149,7 +149,7 @@ class WafMetricCollectorTest extends DDSpecification { when: (0..limit*2).each { - collector.wafUpdates("bar") + collector.wafUpdates("bar", true) } then: @@ -298,7 +298,7 @@ class WafMetricCollectorTest extends DDSpecification { def "test Rasp #ruleType metrics"() { when: - WafMetricCollector.get().wafInit('waf_ver1', 'rules.1') + WafMetricCollector.get().wafInit('waf_ver1', 'rules.1', true) WafMetricCollector.get().raspRuleEval(ruleType) WafMetricCollector.get().raspRuleEval(ruleType) WafMetricCollector.get().raspRuleMatch(ruleType) diff --git a/telemetry/src/test/groovy/datadog/telemetry/metric/WafMetricPeriodicActionSpecification.groovy b/telemetry/src/test/groovy/datadog/telemetry/metric/WafMetricPeriodicActionSpecification.groovy index 375a09d2244..329dcb18c14 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/metric/WafMetricPeriodicActionSpecification.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/metric/WafMetricPeriodicActionSpecification.groovy @@ -11,9 +11,9 @@ class WafMetricPeriodicActionSpecification extends DDSpecification { void 'push waf metrics into the telemetry service'() { setup: - WafMetricCollector.get().wafInit('0.0.0', 'rules_ver_1') - WafMetricCollector.get().wafUpdates('rules_ver_2') - WafMetricCollector.get().wafUpdates('rules_ver_3') + WafMetricCollector.get().wafInit('0.0.0', 'rules_ver_1', true) + WafMetricCollector.get().wafUpdates('rules_ver_2', true) + WafMetricCollector.get().wafUpdates('rules_ver_3', true) when: periodicAction.doIteration(telemetryService) @@ -23,26 +23,26 @@ class WafMetricPeriodicActionSpecification extends DDSpecification { metric.namespace == 'appsec' && metric.metric == 'waf.init' && metric.points[0][1] == 1 && - metric.tags == ['waf_version:0.0.0', 'event_rules_version:rules_ver_1'] + metric.tags == ['waf_version:0.0.0', 'event_rules_version:rules_ver_1', 'success:true'] } ) 1 * telemetryService.addMetric( { Metric metric -> metric.namespace == 'appsec' && metric.metric == 'waf.updates' && metric.points[0][1] == 1 && - metric.tags == ['waf_version:0.0.0', 'event_rules_version:rules_ver_2'] + metric.tags == ['waf_version:0.0.0', 'event_rules_version:rules_ver_2', 'success:true'] } ) 1 * telemetryService.addMetric( { Metric metric -> metric.namespace == 'appsec' && metric.metric == 'waf.updates' && metric.points[0][1] == 2 && - metric.tags == ['waf_version:0.0.0', 'event_rules_version:rules_ver_3'] + metric.tags == ['waf_version:0.0.0', 'event_rules_version:rules_ver_3', 'success:true'] } ) 0 * _._ } void 'push waf request metrics and push into the telemetry'() { when: - WafMetricCollector.get().wafInit('0.0.0', 'rules_ver_1') + WafMetricCollector.get().wafInit('0.0.0', 'rules_ver_1', true) WafMetricCollector.get().wafRequest() WafMetricCollector.get().wafRequestTriggered() WafMetricCollector.get().wafRequest() @@ -108,7 +108,7 @@ class WafMetricPeriodicActionSpecification extends DDSpecification { 0 * _._ when: 'waf.updates happens' - WafMetricCollector.get().wafUpdates('rules_ver_2') + WafMetricCollector.get().wafUpdates('rules_ver_2', true) WafMetricCollector.get().wafRequest() WafMetricCollector.get().wafRequestTriggered() WafMetricCollector.get().wafRequestBlocked()