appsec: RFC-1012 metrics improvements (duration distributions, rule_variant, tag fixes)

.claude/ci/index.md

@@ -244,6 +244,10 @@ Note that path — it is the output file for the next step.
 .claude/ci/ci-watch [--start-offset N] OUTPUT_FILE
 ```
 
+**`OUTPUT_FILE` must be the output file from a `check-ci` process** — not an
+arbitrary background task. `ci-watch` parses `check-ci`'s structured
+`FAILED:` / `SUCCESS:` lines and exits silently on anything else.
+
 `ci-watch` tails the output file and exits when there is something to
 act on. Run it with `run_in_background: true` — you will be notified
 when it completes. While it runs, you can do other work.

appsec/cmake/clang-tidy.cmake

@@ -1,17 +1,17 @@
-# Prefer a locally installed LLVM 19 run-clang-tidy (e.g. via brew install llvm@19)
+# Prefer a locally installed LLVM 17 run-clang-tidy (e.g. via brew install llvm@17)
 # over the Docker-based wrapper, since native execution avoids SDK incompatibilities.
-set(_LLVM19_BIN /opt/homebrew/opt/llvm@19/bin)
-set(_LLVM19_TIDY ${_LLVM19_BIN}/run-clang-tidy)
+set(_LLVM17_BIN /opt/homebrew/opt/llvm@17/bin)
+set(_LLVM17_TIDY ${_LLVM17_BIN}/run-clang-tidy)
 set(CLANG_TIDY_BINARY_OPT "")
-if(EXISTS ${_LLVM19_TIDY})
-    set(CLANG_TIDY ${_LLVM19_TIDY})
-    set(CLANG_TIDY_BINARY_OPT -clang-tidy-binary ${_LLVM19_BIN}/clang-tidy)
-    message(STATUS "Using Homebrew LLVM 19 run-clang-tidy: ${CLANG_TIDY}")
+if(EXISTS ${_LLVM17_TIDY})
+    set(CLANG_TIDY ${_LLVM17_TIDY})
+    set(CLANG_TIDY_BINARY_OPT -clang-tidy-binary ${_LLVM17_BIN}/clang-tidy)
+    message(STATUS "Using Homebrew LLVM 17 run-clang-tidy: ${CLANG_TIDY}")
 else()
-    find_program(_RCT_VERSIONED run-clang-tidy-19)
+    find_program(_RCT_VERSIONED run-clang-tidy-17)
     if(NOT _RCT_VERSIONED STREQUAL _RCT_VERSIONED-NOTFOUND)
         set(CLANG_TIDY ${_RCT_VERSIONED})
-        find_program(_CT_VERSIONED clang-tidy-19)
+        find_program(_CT_VERSIONED clang-tidy-17)
         if(NOT _CT_VERSIONED STREQUAL _CT_VERSIONED-NOTFOUND)
             set(CLANG_TIDY_BINARY_OPT -clang-tidy-binary ${_CT_VERSIONED})
         endif()
@@ -37,7 +37,7 @@ else()
     if(NOT CLANG_TIDY)
         set(CLANG_TIDY ${CMAKE_CURRENT_LIST_DIR}/clang-tools/run-clang-tidy)
         if(NOT EXISTS ${CLANG_TIDY})
-            message(STATUS "Cannot find clang-tidy version 19, either set CLANG_TIDY or make it discoverable")
+            message(STATUS "Cannot find clang-tidy version 17, either set CLANG_TIDY or make it discoverable")
             return()
         endif()
         message(STATUS "Using Docker-based run-clang-tidy wrapper: ${CLANG_TIDY}")

appsec/helper-rust/src/client.rs

@@ -467,6 +467,8 @@ async fn do_request_loop_iter(
                     .run_waf(data, &protocol::RequestExecOptions::regular())
                     .map_err(FatalRequestError)?;
 
+                req_ctx.record_shutdown_context(req.input_truncated);
+
                 // span metrics / meta
                 let mut span_submitter = metrics::CollectingMetricsSubmitter::default();
                 req_ctx.generate_span_metrics(&mut span_submitter);
@@ -486,7 +488,7 @@ async fn do_request_loop_iter(
                     });
                 send_command_resp(framed, resp).await?;
 
-                submit_context_telemetry_metrics(client, &mut req_ctx, req.input_truncated);
+                submit_context_telemetry_metrics(client, &mut req_ctx);
                 submit_service_telemetry(client, service);
 
                 break;
@@ -524,11 +526,7 @@ fn submit_service_telemetry(client: &Client, service: &Service) {
     }
 }
 
-fn submit_context_telemetry_metrics(
-    client: &Client,
-    req_ctx: &mut ReqContext,
-    input_truncated: bool,
-) {
+fn submit_context_telemetry_metrics(client: &Client, req_ctx: &mut ReqContext) {
     let Some(ref sidecar_settings) = client.sidecar_settings else {
         warn!("Cannot submit context telemetry metrics: sidecar_settings not set");
         return;
@@ -542,7 +540,7 @@ fn submit_context_telemetry_metrics(
         &client.metrics_last_registered,
     );
 
-    let waf_metrics = req_ctx.take_waf_metrics(input_truncated);
+    let waf_metrics = req_ctx.take_waf_metrics();
     waf_metrics.generate_telemetry_metrics(&mut *tel_metric_submitter);
 }
 
@@ -631,8 +629,15 @@ impl ReqContext {
             WafRunType::NonRasp => {
                 self.waf_metrics.record_non_rasp_eval(&run_output);
             }
-            WafRunType::RaspRule(rule_type) => {
-                self.waf_metrics.record_rasp_eval(rule_type, &run_output);
+            WafRunType::RaspRule {
+                rule_type,
+                rule_variant,
+            } => {
+                self.waf_metrics.record_rasp_eval(
+                    rule_type,
+                    rule_variant.as_deref().unwrap_or(""),
+                    &run_output,
+                );
             }
         }
 
@@ -696,17 +701,19 @@ impl ReqContext {
     }
 
     fn should_force_keep(&mut self, service: &Service, waf_keep: bool) -> bool {
-        // cache limiter result (called once per request)
-        let limiter_allows = match self.limiter_result {
+        // The limiter slot is only consumed when the WAF triggered; clean requests
+        // must not burn a slot even if this function is called multiple times.
+        if !waf_keep {
+            return false;
+        }
+        match self.limiter_result {
             Some(result) => result,
             None => {
                 let result = service.should_force_keep();
                 self.limiter_result = Some(result);
                 result
             }
-        };
-
-        limiter_allows && waf_keep
+        }
     }
 
     fn settings(&self) -> HashMap<&'static str, String> {
@@ -716,8 +723,14 @@ impl ReqContext {
         )])
     }
 
-    pub fn take_waf_metrics(&mut self, input_truncated: bool) -> metrics::WafMetrics {
+    pub fn record_shutdown_context(&mut self, input_truncated: bool) {
         self.waf_metrics.set_input_truncated(input_truncated);
+    }
+
+    pub fn take_waf_metrics(&mut self) -> metrics::WafMetrics {
+        self.waf_metrics
+            .set_rate_limited(matches!(self.limiter_result, Some(false)));
+
         std::mem::take(&mut self.waf_metrics)
     }
 }

appsec/helper-rust/src/client/metrics.rs

@@ -53,17 +53,25 @@ pub struct WafMetrics {
     /// Count of RASP timeouts
     rasp_timeouts: u32,
 
-    /// Per-rule-type RASP metrics for telemetry
-    rasp_per_rule: HashMap<String, RaspRuleMetrics>,
+    /// Per-(rule_type, rule_variant) RASP metrics for telemetry
+    rasp_per_rule: HashMap<(String, String), RaspRuleMetrics>,
 
     /// Whether the WAF triggered any rules
     had_triggers: bool,
 
     /// Whether the request was blocked
     request_blocked: bool,
 
-    /// Whether the input was truncated by the extension
+    /// Whether the input was truncated by the extension.
+    /// Used as a tag on waf.requests. The separate appsec.waf.input_truncated
+    /// metric was deprecated by RFC-1089, as was appsec.waf.truncated_value_size.
+    /// Neither is implemented.
     input_truncated: bool,
+
+    /// Whether the trace was rate-limited by the appsec event rate limiter
+    /// (i.e. the limiter prevented force-keeping a trace that would otherwise
+    /// have been force-kept).
+    rate_limited: bool,
 }
 
 #[derive(Default, Debug, Clone)]
@@ -87,13 +95,18 @@ impl WafMetrics {
             had_triggers: false,
             request_blocked: false,
             input_truncated: false,
+            rate_limited: false,
         }
     }
 
     pub fn set_input_truncated(&mut self, input_truncated: bool) {
         self.input_truncated = input_truncated;
     }
 
+    pub fn set_rate_limited(&mut self, rate_limited: bool) {
+        self.rate_limited = rate_limited;
+    }
+
     pub fn record_non_rasp_error_eval(&mut self) {
         self.waf_hit_error = true;
     }
@@ -112,15 +125,23 @@ impl WafMetrics {
         }
     }
 
-    pub fn record_rasp_eval(&mut self, rule_type: &str, run_output: &libddwaf::RunOutput) {
+    pub fn record_rasp_eval(
+        &mut self,
+        rule_type: &str,
+        rule_variant: &str,
+        run_output: &libddwaf::RunOutput,
+    ) {
         self.rasp_duration += run_output.duration();
         self.rasp_rule_evals += 1;
 
         if run_output.timeout() {
             self.rasp_timeouts += 1;
         }
 
-        let entry = self.rasp_per_rule.entry(rule_type.to_string()).or_default();
+        let entry = self
+            .rasp_per_rule
+            .entry((rule_type.to_string(), rule_variant.to_string()))
+            .or_default();
         entry.evals += 1;
         if run_output.has_events() {
             entry.matches += 1;
@@ -157,30 +178,67 @@ impl telemetry::TelemetryMetricsGenerator for WafMetrics {
         submitter: &mut dyn telemetry::TelemetryMetricSubmitter,
     ) {
         // waf.requests metrics
+        // RFC-1012: all boolean tags must be emitted regardless of value.
         let mut tags = telemetry::TelemetryTags::new();
         tags.add("waf_version", crate::service::Service::waf_version());
-        if let Some(ref rules_ver) = self.rules_version {
-            tags.add("event_rules_version", rules_ver);
-        }
-        if self.had_triggers {
-            tags.add("rule_triggered", "true");
-        }
-        if self.request_blocked {
-            tags.add("request_blocked", "true");
-        }
-        if self.waf_hit_timeout {
-            tags.add("waf_timeout", "true");
+        tags.add(
+            "event_rules_version",
+            self.rules_version.as_deref().unwrap_or("unknown"),
+        );
+        tags.add("rule_triggered", bool_tag(self.had_triggers));
+        // block_failure is not tracked: the PHP layer is assumed to always succeed at blocking.
+        // Therefore request_blocked == "WAF requested a block" == "block succeeded".
+        // request_excluded is not tracked: libddwaf applies exclusion filters internally and
+        // does not expose whether a request was excluded in RunOutput.
+        tags.add("request_blocked", bool_tag(self.request_blocked));
+        tags.add("waf_error", bool_tag(self.waf_hit_error));
+        tags.add("waf_timeout", bool_tag(self.waf_hit_timeout));
+        tags.add("input_truncated", bool_tag(self.input_truncated));
+        tags.add("rate_limited", bool_tag(self.rate_limited));
+        submitter.submit_metric(telemetry::WAF_REQUESTS, 1.0, tags);
+
+        // waf.duration distribution: one observation per request, value in microseconds
+        if !self.waf_duration.is_zero() {
+            let mut dur_tags = telemetry::TelemetryTags::new();
+            dur_tags.add("waf_version", crate::service::Service::waf_version());
+            dur_tags.add(
+                "event_rules_version",
+                self.rules_version.as_deref().unwrap_or("unknown"),
+            );
+            submitter.submit_metric(
+                telemetry::WAF_DURATION_DIST,
+                self.waf_duration.as_micros() as f64,
+                dur_tags,
+            );
         }
-        if self.input_truncated {
-            tags.add("input_truncated", "true");
+
+        // rasp.duration distribution: cumulative internal libddwaf runtime per request, in microseconds
+        if !self.rasp_duration.is_zero() {
+            let mut dur_tags = telemetry::TelemetryTags::new();
+            dur_tags.add("waf_version", crate::service::Service::waf_version());
+            dur_tags.add(
+                "event_rules_version",
+                self.rules_version.as_deref().unwrap_or("unknown"),
+            );
+            submitter.submit_metric(
+                telemetry::RASP_DURATION_DIST,
+                self.rasp_duration.as_micros() as f64,
+                dur_tags,
+            );
         }
-        submitter.submit_metric(telemetry::WAF_REQUESTS, 1.0, tags);
 
         // Rasp rule metrics
-        for (rule_type, metrics) in &self.rasp_per_rule {
+        for ((rule_type, rule_variant), metrics) in &self.rasp_per_rule {
             let mut tags = telemetry::TelemetryTags::new();
             tags.add("rule_type", rule_type);
+            if !rule_variant.is_empty() {
+                tags.add("rule_variant", rule_variant);
+            }
             tags.add("waf_version", crate::service::Service::waf_version());
+            tags.add(
+                "event_rules_version",
+                self.rules_version.as_deref().unwrap_or("unknown"),
+            );
 
             if metrics.evals > 0 {
                 submitter.submit_metric(
@@ -207,12 +265,15 @@ impl telemetry::TelemetryMetricsGenerator for WafMetrics {
 impl telemetry::SpanMetricsGenerator for WafMetrics {
     fn generate_span_metrics(&'_ self, submitter: &mut dyn telemetry::SpanMetricsSubmitter) {
         if !self.waf_duration.is_zero() {
-            submitter.submit_metric(telemetry::WAF_DURATION, self.waf_duration.duration_ms_f64());
+            submitter.submit_metric(
+                telemetry::WAF_DURATION,
+                self.waf_duration.as_micros() as f64,
+            );
         }
         if !self.rasp_duration.is_zero() {
             submitter.submit_metric(
                 telemetry::RAST_DURATION,
-                self.rasp_duration.duration_ms_f64(),
+                self.rasp_duration.as_micros() as f64,
             );
         }
         if self.rasp_rule_evals > 0 {
@@ -224,6 +285,14 @@ impl telemetry::SpanMetricsGenerator for WafMetrics {
     }
 }
 
+fn bool_tag(value: bool) -> &'static str {
+    if value {
+        "true"
+    } else {
+        "false"
+    }
+}
+
 trait DurationExt {
     fn duration_ms_f64(&self) -> f64;
 }