-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
DLQ-ing events that trigger an conditional evaluation error. #16423
base: main
Are you sure you want to change the base?
DLQ-ing events that trigger an conditional evaluation error. #16423
Conversation
…ct the full stack trafe and debug log it, the other is send to DLQ
@@ -190,6 +191,15 @@ public void notify(ConditionalEvaluationError err) { | |||
} catch (IOException ioex) { | |||
LOGGER.warn("Invalid operation on closing internal resources", ioex); | |||
} | |||
|
|||
// if pipeline has DLQ enabled, send also there the event | |||
if (javaDlqWriter != null) { |
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.
We should change the log entry to mention that the event was sent to the DLQ instead of "Event was dropped, enable debug logging to see the event's payload."
logstash-core/src/main/java/org/logstash/execution/AbstractPipelineExt.java
Show resolved
Hide resolved
@@ -180,10 +181,32 @@ public final class LogErrorEvaluationListener implements ConditionalEvaluationLi | |||
@Override | |||
public void notify(ConditionalEvaluationError err) { | |||
lastErrorEvaluationReceived = err.getCause().getMessage(); | |||
LOGGER.warn("{}. Event was dropped, enable debug logging to see the event's payload.", lastErrorEvaluationReceived); | |||
if (isDLQEnabled()) { | |||
LOGGER.warn("{}. Failing event was sent to dead letter queue", lastErrorEvaluationReceived); |
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.
Any reason for one to end on a period (.
) and the other not?
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.
No, was a typo
Quality Gate passedIssues Measures |
💚 Build Succeeded
History
cc @andsel |
Release notes
When a conditional evaluation encounter an error in the expression the event that triggered the issue is sent to pipeline's DLQ, if enabled for the executing pipeline.
What does this PR do?
This PR engage with the work done in #16322, the
ConditionalEvaluationListener
that is receives notifications about if-statements evaluation failure, is improved to also send the event to DLQ (if enabled in the pipeline) and not just logging it.Why is it important/What is the impact to the user?
Let the user that encounter errors in the evaluation of conditional statements to enqueue in DLQ for a subsequent processing.
Checklist
[ ] I have made corresponding change to the default configuration files (and/or docker env variables)Author's Checklist
How to test this PR locally
Use a pipeline that would trigger an error in conditional evaluation, like:
And enable DLQ, so in
config/logstash.yml
setRelated issues
Use cases
Screenshots
Logs