[otel-agent] Use zstd compression for all signals - #52758
[otel-agent] Use zstd compression for all signals#52758gh-worker-dd-mergequeue-cf854d[bot] merged 10 commits into
Conversation
7a7b1a1 to
044b22b
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Go Package Import DifferencesBaseline: d547014
|
|
🎯 Code Coverage (details) 🔗 Commit SHA: e47d8fc | Docs | Datadog PR Page | Give us feedback! |
Files inventory check summaryFile checks results against ancestor d547014c: Results for datadog-agent_7.84.0~devel.git.134.e47d8fc.pipeline.130417074-1_amd64.deb:No change detected Results for datadog-iot-agent_7.84.0~devel.git.134.e47d8fc.pipeline.130417074-1_amd64.deb:No change detected |
Static quality checks✅ Please find below the results from static quality gates 33 successful checks with minimal change (< 2 KiB)
|
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 5ba66e3 Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_private_action_runner | memory utilization | +0.11 | [-0.02, +0.23] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | +0.01 | [-0.04, +0.05] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | -0.08 | [-0.15, -0.01] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | -0.09 | [-0.24, +0.05] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle | memory utilization | -0.17 | [-0.29, -0.05] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_idle | memory utilization | -0.23 | [-0.34, -0.12] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_logs | % cpu utilization | -0.50 | [-1.36, +0.35] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | -1.36 | [-1.61, -1.11] | 1 | Logs bounds checks dashboard |
Bounds Checks: ❌ Failed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | quality_gate_idle | intake_connections | 10/10 | 4 = 4 | bounds checks dashboard |
| ❌ | quality_gate_idle | memory_usage | 0/10 | 174.59MiB > 154MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 730.76KiB ≤ 819.20KiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | intake_connections | 10/10 | 4 = 4 | bounds checks dashboard |
| ❌ | quality_gate_idle_all_features | memory_usage | 8/10 | 514.18MiB > 512MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 1.12MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 18 ≤ 40 | bounds checks dashboard |
| ❌ | quality_gate_logs | memory_usage | 1/10 | 205.66MiB > 195MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 264.18MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 354.48 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 18 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 389.49MiB ≤ 430MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | total_bytes_received | 10/10 | 0.94GiB ≤ 1.04GiB | bounds checks dashboard |
| ✅ | quality_gate_private_action_runner | memory_usage | 10/10 | 72.41MiB ≤ 75MiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 31.82 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 323.55MiB ≤ 330MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 70.76 ≤ 200 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 302.41MiB ≤ 310MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 23.03 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 315.53MiB ≤ 320MiB | bounds checks dashboard |
Explanation
Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%
Performance changes are noted in the perf column of each table:
- ✅ = significantly better comparison variant performance
- ❌ = significantly worse comparison variant performance
- ➖ = no significant change in performance
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
Replicate Execution Details
We run multiple replicates for each experiment/variant. However, we allow replicates to be automatically retried if there are any failures, up to 8 times, at which point the replicate is marked dead and we are unable to run analysis for the entire experiment. We call each of these attempts at running replicates a replicate execution. This section lists all replicate executions that failed due to the target crashing or being oom killed.
Note: In the below tables we bucket failures by experiment, variant, and failure type. For each of these buckets we list out the replicate indexes that failed with an annotation signifying how many times said replicate failed with the given failure mode. In the below example the baseline variant of the experiment named experiment_with_failures had two replicates that failed by oom kills. Replicate 0, which failed 8 executions, and replicate 1 which failed 6 executions, all with the same failure mode.
| Experiment | Variant | Replicates | Failure | Logs | Debug Dashboard |
|---|---|---|---|---|---|
| experiment_with_failures | baseline | 0 (x8) 1 (x6) | Oom killed | Debug Dashboard |
The debug dashboard links will take you to a debugging dashboard specifically designed to investigate replicate execution failures.
❌ Retried Profiling Replicate Execution Failures (ddprof)
Note: Profiling replicas may still be executing. See the debug dashboard for up to date status.
| Experiment | Variant | Replicates | Failure | Debug Dashboard |
|---|---|---|---|---|
| quality_gate_idle_all_features | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_logs | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_logs | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_security_idle | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_security_mean_fs_load | comparison | 10 | Crashed (exit code: 134) | Debug Dashboard |
CI Pass/Fail Decision
❌ Failed. Some Quality Gates were violated.
- quality_gate_metrics_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check memory_usage: 8/10 replicas passed. Failed 2 which is > 0. Gate FAILED.
- quality_gate_private_action_runner, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check memory_usage: 0/10 replicas passed. Failed 10 which is > 0. Gate FAILED.
- quality_gate_logs, bounds check memory_usage: 1/10 replicas passed. Failed 9 which is > 0. Gate FAILED.
- quality_gate_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
a5055d3 to
e69c344
Compare
The DDOT collector (otel-agent) previously used a different compressor per signal: zlib for metrics, gzip for traces, and zstd for logs. Switch all signals to zstd for a consistent compression algorithm. - Metrics: use the config-driven metrics compressor (metricscompression/fx) instead of the hardcoded zlib module; defaults to zstd level 3, overridable via serializer_zstd_compressor_level. - Logs: pin zstd and default zstd_compression_level to 3 (overridable via logs_config.zstd_compression_level). - Traces: use the existing fx-zstd module (zstd at BestSpeed); the trace compression level is intentionally fixed. The v2 metrics intake is preserved (use_v3_api.series.enabled stays false); moving series to v3 is a separate effort. Changes are scoped to cmd/otel-agent and do not affect the core Agent, trace-agent, host-profiler, or OSS exporter. Tests: - Unit guard that metrics and logs resolve to the same algorithm and the level stays overridable. - Integration test verifies traces ship valid zstd. - E2E test asserts metrics, traces, and logs reach fakeintake as zstd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
e69c344 to
af1aa56
Compare
|
This pull request has been automatically marked as stale because it has not had activity in the past 15 days. It will be closed in 30 days if no further activity occurs. If this pull request is still relevant, adding a comment or pushing new commits will keep it open. Also, you can always reopen the pull request if you missed the window. Thank you for your contributions! |
Merging main surfaced two breakages (the diff was clean, but referenced symbols/deps changed underneath it): - pkgconfigsetup.DefaultCompressorKind / DefaultLogCompressionKind moved to the pkg/config/setup/constants package; reference them there. - Regenerate BUILD.bazel for the fx-otel->fx and fx-gzip->fx-zstd import swaps and the new DataDog/zstd test dependency (gazelle). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…azel The constants import added to agent_config.go requires the corresponding gazelle dep in cmd/otel-agent/config/BUILD.bazel. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…compressors Make the compression tests validate the *expected* compressor per signal rather than assuming zstd, so they track configurable compression: - e2e utils.TestCompression is now expected-compressor-driven (SignalCompression + DefaultDDOTCompression), adds sketches (/api/beta/sketches) and APM stats (/api/v0.2/stats, gzip) as assert-if-present, and logs which signals were actually observed so an absent optional signal is not a silent pass. - unit TestDDOTSupportedCompressors covers every supported algorithm per signal (metrics: zstd/zlib/gzip/none; logs: zstd/gzip): asserts each is selectable via the DD_* config surface and pins the on-the-wire Content-Encoding each emits (zstd->zstd, zlib->deflate, gzip->gzip, none->identity). Traces are compile-time (fx module), out of scope for the config-level test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
truthbk
added
qa/rc-required
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddfffac979
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| logscompressionfx "github.com/DataDog/datadog-agent/comp/serializer/logscompression/fx" | ||
| metricscompression "github.com/DataDog/datadog-agent/comp/serializer/metricscompression/def" | ||
| metricscompressionfx "github.com/DataDog/datadog-agent/comp/serializer/metricscompression/fx-otel" | ||
| metricscompressionfx "github.com/DataDog/datadog-agent/comp/serializer/metricscompression/fx" |
There was a problem hiding this comment.
With this production switch to the config-driven metrics compressor, the in-process DDOT integration test still imports and wires metricscompression/fx-otel, which hardcodes zlib, while only traces were switched to zstd there. In PR/local integration runs this leaves the new metrics /api/v2/series zstd path unexercised until the E2E suite, so regressions in the changed production compressor can slip through; mirror this module in the integration harness and assert the metric Content-Encoding as well.
Useful? React with 👍 / 👎.
|
The outdated note in comp/otelcol/otlp/components/exporter/serializerexporter/serializer.go:93 (“forces zlib compression”) is misleading when reading the code — trust what is actually on the wire (Content-Encoding), not the comment. |
…harness Address Codex review feedback: the in-process integration harness wired the hardcoded-zlib metrics compressor (metricscompression/fx-otel) while production DDOT uses the config-driven module, so the metrics /api/v2/series zstd path was only covered by the (non-per-PR) e2e suite. - Wire metricscompression/fx (config-driven), exactly as command.go does. The harness config comes from NewConfigComponent (serializer_compressor_kind=zstd). - Add zlib,zstd to the integration test build tags. selector.FromConfig is //go:build zlib && zstd gated; without those tags it links its noop variant and metrics ship "identity" (uncompressed). These match the shipped OTEL_AGENT_TAGS. - Record /api/v2/series and /api/beta/sketches with non-blocking handlers (respond 200 immediately, capture Content-Encoding) and assert a metric payload ships zstd. Blocking channel recorders would stall the metrics forwarder and server.Close(). bazel does not run this test (//go:build !race excludes it from the --config=gorace bazel jobs), so gotags_sets stays [["otlp"]]. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… feedback) Address RiantZ review feedback: - getZstdReader returned io.Reader, hiding Close() on a cgo-backed io.ReadCloser (a ZSTD_DStream* plus a pooled buffer). Each trace-loop iteration leaked the C allocation and drained a buffer from the shared sync.Pool. Return io.ReadCloser and Close() immediately after reading. - Replace the zstd level literal 3 duplicated across four sites with a named const ddotZstdCompressionLevel in agent_config.go, referenced by the two config Set calls and the two test assertions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The comment read "forces zlib compression" right after setting serializer_compressor_kind=zstd, which is contradictory on the page. The exporter wires metricscompression/fx-otel (NewCompressorReqOtel -> zlib.New()), which hardcodes zlib and ignores serializer_compressor_kind, so series go out zlib-compressed (Content-Encoding: deflate) and v3 stays disabled. Spell that out so the wire behavior matches the comment. Per RiantZ review feedback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@RiantZ re: the |
github-actions
Bot
added
long review
Reword the compression note to be explicit that setupSerializer/initSerializerInternal run only for the standalone OSS Datadog exporter (opentelemetry-collector-contrib), the single path where serializerexporter builds its own serializer (f.s == nil). DDOT and core-Agent OTLP ingestion inject an already-built serializer and never reach it. State plainly that the fx-otel compressor hardcodes zlib.New(), so the serializer_compressor_kind=zstd set just above is inert and series ship deflate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
The expected merge time in
|
4 participants
What does this PR do?
Makes the DDOT collector (
otel-agent) compress every signal withzstd, replacing the previous per-signal mix (zlib for metrics, gzip for traces, zstd for logs):metricscompression/fx) instead of the hardcoded zlib module; defaults to zstd level 3, overridable viaserializer_zstd_compressor_level.logs_config.zstd_compression_levelto 3 (overridable).fx-zstdmodule (zstd atBestSpeed); the trace level is intentionally fixed.Also adds tests that guard against future per-signal compression divergence.
Motivation
The DDOT exporter used a different compression algorithm per signal, which is inconsistent and harder to reason about. Standardizing on
zstdgives a consistent algorithm across metrics/traces/logs and a better compression ratio, with the level configurable per signal.Describe how you validated your changes
dda inv otel-agent.buildpasses;dda inv linter.go→ 0 issues on changed packages;gofmtclean.cmd/otel-agent/config): asserts metrics and logs resolve to the same algorithm (zstd) at level 3, that the level stays env-overridable, and that host metadata uses that same serializer compressor (so it is zstd too).comp/otelcol/otlp/integrationtest): now decodes trace payloads as zstd — verifies traces ship valid zstd end-to-end. Passes locally (dda inv otel-agent.integration-test).TestOTelAgentComplete/TestOTLPCompression): asserts metrics, traces, and logs reach fakeintake withContent-Encoding: zstd(requires the e2e infra to run).Additional Notes
otel-agent. The core Agent, trace-agent, host-profiler, and OSS exporter are unaffected — they use their own compression modules (metricscompression/fx,fx-zstd,fx-gzip,fx-otelrespectively).use_v3_api.series.enabledstaysfalse); moving series to v3 is a separate effort./api/v2/series) acceptsContent-Encoding: zstd— the core Agent uses zstd→v3 for series by default, so this exact combination isn't exercised today. The new e2e test validates it against fakeintake.pkg/trace/writer/stats.go, shared with the trace-agent; out of scope here, separate conversation). The orchestrator/K8s-objects sub-exporter already uses zstd and is disabled by default. Internal exporter telemetry (metricsclient, COAT/gateway gauges) is scraped Prometheus/OTel-meter telemetry, not an intake submission.🤖 Generated with Claude Code