Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

better PromQL syntax documentation #3697

Closed
rogpeppe opened this issue Jan 17, 2018 · 14 comments
Closed

better PromQL syntax documentation #3697

rogpeppe opened this issue Jan 17, 2018 · 14 comments

Comments

@rogpeppe
Copy link

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 that rate(jem_monitor_errors_count[1m]){instance="instance-name"} would work, but I get parse error at char 37: could not parse remaining input \"{instance=\\\"in\"...", which I find odd because jem_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.

@deeTEEcee
Copy link

deeTEEcee commented Aug 30, 2018

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.

@brian-brazil
Copy link
Contributor

Yip, looks like we forgot to document many-to-many matching.

@douardda
Copy link

douardda commented Dec 11, 2018

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

@brian-brazil
Copy link
Contributor

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.

@douardda
Copy link

@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.

@brian-brazil
Copy link
Contributor

They are, however a vector selector is not the same as a vector.

@douardda
Copy link

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.

douardda added a commit to douardda/prometheus that referenced this issue Dec 13, 2018
…s to

a very partial response to prometheus#3697

Signed-off-by: David Douard <david.douard@sdfa3.org>
@cben
Copy link

cben commented Jul 1, 2019

Two more syntax corners I discovered experimentally, but can't find in documentation:

  1. 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>)]
    
  2. 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 👍
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 😉. But I suppose I shouldn't spread it, if only because uniform style is good for a community.

@brian-brazil
Copy link
Contributor

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.

@pavansai1
Copy link

Hey, I'm interested in working on this issue :)

@brian-brazil
Copy link
Contributor

Please go ahead. I'd suggest focus mainly on things that are completely missing from the docs, as these are reference docs.

@slrtbtfs
Copy link
Contributor

slrtbtfs commented Feb 5, 2020

There also exists an EBNF grammar for PromQL now, which could help: https://github.com/prometheus/prometheus/blob/2aacd807b3ec6ddd90ae55f3a42f4cffed561ea9/promql/generated_parser.y

@jonas-voi
Copy link

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.

@beorn7
Copy link
Member

beorn7 commented Feb 27, 2024

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.

@beorn7 beorn7 closed this as completed Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants