Hi Bruno, Thanks for your comments.
1) I updated the header in the public interfaces section. 2) Well, the overloads were there initially but then it was suggested to remove them during PR review: https://github.com/apache/kafka/pull/12121#discussion_r875156714 3) I updated the KIP with the function name you suggested. Thanks! Sagar. On Tue, May 24, 2022 at 9:34 PM Bruno Cadonna <cado...@apache.org> wrote: > Sagar, Thanks for the KIP! > > > 1. Could you please add the class to which you want to add the method in > section "Public Interfaces"? Ideally, in the header of the code part > that contains "prefixScan" currently. > > 2. Do you also plan to add overloads for metricOrElseCreate() similar as > for addMetric()? > > 3. I would propose to call the method getMetricOrElseCreate(). > > Best, > Bruno > > > On 24.05.22 13:47, Sagar wrote: > > Thanks Guozhang. I made the edit. Let me open a VOTE thread. > > > > Thanks! > > Sagar. > > > > On Mon, May 23, 2022 at 10:19 PM Guozhang Wang <wangg...@gmail.com> > wrote: > > > >> Thanks Sagar, > >> > >> The KIP looks good to me overall, I just have a minor comment on the > >> motivation of it: "The Metrics registry is often used by concurrent > >> threads" -> maybe we can be more clearer, by saying that "Concurrent > thread > >> may try to access the Metrics registry to create the same, > instance-level > >> metrics". > >> > >> Otherwise, +1 from me. Since this is a very straight-forward KIP I > think we > >> can move on to the VOTE phase directly. > >> > >> > >> Guozhang > >> > >> On Mon, May 23, 2022 at 5:46 AM Sagar <sagarmeansoc...@gmail.com> > wrote: > >> > >>> Hi All, > >>> > >>> I would like to start a discussion thread on the following KIP: > >>> > >>> > >>> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-843%3A+Adding+metricOrElseCreate+method+to+Metrics > >>> > >>> PS: Note that I sent another email without the KIP number in the > >>> subject line. Plz use this email for discussion and ignore the other > one. > >>> Sorry about the spamming. > >>> > >>> Thanks! > >>> Sagar. > >>> > >> > >> > >> -- > >> -- Guozhang > >> > > >
