chore(profiler): better compression testing and minor fixes#4697
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
🚀 New features to boost your workflow:
|
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 18fcb7c | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-04-29 07:56:33 Comparing candidate commit 18fcb7c in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 224 metrics, 0 unstable metrics.
|
### What does this PR do? Adds a functional gzip compression mode for the profiler via `DD_PROFILING_DEBUG_COMPRESSION_SETTINGS=gzip`. This was already kinda working, but not really, hence the "fix" title. - Defaults `gzip` to gzip-6, matching the gzip level legacy mode applies when it compresses profiler-produced profiles. - Adds gzip-to-gzip recompression so gzip mode works for runtime profiles that are already gzip-compressed at gzip-1. - Adds tests for gzip mode and gzip recompression. See #4697 for some follow-up cleanups. ### Motivation This is done for incident-53386, where zstd caused a memory usage increase. The gzip mode provides an alternative to the default zstd compression while keeping profiler uploads compressed. ### Reviewer's Checklist - [x] Changed code has unit tests for its functionality at or near 100% coverage. - [ ] [System-Tests](https://github.com/DataDog/system-tests/) covering this feature have been added and enabled with the va.b.c-dev version tag. - [ ] There is a benchmark for any new code, or changes to existing code. - [ ] If this interacts with the agent in a new way, a system test has been added. - [ ] New code is free of linting errors. You can check this by running `make lint` locally. - [x] New code doesn't break existing tests. You can check this by running `make test` locally. - [ ] Add an appropriate team label so this PR gets put in the right place for the release notes. - [ ] All generated files are up to date. You can check this by running `make generate` locally. - [ ] Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild. Make sure all nested modules are up to date by running `make fix-modules` locally. Tests run: - `go test ./profiler -run 'TestNewCompressionPipeline|TestDebugCompressionEnv/explicit-gzip' -count=1` Co-authored-by: felix.geisendoerfer <[email protected]>
Close gzip readers after io.Copy in the gzip recompression paths so any final decompressor error state is observed. This applies to both gzip->gzip and gzip->zstd recompression because both use a gzip reader for the input stream. This is not believed to fix any real issues, it's just done to follow the proper life cycle.
Both recompressors decompress gzip input via an io.Pipe and forward the decoded bytes to an output compressor; only the output type differed. Collapse them into a single gzipDecodingRecompressor parameterized over the compressor interface, which both *kgzip.Writer and *sharedZstdEncoder already satisfy.
If kgzip.NewReader fails (e.g. on a corrupt or non-gzip stream) the goroutine inside gzipDecodingRecompressor used to exit immediately, leaving io.Pipe with no reader. Any in-flight or subsequent pw.Write from the caller would then block forever, also preventing Close from ever draining the error channel. Close the read end of the pipe with the final error before sending it on the err channel, so pending and future writes fail fast with that error and the caller can complete normally. Includes a regression test that runs inside a testing/synctest bubble so the deadlock is detected deterministically rather than via a wall-clock timeout: synctest.Wait returns once every goroutine in the bubble is durably blocked or has exited, and the test then asserts that Write actually returned with an error.
6775043 to
18fcb7c
Compare
2743f8e
into
main
### What does this PR do? Adds a functional gzip compression mode for the profiler via `DD_PROFILING_DEBUG_COMPRESSION_SETTINGS=gzip`. This was already kinda working, but not really, hence the "fix" title. - Defaults `gzip` to gzip-6, matching the gzip level legacy mode applies when it compresses profiler-produced profiles. - Adds gzip-to-gzip recompression so gzip mode works for runtime profiles that are already gzip-compressed at gzip-1. - Adds tests for gzip mode and gzip recompression. See #4697 for some follow-up cleanups. ### Motivation This is done for incident-53386, where zstd caused a memory usage increase. The gzip mode provides an alternative to the default zstd compression while keeping profiler uploads compressed. ### Reviewer's Checklist - [x] Changed code has unit tests for its functionality at or near 100% coverage. - [ ] [System-Tests](https://github.com/DataDog/system-tests/) covering this feature have been added and enabled with the va.b.c-dev version tag. - [ ] There is a benchmark for any new code, or changes to existing code. - [ ] If this interacts with the agent in a new way, a system test has been added. - [ ] New code is free of linting errors. You can check this by running `make lint` locally. - [x] New code doesn't break existing tests. You can check this by running `make test` locally. - [ ] Add an appropriate team label so this PR gets put in the right place for the release notes. - [ ] All generated files are up to date. You can check this by running `make generate` locally. - [ ] Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild. Make sure all nested modules are up to date by running `make fix-modules` locally. Tests run: - `go test ./profiler -run 'TestNewCompressionPipeline|TestDebugCompressionEnv/explicit-gzip' -count=1` Co-authored-by: felix.geisendoerfer <[email protected]>
What does this PR do?
See commits.
Motivation
Follow-up cleanups to #4696 suggested by my clanker. Separate PR to make the main one easier to review.
Reviewer's Checklist
make lintlocally.make testlocally.make generatelocally.make fix-moduleslocally.Unsure? Have a question? Request a review!