[shim] Revamp logging and CLI #2176
Conversation
* Migrate to context-aware logging facility
* Write to `shim.log` internally (don't rely on systemd/shell
std{out,err} redirects, preserves shim logs in journald)
* Add `--shim-log-level`
* Rename `--home` to `--shim-home` for consistency
* Drop `docker` command (it is _the only_ command)
| if !t.mu.TryLock() { | ||
| log.Fatalf("task %s already locked!", t.ID) | ||
| log.Fatal(ctx, "already locked!", "task", t.ID) |
There was a problem hiding this comment.
log.Fatal calls exit(1) and defers do not run, i.e. basically panics. Is it intended here?
There was a problem hiding this comment.
log.Fatal calls exit(1) and defers do not run
Didn't know that, but in this specific case it's fine I think, this should never happen, fatal here indicates a serious programmatic error (which otherwise would lead to a deadlock, as mutexes are not reentrant) and should abort execution, even if not gracefully
| Handler: r, | ||
| Addr: address, | ||
| Handler: r, | ||
| BaseContext: func(l net.Listener) context.Context { return ctx }, |
There was a problem hiding this comment.
Does it mean the same context will be used for all requests? I assume it will be a problem if we need to store per-request values in context.
There was a problem hiding this comment.
If I understand correctly, base context (as its name implies) is not used directly but to derive a new per-request context: https://github.com/golang/go/blob/194de8fbfaf4c3ed54e1a3c1b14fc67a830b8d95/src/net/http/server.go#L3318-L3328
shim.loginternally (don't rely on systemd/shell std{out,err} redirects, preserves shim logs in journald)--shim-log-level--hometo--shim-homefor consistencydockercommand (it is the only command)