-
Notifications
You must be signed in to change notification settings - Fork 73
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
chore(parse_groks): expose internal errors #1113
chore(parse_groks): expose internal errors #1113
Conversation
811d3bd
to
3c84964
Compare
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.
Thanks, I agree that we should introduce this breaking change to fix the data loss scenario.
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.
makes sense to me, thanks for the changes 🙏 should we remove the bug label with the latest update?
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'm still confused by the error type naming, but I will approve this to unblock further work.
Summary
parse_grok
follows the Datadog Log's Grok algorithm's behavior where errors in applying filters are non fatal.This change surfaces those filter errors to the caller, and differentiates between fatal and non fatal, as outlined by the Datadog Log's Grok algorithm, allowing the caller to decide whether to align with that or differ.
The stdlib
parse_groks
was unmodified, as such this is not a change to user facing behavior. If we decide to change that, we can do it in a follow up change.Change Type
Is this a breaking change?
How did you test this PR?
Added/corrected unit tests
Does this PR include user facing changes?
our guidelines.
Checklist
run
dd-rust-license-tool write
and commit the changes. More details here.References