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.there...@gmail.com> wrote: > Thanks for starting the discussion on these KIPs Aarti. > > Eno > > On Sunday, January 1, 2017, Aarti Gupta <aartigup...@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.rosenbl...@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 <aartigup...@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
