Closed rogpeppe closed 6 months ago
I figure I'd just add on to this for more syntax/documentation-related issues.
Background So we had just started using prometheus and I added a few alerts, one of the using multiple conditoins with the AND keyword. Then we started having magical alert issues and I wasn't sure why. After analyzing graphs for days, I realized that queries were mixed with the labels (but I had wrongfully assumed that Prometheus alerts would isolate the labels) So then I learned about the "on" keyword (if you google for this, you will find at least a blog link which led me back to check the prometheus page.)
Suggestion
Related page: https://prometheus.io/docs/prometheus/latest/querying/operators/
Would it make sense to put the keywords ignoring
and on
under the "Logical/set binary operators" along with examples on how to use those in the section? Because I figure that's where it's important.
Yip, looks like we forgot to document many-to-many matching.
I agree that a proper syntax definition is missing. In my case, reading the doc, I see no reason why
(a + b)[5m]
fails with a syntax error. Reading the unit tests code, this case is explicitly a fail but there is no way I could figure it out from the doc. As a side note, I don't understand how to do such a thing as building a range vector from an instant vector resulting from a binary op.
edit: this side note part of my comment seems to be the subject of #1227
In my case, reading the doc, I see no reason why (a + b)[5m] fails with a syntax error.
This is documented, "Syntactically, a range duration is appended in square brackets ([]) at the end of a vector selector ". (a + b)
is not a vector selector.
@brian-brazil you are right, but for a promql newbie (like me), this is far from obvious. Naively I was expecting selected instant vector and "built" ones (typically resulting from the application of a binary op) to be the same class of citizen.
They are, however a vector selector is not the same as a vector.
Yes, I do now understand that the range vector is built from an instant vector selector and and not an instant vector. And if I don't really understand why it is that way rather than the other (range is built from an instant vector), reading at #1227 it looks this is more difficult than it may see. I get it. May be a more explicit warning in the documentation would help. Anyway thanks for your quick responses.
Two more syntax corners I discovered experimentally, but can't find in documentation:
In aggregation operators, it's possible to place the without
|by
clause before the call: sum by (foo)(some_metric)
rather than usual sum(some_metric) by (foo)
.
Documentation only mentions the form:
<aggr-op>([parameter,] <vector expression>) [without|by (<label list>)]
In a label list, it's possible to have no labels – some_metric{}
, and possible to have a trailing comma — some_metric{foo="bar",}
.
All these are convenient for building up queries automatically in code :+1:
They allow simple code like to fmt.Sprintf(
%s(%s{%s}), function, metric, labels)
to accomodate aggregation operators by considering the by/without clause as part of the function name: buildPromQL("sum by (foo)", "some_metric", "")
, to add labels selector unconditionally even if empty {}
, and allows callers to concat selectors as buildPromQL(..., "some_metric", `foo="bar",` + extra_labels)
where it's OK for extra_labels
to be empty.
(This particular interface is not best, just an example how these allow various sloppy query building to still work...)
But they're hard to discover, and I would prefer to use only documented syntax...
P.S. personally I also find (1) order cleaner than putting the by/without after the call and started using it in my own queries :wink:. But I suppose I shouldn't spread it, if only because uniform style is good for a community.
I suppose I shouldn't spread it, if only because uniform style is good for a community.
Odd that we didn't document that, it's preferred as it's hard to understand complex expressions otherwise.
Hey, I'm interested in working on this issue :)
Please go ahead. I'd suggest focus mainly on things that are completely missing from the docs, as these are reference docs.
There also exists an EBNF grammar for PromQL now, which could help: https://github.com/prometheus/prometheus/blob/2aacd807b3ec6ddd90ae55f3a42f4cffed561ea9/promql/generated_parser.y
Would be great if the EBNF bits in the parser logic (linked by slrtbtfs above) could be extracted to a EBNF document to make it easier to read and comprehend. As-is the y-file is a mixture of bnf-like elements and code, intertwined. With enough mental effort and time, someone may eventually deduce the syntax rules from the parser file, but if users could be spared that effort it would improve the usability and cost efficiency of using the query language.
Personally whenever there is a new DSL that I try to figure out, and I run into a parsing / syntax error, the first thing I look for is a formal definition of the language. When such is available it immediately tells me what I'm missing, or what other language elements I need to leverage in order to make the expression I'm seeking to formulate compatible with the DSL's parser rules.
Hello from the bug scrub.
This issue touches many different aspect. There is the overarching and long-lasting issue of creating better documentation, but that's a project over in https://github.com/prometheus/docs
I guess a lot of the hassle this touches has been alleviated by the syntax completion and tool tips in the UI.
Several incremental doc improvements have also happened.
In short, this issue isn't really helpful in its current form. We'll close it. Feel free to open issues (or create improvements) for more concrete aspects.
I recently tried to form a query that failed with a parse error, but I'm not sure why it did. As the query language documentation is mostly by example, I have no clear idea what the syntax of the query language actually is.
My specific example was that it's not clear what syntactic role an instance vector selector takes and hence how it can be applied. I had a vector expression
rate(jem_monitor_errors_count[1m])
that I wanted only one component of, so I thought thatrate(jem_monitor_errors_count[1m]){instance="instance-name"}
would work, but I getparse error at char 37: could not parse remaining input \"{instance=\\\"in\"..."
, which I find odd becausejem_monitor_errors_count{instance="instance-name"}
works fine.It would be great to have the PromQL grammar available as a reference doc (ideally in some kind of BNF notation), so people like me don't come bugging the prometheus issues with probably-stupid questions like this.
PS I realised that I could fix the issue by applying the instance vector selector to the inner expression, but I think my point remains - the syntax is not clearly documented so we have to guess.