Report waf init and update success and failure (#5388)

* Report waf init and update success

* handle waf init failure

* do not report ecent rules when success fails

* remove default value of diagnosticsRules

* use default args to report init

Author: IlyasShabi

packages/dd-trace/src/appsec/reporter.js

@@ -88,16 +88,18 @@ function formatHeaderName (name) {
     .toLowerCase()
 }
 
-function reportWafInit (wafVersion, rulesVersion, diagnosticsRules = {}) {
-  metricsQueue.set('_dd.appsec.waf.version', wafVersion)
-
-  metricsQueue.set('_dd.appsec.event_rules.loaded', diagnosticsRules.loaded?.length || 0)
-  metricsQueue.set('_dd.appsec.event_rules.error_count', diagnosticsRules.failed?.length || 0)
-  if (diagnosticsRules.failed?.length) {
-    metricsQueue.set('_dd.appsec.event_rules.errors', JSON.stringify(diagnosticsRules.errors))
+function reportWafInit (wafVersion, rulesVersion, diagnosticsRules = {}, success = false) {
+  if (success) {
+    metricsQueue.set('_dd.appsec.waf.version', wafVersion)
+
+    metricsQueue.set('_dd.appsec.event_rules.loaded', diagnosticsRules.loaded?.length || 0)
+    metricsQueue.set('_dd.appsec.event_rules.error_count', diagnosticsRules.failed?.length || 0)
+    if (diagnosticsRules.failed?.length) {
+      metricsQueue.set('_dd.appsec.event_rules.errors', JSON.stringify(diagnosticsRules.errors))
+    }
   }
 
-  incrementWafInitMetric(wafVersion, rulesVersion)
+  incrementWafInitMetric(wafVersion, rulesVersion, success)
 }
 
 function reportMetrics (metrics, raspRule) {

packages/dd-trace/src/appsec/telemetry/index.js

@@ -74,16 +74,16 @@ function updateWafRequestsMetricTags (metrics, req) {
   return trackWafMetrics(store, metrics)
 }
 
-function incrementWafInitMetric (wafVersion, rulesVersion) {
+function incrementWafInitMetric (wafVersion, rulesVersion, success) {
   if (!enabled) return
 
-  incrementWafInit(wafVersion, rulesVersion)
+  incrementWafInit(wafVersion, rulesVersion, success)
 }
 
-function incrementWafUpdatesMetric (wafVersion, rulesVersion) {
+function incrementWafUpdatesMetric (wafVersion, rulesVersion, success) {
   if (!enabled) return
 
-  incrementWafUpdates(wafVersion, rulesVersion)
+  incrementWafUpdates(wafVersion, rulesVersion, success)
 }
 
 function incrementWafRequestsMetric (req) {

packages/dd-trace/src/appsec/telemetry/waf.js

@@ -91,16 +91,22 @@ function getOrCreateMetricTags (store, versionsTags) {
   return metricTags
 }
 
-function incrementWafInit (wafVersion, rulesVersion) {
+function incrementWafInit (wafVersion, rulesVersion, success) {
   const versionsTags = getVersionsTags(wafVersion, rulesVersion)
+  appsecMetrics.count('waf.init', { ...versionsTags, success }).inc()
 
-  appsecMetrics.count('waf.init', versionsTags).inc()
+  if (!success) {
+    appsecMetrics.count('waf.config_errors', versionsTags).inc()
+  }
 }
 
-function incrementWafUpdates (wafVersion, rulesVersion) {
+function incrementWafUpdates (wafVersion, rulesVersion, success) {
   const versionsTags = getVersionsTags(wafVersion, rulesVersion)
+  appsecMetrics.count('waf.updates', { ...versionsTags, success }).inc()
 
-  appsecMetrics.count('waf.updates', versionsTags).inc()
+  if (!success) {
+    appsecMetrics.count('waf.config_errors', versionsTags).inc()
+  }
 }
 
 function incrementWafRequests (store) {

packages/dd-trace/src/appsec/waf/waf_manager.js

@@ -11,20 +11,23 @@ class WAFManager {
     this.config = config
     this.wafTimeout = config.wafTimeout
     this.ddwaf = this._loadDDWAF(rules)
-    this.ddwafVersion = this.ddwaf.constructor.version()
     this.rulesVersion = this.ddwaf.diagnostics.ruleset_version
 
-    Reporter.reportWafInit(this.ddwafVersion, this.rulesVersion, this.ddwaf.diagnostics.rules)
+    Reporter.reportWafInit(this.ddwafVersion, this.rulesVersion, this.ddwaf.diagnostics.rules, true)
   }
 
   _loadDDWAF (rules) {
     try {
       // require in `try/catch` because this can throw at require time
       const { DDWAF } = require('@datadog/native-appsec')
+      this.ddwafVersion = DDWAF.version()
 
       const { obfuscatorKeyRegex, obfuscatorValueRegex } = this.config
       return new DDWAF(rules, { obfuscatorKeyRegex, obfuscatorValueRegex })
     } catch (err) {
+      this.ddwafVersion = this.ddwafVersion || 'unknown'
+      Reporter.reportWafInit(this.ddwafVersion, 'unknown')
+
       log.error('[ASM] AppSec could not load native package. In-app WAF features will not be available.')
 
       throw err
@@ -49,13 +52,19 @@ class WAFManager {
   }
 
   update (newRules) {
-    this.ddwaf.update(newRules)
+    try {
+      this.ddwaf.update(newRules)
 
-    if (this.ddwaf.diagnostics.ruleset_version) {
-      this.rulesVersion = this.ddwaf.diagnostics.ruleset_version
-    }
+      if (this.ddwaf.diagnostics.ruleset_version) {
+        this.rulesVersion = this.ddwaf.diagnostics.ruleset_version
+      }
 
-    Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion)
+      Reporter.reportWafUpdate(this.ddwafVersion, this.rulesVersion, true)
+    } catch (error) {
+      Reporter.reportWafUpdate(this.ddwafVersion, 'unknown', false)
+
+      throw error
+    }
   }
 
   destroy () {

packages/dd-trace/test/appsec/index.spec.js

@@ -646,7 +646,7 @@ describe('AppSec Index', function () {
         responseBody.publish({ req, res, body })
 
         expect(apiSecuritySampler.sampleRequest).to.have.been.calledOnceWith(req, res)
-        expect(waf.run).to.been.calledOnceWith({
+        expect(waf.run).to.have.been.calledOnceWith({
           persistent: {
             [addresses.HTTP_INCOMING_RESPONSE_BODY]: body
           }

packages/dd-trace/test/appsec/reporter.spec.js

@@ -101,7 +101,7 @@ describe('reporter', () => {
     }
 
     it('should add some entries to metricsQueue', () => {
-      Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules)
+      Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, true)
 
       expect(Reporter.metricsQueue.get('_dd.appsec.waf.version')).to.be.eq(wafVersion)
       expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.loaded')).to.be.eq(3)
@@ -110,23 +110,32 @@ describe('reporter', () => {
         .to.be.eq(JSON.stringify(diagnosticsRules.errors))
     })
 
+    it('should not add entries to metricsQueue with success false', () => {
+      Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, false)
+
+      expect(Reporter.metricsQueue.get('_dd.appsec.waf.version')).to.be.undefined
+      expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.loaded')).to.be.undefined
+      expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.error_count')).to.be.undefined
+      expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.errors')).to.be.undefined
+    })
+
     it('should call incrementWafInitMetric', () => {
-      Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules)
+      Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, true)
 
-      expect(telemetry.incrementWafInitMetric).to.have.been.calledOnceWithExactly(wafVersion, rulesVersion)
+      expect(telemetry.incrementWafInitMetric).to.have.been.calledOnceWithExactly(wafVersion, rulesVersion, true)
     })
 
     it('should not fail with undefined arguments', () => {
       const wafVersion = undefined
       const rulesVersion = undefined
       const diagnosticsRules = undefined
 
-      Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules)
+      Reporter.reportWafInit(wafVersion, rulesVersion, diagnosticsRules, true)
 
       expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.loaded')).to.be.eq(0)
       expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.error_count')).to.be.eq(0)
 
-      expect(telemetry.incrementWafInitMetric).to.have.been.calledOnceWithExactly(wafVersion, rulesVersion)
+      expect(telemetry.incrementWafInitMetric).to.have.been.calledOnceWithExactly(wafVersion, rulesVersion, true)
     })
   })
 
@@ -380,9 +389,9 @@ describe('reporter', () => {
 
   describe('reportWafUpdate', () => {
     it('should call incrementWafUpdatesMetric', () => {
-      Reporter.reportWafUpdate('0.0.1', '0.0.2')
+      Reporter.reportWafUpdate('0.0.1', '0.0.2', true)
 
-      expect(telemetry.incrementWafUpdatesMetric).to.have.been.calledOnceWithExactly('0.0.1', '0.0.2')
+      expect(telemetry.incrementWafUpdatesMetric).to.have.been.calledOnceWithExactly('0.0.1', '0.0.2', true)
     })
   })
 

packages/dd-trace/test/appsec/telemetry/waf.spec.js

@@ -185,21 +185,22 @@ describe('Appsec Waf Telemetry metrics', () => {
 
     describe('incWafInitMetric', () => {
       it('should increment waf.init metric', () => {
-        appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion)
+        appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion, true)
 
         expect(count).to.have.been.calledOnceWithExactly('waf.init', {
           waf_version: wafVersion,
-          event_rules_version: rulesVersion
+          event_rules_version: rulesVersion,
+          success: true
         })
         expect(inc).to.have.been.calledOnce
       })
 
       it('should increment waf.init metric multiple times', () => {
         sinon.restore()
 
-        appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion)
-        appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion)
-        appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion)
+        appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion, true)
+        appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion, true)
+        appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion, true)
 
         const { metrics } = appsecNamespace.toJSON()
         expect(metrics.series.length).to.be.eq(1)
@@ -208,26 +209,45 @@ describe('Appsec Waf Telemetry metrics', () => {
         expect(metrics.series[0].points[0][1]).to.be.eq(3)
         expect(metrics.series[0].tags).to.include('waf_version:0.0.1')
         expect(metrics.series[0].tags).to.include('event_rules_version:0.0.2')
+        expect(metrics.series[0].tags).to.include('success:true')
+      })
+
+      it('should increment waf.init and waf.config_errors on failed init', () => {
+        sinon.restore()
+
+        appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion, false)
+
+        const { metrics } = appsecNamespace.toJSON()
+        expect(metrics.series.length).to.be.eq(2)
+        expect(metrics.series[0].metric).to.be.eq('waf.init')
+        expect(metrics.series[0].tags).to.include('waf_version:0.0.1')
+        expect(metrics.series[0].tags).to.include('event_rules_version:0.0.2')
+        expect(metrics.series[0].tags).to.include('success:false')
+
+        expect(metrics.series[1].metric).to.be.eq('waf.config_errors')
+        expect(metrics.series[1].tags).to.include('waf_version:0.0.1')
+        expect(metrics.series[1].tags).to.include('event_rules_version:0.0.2')
       })
     })
 
     describe('incWafUpdatesMetric', () => {
       it('should increment waf.updates metric', () => {
-        appsecTelemetry.incrementWafUpdatesMetric(wafVersion, rulesVersion)
+        appsecTelemetry.incrementWafUpdatesMetric(wafVersion, rulesVersion, true)
 
         expect(count).to.have.been.calledOnceWithExactly('waf.updates', {
           waf_version: wafVersion,
-          event_rules_version: rulesVersion
+          event_rules_version: rulesVersion,
+          success: true
         })
         expect(inc).to.have.been.calledOnce
       })
 
       it('should increment waf.updates metric multiple times', () => {
         sinon.restore()
 
-        appsecTelemetry.incrementWafUpdatesMetric(wafVersion, rulesVersion)
-        appsecTelemetry.incrementWafUpdatesMetric(wafVersion, rulesVersion)
-        appsecTelemetry.incrementWafUpdatesMetric(wafVersion, rulesVersion)
+        appsecTelemetry.incrementWafUpdatesMetric(wafVersion, rulesVersion, true)
+        appsecTelemetry.incrementWafUpdatesMetric(wafVersion, rulesVersion, true)
+        appsecTelemetry.incrementWafUpdatesMetric(wafVersion, rulesVersion, true)
 
         const { metrics } = appsecNamespace.toJSON()
         expect(metrics.series.length).to.be.eq(1)
@@ -236,6 +256,24 @@ describe('Appsec Waf Telemetry metrics', () => {
         expect(metrics.series[0].points[0][1]).to.be.eq(3)
         expect(metrics.series[0].tags).to.include('waf_version:0.0.1')
         expect(metrics.series[0].tags).to.include('event_rules_version:0.0.2')
+        expect(metrics.series[0].tags).to.include('success:true')
+      })
+
+      it('should increment waf.updates and waf.config_errors on failed update', () => {
+        sinon.restore()
+
+        appsecTelemetry.incrementWafUpdatesMetric(wafVersion, rulesVersion, false)
+
+        const { metrics } = appsecNamespace.toJSON()
+        expect(metrics.series.length).to.be.eq(2)
+        expect(metrics.series[0].metric).to.be.eq('waf.updates')
+        expect(metrics.series[0].tags).to.include('waf_version:0.0.1')
+        expect(metrics.series[0].tags).to.include('event_rules_version:0.0.2')
+        expect(metrics.series[0].tags).to.include('success:false')
+
+        expect(metrics.series[1].metric).to.be.eq('waf.config_errors')
+        expect(metrics.series[1].tags).to.include('waf_version:0.0.1')
+        expect(metrics.series[1].tags).to.include('event_rules_version:0.0.2')
       })
     })
 
@@ -333,7 +371,7 @@ describe('Appsec Waf Telemetry metrics', () => {
         metrics: true
       })
 
-      appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion)
+      appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion, true)
 
       expect(count).to.not.have.been.called
       expect(inc).to.not.have.been.called
@@ -345,7 +383,7 @@ describe('Appsec Waf Telemetry metrics', () => {
         metrics: false
       })
 
-      appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion)
+      appsecTelemetry.incrementWafInitMetric(wafVersion, rulesVersion, true)
 
       expect(count).to.not.have.been.called
       expect(inc).to.not.have.been.called