Generally, I agree with Ismael that having a new, weird name will make it hard to keep them straight. Then again, we need to make them different to prevent confusion about their semantics. To be clear, I'll be a +1 regardless of how we break this dilemma.
One suggestion: We currently have addMetric to add a new metric. We can take some inspiration from the Java Map interface and call this new method `addMetricIfAbsent`. Having the same prefix should help discovery, and following the Map convention should help confusion. Thanks all, -John On Tue, May 31, 2022, at 12:13, Sagar wrote: > Oh yeah there's another metric function which is get-only. I think we > should go ahead with getOrCreateMetric. > > Thanks! > Sagar. > > On Tue, May 31, 2022 at 10:02 PM Guozhang Wang <wangg...@gmail.com> wrote: > >> I'd prefer the getOrCreateMetric function name, since for the existing " >> sensor(String name)" function that only takes a single `String` parameter, >> its semantics is already "get or create". Whereas the existing >> "metric(MetricName)" function's semantics is "get" only. So in my mind, the >> inconsistent conventions in function signatures already exist today. And >> with the other option we would need to educate users that "all the `sensor` >> functions are get-or-create, but, please remember that the `metric` >> function with just the metric name is get-only, while other `metric` >> overrides with more parameters are get-or-create", which I think is even >> more confusing. >> >> >> Guozhang >> >> >> On Mon, May 30, 2022 at 9:51 PM Sagar <sagarmeansoc...@gmail.com> wrote: >> >> > Hi Ismael, >> > >> > I guess in that case, we will have to go with the name *metric*- similar >> to >> > *sensor* - which David pointed out above because I think that's the >> closest >> > method which either gets or creates a new sensor. Current addMetric in >> the >> > Metrics class throw an IllegalArguementException when the metric already >> > exists and that's why I still think getOrCreateMetric still signifies the >> > action correctly. Or how about addOrGetMetric or getOrAddMetric, just >> > replacing create with add to keep it similar to the already present >> > addMetric method. >> > >> > Thanks! >> > Sagar. >> > >> > On Tue, May 31, 2022 at 1:19 AM Ismael Juma <ism...@juma.me.uk> wrote: >> > >> > > I think it's confusing to use two completely different naming >> conventions >> > > in the same class. We either stick with the existing convention or we >> > > create a new one and deprecate old method(s). I am not sure there is >> > enough >> > > value in this case for the latter, but it would be good to hear what >> > others >> > > think. >> > > >> > > Ismael >> > > >> > > On Mon, May 30, 2022, 2:08 AM Bruno Cadonna <cado...@apache.org> >> wrote: >> > > >> > > > Hi, >> > > > >> > > > I would also lean towards getOrCreateMetric() for the reasons pointed >> > > > out by Sagar. But I am fine either way. >> > > > >> > > > Best, >> > > > Bruno >> > > > >> > > > On 30.05.22 10:54, Sagar wrote: >> > > > > Hi Bruno/David, >> > > > > >> > > > > Thanks for the suggestions. I would personally lean towards using >> > > > > getOrCreateMetric as it clearly explains the intent. Having said >> > that, >> > > if >> > > > > we want to use just metric(similar to sensor), that should also be >> > ok. >> > > > Just >> > > > > that I feel getOrCreateMetric is easily understandable. >> > > > > >> > > > > Thanks! >> > > > > Sagar. >> > > > > >> > > > > On Mon, May 30, 2022 at 2:16 PM David Jacot >> > > <dja...@confluent.io.invalid >> > > > > >> > > > > wrote: >> > > > > >> > > > >> Hi all, >> > > > >> >> > > > >> Looking at the current Metrics' API, we have `sensor` which gets >> or >> > > > creates >> > > > >> a sensor. How about using `metric` to follow the same naming >> > > convention? >> > > > >> >> > > > >> Best, >> > > > >> David >> > > > >> >> > > > >> On Mon, May 30, 2022 at 9:18 AM Bruno Cadonna <cado...@apache.org >> > >> > > > wrote: >> > > > >>> >> > > > >>> Hi Sagar, >> > > > >>> Hi Ismael, >> > > > >>> >> > > > >>> what about getOrCreateMetric()? >> > > > >>> >> > > > >>> Best, >> > > > >>> Bruno >> > > > >>> >> > > > >>> >> > > > >>> On 28.05.22 18:56, Sagar wrote: >> > > > >>>> Hi Ismael, >> > > > >>>> >> > > > >>>> Actually Bruno suggested renaming it to getMetricOrElseCreate >> and >> > we >> > > > >>>> decided to go ahead with that one. These were the only names >> that >> > we >> > > > >>>> considered for the KIP. >> > > > >>>> >> > > > >>>> Thanks! >> > > > >>>> Sagar. >> > > > >>>> >> > > > >>>> >> > > > >>>> On Sat, May 28, 2022 at 8:19 PM Ismael Juma <ism...@juma.me.uk> >> > > > wrote: >> > > > >>>> >> > > > >>>>> Thanks for the KIP. The method makes sense, but the name is a >> bit >> > > > >> verbose. >> > > > >>>>> Have we considered a more concise name? >> > > > >>>>> >> > > > >>>>> Ismael >> > > > >>>>> >> > > > >>>>> On Tue, May 24, 2022, 4:49 AM Sagar <sagarmeansoc...@gmail.com >> > >> > > > >> wrote: >> > > > >>>>> >> > > > >>>>>> Hi All, >> > > > >>>>>> >> > > > >>>>>> I would like to open a voting thread for the following KIP: >> > > > >>>>>> >> > > > >>>>>> >> > > > >>>>>> >> > > > >>>>> >> > > > >> >> > > > >> > > >> > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-843%3A+Adding+metricOrElseCreate+method+to+Metrics >> > > > >>>>>> >> > > > >>>>>> Thanks! >> > > > >>>>>> Sagar. >> > > > >>>>>> >> > > > >>>>> >> > > > >>>> >> > > > >> >> > > > > >> > > > >> > > >> > >> >> >> -- >> -- Guozhang >>
