Skip to content
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

Enhancement : Add "discard_kafka_delivery_failed_regex" option #496

Merged

Conversation

kubotat
Copy link
Contributor

@kubotat kubotat commented Oct 10, 2023

When with Forwarder and Aggregator architecture, forwarders sometimes send invalid data which cause delivery failure at aggregators. out_rdkafka2 plugin hasdiscard_kafka_delivery_failed but it potentially discards not only unnecessary events but also required events, e.g. users expect Fluentd to keep events in buffer files during the outage of Kafka cluster (due to maintenance for instance) but all events are discarded when with discard_kafka_delivery_failed.
This enhancement request proposes having discard_kafka_delivery_failed_regex option on out_rdkafka2 plugin to nullify invalid data by checking the error message with given regexp pattern.

Here is a sample use case:
In the following configuration, dummy events are generated with tag test-topic0001 and Fluentd try to ship message to test-topic0001. If test-topic0001 does not exist in Kafka cluster, out_rdkafka2 emits a Local: Unknown topic (unknown_topic) warning message. With discard_kafka_delivery_failed_regex /Unknown topic/, out_rdkafka2 discards events which emits a Local: Unknown topic (unknown_topic) warning message.

<source>
  @type dummy
  sample {"Hello":"World"}
  tag test-topic0001
</source>

<match dummy>
    @type rdkafka2
    @id "rdkafka2"
    bokers kafka01.demo.local:9093, kafka02.demo.local:9093
    discard_kafka_delivery_failed_regex /Unknown topic/
    required_acks 1
    ack_timeout 20
    share_producer true
    ## Buffer settings
    <buffer tag>
      @type file
      path ./buffer/
      flush_interval 5s
    </buffer>
    <format>
      @type json
    </format>
</match>

@ashie ashie self-requested a review October 10, 2023 22:16
@ashie ashie marked this pull request as ready for review October 11, 2023 06:50
Copy link
Member

@ashie ashie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems nice, thanks for your proposal!
To merge this, could you address following things?

As a future task for us, it would be nice if we can match errors by error codes of Rdkafka to avoid ambiguity: https://docs.confluent.io/platform/current/clients/librdkafka/html/rdkafkacpp_8h.html#a4c6b7af48c215724c323c60ea4080dbf
but it's not needed in this pull request.

…failed events by regexp

Signed-off-by: kubotat <tkubota@ctc-america.com>
…failed events by regexp

Signed-off-by: kubotat <tkubota@ctc-america.com>
Signed-off-by: kubotat <tkubota@ctc-america.com>
@kubotat kubotat force-pushed the enhancement/discard_kafka_delivery_failed_regex branch from a94c27f to 3a9191c Compare October 11, 2023 18:08
@kubotat
Copy link
Contributor Author

kubotat commented Oct 11, 2023

@ashie Thanks for reviewing. I've done two items you mentioned. I will keep working on a test code.

It seems some of the check processes were failed after updating the contents. I would appreciate it if you could give me an advice on how to pass the check process.

@kubotat kubotat changed the title Enhancement : Add "discard_kafka_delivery_failed_regexp" option Enhancement : Add "discard_kafka_delivery_failed_regex" option Oct 11, 2023
@ashie
Copy link
Member

ashie commented Oct 12, 2023

It seems some of the check processes were failed after updating the contents. I would appreciate it if you could give me an advice on how to pass the check process.

Don't worry, they aren't caused by this change.
They are always unstable, need to rerun when failed.
We should tackle on this as another issue.

I will keep working on a test code.

Please let me know if it's hard to add. I'll merge & release it without test for now.

@kubotat
Copy link
Contributor Author

kubotat commented Oct 12, 2023

@ashie Thanks. I would appreciate it if could merge my request.

@ashie ashie merged commit 20ba524 into fluent:master Oct 13, 2023
9 checks passed
@ashie
Copy link
Member

ashie commented Oct 13, 2023

Thanks!

@ashie
Copy link
Member

ashie commented Oct 13, 2023

I've released v0.19.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants