mirror of
https://github.com/prometheus/prometheus.git
synced 2025-12-06 10:01:02 +01:00
Fix serialization for empty ignoring() in combination with group_x()
Currently both the backend and frontend printers/formatters/serializers incorrectly transform the following expression: ``` up * ignoring() group_left(__name__) node_boot_time_seconds ``` ...into: ``` up * node_boot_time_seconds ``` ...which yields a different result (including the metric name in the result vs. no metric name). We need to keep empty `ignoring()` modifiers if there is a grouping modifier present. Signed-off-by: Julius Volz <julius.volz@gmail.com>
This commit is contained in:
parent
f6ca7145ca
commit
39e11f50b2
@ -147,12 +147,14 @@ func (node *BinaryExpr) ShortString() string {
|
||||
func (node *BinaryExpr) getMatchingStr() string {
|
||||
matching := ""
|
||||
vm := node.VectorMatching
|
||||
if vm != nil && (len(vm.MatchingLabels) > 0 || vm.On) {
|
||||
vmTag := "ignoring"
|
||||
if vm.On {
|
||||
vmTag = "on"
|
||||
if vm != nil {
|
||||
if len(vm.MatchingLabels) > 0 || vm.On || vm.Card == CardManyToOne || vm.Card == CardOneToMany {
|
||||
vmTag := "ignoring"
|
||||
if vm.On {
|
||||
vmTag = "on"
|
||||
}
|
||||
matching = fmt.Sprintf(" %s (%s)", vmTag, strings.Join(vm.MatchingLabels, ", "))
|
||||
}
|
||||
matching = fmt.Sprintf(" %s (%s)", vmTag, strings.Join(vm.MatchingLabels, ", "))
|
||||
|
||||
if vm.Card == CardManyToOne || vm.Card == CardOneToMany {
|
||||
vmCard := "right"
|
||||
|
||||
@ -94,6 +94,25 @@ func TestExprString(t *testing.T) {
|
||||
in: `a - ignoring() c`,
|
||||
out: `a - c`,
|
||||
},
|
||||
{
|
||||
// This is a bit of an odd case, but valid. If the user specifies ignoring() with
|
||||
// no labels, it means that both label sets have to be exactly the same on both
|
||||
// sides (except for the metric name). This is the same behavior as specifying
|
||||
// no matching modifier at all, but if the user wants to include the metric name
|
||||
// from either side in the output via group_x(__name__), they have to specify
|
||||
// ignoring() explicitly to be able to do so, since the grammar does not allow
|
||||
// grouping modifiers without either ignoring(...) or on(...). So we need to
|
||||
// preserve the empty ignoring() clause in this case.
|
||||
//
|
||||
// a - group_left(__name__) c <--- Parse error
|
||||
// a - ignoring() group_left(__name__) c <--- Valid
|
||||
in: `a - ignoring() group_left(__metric__) c`,
|
||||
out: `a - ignoring () group_left (__metric__) c`,
|
||||
},
|
||||
{
|
||||
in: `a - ignoring() group_left c`,
|
||||
out: `a - ignoring () group_left () c`,
|
||||
},
|
||||
{
|
||||
in: `up > bool 0`,
|
||||
},
|
||||
|
||||
@ -266,22 +266,19 @@ const formatNodeInternal = (
|
||||
let matching = <></>;
|
||||
let grouping = <></>;
|
||||
const vm = node.matching;
|
||||
if (vm !== null && (vm.labels.length > 0 || vm.on)) {
|
||||
if (vm.on) {
|
||||
if (vm !== null) {
|
||||
if (
|
||||
vm.labels.length > 0 ||
|
||||
vm.on ||
|
||||
vm.card === vectorMatchCardinality.manyToOne ||
|
||||
vm.card === vectorMatchCardinality.oneToMany
|
||||
) {
|
||||
matching = (
|
||||
<>
|
||||
{" "}
|
||||
<span className="promql-keyword">on</span>
|
||||
<span className="promql-paren">(</span>
|
||||
{labelNameList(vm.labels)}
|
||||
<span className="promql-paren">)</span>
|
||||
</>
|
||||
);
|
||||
} else {
|
||||
matching = (
|
||||
<>
|
||||
{" "}
|
||||
<span className="promql-keyword">ignoring</span>
|
||||
<span className="promql-keyword">
|
||||
{vm.on ? "on" : "ignoring"}
|
||||
</span>
|
||||
<span className="promql-paren">(</span>
|
||||
{labelNameList(vm.labels)}
|
||||
<span className="promql-paren">)</span>
|
||||
|
||||
@ -136,11 +136,14 @@ const serializeNode = (
|
||||
let matching = "";
|
||||
let grouping = "";
|
||||
const vm = node.matching;
|
||||
if (vm !== null && (vm.labels.length > 0 || vm.on)) {
|
||||
if (vm.on) {
|
||||
matching = ` on(${labelNameList(vm.labels)})`;
|
||||
} else {
|
||||
matching = ` ignoring(${labelNameList(vm.labels)})`;
|
||||
if (vm !== null) {
|
||||
if (
|
||||
vm.labels.length > 0 ||
|
||||
vm.on ||
|
||||
vm.card === vectorMatchCardinality.manyToOne ||
|
||||
vm.card === vectorMatchCardinality.oneToMany
|
||||
) {
|
||||
matching = ` ${vm.on ? "on" : "ignoring"}(${labelNameList(vm.labels)})`;
|
||||
}
|
||||
|
||||
if (
|
||||
|
||||
@ -192,8 +192,7 @@ describe("serializeNode and formatNode", () => {
|
||||
anchored: false,
|
||||
smoothed: false,
|
||||
},
|
||||
output:
|
||||
'{label1="value1"}',
|
||||
output: '{label1="value1"}',
|
||||
},
|
||||
|
||||
// Anchored and smoothed modifiers.
|
||||
@ -722,6 +721,46 @@ describe("serializeNode and formatNode", () => {
|
||||
output: "… + ignoring(label1, label2) …",
|
||||
prettyOutput: ` …
|
||||
+ ignoring(label1, label2)
|
||||
…`,
|
||||
},
|
||||
{
|
||||
// Empty ignoring() without group modifiers can be stripped away.
|
||||
node: {
|
||||
type: nodeType.binaryExpr,
|
||||
op: binaryOperatorType.add,
|
||||
lhs: { type: nodeType.placeholder, children: [] },
|
||||
rhs: { type: nodeType.placeholder, children: [] },
|
||||
matching: {
|
||||
card: vectorMatchCardinality.oneToOne,
|
||||
labels: [],
|
||||
on: false,
|
||||
include: [],
|
||||
},
|
||||
bool: false,
|
||||
},
|
||||
output: "… + …",
|
||||
prettyOutput: ` …
|
||||
+
|
||||
…`,
|
||||
},
|
||||
{
|
||||
// Empty ignoring() with group modifiers may not be stripped away.
|
||||
node: {
|
||||
type: nodeType.binaryExpr,
|
||||
op: binaryOperatorType.add,
|
||||
lhs: { type: nodeType.placeholder, children: [] },
|
||||
rhs: { type: nodeType.placeholder, children: [] },
|
||||
matching: {
|
||||
card: vectorMatchCardinality.manyToOne,
|
||||
labels: [],
|
||||
on: false,
|
||||
include: ["__name__"],
|
||||
},
|
||||
bool: false,
|
||||
},
|
||||
output: "… + ignoring() group_left(__name__) …",
|
||||
prettyOutput: ` …
|
||||
+ ignoring() group_left(__name__)
|
||||
…`,
|
||||
},
|
||||
{
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user