Skip to content

/prometheus

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

`on()` should work #1597

Closed
brian-brazil opened this Issue Apr 26, 2016 · 8 comments

Comments

Assignees
No one assigned
Labels
kind/enhancement
Projects
None yet
Milestone
  v1.0.0
4 participants
@brian-brazil @beorn7 @juliusv @fabxc
@brian-brazil
Copy link
Member

brian-brazil commented Apr 26, 2016

We should accept an empty list of labels for aggregation and vector matching. The only place this offers new semantics is with on, as it'll let us do on() rather than on(dummy).

@brian-brazil brian-brazil added the kind/enhancement label May 6, 2016

@brian-brazil brian-brazil added this to the v1.0.0 milestone May 19, 2016

@brian-brazil brian-brazil referenced this issue May 27, 2016

Closed

Empty `group_left`/`group_right` isn't working in rules #1676

@beorn7

This comment has been minimized.

Sign in to view
Copy link
Member

beorn7 commented May 27, 2016

Would on be valid, too, then? I.e. on == on() == on(does_not_exist) ?
@brian-brazil

This comment has been minimized.

Sign in to view
Copy link
Member Author

brian-brazil commented May 27, 2016

Hmm, for clarity we may want to keep it to only on()
@juliusv

This comment has been minimized.

Sign in to view
Copy link
Member

juliusv commented May 27, 2016

👍 to always requiring the parentheses.
@brian-brazil

This comment has been minimized.

Sign in to view
Copy link
Member Author

brian-brazil commented May 28, 2016

Should we also require them for group_left for consistency? My instinct says no.
@beorn7

This comment has been minimized.

Sign in to view
Copy link
Member

beorn7 commented May 28, 2016

From a natural language perspective, group_left/group_right totally makes sense without parentheses, while on would imply that the next token is what you join "on". From a purely syntactical perspective, it's however inconsistent that sometimes modifier() is equivalent to modifier and sometimes not.

Another point is that group_left/group_right will be used most of the time without any following labels, while it's the other way round with on (usually you would list at least one label, and the case without a label is pretty special).

In conclusion, I'd definitely allow group_left/group_right without parentheses. Whether we want allow on without parentheses for consistency or require on() to avoid misreading of queries is a matter of discussion.

beorn7 added a commit that referenced this issue May 28, 2016

@beorn7
Correctly stringify GROUP_x modifiers without labels 
Since rule evaluations work via String(), this fixes evaluation of
rules containing GROUP_x modifiers without labels. This change is the
minimal bugfix (so that we can release a fixed version without
risk). It does not intend to implement any additional features (like
allowing `GROUP_LEFT()` or `ON()` or even `ON` - see discussion in
#1597 ).
Loading status checks…
1f96cc3

@beorn7 beorn7 referenced this issue May 28, 2016

Closed

Correctly stringify GROUP_x modifiers without labels #1684

beorn7 added a commit that referenced this issue May 28, 2016

@beorn7
Correctly stringify GROUP_x modifiers without labels 
Since rule evaluations work via String(), this fixes evaluation of
rules containing GROUP_x modifiers without labels. This change is the
minimal bugfix (so that we can release a fixed version without
risk). It does not intend to implement any additional features (like
allowing `GROUP_LEFT()` or `ON()` or even `ON` - see discussion in
#1597 ).
Loading status checks…
5408666

@beorn7 beorn7 referenced this issue May 28, 2016

Merged

Correctly stringify GROUP_x modifiers without labels #1685

@fabxc

This comment has been minimized.

Sign in to view
Copy link
Member

fabxc commented Jun 23, 2016

What's the status here?
How necessary is it really for 1.0?
@brian-brazil

This comment has been minimized.

Sign in to view
Copy link
Member Author

brian-brazil commented Jun 23, 2016

I hope to take care of this in the next few weeks.

This isn't critical for 1.0, it cleans up a not uncommon use case.

@brian-brazil brian-brazil referenced this issue Jun 23, 2016

Merged

Support on() #1765

brian-brazil added a commit that referenced this issue Jun 24, 2016

@brian-brazil
Allow on, ignoring, by and without wit empty laberls. 
This offers new semantics in allowing on() for matching
two single-element vectors with no known common labels.
Previosuly this was often done using on(dummy).

This also allows making it explict that you meant
to do an aggregation without labels via by().

Fixes #1597.
Loading status checks…
3b89616

@brian-brazil brian-brazil closed this in #1765 Jun 24, 2016

gouthamve pushed a commit to gouthamve/promql that referenced this issue Mar 28, 2018

@beorn7
Correctly stringify GROUP_x modifiers without labels 
Since rule evaluations work via String(), this fixes evaluation of
rules containing GROUP_x modifiers without labels. This change is the
minimal bugfix (so that we can release a fixed version without
risk). It does not intend to implement any additional features (like
allowing `GROUP_LEFT()` or `ON()` or even `ON` - see discussion in
prometheus/prometheus#1597 ).
95713f3
@lock

This comment has been minimized.

Sign in to view
Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.