Replace compatibility table with a link to the official Kafka compati… #237
Conversation
…bility table. Implementation of logstash-plugins/logstash-output-kafka#156
docs/index.asciidoc
Outdated
| is compatible with both the 0.8 consumer and 0.9 consumer APIs, but not the other way around. | ||
| This input will read events from a Kafka topic. | ||
|
|
||
| This plugin uses Kafka Client 0.11.0. For broker compatibility, see the official https://cwiki.apache.org/confluence/display/KAFKA/Compatibility+Matrix[Kafka compatibility reference]. |
There was a problem hiding this comment.
Should this be a call-out/note thing instead of a paragraph here?
There was a problem hiding this comment.
I think it's good as a paragraph.
There was a problem hiding this comment.
++ @dedemorton perhaps we can have a "Broker Compatibility" header/title here so its not missed. Thoughts?
There was a problem hiding this comment.
Also, for the kafka client, can we use 0.11 instead of 0.11.0? I'm not sure what 0.11.0 is, since their versioning is w.x.y.z.
There was a problem hiding this comment.
I don't know if that's more or less confusing. 0.11 is less precise than 0.11.0. I'd lean towards more precise and say 0.11.0.0 -- what's the goal of 0.11?
docs/index.asciidoc
Outdated
|
|
||
| This plugin uses Kafka Client 0.11.0. For broker compatibility, see the official https://cwiki.apache.org/confluence/display/KAFKA/Compatibility+Matrix[Kafka compatibility reference]. | ||
|
|
||
| If you are using a newer version of this plugin, you will want to read the https://www.elastic.co/guide/en/logstash/master/plugins-inputs-kafka.html[latest plugin version documentation]. |
There was a problem hiding this comment.
This is a workaround. We may not even need this. I hope we don't need this, actually, because we may not need any future major revisions of this plugin given Kafka's promise.
There was a problem hiding this comment.
@acchen97 If we believe Kafka will continue its promise of client compatibility, then I believe we can probably remove this line about 'newer version' because it is (hopefully) unlikely that we will need to chase Kafka versions and will avoid frequent major version bumps of this plugin.
There was a problem hiding this comment.
@jordansissel I'm a little confused. This statement is true for all of the plugin docs. Are you pointing to master here in case we need to add more about compatibility? If so, I'd make that clear in your statement. Maybe say something like:
If you're using a plugin version that was released after {version}, see the https://www.elastic.co/guide/en/logstash/master/plugins-inputs-kafka.html[latest plugin documentation] for updated information about Kafka compatibility.
{version} resolves to the actual plugin version so there will be no ambiguity about what we mean by "newer".
There was a problem hiding this comment.
I'm a little confused
Right, so this plugin is special. A significant amount of our customers have difficulty using Kafka, so we get lots of questions about Kafka versions, compatibility, etc. Today, we maintain 3 (I think?) branches of this plugin, so we have 3 active plugin versions as "latest". The confusion we created is that Logstash 5.x ships with the "not latest" plugin, and we have customers constantly asking support, who escalate to dev, "Where are the docs for version X.Y?" etc.
This paragraph is a hack. It's hopefully a shortcut for customers who have no obvious way to help themselves answer a question, so they ask support, who ask us. I am hoping this paragraph gives these customers a shortcut to save themselves time asking support/us.
(If I'm not making sense, let me know, I'm not sure I use the right words to describe the intent)
I like your suggested copy, and I'll update the PR to use that.
There was a problem hiding this comment.
Yup, I get that. I understand that users are confused; I'd just forgotten about the complexity of all our versions of this plugin. The matrix has been a major pain to maintain, so if this works, that's great! Incidentally João and I have been discussion the project to create versioned plugin docs. I'm going to come up with a little POC (hopefully today) of what that would look like. When we get that in place, users will be able to find x.y version of the docs.
There was a problem hiding this comment.
I am delighted to hear this! :)
|
@dedemorton I could use your excellence in reviewing this. I added some inline comments with some kind of questions. |
|
Alright, I think we're ready for another review. After this is merged, we will replicate the same change on all supported branches of our documentation for both input and output kafka plugins. |
|
LGTM |
|
@jordansissel added a few comments. Thanks for helping drive this. |
In anticipation of this upgrade - logstash-plugins/logstash-output-kafka#164
|
Gonna wait until logstash-plugins/logstash-output-kafka#164 (upgrade client to 1.0.0) is merged. Then I'll merge w/ @acchen97's suggestions. After that, I'll open an issue to track porting this change into all active branches of both input and output plugins. |
|
@jordansissel following up from a previous comment (irrelevant now for master since its Kafka 1.0.0), I'm good with the older kafka client releases to be more explicit in the version i.e. 0.11.0.0. |
In anticipation of this upgrade - logstash-plugins/logstash-output-kafka#164 Fixes #237
Implementation of logstash-plugins/logstash-output-kafka#156
This is a draft implementation that has a few major improvements over the current matrix:
I am hoping that users find this easier to understand than the difficult table we were maintaining before. Further, new Kafka releases will not require updating the now-deleted compatibility table, which is very exciting to me.