Add support for log file compression #43
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
May 22, 2017 17:58
It is insufficient to just check the length of test files, especially given that many of the tests result in multiple files that have the same content/length. Instead, actually check that the file content is what it is expected to be. Vary the content that is being written so that the test failures become apparent. This also fixes a case where the length of the wrong value is checked following a write (it happens to work since the length of the value checked is the same as that written).
Simplify the timeFromName parsing (we only need to slice once, not twice) and actually parse the extracted time in the timeFromName function rather than returning an abitrary string that may or may not be a time. Also conver the timeFromName tests into table driven tests.
4a6f656c
force-pushed
the
compress
branch
2 times, most recently
from
afff456
to
179441b
Compare
Rather than scanning for old log files (under lock) when a rotation occurs, a goroutine is started when we first open or create a log file. Post-rotation compression (if enabled) and removal of stale log files is now designated to this goroutine. Scanning, removal and compression are run in the same goroutine in order to minimise background disk I/O, with removals being processed prior to compression in order to free up disk space. This results in a small change in existing behaviour - previously only logs would be removed when the first rotation occurs, whereas now logs will potentially be removed when logging first starts.
|
First two commits look good, thanks for the cleanup. Looking at the compression code now (obviously a bigger and more complicate change). |
|
Ack, thanks. |
|
@natefinch any further progress? Anything I can assist with or clarify? |
|
Hey @natefinch, was really hoping to get this into Juju 2.2-rc1 (tomorrow). |
|
@howbazaar crap, sorry. Reviewing now. |
|
Thanks @natefinch, I really appreciate it. |
|
@4a6f656c tested this with Juju and the fill-logs charm that the CI test uses. We'd want the rolled file to have the same owner/group as the source file. |
|
@natefinch I don't suppose you recall how the syslog user is set as the file owner/group in Juju do you? |
|
ahh yeah, the file is initially created by something else as syslog, I forget what, and there's code in lumberjack when it rotates a file to copy over the mode and chown it to what the last file was. That mode and chown code will need to be reused for the zipped file. See this chunk here: https://github.com/natefinch/lumberjack/blob/v2.0/lumberjack.go#L206 Other than that, this looks good. |
added 2 commits
June 1, 2017 01:46
Previously the test only verified that the code called Chown but failed to verify what it actually called Chown on. This reworks the code so that we have a fake file system that tracks file ownership. This also simplifies upcoming additional tests.
Clone the log file owner and the log file mode to the compressed log file. Add tests to ensure that this is handled correctly.
|
@natefinch, @howbazaar - done. |
|
Thanks for all your work, Joel. This looks really good. |
fern4lvarez
added a commit
to oscillatingworks/caddy
that referenced
this pull request
Jun 27, 2017
This directive will enable gzip compression provided by [Lumberjack](natefinch/lumberjack#43). The directive `rotate_compress` can be `true` or `false`, being `false` by default.
fern4lvarez
added a commit
to oscillatingworks/caddy
that referenced
this pull request
Jun 27, 2017
This directive will enable gzip compression provided by [Lumberjack](natefinch/lumberjack#43). The directive `rotate_compress` can be `true` or `false`, being `false` by default.
fern4lvarez
added a commit
to oscillatingworks/caddy
that referenced
this pull request
Jun 28, 2017
This directive will enable gzip compression provided by [Lumberjack](natefinch/lumberjack#43). The directive `rotate_compress` can be `true` or `false`, being `false` by default.
mholt
pushed a commit
to caddyserver/caddy
that referenced
this pull request
Jun 28, 2017
* vendor: update Lumberjack dep * httpserver/roller: introduce rotate_compress directive This directive will enable gzip compression provided by [Lumberjack](natefinch/lumberjack#43). The directive `rotate_compress` can be `true` or `false`, being `false` by default. * httpserver/roller: remove need to set bool with rotate_compress option
yangchangshun
pushed a commit
to yangchangshun/lumberjack
that referenced
this pull request
Nov 22, 2018
* Check test file content, not just length. It is insufficient to just check the length of test files, especially given that many of the tests result in multiple files that have the same content/length. Instead, actually check that the file content is what it is expected to be. Vary the content that is being written so that the test failures become apparent. This also fixes a case where the length of the wrong value is checked following a write (it happens to work since the length of the value checked is the same as that written). * Make timeFromName actually return a time. Simplify the timeFromName parsing (we only need to slice once, not twice) and actually parse the extracted time in the timeFromName function rather than returning an abitrary string that may or may not be a time. Also conver the timeFromName tests into table driven tests. * Add support for compressing log files. Rather than scanning for old log files (under lock) when a rotation occurs, a goroutine is started when we first open or create a log file. Post-rotation compression (if enabled) and removal of stale log files is now designated to this goroutine. Scanning, removal and compression are run in the same goroutine in order to minimise background disk I/O, with removals being processed prior to compression in order to free up disk space. This results in a small change in existing behaviour - previously only logs would be removed when the first rotation occurs, whereas now logs will potentially be removed when logging first starts. * Rework file ownership test. Previously the test only verified that the code called Chown but failed to verify what it actually called Chown on. This reworks the code so that we have a fake file system that tracks file ownership. This also simplifies upcoming additional tests. * Clone file owner and mode on compressed log. Clone the log file owner and the log file mode to the compressed log file. Add tests to ensure that this is handled correctly.
xukgo
pushed a commit
to xukgo/lumberjack
that referenced
this pull request
Mar 5, 2020
* Check test file content, not just length. It is insufficient to just check the length of test files, especially given that many of the tests result in multiple files that have the same content/length. Instead, actually check that the file content is what it is expected to be. Vary the content that is being written so that the test failures become apparent. This also fixes a case where the length of the wrong value is checked following a write (it happens to work since the length of the value checked is the same as that written). * Make timeFromName actually return a time. Simplify the timeFromName parsing (we only need to slice once, not twice) and actually parse the extracted time in the timeFromName function rather than returning an abitrary string that may or may not be a time. Also conver the timeFromName tests into table driven tests. * Add support for compressing log files. Rather than scanning for old log files (under lock) when a rotation occurs, a goroutine is started when we first open or create a log file. Post-rotation compression (if enabled) and removal of stale log files is now designated to this goroutine. Scanning, removal and compression are run in the same goroutine in order to minimise background disk I/O, with removals being processed prior to compression in order to free up disk space. This results in a small change in existing behaviour - previously only logs would be removed when the first rotation occurs, whereas now logs will potentially be removed when logging first starts. * Rework file ownership test. Previously the test only verified that the code called Chown but failed to verify what it actually called Chown on. This reworks the code so that we have a fake file system that tracks file ownership. This also simplifies upcoming additional tests. * Clone file owner and mode on compressed log. Clone the log file owner and the log file mode to the compressed log file. Add tests to ensure that this is handled correctly.
crabio
added a commit
to crabio/woodpecker
that referenced
this pull request
Aug 22, 2021
* v2 is go! * update readme and mention gopkg.in in godoc * remove travis.yml, update badges, use drone.io for builds * fix link in badge * fix link in badge * comment to make MaxAge units more obvious * fix a spot where an error was not properly returned * add changes to maintain perms and owner of logfile * fix test failures on windows * Update README.md add badge for windows build * Fixed import in example test to use gopkg.in. * Fix bug natefinch#12 Fixes bug natefinch#12. If the first write to a file would cause it to rotate, instead of rotating, we'd just move it aside. This change fixes that problem by ensuring that we just run rotate in this situation, which does the right thing (open new and then cleanup.) Also added test to verify the fix. * add coverage badge * Switch to using gopkg.in/yaml.v2 * Update rotate_test.go to use v2 of project Hi there. I thought it would be nice for the rotate example to use v2 of the package. * Use gopkg.in provider instead of github * fix filemode in tests (natefinch#28) This fixes natefinch#20 by using a more restrictive filemode during tests. * update docs w/ backup format info * Add support for log file compression (natefinch#43) * Check test file content, not just length. It is insufficient to just check the length of test files, especially given that many of the tests result in multiple files that have the same content/length. Instead, actually check that the file content is what it is expected to be. Vary the content that is being written so that the test failures become apparent. This also fixes a case where the length of the wrong value is checked following a write (it happens to work since the length of the value checked is the same as that written). * Make timeFromName actually return a time. Simplify the timeFromName parsing (we only need to slice once, not twice) and actually parse the extracted time in the timeFromName function rather than returning an abitrary string that may or may not be a time. Also conver the timeFromName tests into table driven tests. * Add support for compressing log files. Rather than scanning for old log files (under lock) when a rotation occurs, a goroutine is started when we first open or create a log file. Post-rotation compression (if enabled) and removal of stale log files is now designated to this goroutine. Scanning, removal and compression are run in the same goroutine in order to minimise background disk I/O, with removals being processed prior to compression in order to free up disk space. This results in a small change in existing behaviour - previously only logs would be removed when the first rotation occurs, whereas now logs will potentially be removed when logging first starts. * Rework file ownership test. Previously the test only verified that the code called Chown but failed to verify what it actually called Chown on. This reworks the code so that we have a fake file system that tracks file ownership. This also simplifies upcoming additional tests. * Clone file owner and mode on compressed log. Clone the log file owner and the log file mode to the compressed log file. Add tests to ensure that this is handled correctly. * switch to travis (natefinch#44) * Update docs, adding `Compress` setting details (natefinch#49) * Fix test timing (natefinch#64) fix test timeout on CI * Make default file permissions more restrictive (natefinch#83) This asures that the process can still read and write its own log file, but that other users cannot. This is a fairly standard mode for log files in linux. * fix a typo (natefinch#62) * use 0755 to create new dir (natefinch#68) * cleanup and module support (natefinch#77) * cleanup and module support * add rotate everyday * remove unused code in unit tests * fix rotate everyday Co-authored-by: Nate Finch <nate.finch@gmail.com> Co-authored-by: Matt Silverlock <matt@eatsleeprepeat.net> Co-authored-by: Martin Packman <martin.packman@canonical.com> Co-authored-by: Tim Potter <tpot@samba.org> Co-authored-by: Joel Sing <joel@sing.id.au> Co-authored-by: Tyler Butters <dapegral@gmail.com> Co-authored-by: Juan Osorio Robles <jaosorior@gmail.com> Co-authored-by: 康晓宁 <kxnmei@163.com> Co-authored-by: Deen <englanq@126.com> Co-authored-by: Lukas Rist <glaslos@gmail.com>
chancez
added a commit
to cilium/lumberjack
that referenced
this pull request
Mar 14, 2022
* cilium/v2.0: cleanup and module support (natefinch#77) use 0755 to create new dir (natefinch#68) fix a typo (natefinch#62) Make default file permissions more restrictive (natefinch#83) Fix test timing (natefinch#64) Update docs, adding `Compress` setting details (natefinch#49) switch to travis (natefinch#44) Add support for log file compression (natefinch#43) Signed-off-by: Chance Zibolski <chance.zibolski@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds support for compressing rotated log files.
Several other clean ups and specifically test improvements are included as
separate commits.
Fixes issue #13