Hey Sagar, thanks for the KIP! And yes, all metrics are considered part of the public API and thus require a KIP to add (or modify, etc...) Although in this particular case, you could probably make a good case for just considering it as an update to the original KIP which added the analogous metric `input-buffer-bytes-total`. For things like this that weren't considered during the KIP proposal but came up during the implementation or review, and are small changes that would have made sense to include in that KIP had they been thought of, you can just send an update to the existing KIP's discussion and.or voting thread that explains what you want to add or modify and maybe a brief description why.
It's always ok to make a new KIP when in doubt, but there are some cases where an update email is sufficient. If there are any concerns or suggestions that significantly expand the scope of the update, you can always go create a new KIP and move the discussion there. I'd say you can feel free to proceed in whichever way you'd prefer for this new proposal -- it just needs to appear in some KIP somewhere, and have given the community thew opportunity to discuss and provide feedback on. On that note, I do have two suggestions: 1) since we need to measure the size of the cache (and the input buffer(s) anyways, we may as well make `cache-size-bytes-total` -- and also the new input-buffer-bytes-total -- an INFO level metric. In general the more metrics the merrier, the only real reason for disabling some are if they have a performance impact or other cost that not everyone will want to pay. In this case we're already computing the value of these metrics, so why not expose it to the user as an INFO metric 2) I think it would be both more natural and easier to implement if this was a store-level metric. A single task could in theory contain multiple physical state store caches and we would have to roll them up to report the size for the task as a whole. It's additional work just to lose some information that the user may want to have Let me know if anything here doesn't make sense or needs clarification. And thanks for the quick followup to get this 2nd metric! -Sophie On Sat, Jan 29, 2022 at 4:27 AM Sagar <sagarmeansoc...@gmail.com> wrote: > Hi All, > > I would like to open a discussion thread on the following KIP: > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=186878390 > > PS: This is about introducing a new metric and I am assuming that it > requires a KIP. If that isn't the case, I can close it. > > Thanks! > Sagar. >
