rust/greptimedb
1
0
Fork 0
mirror of https://github.com/GreptimeTeam/greptimedb.git synced 2026-10-12 15:02:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(promql): keep metric name for comparison operators (#9332)

Browse Source 
* fix(promql): keep metric name for comparison operators

`collect_metric_names` dropped the response `__name__` for every binary
operator except `and` / `or` / `unless`, so comparisons such as
`node_load1 > 5` came back without the metric name. Grafana rendered them
as `{instance="..."}` and alert templates reading `$labels.__name__` got
nothing.

Follow Prometheus' `shouldDropMetricName` / `resultMetric`: arithmetic
operators drop the name, comparisons keep the left-hand name, except when
the comparison returns a bool or is a one-to-one `on(...)` match. A
`group_left` / `group_right` modifier does not reduce the labels, so the
name is kept there.

Both the HTTP PromQL API and the gRPC gateway share
`retrieve_metric_name_and_result_type`, so one fix covers both.

Signed-off-by: Aumnertic <hznuzc@gmail.com>

* docs: document binary_keeps_metric_name

Signed-off-by: Aumnertic <hznuzc@gmail.com>

* fix(promql): handle on(__name__), scalar lhs and group_right

Signed-off-by: Aumnertic <hznuzc@gmail.com>

* chore(promql): correct result-side docs and drop unreachable String arm

Signed-off-by: Aumnertic <hznuzc@gmail.com>

* fix(promql): drop metric name when explicitly ignored

Handle ignoring(__name__) for one-to-one comparisons and cover
comparison, group_left, group_right and unless behavior.

Signed-off-by: Aumnertic <hznuzc@gmail.com>

---------

