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
>

Reply via email to