Hi all,

I’m closing the voting on this one. We already had 3 binding votes on this
from Guozhang, Bruno and John.

Thanks everyone!

Sagar.

On Sun, 5 Jun 2022 at 1:53 PM, Sagar <sagarmeansoc...@gmail.com> wrote:

> Hi All,
>
> I am planning to use the name of the new function as addMetricIfAbsent.
> Let me know if there are any other suggestions/objections to this.
>
> Also, @Ismael, I agree with Guozhang about not deprecating the existing
> functions. Plz let me know if you have any reservations about that.
>
> If we all agree to these 2 points, then I would mark the KIP as voted.
>
> Thanks!
> Sagar.
>
> On Wed, Jun 1, 2022 at 11:47 PM Guozhang Wang <wangg...@gmail.com> wrote:
>
>> I think `addMetricIfAbsent` is a good function name, and like John said
>> people in the Java world are familiar with its return value semantics as
>> well.
>>
>> Regarding deprecating the existing functions, I feel it is not necessary
>> just for the function semantics difference between `sensors` and `metrics`
>> (Ismael may chime in here if you have other good reasons).
>>
>>
>> Guozhang
>>
>> On Wed, Jun 1, 2022 at 9:50 AM Sagar <sagarmeansoc...@gmail.com> wrote:
>>
>> > So, we have multiple options in terms of names, at this point I actually
>> > liked John's suggestion to use addMetricIfAbsent or something along
>> those
>> > lines.
>> >
>> > Regarding the deprecation of sensor/metric method, I am not sure...
>> Would
>> > like to know others' thoughts.
>> >
>> > Thanks!
>> > Sagar.
>> >
>> > On Wed, Jun 1, 2022 at 2:28 AM Guozhang Wang <wangg...@gmail.com>
>> wrote:
>> >
>> > > Hey Ismael, just checking do you mean the `metric` method instead?
>> > >
>> > > On Tue, May 31, 2022 at 1:45 PM Ismael Juma <ism...@juma.me.uk>
>> wrote:
>> > >
>> > > > Should we deprecate the `sensor` method? One other thing to take
>> into
>> > > > account is that these methods are meant to be used like a dsl for
>> > > > configuring sensors and metrics. So brevity is a plus (but clarity
>> is
>> > > > critical still).
>> > > >
>> > > > Ismael
>> > > >
>> > > > On Tue, May 31, 2022 at 11:09 AM John Roesler <vvcep...@apache.org>
>> > > wrote:
>> > > >
>> > > > > 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
>> > > > > >>
>> > > > >
>> > > >
>> > >
>> > >
>> > > --
>> > > -- Guozhang
>> > >
>> >
>>
>>
>> --
>> -- Guozhang
>>
>

Reply via email to