-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Don't timestamp active log file #11070
Conversation
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.
Thanks @henrikjohansen for reporting the issue and @lgfa29 for fixing it!
* don't timestamp active log file * website: update log_file default value * changelog: add entry for #11070 * website: add upgrade instructions for log_file in v1.14 and v1.2.0
* don't timestamp active log file * website: update log_file default value * changelog: add entry for #11070 * website: add upgrade instructions for log_file in v1.14 and v1.2.0
This patch updates the file_sink to *not* append the timestamp on the current log file. The timestamp is only appended when the file is rotated. This matches the behavior: - Implemented in Nomad v1.1.4 with hashicorp/nomad#11070 - Requested in hashicorp/nomad#11061 - Referenced in Consul's documentation: https://www.consul.io/docs/enterprise/audit-logging > The following example configures a destination called "My Sink" which > stores audit events at the file `/tmp/audit.json`. Nomad's documentation is a little vague but implies the prior behavior of always appending a timestamp. While this is a *backward incompatible change* I think it's worth it for usability and matching Consul's documentation. Nomad's documentation can be fixed and a notice placed in the Upgrade Guide.
Backward compatible version of #62 -- This version does *not* change the default behavior. Users must opt in to new behavior. This patch updates the file sink to allow *not* appending the timestamp on the current log file. In this mode the timestamp is only appended when the file is rotated. This matches the behavior: - Implemented in Nomad v1.1.4 with hashicorp/nomad#11070 - Requested in hashicorp/nomad#11061 - Referenced in Consul's documentation: https://www.consul.io/docs/enterprise/audit-logging > The following example configures a destination called "My Sink" which > stores audit events at the file `/tmp/audit.json`. Nomad's documentation is a little vague but implies the prior behavior of always appending a timestamp. While this is a *backward incompatible change* I think it's worth it for usability and matching Consul's documentation. Nomad's documentation can be fixed and a notice placed in the Upgrade Guide. In a bid to try to win your support I labeled this PR a bug despite the tests and comments being very clear the old behavior (always append a timestamp) *was* intentional.
Backward compatible version of #62 -- This version does *not* change the default behavior. Users must opt in to new behavior. This patch updates the file sink to allow *not* appending the timestamp on the current log file. In this mode the timestamp is only appended when the file is rotated. This matches the behavior: - Implemented in Nomad v1.1.4 with hashicorp/nomad#11070 - Requested in hashicorp/nomad#11061 > The following example configures a destination called "My Sink" which > stores audit events at the file `/tmp/audit.json`. Nomad's documentation is a little vague but implies the prior behavior of always appending a timestamp.
* allow not appending timestamp to current log file Backward compatible version of #62 -- This version does *not* change the default behavior. Users must opt in to new behavior. This patch updates the file sink to allow *not* appending the timestamp on the current log file. In this mode the timestamp is only appended when the file is rotated. This matches the behavior: - Implemented in Nomad v1.1.4 with hashicorp/nomad#11070 - Requested in hashicorp/nomad#11061 > The following example configures a destination called "My Sink" which > stores audit events at the file `/tmp/audit.json`. Nomad's documentation is a little vague but implies the prior behavior of always appending a timestamp. Also fixed one flaky test: * time may be either the same of after While Go compares time using a monotonic clock that only prevents time from moving *backward,* it does *not* guarantee time will move forward between subsequent calls to `time.Now()`. The After assertion failed in a test run on Github because it is missing the `Equal` comparison on the preceeding assertion.
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
The
log_file
configuration option allows setting the log file name to use. In practice, however, this value was being used as a filename prefix with a timestamp appended.This PR changes the log file creation logic to always use the value set in
log_file
as the active log file name.Closes #11061 #9582