Hi John, Thank you for the KIP.
LGTM I also do not like CumulativeSum/Count so much. I propose to just call it Sum and Count. I understand that you want to unequivocally distinguish the two metric functions by their names, but I have the feeling the names become artificially complex. The exact semantics can also be documented in the javadocs, which btw could also be improved in those classes. Best, Bruno On Thu, Jul 11, 2019 at 8:25 PM Matthias J. Sax <matth...@confluent.io> wrote: > > Thanks for the KIP. Overall LGTM. > > The only though I have is, if we may want to use `TotalSum` and > `TotalCount` instead of `CumulativeSum/Count` as names? > > > -Matthias > > > On 7/11/19 9:31 AM, John Roesler wrote: > > Hi Kafka devs, > > > > I'd like to propose KIP-488 as a minor cleanup of some of our metric > > implementations. > > > > KIP-488: https://cwiki.apache.org/confluence/x/kkAyBw > > > > Over time, iterative updates to these metrics has resulted in a pretty > > confusing little collection of classes, and I've personally been > > involved in three separate moderately time-consuming iterations of me > > or someone else trying to work out which metrics are available, and > > which ones are desired for a given use case. One of these was actually > > a long-running bug in Kafka Streams' metrics, so not only has this > > confusion been a time sink, but it has also led to bugs. > > > > I'm hoping this change won't be too controversial. > > > > Thanks, > > -John > > >
