perf: combined processor allocation optimizations (do not merge)#49723
perf: combined processor allocation optimizations (do not merge)#49723strawgate wants to merge 4 commits intoelastic:mainfrom
Conversation
|
This pull request doesn't have a |
🤖 GitHub commentsJust comment with:
|
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
📝 WalkthroughWalkthroughThis PR adds many tests and benchmarks across libbeat and implements behavioral and performance changes: expanded Event.DeepUpdate tests/benchmarks; add_fields fast-paths and corresponding tests/benchmarks; add_cloud_metadata per-value cloning and tests/benchmarks; host metadata cache rewritten for atomic timestamp + lock-free fast path and tests; kubernetes annotator cloning/refactoring for container sub-map; processing pipeline and timestamp benchmarks; timestamp parse-path refactor with centralized parseFailure; rename processor conditional event-clone-on-failure optimization; and smaller test updates and microbenchmarks. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
91a585e to
15b9ace
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libbeat/processors/actions/rename.go`:
- Around line 81-83: The rollback logic incorrectly only clones the event when
f.config.FailOnError && len(f.config.Fields) > 1, which leaves single-field
renames vulnerable to partial mutation if renameField fails; update the code so
that when f.config.FailOnError is true you always clone the event (create backup
= event.Clone()) before any mutation occurs (i.e., remove the
len(f.config.Fields) > 1 condition and perform the clone prior to calling
renameField/Delete/PutValue) so that the restore path can reliably restore the
original event on error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b02f807e-ad6c-4605-8676-8d73aadb8307
📒 Files selected for processing (2)
libbeat/processors/actions/rename.golibbeat/processors/actions/rename_benchmark_test.go
✅ Files skipped from review due to trivial changes (1)
- libbeat/processors/actions/rename_benchmark_test.go
f7ea598 to
d3f17e3
Compare
This comment has been minimized.
This comment has been minimized.
aba3991 to
9caf724
Compare
This comment has been minimized.
This comment has been minimized.
d49a156 to
81c4ed0
Compare
f379341 to
b402585
Compare
This comment has been minimized.
This comment has been minimized.
b402585 to
588cf93
Compare
This comment has been minimized.
This comment has been minimized.
588cf93 to
0fafb6c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
78930e9 to
a1e01e5
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
6a7c9f9 to
2919d32
Compare
This comment has been minimized.
This comment has been minimized.
2919d32 to
dc45324
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
dc45324 to
85dd799
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
TL;DRBuildkite failures are all the same root cause: processor error-message casing changed ( Remediation
Investigation detailsRoot CauseThis is a test failure (not infra): assertions compare full From failed logs, actual events contain lowercase prefixes:
But tests expect uppercase prefixes:
This causes:
Evidence
Verification
Follow-upPick one convention and apply it in both implementation and tests. If the PR intentionally normalized messages to lowercase (Go error style), update expected strings in tests accordingly. Note 🔒 Integrity filtering filtered 1 itemIntegrity filtering activated and filtered the following item during workflow execution.
What is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
Remove wasteful Clone() calls and defer allocations in docker metadata, kubernetes metadata, timestamp, and publisher/processing processors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace Clone()+DeepUpdate() with single-pass DeepCopyUpdate() in add_fields, cloud/host/observer metadata, rename, processing, and heartbeat eventext. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Eliminate event.Clone() in dissect (check-then-write), rename, copy, replace, truncate, urldecode, extract_array, decode_csv, decode_base64, decompress_gzip, and append processors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
f83bcdf to
d307bd7
Compare
Combined performance PR (do not merge)
This PR combines all three processor allocation optimization PRs into a single branch for unified benchmarking and profiling. Each commit corresponds to one PR:
perf: remove unnecessary allocations in hot-path processors(perf: remove unnecessary allocations in hot-path processors #49761) — Remove wasteful Clone() calls in docker metadata, kubernetes metadata, timestamp, and publisher/processing.perf: use DeepCopyUpdate to eliminate Clone+DeepUpdate allocations([DNM] perf: use DeepCopyUpdate to eliminate Clone+DeepUpdate allocations #49762) — Single-pass DeepCopyUpdate in add_fields, cloud/host/observer metadata, rename, processing, and heartbeat. Depends on feat(mapstr): add DeepCopyUpdate for single-pass clone+merge elastic-agent-libs#390.perf: skip unnecessary event.Clone() in processors(perf: skip unnecessary event.Clone() in processors for single-field ops #49777) — Eliminate event.Clone() in dissect (check-then-write) and 10 other single-field processors.Merge via individual PRs
Do not merge this PR directly. Merge the individual PRs above instead — they have full descriptions, benchmark data, and test plans.
🤖 Generated with Claude Code