Skip to content

add support for resolutions for subquery ranges#5

Open
DoctorVin wants to merge 2 commits into
vthriller:masterfrom
DoctorVin:drvince/subqueries
Open

add support for resolutions for subquery ranges#5
DoctorVin wants to merge 2 commits into
vthriller:masterfrom
DoctorVin:drvince/subqueries

Conversation

@DoctorVin
Copy link
Copy Markdown

I refactored the vector parser a bit to support PromQL subquery syntax (e.g. label_replace(some_vector, "replacement", "$1", "label", "label_regex(.*)")[5m:]). Prior to this change the terminal colon (or the other subquery syntax [5m:10m]) fails a parse.

Comment thread src/vec.rs Outdated
Comment thread src/vec.rs Outdated
Instead of making delimited_range try and handle both regular- and
subquery vector range selectors, add a specific variant for each range
type and allow the vector parser to take the range selector that is
well-formed.
Comment thread src/vec.rs
Comment thread src/vec.rs
range: opt!(
delimited!(char!('['), range_literal, char!(']'))
) >>
range: opt!(alt!(call!(range_selector) | call!(subquery_range_selector))) >>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I know one of the reasons why you got confused while implementing parametrized range parser.

Here's a thing: this change is not enough for label_replace(…) [5m:] to be accepted as a valid expression since label_replace() is not a vector but an expression. vector{} alone could be parsed as a full (subquery) expression so it accepts both [5m] (a vector modifier) and [5m:] (an expression modifier that turns it into proper subquery), but the reverse is not true (sum() can be parsed as an expression and thus accept [5m:], but it's not a vector so [5m] is invalid here).

So what you should do instead is introduce expr::Node::Subquery and modify expression parser accordingly. (Bonus points for being consistent with Prometheus at discarding expressions like 123[5m:] or foo[5m][5m:], but it's no big deal to leave that to future pull requests.)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience here. I'm pretty new to PromQL and this particular usage threw me.

I'm working off docs at prometheus.io but it's fairly basic. Is there a PromQL language spec I should review instead? I appreciate your help and I don't want to try your patience iterating on this.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a PromQL language spec I should review instead?

I don't know any, unfortunately, it seems docs and source code is all there is. I guess I just have written enough queries in the past to have pretty strong mental model of how this thing works (or supposed to) and why some things are the way they are (e.g. some people see != and =~ as something random and ad-hoc when in reality one can just pretend that = is a shorthand to ==, and then it makes total sense: ==/!=, =~/!~).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants