Skip to content

feat(profiler): switch to zstd compression by default#3970

Merged
dd-mergequeue[bot] merged 2 commits into
mainfrom
felix.geisendoerfer/PROF-11815-profiling-enable-zstd-by-default
Sep 17, 2025
Merged

feat(profiler): switch to zstd compression by default#3970
dd-mergequeue[bot] merged 2 commits into
mainfrom
felix.geisendoerfer/PROF-11815-profiling-enable-zstd-by-default

Conversation

@felixge

@felixge felixge commented Sep 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

What does this PR do?

This should decrease the amount of egress traffic from the profiler by 20-30%.

If necessary, the old compression strategy (a mix of gzip-1, gzip-6 and none) can be restored by setting the following environment variable:

DD_PROFILING_DEBUG_COMPRESSION_SETTINGS=legacy

Motivation

Reduce network egress for customers and ingress for Datadog. See JIRA.

Reviewer Notes

This is the minimal viable patch to achieve the goal. Some of the existing tests made strong assumptions about the legacy compression strategy, so I fixed them up via DD_PROFILING_DEBUG_COMPRESSION_SETTINGS=legacy for now. We could invest in refactoring them to work regardless of the default compression. I think it's unnecessary, but could be convinced otherwise.

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • 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 ./scripts/lint.sh locally.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

@felixge
feat(profiler): switch to zstd compression by default
9fae39f 
This should decrease the amount of egress traffic from the profiler by
20-30%.

If neccessary, the old compression strategy (a mix of gzip-1, gzip-6
and none) can be restored by setting the following environment variable:

DD_PROFILING_DEBUG_COMPRESSION_SETTINGS=legacy
@felixge
felixge requested a review from a team as a code owner September 16, 2025 06:20
@pr-commenter

pr-commenter Bot commented Sep 16, 2025 •
edited
Loading

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2025-09-16 18:00:23

Comparing candidate commit f825e9e in PR branch felix.geisendoerfer/PROF-11815-profiling-enable-zstd-by-default with baseline commit ce9b743 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 24 metrics, 0 unstable metrics.

@felixge

felixge commented Sep 16, 2025

Copy link
Copy Markdown
Member Author

Note encountered a potential flake. Re-running.

2025-09-16 feat(profiler) switch to zstd compression by default · DataDogdd-trace-go@9fae39f at 08 23 39@2x

@felixge

felixge commented Sep 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Another flake:

2025-09-16 feat(profiler) switch to zstd compression by default · DataDogdd-trace-go@9fae39f at 08 38 35@2x

Edit: Took a shot at this, see: #3971

@felixge

felixge commented Sep 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Got the same flake I got earlier again.

2025-09-16 feat(profiler) switch to zstd compression by default · DataDogdd-trace-go@9fae39f at 08 50 29@2x

Edit: Turns out this is a hard fail, not a flake. But don't have time to fix right now :(

@felixge
felixge requested a review from nsrip-dd September 16, 2025 08:37
@felixge

felixge commented Sep 16, 2025

Copy link
Copy Markdown
Member Author

/merge

@dd-devflow-routing-codex

dd-devflow-routing-codex Bot commented Sep 16, 2025 •
edited
Loading

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2025-09-16 17:50:11 UTC ℹ️ Start processing command /merge

2025-09-16 17:50:26 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2025-09-16 21:51:08 UTC ⚠️ MergeQueue: This merge request was unqueued

devflow unqueued this merge request: It did not become mergeable within the expected time

@felixge

felixge commented Sep 16, 2025

Copy link
Copy Markdown
Member Author

More flakes again 😢

@felixge

felixge commented Sep 17, 2025

Copy link
Copy Markdown
Member Author

/merge

@dd-devflow-routing-codex

dd-devflow-routing-codex Bot commented Sep 17, 2025 •
edited
Loading

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2025-09-17 04:58:11 UTC ℹ️ Start processing command /merge

2025-09-17 04:58:16 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 16m (p90).

2025-09-17 05:13:11 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue
dd-mergequeue Bot merged commit 5abe3af into main Sep 17, 2025
269 of 273 checks passed
@dd-mergequeue
dd-mergequeue Bot deleted the felix.geisendoerfer/PROF-11815-profiling-enable-zstd-by-default branch September 17, 2025 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nsrip-dd nsrip-dd nsrip-dd approved these changes

Assignees

No one assigned

Labels

mergequeue-status: done

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@felixge @nsrip-dd