-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Support label matchers in labels API #3566
Conversation
E2E is flaky #3570 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
matcherSets = append(matcherSets, matchers) | ||
} | ||
|
||
q, err := qapi.queryableCreate(true, nil, storeDebugMatchers, 0, enablePartialResponse, true). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a nit, just wanted to ask a question. Why did you flip the last argument (skipChunks
) to true
, and what it does exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A query result contains two parts, the series metadata like label sets, and the sample values. In this case, since we are just querying labels, we can set skipChunks
to not return samples, only labels are fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the original issue about this option #1822.
Also this Prometheus pr prometheus/prometheus#8050 has the same idea to improve the performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks for the answer. :)
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
2783aaf
to
bbde4ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks. It's not very performant though, but we have TODO. Can we add issue as well? 🤗
Signed-off-by: Ben Ye [email protected]
Changes
Fixes #3351
Add
match[]
param for LabelNames and LabelValues queries.Now this is implemented via the
Series
matching. We can do matchers push down to improve this later.TODO: query frontend also needs
matchers
type for Label names and label values query. Will add them in the next pr.Verification
Add unit tests and E2E tests for this feature.