diff --git a/src/engine.rs b/src/engine.rs index 802d909..f30eac6 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -746,6 +746,23 @@ fn compile_metric_comparison( ) })?; + // Validate `param` against the metric's actual declared `label_params` (ticket 037's unified + // `MetricDeclaration`) rather than just reading whatever key happens to be present in a + // bucket's `labels` map — a typo or a param that was never declared as a label on this metric + // would otherwise silently match nothing instead of being rejected up front. A metric name + // with no declaration at all is left alone here (`bucket_values` below simply yields nothing + // for it, matching prior behavior) since there's no label_params list to validate against. + if let Some(DataValue::MetricDeclaration { label_params, .. }) = + index.get(&IndexKey::MetricDeclaration { name: metric.to_string() }) + { + if !label_params.iter().any(|p| p == param) { + return Err(format!( + "param '{param}' is not a label param of metric '{metric}'; declared label params: [{}]", + label_params.join(", ") + )); + } + } + let mut matching = HashSet::new(); for (labels, (sequence, _, _)) in bucket_values(index, metric, None) { let Some(primitive) = labels.get(param) else { @@ -2941,4 +2958,52 @@ mod tests { _ => panic!("expected an Error response for a metric comparison without [param]"), } } + + #[test] + fn metric_comparison_with_undeclared_param_is_rejected_naming_real_label_params() { + let mut index = BTreeMap::new(); + let pattern = r"level=(?P\w+) duration=(?P\d+)"; + declare_test_param(&mut index, "level", pattern, "level", MetricPrimitiveKind::String); + declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number); + declare_metric(&mut index, metric_request("requests", pattern, vec!["level", "duration"])).unwrap(); + ingest_log(&mut index, 0, "level=error duration=5"); + + match handle_query(&index, logs_query(Some("requests[nonexistent_param]==\"x\"")), 0) { + EngineResponse::Error(message) => { + assert!( + message.contains("nonexistent_param"), + "error should name the offending param, got: {message}" + ); + assert!( + message.contains("requests"), + "error should name the metric, got: {message}" + ); + assert!( + message.contains("level") && message.contains("duration"), + "error should list the metric's real declared label params, got: {message}" + ); + } + _ => panic!("expected an Error response for an undeclared param"), + } + } + + #[test] + fn metric_comparison_with_valid_label_param_still_matches() { + let mut index = BTreeMap::new(); + let pattern = r"level=(?P\w+) duration=(?P\d+)"; + declare_test_param(&mut index, "level", pattern, "level", MetricPrimitiveKind::String); + declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number); + declare_metric(&mut index, metric_request("requests", pattern, vec!["level", "duration"])).unwrap(); + + ingest_log(&mut index, 0, "level=error duration=5"); + ingest_log(&mut index, 1, "level=info duration=2"); + + match handle_query(&index, logs_query(Some("requests[level]==error")), 0) { + EngineResponse::Logs(entries) => { + let sequences: Vec = entries.iter().map(|e| e.sequence).collect(); + assert_eq!(sequences, vec![0]); + } + _ => panic!("expected Logs response"), + } + } }