tk: Format logs with the console log formatter if outputting to a tty #776
Conversation
Since the switch to structured logging in cc21323 we now output evaluation errors all on one line, with escaped newlines: ``` {"level":"fatal","time":"2022-10-07T12:50:42+01:00","message":"Error: evaluating jsonnet: RUNTIME ERROR: function expected 3 positional argument(s), but got 4\n\t/home/laney/temp/tk/environments/default/main.jsonnet:3:14-56\tobject <anonymous>\n\tField \"bar\"\t\n\tDuring manifestation\t\n"} ``` It's pretty unreadable. Instead, what we can do is turn on zerolog's console writer when we're running interactively (precisely, when stderr is a tty). Then the above message becomes: ``` 12:52PM FTL Error: evaluating jsonnet: RUNTIME ERROR: function expected 3 positional argument(s), but got 4 /home/laney/temp/tk/environments/default/main.jsonnet:3:14-56 object <anonymous> Field "bar" During manifestation ``` which is legible again.
|
|
cmd/tk/main.go
Outdated
| @@ -12,7 +12,7 @@ import ( | |||
| "github.com/grafana/tanka/pkg/tanka" | |||
| ) | |||
|
|
|||
| var interactive = term.IsTerminal(int(os.Stdout.Fd())) | |||
| var interactive = term.IsTerminal(int(os.Stderr.Fd())) | |||
There was a problem hiding this comment.
zerolog outputs to stderr, this is the relevant fd to check
There was a problem hiding this comment.
This is used elsewhere though (to tell whether or not the pager should be activated), does checking stdout and stderr do the same thing, or should it be two different vars?
There was a problem hiding this comment.
that's a good point, I guess the error one should check stderr and the other one can keep on looking at stdout
|
It's not really a TTY problem. We have the same problem in CI. The logs are ugly since #774, the fix should've been to add |
|
I don't really mind if you want to close this, but this has the effect of making all logs look nice. It's a decent practice if you ask me. But I'm not that wedded to it. |
cmd/tk/main.go
Outdated
| @@ -12,7 +12,7 @@ import ( | |||
| "github.com/grafana/tanka/pkg/tanka" | |||
| ) | |||
|
|
|||
| var interactive = term.IsTerminal(int(os.Stdout.Fd())) | |||
| var interactive = term.IsTerminal(int(os.Stderr.Fd())) | |||
There was a problem hiding this comment.
This is used elsewhere though (to tell whether or not the pager should be activated), does checking stdout and stderr do the same thing, or should it be two different vars?
inteactive -> stdoutIsTTY add stderrIsTTY Use the latter for checking if logs should be pretty printed, the former for other checks, e.g. whether to use a pager.
|
@julienduchesne can you help me with the CI failure please? 🙈 |
Since the switch to structured logging in
cc21323 we now output evaluation errors
all on one line, with escaped newlines:
It's pretty unreadable. Instead, what we can do is turn on zerolog's
console writer when we're running interactively (precisely, when stderr
is a tty). Then the above message becomes:
which is legible again.