[WIP] Refactor Container Logs and support TTY for Service Logs#31952
[WIP] Refactor Container Logs and support TTY for Service Logs#31952dperny wants to merge 2 commits intomoby:mastermoby/moby:masterfrom dperny:service-logs-support-ttydperny/docker:service-logs-support-ttyCopy head branch name to clipboard
Conversation
Refactors container logs to expose a method allowing us to directly get the low-level struct log messages. Refactors service logs to use that lower-level method instead of trying to parse an ugly-ass byte stream. Signed-off-by: Drew Erny <drew.erny@docker.com>
Adds tentative, not-quite-right support for TTY logs. You can get em even if they don't get formatted all neat-like Signed-off-by: Drew Erny <drew.erny@docker.com>
| // ContainerLogs hooks up a container's stdout and stderr streams | ||
| // configured with the given struct. | ||
| func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, config *backend.ContainerLogsConfig, started chan struct{}) error { | ||
| // ContainerLogsRawChan, given a channel, returns raw log message structs from |
There was a problem hiding this comment.
golint/janky seem unhappy with the comma after ContainerLogsRawChan,
Errors from golint:
13:55:32 daemon/logs.go:21:1: comment on exported method Daemon.ContainerLogsRawChan should be of the form "ContainerLogsRawChan ..."| } | ||
| } | ||
| } | ||
|
|
| defer writer.Close() | ||
| c.backend.ContainerLogs(ctx, c.container.name(), apiOptions, chStarted) | ||
| c.backend.ContainerLogsRawChan(ctx, c.container.name(), apiOptions, chMsgs) | ||
| }() |
There was a problem hiding this comment.
Could be just go c.backend.ContainerLogsRawChan(ctx, c.container.name(), apiOptions, chMsgs)
| func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, config *backend.ContainerLogsConfig, started chan struct{}) error { | ||
| // ContainerLogsRawChan, given a channel, returns raw log message structs from | ||
| // the container logger. | ||
| func (daemon *Daemon) ContainerLogsRawChan(ctx context.Context, containerName string, config *backend.ContainerLogsConfig, msgs chan *logger.Message) error { |
There was a problem hiding this comment.
Maybe this should be ContainerLogs and the other function should be moved into the API server implementation. Not sure if there are any obstacles, but it seems like the other one is more of a wrapper that doesn't need to be part of the backend.
There was a problem hiding this comment.
There is a good reason to do it like this: it the API server implementation doesn't have to be aware of the details of the logging format, and doesn't have logger as a dependency.
There was a problem hiding this comment.
But the logging format is API-specific, right? I'm not sure the backend is the right place to encode it in this format.
|
Closing this PR, there are better things coming... |
This PR is two commits. The first commit is an almost-substantial refactoring of container logs that needs design review before I proceed. It's the main focus of this change.
The second commit is an example of what this refactoring is useful for.
Before merging, there may be further work to support
Detailsin service logs, and all of the work will be rebased to 1 commit./cc @aluzzardi
- What I did
Support TTY logs in
docker service logs, facilitated through internal refactoring.- How I did it
I refactored the daemon to expose a new method,
ContainerLogsRawChan, which sends over a passed-in channel log messages as structs straight from the container logger. I refactoredContainerLogsto use this new, lower-level method. I expanded the backend interface that the exec package uses to exposeContainerLogsRawChaninstead ofContainerLogs. I refactoredServiceLogsto use this new, lower-level method and avoid having to parse a barely-structured ambiguous byte string.In a second commit, I made small changes to show how TTY logs would be supported through this change.
- How to verify it
Things build, run, and work, but I'm not making guarantees about correctness or passing tests at this time, because I'm anticipating people asking me to make substantial changes to this design (because it involves changing internal APIs). When the design looks good, I'll clean things up, address my own TODOs, and fix the tests.
- A picture of a cute animal (not mandatory but encouraged)
this blurry dog should not be driving, because i do not think he has a license.