Ralph/alp log querying benchmark (OTTL + jq + VRL)#53809
Draft
rhy988 wants to merge 8 commits into
Draft
Conversation
… rebased onto main) Combines OTTL (exclude_at_match_ottl/include_at_match_ottl) and jq (exclude_at_jq_match/include_at_jq_match via fastjq) log-filtering engines for benchmarking against the regex baseline. Includes the SMP regression test cases (logs_filter_baseline/ottl/jq). Re-squashed and rebased onto current main (previous rebase had gone stale again after a few days).
…for benchmarking Combines all three log-filtering engine implementations for benchmarking against the regex baseline: OTTL (exclude_at_match_ottl/include_at_match_ottl), jq (exclude_at_jq_match/include_at_jq_match via fastjq), and VRL (exclude_at_vrl_match/include_at_vrl_match/mask_vrl). Includes the SMP regression test cases (logs_filter_baseline/ottl/jq) and the AGENT_TAGS fix so the full agent build compiles the real VRL cgo engine instead of the stub. Combined from two independently-rebased-onto-main branches (ralph/alp-log-querying-benchmark-updated, ralph/alp-vrl-log-processing-updated), each already current with main, so this merge only needed to reconcile the actual OTTL/jq vs VRL code (processing_rules.go, processor.go, processor_test.go — all additive, no real conflicts) plus mechanical go.mod/go.sum churn.
… error messages Fixes perfsprint linter findings caught by the pre-push hook.
…lwin/go.mod Caught by the update-go pre-push hook.
Contributor
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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.
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 👍 / 👎.
Comment on lines
+251
to
+254
| buf := make([]byte, 0, 4096) | ||
| return func(input []byte) (bool, error) { | ||
| matched := false | ||
| buf = buf[:0] |
There was a problem hiding this comment.
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.
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.
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 👍 / 👎.
rhy988
changed the title
Contributor
Go Package Import DifferencesBaseline: 3114c9b
|
- rust/LICENSE-3rdparty.csv: regenerated to include vrl and its new transitive deps (added by the cargo generate-lockfile runs during the VRL rebase) - rust/deny.toml: allow CC0-1.0 (base16) and MIT-0 (borrow-or-share, via vrl's jsonschema dep) - both permissive licenses, not previously encountered - .claude-plugins/datadog-agent-gopls/.claude-plugin/plugin.json: regenerated to include the vrl build tag (dda inv claude.set-buildtags) Fixes CI: lint_rust_licenses, lint_gopls_plugin
comp/logs/agent/config unconditionally imports pkg/logs/vrl (build-tag gating happens inside pkg/logs/vrl itself, not at the import site), and several otel-labeled modules (datadogexporter, logsagentpipeline, etc.) import comp/logs/agent/config transitively. This makes pkg/logs/vrl an unavoidable transitive dependency of the OTel build's module graph, so it needs the same used_by_otel label as its importers - a module-graph consistency requirement, not a claim that OTel/OTTL functionally uses VRL. Fixes CI: validate_modules
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds 4 SMP experiment cases to benchmark the performance of different log filtering rule implementations against the standard Datadog log filtering baseline:
logs_filter_baseline— standard Datadog regex-based log processing rules (the baseline)logs_filter_ottl— OTTL-based filtering via the OpenTelemetry Collector transform processorlogs_filter_jq— jq-based filtering via fastjqlogs_filter_vrl— VRL-based filtering via a curated cgo bridge (see below)All implementations live on the same branch and are bundled into a single agent image. Each case applies the same filtering logic and is run against the same synthetic JSON log workload, with only the active filter method differing per case.
Update: VRL is now included. It was originally dropped (see #53105, now closed/superseded by this PR) after two blockers:
compilerfeature, notstdlib— meaning none of VRL's standard library functions (parse_json,contains, etc.) were actually available, so it couldn't do JSON-field-aware filtering at all. Enablingstdlib/stdlib-basehits an unresolved Bazel/Cargo toolchain gap (native C build scripts 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) sidesteps the toolchain gap entirely (nostdlibdependency needed), and there's now full test coverage (pkg/logs/vrl/vrl_test.go,comp/logs/agent/config/processing_rules_vrl_test.go, plus VRL-specific cases incomp/logs-library/processor/processor_test.go).Rule types added:
exclude_at_vrl_match,include_at_vrl_match,mask_vrl(masking is not exercised by this benchmark — only the exclude-style filtering rule, matching what's tested for regex/OTTL/jq).Also includes a benchmark-branch-only fix to
tasks/build_tags.bzl:vrlwas registered in the tag validation allow-list (ALL_TAGS) but not inAGENT_TAGS(the actual tag set used 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 addingvrltoAGENT_TAGS.Motivation
We are evaluating which log filtering implementation to adopt as a replacement or complement to the current regex-based processing rules. OTTL, jq, and VRL each offer different ergonomics/performance tradeoffs for structured/JSON logs vs regex.
Running all 4 cases in a single SMP job 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 cgo implementation (confirmed viago version -mshowing-tags=vrlandpkg/logs/vrl (devel), not the stub).comp/logs/agent/config(234 tests),comp/logs-library/processor(77 tests, including VRL exclude/include/mask-ordering cases),pkg/logs/vrl(7 tests).Additional Notes