-
Notifications
You must be signed in to change notification settings - Fork 680
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
If log driver is not 'json-file' use /attach API to get the log stream #94
base: master
Are you sure you want to change the base?
Conversation
The race condition is real and we've seen it. Sometimes just losing the On Sun, Apr 26, 2015 at 4:43 PM, Christian Beedgen <[email protected]
Jeff Lindsay |
yeah :( - i guess that's the nature of race conditions after all. on some level, the user is already configuring this, i guess, by not using i thought about alternatives - what i could come up with so far:
|
@raychaser yeah, if you're testing with a "real" app with a non-trival startup time you may not see the race condition, but something like |
The discussion on the Docker issue (moby/moby#12790) made me think that maybe a simple Logspout friendly logging driver would be a driver that can detect when the streaming logs endpoint is used and switch from |
ok. let me take a look tonight and see how this could be wired. catching up on the discussion on the Docker issue now as well. thanks! |
The problem is I don't think the PR to Docker would just be a log driver, it would involve cooperation with (ie changes to) the logs API for this to work. |
I think using /attach instead of /logs changes the behaviour in a significant way when the log consumer is down or can't keep up. With /logs there's no impact on the application. With /attach the application will block potentially causing an outage. See this commit to the docker docs and the issue that prompted it. Ideally this PR should include docs that spell-out the issue and possible impact. (Along with the race-hazard noted earlier.) More significantly, the current PR assumes that "Any other log driver disables the /logs endpoint", which I'm pretty sure isn't true. The Configure logging drivers docker docs say "The docker logs command is available only for the json-file and journald logging drivers." (since docker 1.9). So the PR needs to be updated to allow other log drivers that support the /logs API. Thankfully both change should be simple. |
I really want to use Logspout without having to go to disk for the logs. I thought I needed to fix this on the Docker side, but the discussion brought about the idea to use the
/attach
API in order to at least get a live stream of logs when the log driver is notjson-file
. Any other log driver disables the/logs
endpoint, but with this patch I can run--log-driver=none
and still forward logs via Logspout.It would appear that there is a race condition: between Logspout getting the (re)start event and actually attaching to the container the container could have outputted logs already. But in my tests so far this actually didn't really happen.