Hi Zhiguo, I think it's really important to distinguish between "a class being public" and "a class being part of Kafka's public API." These really are two completely different concepts, unfortunately.
The best explanation of interface annotations is probably here: https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/annotation/InterfaceStability.java Does anyone know if there is a better one? If not, we should consider adding better documentation about this. The short summary is that because of limitations in Java, we are forced to make a lot of classes public that are not intended to be used outside of the Kafka project. Any class intended to be used outside of a single Java module needs to be public, which means almost everything ends up public. Metrics is one of those classes, currently. If we make it part of the API, that means we need to add interface annotations to all parts of it. It also means changes to those APIs will need a KIP. We do this to avoid having stuff break all the time when people upgrade. If users perform a minor upgrade and their security plugins all break because they're relying on some Metrics function that got removed, they won't be happy. Providing a plugin API means we have to really commit to maintaining that API for years to come. If we don't want to make that kind of commitment, it could be worth exploring different ways of doing what we want to do here. best, Colin On Mon, May 18, 2020, at 11:39, Zhiguo Huang wrote: > Hi Ismael, > The purpose for this KIP allows plugins to use the same Metrics instance > from the broker without recreating it. Metrics is public class so we could > use it. Current Metrics is not implementing an interface related to any > metrics. Expose all fields through methods through interface is big > challenge. We do have scenarios that expose public class through interface. > > Jeff. > > > 11:if Metrics updated we have to update component to use it due to Metrics > is public class > > On Mon, May 18, 2020 at 11:07 AM Ismael Juma <ism...@juma.me.uk> wrote: > > > Hi Jeff, > > > > I think the idea is good, but I have 2 questions/comments: > > > > 1. The monitor method is documented to "get" metrics, but it takes a > > `Metrics` instance. Either the method name/signature needs to be adjusted > > or the documentation. > > 2. Instead of exposing the Metrics class, which is currently not exposed, I > > think we should consider introducing an interface with the methods we want > > to expose public. > > > > Ismael > > > > On Mon, May 18, 2020 at 10:47 AM Rajini Sivaram <rajinisiva...@gmail.com> > > wrote: > > > > > +1 (binding) > > > Thanks for the KIP, Jeff! > > > > > > Regards, > > > > > > Rajini > > > > > > > > > On Mon, May 18, 2020 at 6:41 PM Jason Gustafson <ja...@confluent.io> > > > wrote: > > > > > > > +1 This looks useful. > > > > > > > > -Jason > > > > > > > > On Mon, May 18, 2020 at 9:59 AM Guozhang Wang <wangg...@gmail.com> > > > wrote: > > > > > > > > > Zhiguo, thanks for the KIP. +1 from me. > > > > > > > > > > > > > > > Guozhang > > > > > > > > > > On Fri, May 15, 2020 at 2:21 PM Zhiguo Huang < > > jeff.hu...@confluent.io> > > > > > wrote: > > > > > > > > > > > Thanks to everyone for their input. I've incorporated the changes, > > > and > > > > I > > > > > > think this is ready for voting. > > > > > > > > > > > > To summarize, the KIP simply proposes to add a feature which expose > > > > > > instance of Kafka Metrics in Kafka Authorizer plugin.The KIP can be > > > > found > > > > > > here: > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-608+-+Expose+Kafka+Metrics+in+Authorizer > > > > > > > > > > > > Thanks a lot! > > > > > > > > > > > > Jeff. > > > > > > > > > > > > > > > > > > > > > -- > > > > > -- Guozhang > > > > > > > > > > > > > > >
