-
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
clarify logcli commands and output #1712
Conversation
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 few nits otherwise LGTM
Thanks for the review @slim-bean! I'd like to wait for @Dieterbe to give this a look before I merge so we get a non-Loki dev's opinion on it. |
cmd/logcli/main.go
Outdated
the performed query and its results. Raw log lines (i.e., without a | ||
label and timestamp) can be retrieved by passing the "-o raw" flag. | ||
The extra information about the query (API URL, set of common labels, | ||
excluded labels) can be suppressed with the --query flag. |
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.
"log line" vs "raw log line" is ambiguous. are they the same thing?
can you structure this information more? that would make this clearer I think
maybe something like:
- log line: contains what exactly? only the output lines from the app?
- log entry: timestamp + labels + log line
query output:
- default: log entries + query metadata + result metadata
-o raw
to show log lines rather than log entries--query
(you probably meant--quiet
here!) to surpress query metadata and result metadata
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.
see comments
Sorry for the delay @Dieterbe, PTAL at the changes. I didn't sync logcli.md with the flag changes yet, but I'll do that once you think the changes look good. |
looks good robert! |
Codecov Report
@@ Coverage Diff @@
## master #1712 +/- ##
==========================================
+ Coverage 64.07% 64.09% +0.02%
==========================================
Files 121 121
Lines 9027 9027
==========================================
+ Hits 5784 5786 +2
+ Misses 2841 2840 -1
+ Partials 402 401 -1
|
This commit attempts to do two things: 1. Differentiate between log entries and raw log lines, where the latter is what is produced by `-oraw`. 2. Explain the difference between `logcli query` and `logcli instant-query`. Fixes grafana#1676.
This commit leaves logcli.md unmodified; that will be done in a later commit if the changes are approved.
6c44f68
to
bda0510
Compare
@Dieterbe I've synced logcli.md with the go file. Can you approve this so we can merge it? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1712 +/- ##
==========================================
+ Coverage 64.60% 64.62% +0.02%
==========================================
Files 125 125
Lines 9462 9462
==========================================
+ Hits 6113 6115 +2
+ Misses 2923 2922 -1
+ Partials 426 425 -1 |
This PR attempts to do two things:
-oraw
.logcli query
andlogcli instant-query
.Fixes #1676.
/cc @Dieterbe PTAL and let me know if this would've been helpful for you earlier on 🙂