Ralph/alp log querying benchmark (OTTL + fastjq + gojq + VRL) - #53809
Ralph/alp log querying benchmark (OTTL + fastjq + gojq + VRL)#53809rhy988 wants to merge 4 commits into
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6611647a4e
ℹ️ 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".
| "systemd", | ||
| "systemprobechecks", | ||
| "trivy", | ||
| "vrl", # benchmark-branch-only: compiles the real VRL cgo engine instead of the stub, see ralph/alp-vrl-log-processing-updated |
There was a problem hiding this comment.
Keep vrl out of default agent tags
With this tag in AGENT_TAGS, dda inv agent.build passes vrl by default (checked tasks/agent.py's build-tag path and tasks/libs/common/go.py's go build -tags invocation). That selects pkg/logs/vrl/vrl.go, whose cgo link flags require pkg/logs/vrl/rust/libvrl_filter.a, but this commit does not add that archive, so a fresh checkout or CI build that has not manually run //pkg/logs/vrl/rust:install_libs fails at link time. Keep this tag opt-in or make the Rust archive part of the normal build before enabling it by default.
Useful? React with 👍 / 👎.
| buf := make([]byte, 0, 4096) | ||
| return func(input []byte) (bool, error) { | ||
| matched := false | ||
| buf = buf[:0] |
There was a problem hiding this comment.
Remove the shared jq buffer state
When logs_config.pipelines > 1 (the default is 4), provider.Start passes the same compiled ProcessingRule slice to every processor, so all pipeline goroutines share this one JQFilter closure. The captured buf slice header is mutated on every log line and is not protected, creating a data race under concurrent jq filtering; since the buffer is never used by RunFunc, remove it or make all state per-call.
Useful? React with 👍 / 👎.
|
|
||
| var m map[string]any | ||
|
|
||
| err := json.Unmarshal(msg.GetContent(), &m) |
There was a problem hiding this comment.
Evaluate OTTL against the current rule content
If an OTTL rule follows a masking/VRL transform rule in the same ordered processing-rule list, this reparses msg.GetContent() even though earlier rules only update the local content variable and call msg.SetContent(content) after the loop. That makes OTTL conditions see the original unmasked log while regex/jq/VRL filters see prior rewrites, breaking rule ordering for configurations that combine transforms with OTTL filters; pass the current content into the OTTL evaluator instead.
Useful? React with 👍 / 👎.
| # Rust library (rlib), used by rust_test. | ||
| rust_library( | ||
| name = "vrl_filter", | ||
| srcs = glob(["src/**/*.rs"]), |
There was a problem hiding this comment.
Replace the recursive Rust source glob
The Bazel review guideline in bazel/codereview_guideline.md says to flag any glob with a ** pattern because recursive globs skip subdirectories once they gain their own BUILD file and hurt incrementality. This new target uses glob(["src/**/*.rs"]) (and repeats it below for the static library), so add explicit sources or package-level BUILD files instead.
Useful? React with 👍 / 👎.
Go Package Import DifferencesBaseline: 1e49b37
|
This comment has been minimized.
This comment has been minimized.
Files inventory check summaryFile checks results against ancestor 1e49b378: Results for datadog-agent_7.83.0~devel.git.320.7f1a93c.pipeline.126336548-1_amd64.deb:No change detected |
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 1e49b37 Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ❌ | logs_filter_ottl | % cpu utilization | +1824.77 | [+1811.69, +1837.85] | 1 | Logs |
| ❌ | logs_filter_jq | % cpu utilization | +440.24 | [+433.15, +447.34] | 1 | Logs |
| ➖ | logs_filter_baseline | % cpu utilization | +2.81 | [+1.38, +4.25] | 1 | Logs |
Fine details of change detection per experiment
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ❌ | logs_filter_ottl | % cpu utilization | +1824.77 | [+1811.69, +1837.85] | 1 | Logs |
| ❌ | logs_filter_jq | % cpu utilization | +440.24 | [+433.15, +447.34] | 1 | Logs |
| ➖ | quality_gate_private_action_runner | memory utilization | +3.52 | [+3.41, +3.64] | 1 | Logs bounds checks dashboard |
| ➖ | logs_filter_baseline | % cpu utilization | +2.81 | [+1.38, +4.25] | 1 | Logs |
| ➖ | quality_gate_idle | memory utilization | +2.00 | [+1.95, +2.06] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | +1.86 | [+1.77, +1.95] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | +1.85 | [+1.80, +1.90] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_idle | memory utilization | +1.79 | [+1.73, +1.85] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | +1.54 | [+1.50, +1.58] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | +0.41 | [+0.17, +0.65] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_logs | % cpu utilization | -0.51 | [-1.51, +0.48] | 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 | 3 ≤ 4 | bounds checks dashboard |
| ✅ | quality_gate_idle | memory_usage | 10/10 | 151.11MiB ≤ 154MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 744.47KiB ≤ 819.20KiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | intake_connections | 10/10 | 3 ≤ 4 | bounds checks dashboard |
| ❌ | quality_gate_idle_all_features | memory_usage | 0/10 | 507.36MiB > 495MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 1.13MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 3 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 188.67MiB ≤ 195MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 263.80MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 375.38 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 4 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 406.04MiB ≤ 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.93GiB ≤ 1.04GiB | bounds checks dashboard |
| ✅ | quality_gate_private_action_runner | memory_usage | 10/10 | 74MiB ≤ 75MiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 30.61 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 304.34MiB ≤ 330MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 72.28 ≤ 200 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 280.15MiB ≤ 310MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 23.14 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 282.46MiB ≤ 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 |
|---|---|---|---|---|
| logs_filter_baseline | baseline | 10 | Oom killed | Debug Dashboard |
| logs_filter_baseline | comparison | 10 | Oom killed | Debug Dashboard |
| logs_filter_jq | baseline | 10 | Oom killed | Debug Dashboard |
| logs_filter_jq | comparison | 10 | Oom killed | Debug Dashboard |
| logs_filter_ottl | baseline | 10 (x2) | Oom killed | Debug Dashboard |
| logs_filter_ottl | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle_all_features | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle_all_features | comparison | 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 | baseline | 10 | Crashed (exit code: 134) | Debug Dashboard |
| quality_gate_security_idle | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_security_mean_fs_load | baseline | 10 | Crashed (exit code: 134) | Debug Dashboard |
| quality_gate_security_mean_fs_load | comparison | 10 | Crashed (exit code: 134) | Debug Dashboard |
| quality_gate_security_no_fs_load | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_security_no_fs_load | comparison | 10 | Oom killed | Debug Dashboard |
CI Pass/Fail Decision
❌ Failed. Some Quality Gates were violated.
- quality_gate_metrics_logs, bounds check intake_connections: 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 cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check memory_usage: 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 intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, 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_private_action_runner, 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_security_mean_fs_load, 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_idle_all_features, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check memory_usage: 0/10 replicas passed. Failed 10 which is > 0. Gate FAILED.
- quality_gate_idle_all_features, bounds check intake_connections: 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 missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
…for benchmarking Rebased onto main (1e49b37) via file-replay: reset to fresh origin/main and reapplied every non-generated file this branch touches (source, Rust vendor tree, otel-patches/coreinternal vendor, SMP regression cases, build tooling), skipping go.mod/go.sum/Cargo.lock/MODULE.bazel.lock/go.work.sum for modules that already existed on main (those are reconciled by the next commit's `dda inv tidy` run instead). New modules' go.mod/go.sum (otel-patches/coreinternal, pkg/logs/vrl) are included as-is since main never had them and there's nothing to reconcile. Avoids replaying 18 rounds of the original branch's own go-mod-tidy/ gazelle-regen commits, whose only content was dependency-lockfile churn that always conflicted with 108 commits of unrelated lockfile churn on main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rk onto main Regenerates go.mod/go.sum/Cargo.lock/MODULE.bazel.lock/deps/go.MODULE.bazel and BUILD.bazel files across the workspace via `dda inv modules.add-all-replace` + `dda inv tidy`, to reconcile the two new local modules (otel-patches/coreinternal, pkg/logs/vrl) and the fastjq/ottl/vrl dependencies with the current state of main. internal/qbranch/anomalydetection-testbench/go.mod needed a hand-added replace for pkg/logs/vrl since `modules.add-all-replace` intentionally skips internal/* modules (tasks/modules.py). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extends the existing mask_vrl masking rule type to OTTL and jq, so the two other benchmark engines can redact log content the same way VRL's redact()-based mask_vrl already does: - mask_ottl compiles its pattern as an OTTL statement (e.g. replace_pattern(attributes["message"], "\d+", "[REDACTED]")) rather than a boolean condition. processor.go gains processOTTLTransform, which builds the same JSON->plog.LogRecord transform context processOTTLmsg already builds for filtering, executes the statement, and re-serializes the (possibly mutated) top-level scalar attributes back into the original document. Same attribute-flattening limitation as the existing OTTL filter rules: only top-level string/bool/number fields are visible. - mask_jq compiles the same way as the existing jq filter rules, but calls fastjq's Program.Run (which returns the transformed JSON bytes directly) instead of the boolean-only RunFunc path makeJQFilter uses. Both new rule types are fail-closed on any runtime error (non-JSON message, parse/eval error, or a jq program producing no output), mirroring mask_vrl's explicit fail-closed policy: a masking rule that can't run shouldn't fall back to shipping unredacted content. This is a deliberate divergence from the filter rule types (exclude/include_at_*), which are fail-open. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
9b75c00 to
35d2c7b
Compare
Adds a second, independent jq-based log-processing implementation backed by github.com/itchyny/gojq (the standard Go port of real jqlang.org jq semantics), alongside the existing fastjq-based exclude_at_jq_match/include_at_jq_match/mask_jq rule types. New rule types: exclude_at_gojq_match, include_at_gojq_match, mask_gojq. Filter rules fail open on non-JSON input or a jq runtime error; mask_gojq is fail-closed (drops the message on error or empty output), matching mask_vrl/mask_ottl's policy of never shipping unredacted content. Unlike fastjq (byte-level, literal-only sub/gsub replacements), gojq decodes into Go values and supports real jq sub/gsub with capture-group interpolation in replacements, at the cost of a decode/encode round-trip per log line — both meaningful, intentional differences for the engine-vs-engine benchmark. Numbers are decoded via json.Decoder.UseNumber() (gojq natively supports json.Number) rather than json.Unmarshal, so large integers (IDs/timestamps beyond 2^53) survive mask_gojq's whole-document re-serialization without precision loss. All new symbols/rule-type keys are GoJQ-prefixed specifically to avoid colliding with the fastjq-based ExcludeAtJQMatch/IncludeAtJQMatch/ MaskJQTransform already on this branch. gojq/timefmt-go were already indirect dependencies (via pkg/util/jsonquery, reused here for its Parse/cache helper) with existing LICENSE-3rdparty.csv entries, so no new third-party approval was needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
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! |
|
This pull request was automatically closed because it has been stale for 15 days with no activity. If this pull request is still relevant, please reopen it or create a new pull request with updated information. Thanks! |
What does this PR do?
Combines all structured-log filtering/masking implementations being benchmarked onto a
single branch/image, so they can be compared under identical conditions:
regex— the standard Datadog regex-based processing rules (the baseline):exclude_at_match/include_at_match/mask_sequencesOTTL— via the OpenTelemetry Collector transform processor:exclude_at_match_ottl/include_at_match_ottl/mask_ottlfastjq— Datadog's internal, pure-Go, zero-alloc jq subset:exclude_at_jq_match/include_at_jq_match/mask_jqgojq—github.com/itchyny/gojq, the standard Go port of real jqlang.orgjqsemantics:
exclude_at_gojq_match/include_at_gojq_match/mask_gojqVRL— via a curated cgo bridge (see below):exclude_at_vrl_match/include_at_vrl_match/mask_vrlAll five engines live on the same branch and are bundled into a single agent image. Each
mask/filter rule type follows the same fail-open (filters) / fail-closed (masking) policy:
a filter rule that can't evaluate (non-JSON content, a runtime error) passes the message
through unchanged, while a masking rule that can't run drops the message rather than risk
shipping unredacted content.
Why two jq engines?
fastjqis Datadog's own byte-level engine — fast, but a subset:sub/gsubonly support literal replacement strings (no capture-group backreferences).gojqis the standard, jqlang.org-faithful Go implementation — it decodes into Go values(so it pays a decode/encode round-trip fastjq avoids) but supports real jq
sub/gsubwith capture-group interpolation in replacements. Comparing them isolates "does the fuller
jq feature set cost more CPU" from "is jq itself competitive with OTTL/VRL/regex". (
gojqwas already an indirect dependency via
pkg/util/jsonquery, so no new third-partyapproval was needed — it's just promoted to a direct import here.)
Update: VRL is now included. It was originally dropped (see #53105, now closed/superseded
by this PR) after two blockers:
compilerfeature, notstdlib— meaningnone of VRL's standard library functions (
parse_json,contains, etc.) were actuallyavailable, so it couldn't do JSON-field-aware filtering at all. Enabling
stdlib/stdlib-basehits an unresolved Bazel/Cargo toolchain gap (native C buildscripts in some of VRL's dependencies fail to compile under this repo's hermetic LLVM
toolchain).
Both are now fixed: a custom curated function library (
parse_json,redact) sidestepsthe toolchain gap entirely (no
stdlibdependency needed), and there's now full testcoverage (
pkg/logs/vrl/vrl_test.go,comp/logs/agent/config/processing_rules_vrl_test.go,plus VRL-specific cases in
comp/logs-library/processor/processor_test.go).Also includes a benchmark-branch-only fix to
tasks/build_tags.bzl:vrlwas registeredin the tag validation allow-list (
ALL_TAGS) but not inAGENT_TAGS(the actual tag setused to build the "full" agent flavor), so the published image would have silently shipped
the VRL stub instead of the real cgo-backed engine. Fixed by adding
vrltoAGENT_TAGS.Motivation
We are evaluating which log filtering/masking implementation to adopt as a replacement or
complement to the current regex-based processing rules. OTTL, fastjq, gojq, and VRL each
offer different ergonomics/performance tradeoffs for structured/JSON logs vs regex, and
fastjq vs gojq specifically isolates the cost of a fuller jq feature set.
Running all cases in a single SMP job/image is a deliberate design choice: because they
share the same job, they run on the same machine in the same time window, making their CPU
and memory metrics directly comparable on the dashboard without any time series alignment.
Describe how you validated your changes
dda inv agent.build --build-include vrlsucceeds and links the real VRL cgoimplementation, OTTL, fastjq, and gojq together in one binary (confirmed via
./bin/agent/agent versionrunning clean post-build).comp/logs/agent/config(252tests) and
comp/logs-library/processor(100 tests), covering exclude/include/maskcorrectness, fail-open vs fail-closed behavior, rule ordering, and (for gojq
specifically) a regression test proving large integer fields (IDs/timestamps beyond
2^53) survive
mask_gojq's whole-document re-serialization without precision loss —json.Unmarshalalone would silently corrupt them.dda inv linter.goclean on all touched packages.multiple loads; VRL and gojq are being folded into that comparison next using this
branch's published image.
Additional Notes
gojq, and the OTTL/fastjq masking rule types included since their blockers are fixed /
they've since been added.