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

Update documentation to clarify the default format for logs #4953

Merged
merged 3 commits into from Jun 26, 2019
Merged

Update documentation to clarify the default format for logs #4953

merged 3 commits into from Jun 26, 2019

Conversation

dduportal
Copy link
Contributor

@dduportal dduportal commented Jun 13, 2019

Signed-off-by: dduportal 1522731+dduportal@users.noreply.github.com

What does this PR do?

This PR clarifies the documentation page about logs:

  • Adding comment to explicit that the default value is common and refers to the default CLF format
  • Clarifies that customizing the fields requires to switch to the JSON format

Motivation

This PR follows up #3942 and is inspired by #3870 where the user was not aware about the mandatory format switch, which could have avoided wasting time searching for a non existent configuration.

Closes #4869.

More

  • [ ] Added/updated tests
  • Added/updated documentation

Additional Notes

sheldon-nitpick

@dduportal
Copy link
Contributor Author

=> To be done: we need to check, there might be an issue classified as "bug/possible" around setting the format to the value common which does not work...

@dduportal
Copy link
Contributor Author

@ldez it looks like that the value common is the right one (by default):

Also, I tried to set accesslog.format (with CLI below, but also tried with traefik.toml) to common, and no error and the behavior is the same as only enabling accesslog alone

version: '3'

services:
  lb:
    image: traefik:v1.7.12
    restart: unless-stopped
    command: --api --docker --accesslog.format=common
    ports:
      - 80:80
      - 8080:8080
    volumes:
      - /var/run/docker.sock:/var/run/docker.sock
    labels:
      - traefik.enable=false

  webapp:
    image: containous/whoami
    labels:
      - "traefik.frontend.rule=Host:localhost"

I'm removing the bot/no-merge and this PR is ready to review (unless I missed something of course!)

Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez added the kind/enhancement a new or improved feature. label Jun 21, 2019
docs/configuration/logs.md Outdated Show resolved Hide resolved
docs/configuration/logs.md Outdated Show resolved Hide resolved
docs/configuration/logs.md Outdated Show resolved Hide resolved
docs/configuration/logs.md Outdated Show resolved Hide resolved
docs/configuration/logs.md Outdated Show resolved Hide resolved
docs/configuration/logs.md Outdated Show resolved Hide resolved
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@SantoDE SantoDE left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jbdoumenjou jbdoumenjou left a comment

Choose a reason for hiding this comment

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

LGTM

dduportal and others added 3 commits June 26, 2019 12:24
…e and how to customize fields

Signed-off-by: dduportal <1522731+dduportal@users.noreply.github.com>
(cherry picked from commit 57fe470dcb00a17f9a5558824d6a19b91d6b1e21)
Co-Authored-By: Ludovic Fernandez <ldez@users.noreply.github.com>
Co-Authored-By: Ludovic Fernandez <ldez@users.noreply.github.com>
@traefiker traefiker merged commit d35d5bd into traefik:v1.7 Jun 26, 2019
@dduportal dduportal deleted the doc/accesslog-clf branch June 27, 2019 11:03
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