-
Notifications
You must be signed in to change notification settings - Fork 5k
add basic grok processor functionality #44215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Robert Paschedag <robert.paschedag@web.de>
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
💚 CLA has been signed |
Signed-off-by: Robert Paschedag <robert.paschedag@web.de>
Hi @rpasche, it's a good start, thanks :) Could you add some tests to it? You should try to cover the happy path and as many failure cases as possible. Due to the similarity, I'd suggest you to check the Please let me know if you need any further help. |
metricbeat/include/list_init.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be part of this PR. Could you remove those changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted those changes. But those had been placed. At least, I did not - manually - do the changes.
if err != nil { | ||
return nil, fmt.Errorf("failed to get field '%s' from event: %w", field, err) | ||
} | ||
|
||
input, ok := field.(string) | ||
if !ok { | ||
return nil, fmt.Errorf("field '%s' is not a string", field) | ||
} | ||
|
||
values, err := u.grok.ParseTypedString(input) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to parse input with grok pattern: %w", err) | ||
} | ||
|
||
for k, v := range values { | ||
_, err := event.PutValue(k, v) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to update event with parsed data: %w", err) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it's better to preserve the original event on failure, that means, instead of returning nil
, return the original event on failure. Also before making permanent changes to the event, clone it, so the backup can be returned if there is any error while applying permanent changes.
Signed-off-by: Robert Paschedag <robert.paschedag@web.de>
Signed-off-by: Robert Paschedag <robert.paschedag@web.de>
Please see: #30073 (comment) |
This pull request is now in conflicts. Could you fix it? 🙏
|
Closing to #44215 (comment) |
Proposed commit message
This PR adds a
grok
processor tobeats
. Mainly, this might only be used withinfilebeat
to parse logfiles the same way, thatlogstash
or thegrok
processor within an ingest pipeline is doing it.Please explain:
The goal is to have more flexibility and ability to parse a log file already on the
client
side, wherefilebeat
might be running, instead of relying on alogstash
or elasticsearchingest pipeline
to do the parsing.Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Disruptive User Impact
Possible impact while using this processor is that a final
mapping
on the final elasticsearchindex
might not match and therefore might cause anindex mapping conflict
.Depending on the number of log entries to parse and the complexity of the
grok
patterns, CPU and memory usage might increase significantly.Author's Checklist
I truly need help to write
tests
. Also requesting some hints, how this processor might be further improved. Maybe to addtags
(if parsing fails)I also need to fully confirm the ingestion into a running demo elasticsearch cluster. So far, I was only able to see the
events
from the internalfilebeat
/logs directly, how they should look. I will try to do this today.How to test this PR locally
This is described in the corresponding ascii docs.
Related issues
Use cases
You do not need to maintain the
ingest pipelines
on elasticsearch itself and allows proper parsing already on theclient
sideScreenshots
Logs