From c716d8a47b35786077d508ba6abb741458671963 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Fri, 12 Jun 2015 09:42:36 +0200 Subject: [PATCH 1/2] promql: fix aggregation expression String() method. Fixes #794. --- promql/printer.go | 6 +++++- promql/printer_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 promql/printer_test.go diff --git a/promql/printer.go b/promql/printer.go index 4a3a8fded..6ff55dfd8 100644 --- a/promql/printer.go +++ b/promql/printer.go @@ -178,7 +178,11 @@ func (es Expressions) String() (s string) { func (node *AggregateExpr) String() string { aggrString := fmt.Sprintf("%s(%s)", node.Op, node.Expr) if len(node.Grouping) > 0 { - return fmt.Sprintf("%s BY (%s)", aggrString, node.Grouping) + format := "%s BY (%s)" + if node.KeepExtraLabels { + format += " KEEPING_EXTRA" + } + return fmt.Sprintf(format, aggrString, node.Grouping) } return aggrString } diff --git a/promql/printer_test.go b/promql/printer_test.go new file mode 100644 index 000000000..87c4330ab --- /dev/null +++ b/promql/printer_test.go @@ -0,0 +1,48 @@ +// Copyright 2015 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package promql + +import ( + "testing" +) + +func TestExprString(t *testing.T) { + // A list of valid expressions that are expected to be + // returned as out when calling String(). If out is empty the output + // is expected to equal the input. + inputs := []struct { + in, out string + }{ + { + in: `sum(task:errors:rate10s{job="s"}) BY (code)`, + }, + { + in: `sum(task:errors:rate10s{job="s"}) BY (code) KEEPING_EXTRA`, + }, + } + + for _, test := range inputs { + expr, err := ParseExpr(test.in) + if err != nil { + t.Fatalf("parsing error for %q: %s", test.in, err) + } + exp := test.in + if test.out != "" { + exp = test.out + } + if expr.String() != exp { + t.Fatalf("expected %q to be returned as:\n%s\ngot:\n%s\n", test.in, exp, expr.String()) + } + } +} From e7659f908cb7a06a0e0df1e0bcea5f25c74636c1 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Fri, 12 Jun 2015 09:44:21 +0200 Subject: [PATCH 2/2] promql: remove DotGraph methods from nodes. --- promql/ast.go | 2 - promql/printer.go | 156 ---------------------------------------------- 2 files changed, 158 deletions(-) diff --git a/promql/ast.go b/promql/ast.go index fc7c26a75..f20ee7358 100644 --- a/promql/ast.go +++ b/promql/ast.go @@ -40,8 +40,6 @@ type Node interface { // String representation of the node that returns the given node when parsed // as part of a valid query. String() string - // DotGraph returns a dot graph representation of the node. - DotGraph() string } // Statement is a generic interface for all statements. diff --git a/promql/printer.go b/promql/printer.go index 6ff55dfd8..1543cfe59 100644 --- a/promql/printer.go +++ b/promql/printer.go @@ -15,7 +15,6 @@ package promql import ( "fmt" - "reflect" "sort" "strings" @@ -246,158 +245,3 @@ func (node *VectorSelector) String() string { sort.Strings(labelStrings) return fmt.Sprintf("%s{%s}", node.Name, strings.Join(labelStrings, ",")) } - -// DotGraph returns a DOT representation of a statement list. -func (ss Statements) DotGraph() string { - graph := "" - for _, stmt := range ss { - graph += stmt.DotGraph() - } - return graph -} - -// DotGraph returns a DOT representation of the alert statement. -func (node *AlertStmt) DotGraph() string { - graph := fmt.Sprintf( - `digraph "Alert Statement" { - %#p[shape="box",label="ALERT %s IF FOR %s"]; - %#p -> %x; - %s - }`, - node, node.Name, strutil.DurationToString(node.Duration), - node, reflect.ValueOf(node.Expr).Pointer(), - node.Expr.DotGraph(), - ) - return graph -} - -// DotGraph returns a DOT representation of the eval statement. -func (node *EvalStmt) DotGraph() string { - graph := fmt.Sprintf( - `%#p[shape="box",label="[%d:%s:%d]"; - %#p -> %x; - %s - }`, - node, node.Start, node.End, node.Interval, - node, reflect.ValueOf(node.Expr).Pointer(), - node.Expr.DotGraph(), - ) - return graph -} - -// DotGraph returns a DOT representation of the record statement. -func (node *RecordStmt) DotGraph() string { - graph := fmt.Sprintf( - `%#p[shape="box",label="%s = "]; - %#p -> %x; - %s - }`, - node, node.Name, - node, reflect.ValueOf(node.Expr).Pointer(), - node.Expr.DotGraph(), - ) - return graph -} - -// DotGraph returns a DOT representation of // DotGraph returns a DOT representation of the record statement. -// DotGraph returns a DOT representation of a statement list. -func (es Expressions) DotGraph() string { - graph := "" - for _, expr := range es { - graph += expr.DotGraph() - } - return graph -} - -// DotGraph returns a DOT representation of the vector aggregation. -func (node *AggregateExpr) DotGraph() string { - groupByStrings := make([]string, 0, len(node.Grouping)) - for _, label := range node.Grouping { - groupByStrings = append(groupByStrings, string(label)) - } - - graph := fmt.Sprintf("%#p[label=\"%s BY (%s)\"]\n", - node, - node.Op, - strings.Join(groupByStrings, ", ")) - graph += fmt.Sprintf("%#p -> %x;\n", node, reflect.ValueOf(node.Expr).Pointer()) - graph += node.Expr.DotGraph() - return graph -} - -// DotGraph returns a DOT representation of the expression. -func (node *BinaryExpr) DotGraph() string { - nodeAddr := reflect.ValueOf(node).Pointer() - graph := fmt.Sprintf( - ` - %x[label="%s"]; - %x -> %x; - %x -> %x; - %s - %s - }`, - nodeAddr, node.Op, - nodeAddr, reflect.ValueOf(node.LHS).Pointer(), - nodeAddr, reflect.ValueOf(node.RHS).Pointer(), - node.LHS.DotGraph(), - node.RHS.DotGraph(), - ) - return graph -} - -// DotGraph returns a DOT representation of the function call. -func (node *Call) DotGraph() string { - graph := fmt.Sprintf("%#p[label=\"%s\"];\n", node, node.Func.Name) - graph += functionArgsToDotGraph(node, node.Args) - return graph -} - -// DotGraph returns a DOT representation of the number literal. -func (node *NumberLiteral) DotGraph() string { - return fmt.Sprintf("%#p[label=\"%v\"];\n", node, node.Val) -} - -// DotGraph returns a DOT representation of the encapsulated expression. -func (node *ParenExpr) DotGraph() string { - return node.Expr.DotGraph() -} - -// DotGraph returns a DOT representation of the matrix selector. -func (node *MatrixSelector) DotGraph() string { - return fmt.Sprintf("%#p[label=\"%s\"];\n", node, node) -} - -// DotGraph returns a DOT representation of the string literal. -func (node *StringLiteral) DotGraph() string { - return fmt.Sprintf("%#p[label=\"'%q'\"];\n", node, node.Val) -} - -// DotGraph returns a DOT representation of the unary expression. -func (node *UnaryExpr) DotGraph() string { - nodeAddr := reflect.ValueOf(node).Pointer() - graph := fmt.Sprintf( - ` - %x[label="%s"]; - %x -> %x; - %s - %s - }`, - nodeAddr, node.Op, - nodeAddr, reflect.ValueOf(node.Expr).Pointer(), - node.Expr.DotGraph(), - ) - return graph -} - -// DotGraph returns a DOT representation of the vector selector. -func (node *VectorSelector) DotGraph() string { - return fmt.Sprintf("%#p[label=\"%s\"];\n", node, node) -} - -func functionArgsToDotGraph(node Node, args Expressions) string { - graph := args.DotGraph() - for _, arg := range args { - graph += fmt.Sprintf("%x -> %x;\n", reflect.ValueOf(node).Pointer(), reflect.ValueOf(arg).Pointer()) - } - return graph -}