@wlan0 Gofmt check failed.
We need design review on new flag, but I sorta like it, not tried to use though.
ping @crosbymichael @tiborvass @jfrazelle

@LK4D4 fixed the gofmt issue.

@wlan0 Haha, tests don't compile :)

@LK4D4 All tests pass now.. There were some unused packages earlier

@wlan0 @LK4D4 I'd much rather have --log-opt like we have --storage-opt.

Also, I think we should not make it a new driver, but instead make the default json-file driver have options with this new flag. So rollover would be done like this: docker run --log-opt max_file=10 --log-opt max_size=1k.

Proposed defaults:

• max_size = 0 (no limit)
• max_file = 1

What do you guys think of this? If we can agree, we could move on to code review.

+1 I like @tiborvass's suggestion.

@tiborvass I'm okay with moving that logic to default driver.

@tiborvass I also agree it should be in the default, we did it as a separate only because we didn't know if you guys would want us screwing with the default logging driver. For example, just keep the default stupid simple because you know it works.

@ibuildthecloud except this PR would introduce log options, and that's a whole new world :)

EDIT: https://www.youtube.com/watch?v=-kl4hJ4j48s

Okay. To sum up.

• Change --log-driver-opts to --log-opts
• Remove "rollover" driver and put logic in json-file
• max_size = 0 should mean unlimited
• Defaults should be max_size=0, max_file=1

Question:

• Should it be max-size or max_size, which is more consistent with existing Docker?
• docker logs should read from all historical logs or just the latest?

@ibuildthecloud

flag would be singular: --log-opt.

Let's make it max-size to make it consistent with -restart on-failure[:max-retry]. Also I generally prefer - rather than _.

If docker logs outputs only the latest file, the default value of --tail would need to change to latest or something like that. I'm not sure what's the best solution for that, I'm still thinking.

@LK4D4 @tiborvass updated the code according to comments.

• Now, the flag is --log-opt
• the options use - instead of _, so max-file and max-size are the options
• the options have been added to the json-file log driver
• the options have default values max-file=1 and max-size=0 (infinite)
• updated docs

example

docker run --log-driver json-file --log-opt max-size=1k --log-opt max-file=10 -d redis

@wlan0 in your example --log-driver json-file is not necessary since it's the default.

I agree with everything you pointed out. Don't forget to update the API as well.

What's left to decide is what to do with docker logs... :S @LK4D4 @jfrazelle @icecrime

EDIT: also, what happens when it reaches maximum? How are the log files named? Do you go back to "file1" or do you remove file1, and write to file11 ?

Great!

👍

@calavera Fixed the doc!

@wlan0 Thanks for the contribution. Please check your docs output with a make docs from the docs dir.

@moxiegirl @thaJeztah I've updated the docs based on your comments.

@thaJeztah @moxiegirl Updated.

Hi,

Thanks for the PR as I have no doubt it adds a very useful feature to Docker. Here are some comments about the doc changes.

• docs/reference/logging/index.md

  • As @thaJeztah mentioned, I think it is worthwhile to clearly document that max-file is meaningless without max-size. At the same time, we should also mention the default value for max-size (which is unlimited).
  • There is a missing space in "The docker logscommand" though it is not this PR's doing.

• docs/reference/commandline/logs.md

  • It is not clear to me why log-opt option is added to docker logs command. What exactly can I pass with this option? I may be missing something here but the options max-size and max-file make sense only for docker run command.
  • If I run docker logs without any options, does it combine all the log files or just return the latest? I ask because the command seems to default to "latest" when an invalid value is passed to --tail option. Similarly, does --since option apply across all the log files?

• docs/reference/commandline/run.md

  It would be beneficial if an example is provided in this file covering the new log options.

@draghuram makes very good points. @wlan0 can you clarify in the docs these points please? Thank you for being so cooperative on making changes. User would appreciate this extra clarification.

@moxiegirl @draghuram Thanks! Absolutely not a problem. I would like to be as clear as possible while describing these options as well.

Firstly, I've updated the docs keeping in mind all of your comments. Secondly, to answer your questions -

1. --log-opt doesn't make sense in logs command. Good catch. Removed this from the logs.md file in the docs.
2. If you run docker logs without options, it only provides the latest logs. Added this to the docs.

I've updated the docs according to the rest of your comments.

@wlan0 Thanks for the changes. In addition to my comment above, I would really like to see an example in run.md but I am ok with the PR being merged without one.

LGTM thanks @wlan0

@thaJeztah for the merge.

LGTM! Thanks so much for being so patient and cooperative, @wlan0. Apologies that it took so long :-)

yaay finally!!! thanks all... :)

+1!!!

Hello,

thanks for this feature!
Is it possible to use it with docker-compose?
How do I specify/call it in the compose.yml

Greetings,
Hendrik

@henfri please don't comment on closed/merged PR's for support questions; the issue tracker is meant for tracking bugs, and reviewing pull requests; your question is better asked;

• in the #docker or #docker-compose) IRC channel on freenode
• the docker-user Google Group (https://groups.google.com/forum/m/#!forum/docker-user
• https://forums.docker.com