-
-
Notifications
You must be signed in to change notification settings - Fork 217
output: add file rotation support #639
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
base: main
Are you sure you want to change the base?
Conversation
f0b4b4a to
4bfbc42
Compare
|
@brb |
|
Thanks, I will take a look in a bit! |
|
Have you considered using https://github.com/natefinch/lumberjack/tree/v3 or similar for traces rotation? |
|
@gyutaeb Please let me know if you plan to work on using the lumberjack pkg? |
@brb Sorry for the late reply. I originally implemented that rotation logic manually to have fine-grained control. However, after testing it with lumberjack as you suggested, I agree that it is much cleaner. I'm running final tests now and will push the updates ASAP! |
|
Perfect, thank you! Just one side note, maybe you could add some benchmarking tests to avoid any hidden costs by that pkg (we have some already in |
|
@brb |
4bfbc42 to
a8eef08
Compare
| for i := 0; i < b.N; i++ { | ||
| output.Print(event) | ||
| } | ||
| func BenchmarkOutputPrint(b *testing.B) { |
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.
❯ go test --bench=. ./internal/pwru/
goos: linux
goarch: arm64
pkg: github.com/cilium/pwru/internal/pwru
BenchmarkOutputPrint/DevNull-8 723668 1519 ns/op
BenchmarkOutputPrint/RegularFile-8 656631 1804 ns/op
BenchmarkOutputPrint/Lumberjack-8 613897 1833 ns/op
|
@brb Thanks for the review. I've switched to using lumberjack and added benchmark tests. |
Add automatic log file rotation when using --output-file flag. Files rotate at 100MB by default with configurable options: - --output-file-max-size: max size in MB before rotation (default 100) - --output-file-max-backups: max rotated files to keep (0 = no limit) - --output-file-max-age: max days to keep rotated files (0 = no limit) - --output-file-compress: compress rotated files with gzip Signed-off-by: Gyutae Bae <[email protected]>
b8212b5 to
fb49a97
Compare
| rightPadding := width - len(s) - leftPadding | ||
| return fmt.Sprintf("%s%s%s", strings.Repeat(" ", leftPadding), s, strings.Repeat(" ", rightPadding)) | ||
| } | ||
|
|
||
| func NewOutput(flags *Flags, printSkbMap, printShinfoMap, printStackMap, printBpfmapMap *ebpf.Map, addr2Name Addr2Name, skbMds, xdpMds []*SkbMetadata, kprobeMulti bool, btfSpec *btf.Spec) (*output, error) { | ||
| writer := os.Stdout | ||
| var writer io.Writer = os.Stdout | ||
| var closer io.Closer | ||
|
|
||
| if flags.OutputFile != "" { | ||
| file, err := os.Create(flags.OutputFile) | ||
| if err != nil { | ||
| return nil, err | ||
| lj := &lumberjack.Logger{ | ||
| Filename: flags.OutputFile, | ||
| MaxSize: flags.OutputFileMaxSize, | ||
| MaxAge: flags.OutputFileMaxAge, | ||
| MaxBackups: flags.OutputFileMaxBackups, | ||
| Compress: flags.OutputFileCompress, | ||
| LocalTime: true, | ||
| } | ||
| writer = file | ||
| writer = lj | ||
| closer = lj |
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.
I decided to drop support for file creation via os.Create, as it would introduce unnecessarily complexity to the code. I also believe we should standardize on lumberjack as the default moving forward.
Summary
--output-file-max-size,--output-file-max-files, and--output-file-compressflagsMotivation
When analyzing issues that require long-running packet tracing (e.g., intermittent connectivity problems, rare race conditions), the output file can grow indefinitely and exhaust disk space. This PR adds file rotation support to safely capture traces over extended periods.
New Flags
--output-file-max-size100M,1G)--output-file-max-files5--output-file-compressfalseExample
pwru --output-file trace.log \ --output-file-max-size 100M \ --output-file-max-files 10 \ --output-file-compress \ 'host 10.0.0.1'