Signed-off-by: Aumnertic <hznuzc@gmail.com>
This commit is contained in:
Aum
2026-10-09 10:41:40 +00:00
committed by GitHub
parent d1151b0ed6
commit c33a6cfa56
1 changed files with 207 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/servers/src/http/prometheus.rs
+207 -11
View File
@@ -49,11 +49,11 @@ use futures::future::join_all;
use futures::{StreamExt, TryStreamExt};
use itertools::Itertools;
use promql_parser::label::{METRIC_NAME, MatchOp, Matcher, Matchers};
use promql_parser::parser::token::{self};
use promql_parser::parser::token::TokenType;
use promql_parser::parser::value::ValueType;
use promql_parser::parser::{
AggregateExpr, BinaryExpr, Call, Expr as PromqlExpr, LabelModifier, MatrixSelector, ParenExpr,
SubqueryExpr, UnaryExpr, VectorSelector,
AggregateExpr, BinModifier, BinaryExpr, Call, Expr as PromqlExpr, LabelModifier,
MatrixSelector, ParenExpr, SubqueryExpr, UnaryExpr, VectorMatchCardinality, VectorSelector,
};
use query::parser::{DEFAULT_LOOKBACK_STRING, PromQuery, QueryLanguageParser, QueryStatement};
use serde::de::{self, MapAccess, Visitor};
@@ -1373,6 +1373,72 @@ fn promql_expr_to_metric_name(expr: &PromqlExpr) -> Option<String> {
}
}
/// Whether the metric name survives a binary operation; see
/// [`binary_metric_name_side`] for the operand that supplies it.
///
/// Follows Prometheus' `shouldDropMetricName` and `resultMetric`: set operators
/// and comparisons keep the name, while arithmetic operators drop it. A
/// comparison also drops it when it returns a bool or is a one-to-one
/// `on(...)` match that does not keep `__name__` or an `ignoring(...)` match
/// that excludes it. Prometheus keeps only the `on(...)` labels or deletes the
/// `ignoring(...)` labels for one-to-one matches.
fn binary_keeps_metric_name(op: &TokenType, modifier: Option<&BinModifier>) -> bool {
if op.is_set_operator() {
return true;
}
if !op.is_comparison_operator() {
return false;
}
let Some(m) = modifier else {
return true;
};
if m.return_bool {
return false;
}
if m.card == VectorMatchCardinality::OneToOne {
match &m.matching {
Some(LabelModifier::Include(labels))
if !labels.labels.contains(&METRIC_NAME.to_string()) =>
{
return false;
}
Some(LabelModifier::Exclude(labels))
if labels.labels.contains(&METRIC_NAME.to_string()) =>
{
return false;
}
_ => {}
}
}
true
}
/// The operand whose labels supply the result metric name of a binary
/// operation.
///
/// Prometheus builds the result from the side whose labels it carries, i.e. the
/// "many" side of the match: `group_right` makes the right-hand operand that
/// side, and a scalar left-hand operand (for example `0.5 < a`) leaves the
/// vector on the right. Every other case takes the left-hand operand.
fn binary_metric_name_side<'a>(
lhs: &'a PromqlExpr,
rhs: &'a PromqlExpr,
modifier: Option<&BinModifier>,
) -> &'a PromqlExpr {
let group_right =
modifier.is_some_and(|m| matches!(m.card, VectorMatchCardinality::OneToMany(_)));
if group_right || matches!(lhs.value_type(), ValueType::Scalar) {
rhs
} else {
lhs
}
}
/// Recursively collect all metric names from a PromQL expression
fn collect_metric_names(expr: &PromqlExpr, metric_names: &mut HashSet<String>) {
match expr {
@@ -1395,14 +1461,16 @@ fn collect_metric_names(expr: &PromqlExpr, metric_names: &mut HashSet<String>) {
collect_metric_names(expr, metric_names)
}
PromqlExpr::Unary(UnaryExpr { .. }) => metric_names.clear(),
PromqlExpr::Binary(BinaryExpr { lhs, op, .. }) => {
if matches!(
op.id(),
token::T_LAND // INTERSECT
| token::T_LOR // UNION
| token::T_LUNLESS // EXCEPT
) {
collect_metric_names(lhs, metric_names)
PromqlExpr::Binary(BinaryExpr {
lhs,
op,
rhs,
modifier,
..
}) => {
if binary_keeps_metric_name(op, modifier.as_ref()) {
let side = binary_metric_name_side(lhs, rhs, modifier.as_ref());
collect_metric_names(side, metric_names)
} else {
metric_names.clear()
}
@@ -3211,6 +3279,134 @@ mod tests {
expected_type: ValueType::Vector,
should_error: false,
},
// Arithmetic operators always drop the metric name.
TestCase {
name: "arithmetic between metrics",
promql: "a + b",
expected_metric: None,
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "arithmetic with scalar",
promql: "a * 2",
expected_metric: None,
expected_type: ValueType::Vector,
should_error: false,
},
// Comparisons keep the metric name from the side whose labels the
// result carries, i.e. the "many" side of the match: normally the
// left operand, but `group_right` takes it from the right operand and
// a scalar left operand leaves the vector on the right. The name is
// dropped when the comparison returns a bool or is a one-to-one
// `on(...)` match that does not keep `__name__` or an `ignoring(...)`
// match that excludes it. A `group_left`/`group_right` modifier does
// not reduce the labels, so the name is kept.
TestCase {
name: "bool comparison between metrics",
promql: "a > bool b",
expected_metric: None,
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "bool comparison with scalar",
promql: "a > bool 0.5",
expected_metric: None,
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "comparison with scalar",
promql: "a > 0.5",
expected_metric: Some("a"),
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "comparison with scalar on the left",
promql: "0.5 < a",
expected_metric: Some("a"),
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "equality comparison with scalar",
promql: "up == 0",
expected_metric: Some("up"),
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "comparison between metrics",
promql: "a > b",
expected_metric: Some("a"),
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "comparison with ignoring",
promql: "a > ignoring(x) b",
expected_metric: Some("a"),
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "comparison with ignoring __name__",
promql: "a > ignoring(__name__) b",
expected_metric: None,
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "comparison with ignoring __name__ and group_left",
promql: "a > ignoring(__name__) group_left b",
expected_metric: Some("a"),
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "comparison with ignoring __name__ and group_right",
promql: "a > ignoring(__name__) group_right b",
expected_metric: Some("b"),
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "unless with ignoring __name__",
promql: "a unless ignoring(__name__) b",
expected_metric: Some("a"),
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "comparison with on",
promql: "a > on(x) b",
expected_metric: None,
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "comparison with on keeping __name__",
promql: "a > on(__name__) b",
expected_metric: Some("a"),
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "comparison with on and group_left",
promql: "a > on(x) group_left b",
expected_metric: Some("a"),
expected_type: ValueType::Vector,
should_error: false,
},
TestCase {
name: "comparison with on and group_right",
promql: "a > on(x) group_right b",
expected_metric: Some("b"),
expected_type: ValueType::Vector,
should_error: false,
},
// Subquery cases
TestCase {
name: "basic subquery",