Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

raychaser
Copy link
Contributor

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 not json-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.

@progrium
Copy link
Contributor

The race condition is real and we've seen it. Sometimes just losing the
first line. Perhaps if we make this configurable, this is the price you pay
for not hitting the disk.

On Sun, Apr 26, 2015 at 4:43 PM, Christian Beedgen <notifications@github.com

wrote:

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 moby/moby#12790 to
use the /attach API in order to at least get a live stream of logs when
the log driver is not json-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.

You can view, comment on, or merge this pull request online at:

#94
Commit Summary

  • If log driver is not 'json-file' use /attach API to get the log
    stream

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#94.

Jeff Lindsay
http://progrium.com

@raychaser
Copy link
Contributor Author

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 json-file.

i thought about alternatives - what i could come up with so far:

  1. have a little ring buffer Docker-side to remember the last N lines. for N=10, or maybe 100... - clearly doesn't fix the race theoretically, but if the common observed case is missing a couple of lines in the beginning, this might help getting there... not sure Docker would like this tho.
  2. also thought about making a log driver for Logspout. maybe have the driver write to a logspout managed unix domain socket? this way we can still "pull" in logspout, but we own the socket. attaching containers to /dev/log for local syslog has caused some issues for me when the syslogd hup's - the container loses the bind-mounted /dev/log when it is replaced (i need to do more research on that, tho, but i have seen it happening...)

@mgood
Copy link

mgood commented Apr 27, 2015

@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 echo foo; sleep 1; echo bar, you will probably miss the "foo" line in the logs.

@progrium
Copy link
Contributor

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 json-file to just sending directly to the log stream connection instead of to disk. This would mean logs would act just as normal with json-file until you do a log -f or Logspout connects, then it switches into a mode where it doesn't write to disk, similar to the none driver. Except ideally, like you suggest, it keeps a minimal capped buffer so that it doesn't lose messages across disconnect/reconnects.

@raychaser
Copy link
Contributor Author

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!

@progrium
Copy link
Contributor

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.

@timbunce
Copy link

timbunce commented Oct 6, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants