grokky : ParseInto - #4729
grokky : ParseInto#4729buixor wants to merge 2 commits into
Conversation
|
@buixor: There are no 'kind' label on this PR. You need a 'kind' label to generate the release automatically.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
|
@buixor: There are no area labels on this PR. You can add as many areas as you see fit.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Tests share mutable destinations across engines, and the PR body lacks the required human review and testing confirmation.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds allocation-efficient grok parsing directly into caller-provided maps for both regex engines.
Changes:
- Introduces
ParseIntoand uses it in parser nodes. - Preserves
Parsecompatibility. - Adds behavioral tests and performance benchmarks.
| File | Description |
|---|---|
pkg/parser/node.go |
Parses captures directly into event data. |
pkg/grok/pattern.go |
Extends the pattern interface. |
pkg/grok/pattern_re2.go |
Implements RE2 ParseInto. |
pkg/grok/pattern_legacy.go |
Implements legacy ParseInto. |
pkg/grok/compile_test.go |
Tests both implementations. |
pkg/grok/bench_test.go |
Benchmarks allocations and throughput. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| require.Equal(t, tc.want, p.ParseInto(tc.input, tc.dest)) | ||
| require.Equal(t, tc.after, tc.dest) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4729 +/- ##
==========================================
+ Coverage 71.04% 71.05% +0.01%
==========================================
Files 536 536
Lines 36259 36261 +2
==========================================
+ Hits 25759 25765 +6
+ Misses 10499 10495 -4
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/kind enhancement |
github-actions
Bot
added
kind/enhancement
Implement
ParseIntofor both RE2 and regexps, and killParse.ParseIntoreduces allocations, as we're writing directly into the provided map, avoid an intermediary alloc. Especially effective when it's a miss, but never more expensive.Allocations
CPU
In terms of pure parsing time, performance impact is neglectable when it's a hit.
ParseParseInto