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

[WIP] Upgrade Kafka client library to v2.8.1 / 3.0.0 (+ update related Kafka client dependencies) #341

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Kami
Copy link

@Kami Kami commented Dec 9, 2021

Description

This pull request updates Kafka client library versions to the latest stable version (3.0.0).

Background, Context

Currently the input plugin is depending on an old Kafka client library (2.3.0).

I assume that was done for broker compatibility reasons. Sadly the Kafka compatibility matrix is vastly out of date (https://cwiki.apache.org/confluence/display/KAFKA/Compatibility+Matrix), but people claim newer versions of the client should also work with older versions of the broker.

We encountered a bug in with CooperativeSticky partition assigner which has been fixed in v3.0.1 and also backported to v2.8.2 (in our case the broker is running Kafka 2.6.x) - https://issues.apache.org/jira/browse/KAFKA-12984.

Upgrading the client should fix the bug (https://issues.apache.org/jira/browse/KAFKA-12984?focusedCommentId=17433029&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17433029), but I'm still working on verifying the changes end to end.

On a related note - it would be good to know what is the minimum version of the Kafka broker this plugin aims to support so we document that and see if this change is viable or not.

TODO

  • Verify the change end to end
  • Changelog entry
  • Bump input ruby gem version

@cla-checker-service
Copy link

cla-checker-service bot commented Dec 9, 2021

💚 CLA has been signed

@Kami
Copy link
Author

Kami commented Dec 14, 2021

Another reason why we should consider upgrading is a known vulnerability in older releases - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-38153.

@Kami Kami force-pushed the upgrade_kafka_client branch from 2396647 to c9d20a1 Compare December 14, 2021 10:02
@Kami
Copy link
Author

Kami commented Dec 14, 2021

I tested the change end to end and it works fine with Kafka broker v2.5.x, but it needed some config value changes (e.g. max_poll_records value needs to be a string, but it used to work fine with an int / long, partition_assignment_strategy values have changed, used to be coperative_sticky, now it needs to be - cooperative-sticky, etc.).

So this will need more work (and some decisions will need to be made on how to handle those option value changes) before it's "production ready".

It was quite a pain to build the plugin though. Having a Dockerfile would make this a lot easier. I needed to update gradle and include some other changes to get the build working under jruby + JDK 11 (I couldn't get it to work under JDK 17).

I can also commit Dockerfile I used.

@Kami
Copy link
Author

Kami commented Jan 7, 2022

Just a quick update.

I also tried to get it to build under JDK 17 and that was a huge pain.

I needed to update gradle config file since there were a lot of breaking changes in latest releases and I also needed to use latest ruby-maven gem from master since the version published on RubyGems is very old and doesn't work with newer JDKs and master branch contains (unpublished) a fix for that (ac040b1, ad3fa7c).

Here is the ruby-maven workaround I used inside the Dockerfile:

...
# Download and install patched ruby maven
RUN git clone --depth 1 https://github.com/jruby/ruby-maven.git --branch master
WORKDIR /tmp/ruby-maven

RUN gem list installed
RUN gem build ruby-maven.gemspec
RUN gem install ruby-maven-3.3.12.gem
RUN gem list installed
...
# Build input plugin here
...

@Kami Kami changed the title [WIP] Upgrade Kafka client library to 3.0.0 (+ update related Kafka client dependencies) [WIP] Upgrade Kafka client library to v2.8.1 / 3.0.0 (+ update related Kafka client dependencies) Jan 10, 2022
@Kami
Copy link
Author

Kami commented Jan 10, 2022

OK, I put together a little PoC / prototype with docker compose which puts everything together and demonstrates how to use Logstash 7.16 with JDK 17 and Kafka Logstash input with Kafka client library v2.8.1 and v3.0.0 - https://github.com/Kami/docker-compose-logstash-kafka-client-v281-v300.

So far I have only tested with Kafka Broker 2.6 and client library v2.8.1 and v3.0.0 and I confirmed it's working correctly (minus some small logstash input config changes which are needed, as noted in README and Dockerfile).

As noted in Dockerfile, it also contains some quick "hacks", like copying jars which are bundled with the gem into global Logstash jar search path, since for some reason, loading jars from local gem vendor directories doesn't appear to work (and I didn't have time yet to dig in).


Having said that, I would appreciate some information on how to continue here and get those changes merged upstream - it's likely not an easy task and will require quite a bit more work, including some kind of CI which tests against multiple Kafka broker versions.

And there are also some small backward incompatible changes in the config values, but those can be handled inside the plugin code and made opaque to the end user.

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

Successfully merging this pull request may close these issues.

2 participants