-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
logcli: adds --analyize-labels to logcli series command and changes how labels are provided to the command #2497
Conversation
Changes the series command to use the common matcher input found in the query and instant-query commands, instead of `logcli series --matcher='{foo="bar"}'` it's now `logcli series '{foo="bar"}'`
Codecov Report
@@ Coverage Diff @@
## master #2497 +/- ##
=======================================
Coverage 63.22% 63.22%
=======================================
Files 162 162
Lines 14115 14115
=======================================
Hits 8924 8924
Misses 4484 4484
Partials 707 707 |
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.
I thin this should continue to take a slice of match clauses, but LGTM asides that.
Start time.Time | ||
End time.Time | ||
Quiet bool | ||
Matcher string |
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 should take a slice of matcher clauses, see https://grafana.com/docs/loki/latest/api/#series
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.
i debated this but honestly I couldn't figure out why that would be useful? I couldn't think of a usecase for this from logcli? I may totally be missing one though.
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.
It's probably ok. I think this is more important for Prometheus which can't combine multiple metric names in the same matcher clause, but loki doesn't have this problem.
Anyways, we can always add it in the future if there's a case where we need it. LGTM
I love this PR. |
Unless there's a query to do ^ :-D |
Changes the series command to use the common matcher input found in the query and instant-query commands, instead of
logcli series --matcher='{foo="bar"}'
it's nowlogcli series '{foo="bar"}'
Adds
--analyze-labels
which prodcues output like this:For debugging high cardinality labels