Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@kafka.apache.org
MIME-Version: 1.0
In-Reply-To: <CAFX5QBqRQeBv-wGf5WhHHGtn9WRvxj8vcEWAbYY+tRFQ-kSKhA@mail.gmail.com>
References: <CAEhVOwHkpxkotbT72Yuc9pZM61ctEZ_sZx33YXqRY4bc8xhx3A@mail.gmail.com>
 <CANTWn9NJ8G8PamyWV8+RTYT-L_kBcCcD7y4=B8zSXk-9ai6WvQ@mail.gmail.com>
 <CAEhVOwF+u-1z+Z61r9SF4j3_Nm-_fkrj9ErDfLzSAi4DRQ=Uew@mail.gmail.com> <CAFX5QBqRQeBv-wGf5WhHHGtn9WRvxj8vcEWAbYY+tRFQ-kSKhA@mail.gmail.com>
From: Guozhang Wang <wangguoz@gmail.com>
Date: Tue, 3 Jan 2017 14:27:03 -0800
Message-ID: <CAHwHRrWxvn0HW34NfpBu2u4_LVeej+xFxG28YTssdiUjbcr0cQ@mail.gmail.com>
Subject: Re: [DISCUSS] KIP-105: Addition of Record Level for Sensors
To: "dev@kafka.apache.org" <dev@kafka.apache.org>
Content-Type: multipart/alternative; boundary=001a114e718ee4c40d0545382a0b
archived-at: Tue, 03 Jan 2017 22:27:18 -0000

--001a114e718ee4c40d0545382a0b
Content-Type: text/plain; charset=UTF-8

LGTM overall. A few comments below:

1. "METRICS_RECORD_LEVEL_CONFIG" is the internal code's variable name, but
not the real config string, I think it is `metrics.record.level` instead?

2. In the Motivation section, as in "This associates each sensor with a
record level ... only if the metric config of the client requires these
metrics to be recorded."

Could you elaborate this a bit more, for example, will the sensor ever be
registered in the reporter if its level is not allowed in the client
config? Or it will be registered but never polled? Or it will be registered
and polled, but recorded?

From PR 1446 it seems to be the last case, i.e. the sensor will have the
default value based on its type, and it will still be polled by the
reporter but not recorded. To an end-user's experience it will mean that
for example the monitoring UI that displays all polled metrics will still
show the metrics graph, with the value consistently shown as the default
value, instead of not showing the graphs at all.

3. Could you elaborate on the "shouldRecord()" function, e.g. which class
it will be added to? Does it contain any parameters?


Guozhang


On Sun, Jan 1, 2017 at 5:31 AM, Eno Thereska <eno.thereska@gmail.com> wrote:

> Thanks for starting the discussion on these KIPs Aarti.
>
> Eno
>
> On Sunday, January 1, 2017, Aarti Gupta <aartiguptaa@gmail.com> wrote:
>
> > Thanks Radai,
> >
> > Yes that is the correct link, my bad
> >
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 105%3A+Addition+of+Record+Level+for+Sensors
> >
> >
> >
> > Aarti
> >
> > On Sat, Dec 31, 2016 at 9:32 PM radai <radai.rosenblatt@gmail.com
> > <javascript:;>> wrote:
> >
> > > link leads to 104. i think this is the correct one -
> > >
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 105%3A+Addition+of+Record+Level+for+Sensors
> > >
> > > ?
> > >
> > >
> > >
> > > On Fri, Dec 30, 2016 at 8:31 PM, Aarti Gupta <aartiguptaa@gmail.com
> > <javascript:;>>
> > > wrote:
> > >
> > >
> > >
> > > > Hi all,
> > >
> > > >
> > >
> > > > I would like to start the discussion on KIP-105: Addition of Record
> > Level
> > >
> > > > for Sensors
> > >
> > > > *https://cwiki.apache.org/confluence/pages/viewpage.action?
> > >
> > > > <
> > > https://cwiki.apache.org/confluence/pages/viewpage.
> > action?pageId=67636480
> > >
> > > > >*
> > >
> > > > *pageId=67636483*
> > >
> > > >
> > >
> > > > Looking forward to your feedback.
> > >
> > > >
> > >
> > > > Thanks,
> > >
> > > > Aarti and Eno
> > >
> > > >
> > >
> > >
> >
>


-- 
-- Guozhang

--001a114e718ee4c40d0545382a0b--