See gh-21890
Comment From: snicoll
Thanks a lot @eddumelendez. Considering that this was added in Spring Kafka 2.5 and this would complement our native metrics story for Kafka added in 2.3.0, I wonder if merging this in 2.3.x would be the right thing to do. I've flagged this for team attention to get more feedback from the team.
Comment From: wilkinsona
I think we should wait for 2.4.
Comment From: garyrussell
The problem is that the previous (deprecated) Micrometer code scraped JMX MBeans so it would automatically find consumer and producer MBeans, including those registered by Kafka streams.
The issue was reported here - because the binder doesn't use Boot's producer/consumer factories.
When fixing that, I realized the same issue would arise for any Boot app that uses Streams.
So 👍 from me for back-porting to 2.3.x.
Comment From: eddumelendez
Should I update the PR and point to 2.3.x?
Comment From: wilkinsona
I think I understand this now. There's a loss in functionality from 2.2 as the streams-related consumer metrics are gone. What is proposed here will bring them back. We think this is worth fixing in 2.3.
@eddumelendez Thanks for the offer, but there's no need to update the PR unless you particularly want to. We can take care of it as part of merging the change.
Comment From: snicoll
@garyrussell can you please let us know when you plan to release Spring Kafka 2.5.3? We can switch to snapshot on master as we are releasing 2.4.0-M1 next week.
Comment From: garyrussell
@snicoll I have scheduled it for next Wednesday.
Comment From: eddumelendez
@snicoll changes were applied and now the target branch is 2.3.x. Once the suggestions is applied in spring-kafka I can submit additional changes.
Comment From: garyrussell
@snicoll @eddumelendez https://github.com/spring-projects/spring-kafka/pull/1515 is merged.
Comment From: eddumelendez
thanks for the notification @garyrussell ! I have introduced new changes in the PR
Comment From: wilkinsona
Thanks very much, @eddumelendez.
Comment From: snicoll
An inner class with an AutoConfiguration suffix seems a bit misleading to me so we should probably rename that, see https://github.com/spring-projects/spring-boot/blob/ecbc8ea2dfa308d6a2860c6490c80f36b64936cf/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/KafkaMetricsAutoConfiguration.java#L76
Comment From: wilkinsona
Well spotted, @snicoll. I've polished that in dfea2f4.