Skip to content

Change syslog format and facility#12391

Merged
jessfraz merged 1 commit intomoby:masterfrom
ibuildthecloud:syslog-format
Apr 15, 2015
Merged

Change syslog format and facility#12391
jessfraz merged 1 commit intomoby:masterfrom
ibuildthecloud:syslog-format

Conversation

@ibuildthecloud
Copy link
Copy Markdown
Contributor

This patch changes three things

  1. Set facility to LOG_DAEMON
  2. Remove ": " from tag so that the tag + pid become a single column in
    the log
  3. Log the container name and not the ID as the ID is often ephemeral
    and hard to correlate with events in the past

This is an implementation of the my comments on #12377

In the end running docker run --name test --rm --log-driver syslog ubuntu echo hi will yield the following log in Ubuntu

Apr 15 00:16:05 inotmac docker<test>[30851]: hi

Fix #12377

@thaJeztah
Copy link
Copy Markdown
Member

Do we need a test involving container rename oldname newname to verify that the logs properly follow a container rename?

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented Apr 15, 2015

@ibuildthecloud I'm -1 on names by default as @phemmer , and I have same concerns as @thaJeztah . I think that there is more usecases about IDs, then about Names. But I'm +1 on log-opts for syslog where tag can be specified.
Also I like format proposed by @phemmer: docker/ID.
Facility ok for me.

@ibuildthecloud
Copy link
Copy Markdown
Contributor Author

I'll like @phemmer format, but still want container names. I think we can agree that there are use cases for both. We will make it configurable, I'll personally put in the PR for that, but 1.6 I still think container name is more friendly.

@ibuildthecloud
Copy link
Copy Markdown
Contributor Author

I update the PR to be "docker/NAME"

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented Apr 15, 2015

Okay, my thoughts about id-name war:

  • I don't really see big difference between docker run --name=cron and docker run --name=cron --log-opts="tag=cron"
  • I want tags to be simple strings and don't want to introduce tag=string: tag=id tag=name
  • names can be changed with rename - and handling this will require significant changes to driver

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented Apr 15, 2015

@jessfraz
Copy link
Copy Markdown
Contributor

ok so my stance on this is id as default and we can have --log-opt added in 1.7 to make configurable
also as @LK4D4 said setting as name will cause problems when you rename a container

and to quote @shykes from another chat "container IDs only please"

This patch changes two things

1. Set facility to LOG_DAEMON
2. Remove ": " from tag so that the tag + pid become a single column in
   the log

Signed-off-by: Darren Shepherd <[email protected]>
@ibuildthecloud
Copy link
Copy Markdown
Contributor Author

Okay, I'm taking my toys and going home.

I updated the PR to do container ID. So the PR just changes the facility and then the final format will be "docker/ID" looking like:

Apr 15 00:16:05 inotmac docker/1234567890AB[30851]: hi

@ibuildthecloud
Copy link
Copy Markdown
Contributor Author

@thaJeztah now that the format is ID, i don't think a rename test is really needed.

@jessfraz
Copy link
Copy Markdown
Contributor

sorry :(

@thaJeztah
Copy link
Copy Markdown
Member

@ibuildthecloud not as part of this PR, no. But I think it will be needed for the 1.7 (1.6.1?) update that supports --log-opts

I realise the outcome here is not what you hoped for (I tried, within reason), but given the short time before the release, I think it's reasonable. "Go go gadget log-opts" for the next release!

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented Apr 15, 2015

LGTM

1 similar comment
@jessfraz
Copy link
Copy Markdown
Contributor

LGTM

jessfraz pushed a commit that referenced this pull request Apr 15, 2015
Change syslog format and facility
@jessfraz jessfraz merged commit 2e4d36e into moby:master Apr 15, 2015
@jessfraz
Copy link
Copy Markdown
Contributor

ok cherry-picked into release bump

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.

Containers Syslog message format.

6 participants