Conversation
1. Used the same logic as elasticsearch output plugin to find out if dlq is enabled or not [execution_context? && execution_context.dlq_writer? && execution_context.dlq_writer is not a dummy writer?] 2. if dlq is enabled, send it to dlq_writer or log and drop the the events otherwise. Fixes logstash-plugins#109 Signed-off-by: RashmiRam <rashmi.ramanathan@freshworks.com>
|
💚 CLA has been signed |
Signed-off-by: RashmiRam <rashmi.ramanathan@freshworks.com>
|
@jsvd Could you please review this when you have time? |
Signed-off-by: RashmiRam <rashmi.ramanathan@freshworks.com>
andsel
left a comment
There was a problem hiding this comment.
Hi @RashmiRam sorry for very late review.
First of all, thank you a lot for this contribution!
I've left some comments.
I think that before moving forward the code base has to be re-aligned to main, to resolve some merge conflicts.
| metadata = event.get("@metadata") | ||
| metadata.each_pair do |key, value| | ||
| event.set("[custom_metadata][#{key}]", value) | ||
| end |
There was a problem hiding this comment.
@metadata field is already stored in the DLQ event, so that it can be read back on the downstream DLQ consumer.
No need to copy the @metadata in custom_metadata field before DLQ write.
| config :retryable_codes, :validate => :number, :list => true, :default => [429, 500, 502, 503, 504] | ||
|
|
||
| # If encountered as response codes, this plugin will write these events to DLQ | ||
| config :dlq_retryable_codes, :validate => :number, :list => true, :default => [400, 403, 404, 401] |
There was a problem hiding this comment.
| config :dlq_retryable_codes, :validate => :number, :list => true, :default => [400, 403, 404, 401] | |
| config :dlq_retryable_codes, :validate => :number, :list => true, :default => [400, 401, 403, 404] |
| Gem::Specification.new do |s| | ||
| s.name = 'logstash-output-http' | ||
| s.version = '5.2.4' | ||
| s.version = '5.2.5' |
There was a problem hiding this comment.
This is a new feature so it deserve a minor version. Actually main has 5.5.0 so the target could be 5.6.0
| s.version = '5.2.5' | |
| s.version = '5.6.0' |
| @@ -1,3 +1,6 @@ | |||
| ## 5.2.5 | |||
There was a problem hiding this comment.
| ## 5.2.5 | |
| ## 5.6.0 |
As per later comment
|
@andsel Thank you for the review. I will have some time this weekend and will try to address all the comments. |
Used the same logic as elasticsearch output plugin to find out if dlq is enabled or not
[execution_context? && execution_context.dlq_writer? && execution_context.dlq_writer is not a dummy writer?]
if dlq is enabled, send it to dlq_writer or log and drop the the events otherwise.
Fixes #109
Signed-off-by: RashmiRam rashmi.ramanathan@freshworks.com
Thanks for contributing to Logstash! If you haven't already signed our CLA, here's a handy link: https://www.elastic.co/contributor-agreement/