Skip to content

Logging technical debt#3615

Merged
mdlinville merged 3 commits into
docker:masterfrom
mdlinville:logging-techdebt
Jun 20, 2017
Merged

Logging technical debt#3615
mdlinville merged 3 commits into
docker:masterfrom
mdlinville:logging-techdebt

Conversation

@mdlinville
Copy link
Copy Markdown

Proposed changes

This addresses several issues with area/engine and logging, though only one of them is actually logging!

PTAL @kencochrane @thaJeztah

@mdlinville mdlinville added this to the engine/17.06 milestone Jun 14, 2017
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! looking good; some minor nits, and suggestion to link to the daemon.json docs

Comment thread engine/admin/logging/awslogs.md Outdated
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. The following
example sets the log driver to `awslogs` and sets the `awslogs-region` option.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should link to the daemon.json reference in the dockerd CLI reference


The `aws-multiline-pattern` option defines a multiline start pattern using a
regular expression. This option is ignored if `awslogs-datetime-format` is also
configured.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should mention what this is useful for, or do you think it's clear from this? (usage can be an application that outputs, e.g. a stack-dump, which would otherwise be logged in multiple entries, and now allows you to capture it in a single log-entry. (could be better examples though)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread engine/admin/logging/gcplogs.md Outdated
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. The following
example sets the log driver to `gcplogs` and sets the `gcp-meta-name` option.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here (link)

Comment thread engine/admin/logging/gelf.md Outdated
|:-------------------------|:----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:--------------------------------------------------|
| `gelf-address` | The address of the GELF server. `udp` is the only supported URI specifier and you must specify the port. | `--log-opt gelf-address=udp://192.168.0.42:12201` |
| `gelf-compression-type` | The type of compression the GELF driver uses to compress each log message. Allowed values are `gzip`, `zlib` and `none`. The default is `gzip`. | `--log-opt gelf-compression-type=gzip` |
| `gelf-compression-level` | The level of compression when `gzip` or `zlib` is the `gelf-compression-type`. An integer in the range of `-1` to `9` (BestCompression). Default value is 1 (BestSpeed). Higher levels provide more compression at lower speed. | `--log-opt gelf-compression-level=2` |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 or 0 is "disable compression" (but not documented here); I know some people ran into issues with compression (used a lot of resources on high-traffic logging), so may be worth mentioning somewhere

Comment thread engine/swarm/services.md
this is not the case, or `localhost` does not resolve to an IP address on your
host, substitute the host's IP address or resolvable host name.

The HTML output is truncated:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this change is not related?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I explain it in the first comment that I just searched for all things that matched "logging" and for some reason those came up so I am fixing them here too. I'll rebase instead of squashing so they will show up as separate commits.

- Disk space used for the container's configuration files, which are typically
small.
- Memory written to disk (if swapping is enabled).
- Checkpoints, if you're using the experimental checkpoint/restore feature.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these changes be in this PR?

@kencochrane
Copy link
Copy Markdown
Contributor

Looks good to me, I had a few of the same comments/suggestions as @thaJeztah so when his comments are addressed, I should be good as well.

Thanks for cleaning them up.

@mdlinville mdlinville merged commit baaead7 into docker:master Jun 20, 2017
@mdlinville mdlinville deleted the logging-techdebt branch June 20, 2017 18:07
@mdlinville mdlinville mentioned this pull request Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants