-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Fix(query-frontend): /label/<name>/values
endpoint to use right set of middlewares
#6072
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -232,7 +232,7 @@ func getOperation(path string) string { | |
return QueryRangeOp | ||
case strings.HasSuffix(path, "/series"): | ||
return SeriesOp | ||
case strings.HasSuffix(path, "/labels") || strings.HasSuffix(path, "/label"): | ||
case strings.HasSuffix(path, "/labels") || strings.HasSuffix(path, "/label") || strings.HasSuffix(path, "/values"): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: Does anyone know why middeware chain was disabled for We in fact have a test to lock that behaviour. I wanted this support so that middlewares like Any thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If no protest, I can remove that test! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I bet it was omitted by mistake |
||
return LabelNamesOp | ||
case strings.HasSuffix(path, "/v1/query"): | ||
return InstantQueryOp | ||
|
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 function feels a bit flimsy, and could introduce some interesting bugs in the future.
We should refactor this at some point
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.
Thanks for adding a test!