Add docs for `local` logging driver. #6717

cpuguy83 · 2018-05-17T19:52:21Z

Proposed changes

Add docs for moby/moby#37092

Unreleased project version (optional)

Depends on the above mentioned PR being merged

cpuguy83 · 2018-05-17T19:52:33Z

ping @anusha-ragunathan

anusha-ragunathan · 2018-05-17T21:02:42Z

config/containers/logging/configure.md

@@ -150,9 +150,12 @@ see more options.
 | [`etwlogs`](etwlogs.md)       | Writes log messages as Event Tracing for Windows (ETW) events. Only available on Windows platforms.           |
 | [`gcplogs`](gcplogs.md)       | Writes log messages to Google Cloud Platform (GCP) Logging.                                                   |
 | [`logentries`](logentries.md) | Writes log messages to Rapid7 Logentries.                                                                     |
+| [`local`](local.md)           | Logs are stored in an custom, efficient logging format designed to cause as little overhead as possible.      |


"an custom" -> "a custom"

Also reword to "designed for minimal overhead"

Thanks that was a mess of a sentence.

anusha-ragunathan · 2018-05-17T21:05:44Z

config/containers/logging/local.md

+
+
+The `local` logging driver captures output from container's stdout/stderr and
+writes them to a local file on disk. The format of these files is unspecified.


WDYM by "format is unspecified" ?

Trying to convey that we are not going to document the format for public consumption because we don't want people messing with these files.
Also that the format itself may change over time.

Not sure if that detail is necessary here. If you feel strongly about it, I'm fine.

I think so because currently people parse the json logs and it makes it impossible to change the internal structure without breaking people.

Thinking a bit more. If we are expecting to change the format and dont provide any tools for parsing, do you think we should make it experimental?

I don't. It's the users choice to use it or not, gating it behind experimental just means no one will use it.

Let's get @thaJeztah thoughts on this

anusha-ragunathan · 2018-05-17T21:06:46Z

config/containers/logging/local.md

+5, for a total of 100MB (when decompressed) of log data.
+
+The goal of this driver is to be as efficient as possible while still
+remaining consistent. As such log files may not be easily parseable with


"not easily parseable with standard CLI tools". This needs more explanation as to why, since it is unexpected and different behavior from the other 2 drivers that work with docker logs

To me the first sentence in the paragraph explains that. Do you have some idea of what else should go here?

Since this is purely to serve as a warning to users, in which case we should add it as a Note/Warning. I prefer Note, since it is less alarming.

anusha-ragunathan · 2018-05-17T21:08:22Z

config/containers/logging/local.md

+and `log-opt` keys to appropriate values in the `daemon.json` file, which is
+located in `/etc/docker/` on Linux hosts or
+`C:\ProgramData\docker\config\daemon.json` on Windows Server. For more about
+configuring Docker using `daemon.json`, see


remove dubious +

anusha-ragunathan · 2018-05-17T21:08:33Z

config/containers/logging/local.md

+located in `/etc/docker/` on Linux hosts or
+`C:\ProgramData\docker\config\daemon.json` on Windows Server. For more about
+configuring Docker using `daemon.json`, see
+[daemon.json](/engine/reference/commandline/dockerd.md#daemon-configuration-file).


remove dubious + here as well

thaJeztah · 2018-05-26T10:02:53Z

Thinking a bit more. If we are expecting to change the format and dont provide any tools for parsing, do you think we should make it experimental?

@anusha-ragunathan I think I get your confusion;

The short answer

We must be explicit that this logging-driver uses an internal storage format that is not for external consumption, and that the format can change at any time. Perhaps something like:

The `local` logging driver captures output from container's stdout/stderr and
writes them to an internal storage that is optimized for performance and disk
use.

By default the `local` driver preserves 100MB of log messages per container and
uses automatic compression to reduce the size on disk.

> *Note*: the `local` logging driver currently uses file-based storage. The
> file-format and storage mechanism are designed to be exclusively accessed by
> the Docker daemon, and should not be used by external tools as the
> implementation may change in future releases.

The long answer:

What it comes down to, is that everything inside /var/lib/docker should be considered an "implementation detail"; this directory is designed to be exclusively accessed by a single daemon, and no external tools should be using those files.

Now comes the tricky bit;

The API endpoint used for docker inspect return the "low level" information of containers. Basically, they directly expose our internal types through the API, which means that any information we keep on a container will be exposed there, and any field that we add becomes visible. This includes information, such as the full path of container's root-mounts, resolv.conf, hosts, but also the LogPath (the file used by the json-file logging driver).
The JSON-file logging-driver uses, well, JSON. So, even though the format/spec is not documented, it's easy to figure out the schema.
The file location was exposed through the API (so, intended for public consumption?)

So, as a result;

Docker didn't support log rotation, so people started writing log-rotate. tools. for docker (sometimes causing issues where the file was kicked from underneath dockerd).
There were no logging-drivers (or plugins), and the logs API has a weird format that's difficult to use, so to aggregate logs, users started to read the log-files directly (this is what Kubernetes does; it creates symlinks to each-and-every logfile).

Because of the above: even though the json-file log-format was intended to be an internal format ("implementation detail"), it has now become an unofficial public API. We can't change it without breaking many people.

So, what we need is;

Be explicit; a published / documented API is a contract, and even though we mention we use an "open schema" for the API (more files may be returned), we haven't been explicit
Don't re-use / expose internal types as API types; changing an internal type should never automatically change the API
If API fields come with descriptions, these should be documented. For example, some information is not part of the API, and their values don't have a defined format (tried doing that for the DriverStatus, and DriverOpts fields).

thaJeztah · 2018-05-26T10:04:51Z

config/containers/logging/local.md

+| Option      | Description                                                                                                                                                                                                   | Example  value                           |
+|:------------|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:-----------------------------------------|
+| `max-size`  | The maximum size of the log before it is rolled. A positive integer plus a modifier representing the unit of measure (`k`, `m`, or `g`). Defaults to 20MB.                                          | `--log-opt max-size=10m`                 |
+| `max-file`  | The maximum number of log files that can be present. If rolling the logs creates excess files, the oldest file is removed. **Only effective when `max-size` is also set.** A positive integer. Defaults to 5. | `--log-opt max-file=3`                   |


I should probably comment this on the implementation PR, but isn't "max file" an implementation detail? Can we abstract storage even more, and just allow people to set "max amount of logs to keep" ?

This stayed as max-file in the implementation.

thaJeztah · 2018-05-26T10:07:34Z

config/containers/logging/local.md

+|:------------|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:-----------------------------------------|
+| `max-size`  | The maximum size of the log before it is rolled. A positive integer plus a modifier representing the unit of measure (`k`, `m`, or `g`). Defaults to 20MB.                                          | `--log-opt max-size=10m`                 |
+| `max-file`  | The maximum number of log files that can be present. If rolling the logs creates excess files, the oldest file is removed. **Only effective when `max-size` is also set.** A positive integer. Defaults to 5. | `--log-opt max-file=3`                   |
+| `compress`  | Toggle compression of rotated log files. Enabled by default. | `--log-opt compress=false` |


Same here; "rotated files" is likely implementation detail; we can describe here why someone would want to disable compression (does it cause a significant overhead? discouraged for high-volume logging? discouraged for "many running containers"?).

Ideally, we'd change this to (e.g.) disable-compression. Booleans with --enablesomething=false are a bit awkward 😅

Should we add labels here as well? (I think it's a common option, so it may be documented as such, but could be useful to include it in every logging driver)

I did not add support for labels/env/etc to the local driver.

thaJeztah · 2018-05-26T10:14:08Z

config/containers/logging/local.md

+
+| Option      | Description                                                                                                                                                                                                   | Example  value                           |
+|:------------|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:-----------------------------------------|
+| `max-size`  | The maximum size of the log before it is rolled. A positive integer plus a modifier representing the unit of measure (`k`, `m`, or `g`). Defaults to 20MB.                                          | `--log-opt max-size=10m`                 |


(Probably better discussed on the implementation PR, I'll comment there as well); is the storage-format suitable for doing time-based rotation as well? (see moby/moby#36853)?

Not tackling this as of yet, but it's definitely possible.

anusha-ragunathan · 2018-08-20T14:25:27Z

Now that the implementation PR is merged, lets get the doc changes done.

@cpuguy83: Can you address @thaJeztah 's concerns?

cpuguy83 · 2018-08-20T16:29:36Z

Updated with explicit wording about the the private nature of the implementation.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

GordonTheTurtle · 2018-08-20T16:35:25Z

Deploy preview for modest-edison-79d82e ready!

Built with commit b096073

https://deploy-preview-6717--modest-edison-79d82e.netlify.com

GordonTheTurtle · 2018-08-20T16:44:21Z

Deploy preview for modest-edison-79d82e ready!

Built with commit 889153d

https://deploy-preview-6717--modest-edison-79d82e.netlify.com

tfoxnc · 2018-08-21T14:29:44Z

@anusha-ragunathan Do you approve of the last change? Are you ready for me to merge now?

anusha-ragunathan · 2018-08-21T14:59:40Z

LGTM

thaJeztah

LGTM

anusha-ragunathan reviewed May 17, 2018

View reviewed changes

cpuguy83 force-pushed the add_local_logger_docs branch from 9e135e2 to 13ef02f Compare May 18, 2018 19:21

gbarr01 added the area/logging label May 24, 2018

thaJeztah requested changes May 26, 2018

View reviewed changes

joaofnfernandes self-requested a review June 15, 2018 23:55

cpuguy83 changed the title ~~[DO NOT MERGE] Add docs for local logging driver.~~ Add docs for local logging driver. Aug 20, 2018

cpuguy83 force-pushed the add_local_logger_docs branch from 13ef02f to b096073 Compare August 20, 2018 16:28

Add docs for local logging driver.

889153d

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

cpuguy83 force-pushed the add_local_logger_docs branch from b096073 to 889153d Compare August 20, 2018 16:32

tfoxnc requested review from tfoxnc and removed request for joaofnfernandes August 21, 2018 14:28

thaJeztah approved these changes Sep 6, 2018

View reviewed changes

paigehargrave merged commit 95550d8 into docker:vnext-engine Dec 28, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add docs for `local` logging driver. #6717

Add docs for `local` logging driver. #6717

cpuguy83 commented May 17, 2018

cpuguy83 commented May 17, 2018

anusha-ragunathan May 17, 2018

cpuguy83 May 18, 2018

anusha-ragunathan May 17, 2018

cpuguy83 May 17, 2018

anusha-ragunathan May 17, 2018

cpuguy83 May 18, 2018

anusha-ragunathan May 21, 2018

cpuguy83 May 21, 2018

anusha-ragunathan May 21, 2018 •

edited

Loading

anusha-ragunathan May 17, 2018 •

edited

Loading

cpuguy83 May 18, 2018

anusha-ragunathan May 21, 2018

anusha-ragunathan May 17, 2018

anusha-ragunathan May 17, 2018

thaJeztah commented May 26, 2018

thaJeztah May 26, 2018

cpuguy83 Aug 20, 2018

thaJeztah May 26, 2018

thaJeztah May 26, 2018

cpuguy83 Aug 20, 2018

thaJeztah May 26, 2018

cpuguy83 Aug 20, 2018

anusha-ragunathan commented Aug 20, 2018

cpuguy83 commented Aug 20, 2018

GordonTheTurtle commented Aug 20, 2018

GordonTheTurtle commented Aug 20, 2018

tfoxnc commented Aug 21, 2018

anusha-ragunathan commented Aug 21, 2018

thaJeztah left a comment



		The `local` logging driver captures output from container's stdout/stderr and
		writes them to a local file on disk. The format of these files is unspecified.

Add docs for local logging driver. #6717

Add docs for local logging driver. #6717

Conversation

cpuguy83 commented May 17, 2018

Proposed changes

Unreleased project version (optional)

cpuguy83 commented May 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anusha-ragunathan May 21, 2018 • edited Loading

Choose a reason for hiding this comment

anusha-ragunathan May 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah commented May 26, 2018

The short answer

The long answer:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anusha-ragunathan commented Aug 20, 2018

cpuguy83 commented Aug 20, 2018

GordonTheTurtle commented Aug 20, 2018

GordonTheTurtle commented Aug 20, 2018

tfoxnc commented Aug 21, 2018

anusha-ragunathan commented Aug 21, 2018

thaJeztah left a comment

Choose a reason for hiding this comment

Add docs for `local` logging driver. #6717

Add docs for `local` logging driver. #6717

anusha-ragunathan May 21, 2018 •

edited

Loading

anusha-ragunathan May 17, 2018 •

edited

Loading