Skip to content
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

Merged
merged 1 commit into from
Aug 13, 2020

Conversation

slim-bean
Copy link
Collaborator

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"}'

Adds --analyze-labels which prodcues output like this:

Total Streams:  25017
Unique Labels:  8

Label Name  Unique Values  Found In Streams
requestId   24653          24979
logStream   1194           25016
logGroup    140            25016
accountId   13             25016
logger      1              25017
source      1              25016
transport   1              25017
format      1              25017

For debugging high cardinality labels

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-commenter
Copy link

Codecov Report

Merging #2497 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2497   +/-   ##
=======================================
  Coverage   63.22%   63.22%           
=======================================
  Files         162      162           
  Lines       14115    14115           
=======================================
  Hits         8924     8924           
  Misses       4484     4484           
  Partials      707      707           

Copy link
Member

@owen-d owen-d left a 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
Copy link
Member

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

Copy link
Collaborator Author

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.

Copy link
Member

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

@slim-bean slim-bean merged commit 82845e4 into master Aug 13, 2020
@slim-bean slim-bean deleted the logcli-analyize-series branch August 13, 2020 15:31
@cyriltovena
Copy link
Contributor

I love this PR.

@ruanbekker
Copy link

--analyze-labels is really neat. It was quite quick to see how many unique labels / total streams per job, in a scripty way:

for label_value in $(logcli labels job); 
do 
  echo "==> job=\"$label_value\""; 
  logcli series '{job="'"$label_value"'"}' --analyze-labels ; 
done

Unless there's a query to do ^ :-D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants