You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Sagar <sa...@gmail.com> on 2022/06/01 16:50:21 UTC
Re: [VOTE] KIP-843: Adding metricOrElseCreate method to Metrics
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 <wa...@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 <is...@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 <vv...@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 <wa...@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 <sa...@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 <is...@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 <cadonna@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
> > > >> > > <djacot@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 <
> > > cadonna@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 <
> > > ismael@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 <
> > > sagarmeansocean@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
>
Re: [VOTE] KIP-843: Adding metricOrElseCreate method to Metrics
Posted by Sagar <sa...@gmail.com>.
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 <sa...@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 <wa...@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 <sa...@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 <wa...@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 <is...@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 <vv...@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 <
>> wangguoz@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 <
>> sagarmeansocean@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 <
>> ismael@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 <
>> > cadonna@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
>> > > > > >> > > <djacot@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 <
>> > > > > cadonna@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 <
>> > > > > ismael@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 <
>> > > > > sagarmeansocean@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
>>
>
Re: [VOTE] KIP-843: Adding metricOrElseCreate method to Metrics
Posted by Sagar <sa...@gmail.com>.
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 <wa...@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 <sa...@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 <wa...@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 <is...@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 <vv...@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 <
> wangguoz@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 <
> sagarmeansocean@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 <
> ismael@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 <
> > cadonna@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
> > > > > >> > > <djacot@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 <
> > > > > cadonna@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 <
> > > > > ismael@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 <
> > > > > sagarmeansocean@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
>
Re: [VOTE] KIP-843: Adding metricOrElseCreate method to Metrics
Posted by Guozhang Wang <wa...@gmail.com>.
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 <sa...@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 <wa...@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 <is...@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 <vv...@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 <wangguoz@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 <sa...@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 <is...@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 <
> cadonna@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
> > > > >> > > <djacot@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 <
> > > > cadonna@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 <
> > > > ismael@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 <
> > > > sagarmeansocean@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