Validate metric[param] filters against declared label params (ticket 039)
compile_metric_comparison used to look up `param` by reading whatever key happened to be present in a bucket's `labels` map at query time, so a typo or a param never declared as a label on that metric silently matched nothing instead of being rejected. Now it looks up the target metric's MetricDeclaration (ticket 037's unified label_params/value_param shape) and rejects the filter up front with an error naming the metric and its actual declared label params when `param` isn't among them. Metric names with no declaration at all are left unvalidated here, matching prior behavior (bucket_values simply yields nothing for them). Added tests: metric[nonexistent_param] is rejected with an error naming the metric and its real label params; existing metric[real_param] filters still match as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
db0d20643b
commit
375aa38ef8
@@ -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();
|
let mut matching = HashSet::new();
|
||||||
for (labels, (sequence, _, _)) in bucket_values(index, metric, None) {
|
for (labels, (sequence, _, _)) in bucket_values(index, metric, None) {
|
||||||
let Some(primitive) = labels.get(param) else {
|
let Some(primitive) = labels.get(param) else {
|
||||||
@@ -2941,4 +2958,52 @@ mod tests {
|
|||||||
_ => panic!("expected an Error response for a metric comparison without [param]"),
|
_ => 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<level>\w+) duration=(?P<duration>\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<level>\w+) duration=(?P<duration>\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<u64> = entries.iter().map(|e| e.sequence).collect();
|
||||||
|
assert_eq!(sequences, vec![0]);
|
||||||
|
}
|
||||||
|
_ => panic!("expected Logs response"),
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user