[receiver/kafkareceiver] move receiver.kafkareceiver.UseFranzGo feature gate to Beta #42181
Conversation
…re gate to Beta Signed-off-by: Paulo Dias <[email protected]>
Signed-off-by: Paulo Dias <[email protected]>
axw
left a comment
axw
left a comment
There was a problem hiding this comment.
Thanks @paulojmdias!
The main issue is that TestComponentStatus is not really testing anything in the franz-go path: it always hits the deadline (without failing!) because we're not reporting component status.
Is that something you can look into?
Sure @axw! I'll look at all of your comments as soon as I can! |
Signed-off-by: Paulo Dias <[email protected]>
Signed-off-by: Paulo Dias <[email protected]>
Signed-off-by: Paulo Dias <[email protected]>
Signed-off-by: Paulo Dias <[email protected]>
axw
left a comment
axw
left a comment
There was a problem hiding this comment.
Looks great, thank you for iterating on it @paulojmdias!
I'm not 100% sure about the component status events we report, but same goes for Sarama. I think keeping parity is good for now, and we can revisit it alter.
I'd like to get to the bottom of #42226 before we merge this.
|
Thank you @axw! 🙏 |
|
@paulojmdias ICYMI, seems the tests are failing now. I think that since #42226 is merged, once tests are green we can mark this as ready to merge. |
|
@axw I'm looking into fixing it now 👀 |
Signed-off-by: Paulo Dias <[email protected]>
|
@axw you can look into it now. The |
axw
left a comment
axw
left a comment
There was a problem hiding this comment.
Thanks @paulojmdias. I think there's a problem with the StatusOK bit, and some possible redundant code, but otherwise it looks good.
Signed-off-by: Paulo Dias <[email protected]>
Signed-off-by: Paulo Dias <[email protected]>
axw
left a comment
axw
left a comment
There was a problem hiding this comment.
Thank you :)
I'm not certain if updating status on each assignment is the best, but it's along the lines of what the Sarama code is doing, and I think it's fine. Maybe we can find another way to go about it later, e.g. using franz-go's hooks.
axw
added
ready to merge
|
@axw Should we open an issue to revisit this later on? |
|
@paulojmdias sounds good, done: #43001 |
#### Description I would like to propose adding @paulojmdias as a code owner of the Kafka components. He has been doing excellent work across all of those components (and more besides!). - #42507 - #42796 - #43105 - #42181 - #42327 - #42507 - #42796 - #43019 - #43083 - #43105 #### Link to tracking issue N/A #### Testing N/A #### Documentation N/A
#### Description I would like to propose adding @paulojmdias as a code owner of the Kafka components. He has been doing excellent work across all of those components (and more besides!). - open-telemetry#42507 - open-telemetry#42796 - open-telemetry#43105 - open-telemetry#42181 - open-telemetry#42327 - open-telemetry#42507 - open-telemetry#42796 - open-telemetry#43019 - open-telemetry#43083 - open-telemetry#43105 #### Link to tracking issue N/A #### Testing N/A #### Documentation N/A
7 participants
Description
This PR moves the
receiver.kafkareceiver.UseFranzGofeature gate to Beta, as it also improves the tests to work with both libraries (saramaandfranz-go) while the feature gate is enabled by default.Link to tracking issue
Fixes #42155
Testing
Improved the tests to work with both libraries (
saramaandfranz-go) while the feature gate is enabled by default.Documentation
Updated the README.md with the reference for the users to opt out of using
franz-go, disabling the feature gate.