-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Enable loss less rotation of log files #2062
Conversation
Could change the base branch of the PR to 1.4? |
855bb2c
to
46b4f5b
Compare
Rebased and changed the base branch of the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Design LGTM
Thanks @marco-jantke
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
For the traefik logs this is accomplished by opening the new log file before closing the previous one. The traefik logs use the global logrus instance and it is already protected by a mutex on rotation. For the access logs such a mutex also had to be introduced.
46b4f5b
to
5eafa55
Compare
There is the chance that Traefik looses log lines for requests during rotation of the log file. This is at the moment true for the traefik log file as well as the access log files. I added two unit tests to prove this. This was also responsible for one flaky behaviour in the integration test here.
For the traefik logs this is accomplished by opening the new log file before closing the previous one. The traefik logs use the global
logrus
instance and it is already protected by a mutex on rotation. For the access logs such a mutex also had to be introduced.I think there is some potential in refactoring and aligning the two logging implementations in order to avoid duplicate code when it comes to the file handling and the logrus configuration. I will do this in a follow-up PR.