Hey Ryan, `kafka.controller:type=KafkaZkController` does not exist today as the current controller uses `kafka.controller:type=KafkaController`. I think that we should stick to the existing one and provide the same metric for both the old and the new controller. Besides this, the KIP looks good to me.
Best, David On Tue, Jun 15, 2021 at 4:28 AM Israel Ekpo <israele...@gmail.com> wrote: > Ryan, > > I think you can add a section under Rejected Alternatives to elaborate on > why you feel combining the metrics for KRaft mode and legacy mode is not a > good idea. That could help clarify future questions such as the one raised > by Xavier. > > > > On Mon, Jun 14, 2021 at 10:20 PM Israel Ekpo <israele...@gmail.com> wrote: > > > Xavier, > > > > I think the reason for doing this is to make them independent so that it > > is easier to design and implement the tracking for legacy mode (with > > Zookeeper) and KRaft Mode (without ZK) > > > > That is my assessment. > > > > On Mon, Jun 14, 2021 at 1:57 PM Xavier Léauté > <xav...@confluent.io.invalid> > > wrote: > > > >> Any reason we need two different metrics for ZK an Quorum based > >> controllers? > >> Wouldn't it make sense to have one metric that abstracts the > >> implementation > >> detail? > >> > >> On Mon, Jun 7, 2021 at 2:29 PM Ryan Dielhenn <rdielh...@confluent.io > >> .invalid> > >> wrote: > >> > >> > Hey Colin and David, > >> > > >> > I added another table for the ZK version of RegisteredBrokerCount. > >> > > >> > Best, > >> > Ryan Dielhenn > >> > > >> > On 2021/06/04 08:21:27, David Jacot <dja...@confluent.io.INVALID> > >> wrote: > >> > > Hi Ryan, > >> > > > >> > > Thanks for the KIP. > >> > > > >> > > +1 for adding RegisteredBrokerCount to the ZK controller as well. > This > >> > > would be really helpful. > >> > > > >> > > Best, > >> > > David > >> > > > >> > > On Fri, Jun 4, 2021 at 12:44 AM Colin McCabe <cmcc...@apache.org> > >> wrote: > >> > > > >> > > > Hi Ryan, > >> > > > > >> > > > Thanks for the KIP. I think it would be good to provide the > >> > > > RegisteredBrokerCount metric for the ZK controller as well as for > >> the > >> > > > Quorum controller. Looks good aside from that! > >> > > > > >> > > > best, > >> > > > Colin > >> > > > > >> > > > On Thu, Jun 3, 2021, at 14:09, Ryan Dielhenn wrote: > >> > > > > Hey kafka-dev, > >> > > > > > >> > > > > I created KIP-748 as a proposal to add broker count metrics to > the > >> > > > > Quorum Controller. > >> > > > > > >> > > > > > >> > > > > >> > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-748%3A+Add+Broker+Count+Metrics#KIP748:AddBrokerCountMetrics > >> > > > > > >> > > > > Best, > >> > > > > Ryan Dielhenn > >> > > > > > >> > > > > >> > > > >> > > >> > > >
