diff --git a/sds/src/scanner/metrics.rs b/sds/src/scanner/metrics.rs index 7ac280b8..3056ceaf 100644 --- a/sds/src/scanner/metrics.rs +++ b/sds/src/scanner/metrics.rs @@ -4,6 +4,8 @@ use metrics::{Counter, counter}; #[derive(Clone)] pub struct RuleMetrics { pub false_positive_excluded_attributes: Counter, + pub match_count: Counter, + pub suppressed_match_count: Counter, } impl RuleMetrics { @@ -13,24 +15,28 @@ impl RuleMetrics { "false_positive.multipass.excluded_match", labels.clone() ), + match_count: counter!("scanning.match_count", labels.clone()), + suppressed_match_count: counter!("scanning.suppressed_match_count", labels.clone()), } } } + /* * Scanning metrics * - * duration_ns: Total time from scan start to completion - * num_scanned_events: Number of scanned events - * match_count: Number of matches found - * suppressed_match_count: Number of matches suppressed - * cpu_duration: Time spent in CPU operations + * Per-scanner (ScannerMetrics, scanner-level labels only): + * scanned_events: Number of scan calls + * scanning.cpu_duration: CPU time spent per scan (excludes async I/O wait) + * + * Per-rule (RuleMetrics, combined scanner+rule labels): + * scanning.match_count: Matches reported to the caller (post-suppression), per rule + * scanning.suppressed_match_count: Matches suppressed before reaching the caller, per rule + * false_positive.multipass.excluded_match: Multipass V0 false positives, per rule * * In case of too high cardinality, please refer to https://github.com/DataDog/logs-backend/blob/prod/domains/commons/shared/libs/telemetry/src/main/java/com/dd/metrics/RegistryCacheTags.java */ pub struct ScannerMetrics { pub num_scanned_events: Counter, - pub match_count: Counter, - pub suppressed_match_count: Counter, pub cpu_duration: Counter, } @@ -38,8 +44,6 @@ impl ScannerMetrics { pub fn new(labels: &Labels) -> Self { ScannerMetrics { num_scanned_events: counter!("scanned_events", labels.clone()), - match_count: counter!("scanning.match_count", labels.clone()), - suppressed_match_count: counter!("scanning.suppressed_match_count", labels.clone()), cpu_duration: counter!("scanning.cpu_duration", labels.clone()), } } diff --git a/sds/src/scanner/mod.rs b/sds/src/scanner/mod.rs index c84c0e4f..b8a1cbda 100644 --- a/sds/src/scanner/mod.rs +++ b/sds/src/scanner/mod.rs @@ -336,6 +336,14 @@ pub trait CompiledRule: Send + Sync { // default is to do nothing } + fn on_match(&self) { + // default is to do nothing + } + + fn on_suppressed_match(&self) { + // default is to do nothing + } + fn as_regex_rule(&self) -> Option<&RegexCompiledRule> { None } @@ -516,10 +524,10 @@ impl Scanner { ) { // Add number of scanned events self.metrics.num_scanned_events.increment(1); - // Add number of matches - self.metrics - .match_count - .increment(output_rule_matches.len() as u64); + // Increment per-rule match counters so dashboards can filter by both scanner and rule tags + for rule_match in output_rule_matches { + self.rules[rule_match.rule_index].on_match(); + } if let Some(io_duration) = io_duration { let total_duration = start.elapsed(); @@ -698,7 +706,7 @@ impl Scanner { ); if match_should_be_suppressed { - self.metrics.suppressed_match_count.increment(1); + self.rules[rule_match.rule_index].on_suppressed_match(); } !match_should_be_suppressed } else { diff --git a/sds/src/scanner/regex_rule/compiled.rs b/sds/src/scanner/regex_rule/compiled.rs index a690147a..c4f1946c 100644 --- a/sds/src/scanner/regex_rule/compiled.rs +++ b/sds/src/scanner/regex_rule/compiled.rs @@ -74,6 +74,14 @@ impl CompiledRule for RegexCompiledRule { self.metrics.false_positive_excluded_attributes.increment(1); } + fn on_match(&self) { + self.metrics.match_count.increment(1); + } + + fn on_suppressed_match(&self) { + self.metrics.suppressed_match_count.increment(1); + } + fn as_regex_rule(&self) -> Option<&RegexCompiledRule> { Some(self) } diff --git a/sds/src/scanner/test/metrics.rs b/sds/src/scanner/test/metrics.rs index 11048597..ac08b0d1 100644 --- a/sds/src/scanner/test/metrics.rs +++ b/sds/src/scanner/test/metrics.rs @@ -2,7 +2,7 @@ use crate::match_action::MatchAction; use crate::scanner::regex_rule::config::{ProximityKeywordsConfig, RegexRuleConfig}; use crate::scanner::scope::Scope; use crate::scanner::{RootRuleConfig, ScannerBuilder}; -use crate::{Path, PathSegment, simple_event::SimpleEvent}; +use crate::{Path, PathSegment, Suppressions, simple_event::SimpleEvent}; use metrics::{Key, Label}; use metrics_util::CompositeKey; use metrics_util::MetricKind::Counter; @@ -45,6 +45,8 @@ fn should_submit_scanning_metrics() { .expect("metric not found"); assert_eq!(metric_value, &(None, None, DebugValue::Counter(1))); + // scanning.match_count is now emitted at rule level; with no custom labels on the rule, + // the key remains name-only (no label dimensions). let metric_name = "scanning.match_count"; let metric_value = snapshot .get(&CompositeKey::new(Counter, Key::from_name(metric_name))) @@ -96,6 +98,113 @@ fn should_submit_excluded_match_metric() { assert_eq!(metric_value, &(None, None, DebugValue::Counter(1))); } +#[test] +fn should_submit_suppressed_match_metric() { + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + + metrics::with_local_recorder(&recorder, || { + let rule_0 = RootRuleConfig::new(RegexRuleConfig::new("bcdef").build()) + .suppressions(Suppressions { + exact_match: vec!["bcdef".to_string()], + ..Default::default() + }) + .match_action(MatchAction::None); + + let scanner = ScannerBuilder::new(&[rule_0]).build().unwrap(); + let mut content = SimpleEvent::Map(BTreeMap::from([( + "key1".to_string(), + SimpleEvent::String("bcdef".to_string()), + )])); + + scanner.scan(&mut content).unwrap(); + }); + + let snapshot = snapshotter.snapshot().into_hashmap(); + + let metric_value = snapshot + .get(&CompositeKey::new( + Counter, + Key::from_name("scanning.suppressed_match_count"), + )) + .expect("suppressed_match_count metric not found"); + assert_eq!(metric_value, &(None, None, DebugValue::Counter(1))); +} + +#[test] +fn match_count_carries_combined_scanner_and_rule_labels() { + use crate::Labels; + + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + + metrics::with_local_recorder(&recorder, || { + let rule = RootRuleConfig::new( + RegexRuleConfig::new("secret") + .with_labels(Labels::new(&[("rule_id", "r1")])) + .build(), + ) + .match_action(MatchAction::None); + + let scanner = ScannerBuilder::new(&[rule]) + .labels(Labels::new(&[("scanner_id", "s1")])) + .build() + .unwrap(); + let mut event = SimpleEvent::String("secret".to_string()); + scanner.scan(&mut event).unwrap(); + }); + + let snapshot = snapshotter.snapshot().into_hashmap(); + + // Both scanner and rule labels must appear on the metric key. + let labels = vec![Label::new("scanner_id", "s1"), Label::new("rule_id", "r1")]; + let key = CompositeKey::new(Counter, Key::from_parts("scanning.match_count", labels)); + assert_eq!( + snapshot.get(&key).expect("metric not found").2, + DebugValue::Counter(1) + ); +} + +#[test] +fn suppressed_match_count_carries_combined_scanner_and_rule_labels() { + use crate::Labels; + + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + + metrics::with_local_recorder(&recorder, || { + let rule = RootRuleConfig::new( + RegexRuleConfig::new("secret") + .with_labels(Labels::new(&[("rule_id", "r1")])) + .build(), + ) + .suppressions(Suppressions { + exact_match: vec!["secret".to_string()], + ..Default::default() + }) + .match_action(MatchAction::None); + + let scanner = ScannerBuilder::new(&[rule]) + .labels(Labels::new(&[("scanner_id", "s1")])) + .build() + .unwrap(); + let mut event = SimpleEvent::String("secret".to_string()); + scanner.scan(&mut event).unwrap(); + }); + + let snapshot = snapshotter.snapshot().into_hashmap(); + + let labels = vec![Label::new("scanner_id", "s1"), Label::new("rule_id", "r1")]; + let key = CompositeKey::new( + Counter, + Key::from_parts("scanning.suppressed_match_count", labels), + ); + assert_eq!( + snapshot.get(&key).expect("metric not found").2, + DebugValue::Counter(1) + ); +} + #[test] fn should_submit_excluded_keywords_metric() { let recorder = DebuggingRecorder::new();