Thanks Mohamed, I think Bruno raised a good point in the ticket that the node name is not well known from within the Processor at the time of init(), so users would basically have to make up a name to pass into the sensor. Maybe this is ok, but it doesn't seem too nice.
Offhand, it seems like the options are: 1. To just expose the node name also in ProcessorContext (such as with a nullable "processorNodeName()" method). 2. To close over the node name in the view of the context and metrics that we pass to the Processor when we call init(). The caller of init() is actually the ProcessorNode itself, so it can easily insert this information into the metrics before invoking Processor#init(). Offhand, it seems option 2 gives a simpler and better experience. My concern would be that it might be "too fancy". Also, if we favor this route, we should reconsider many of the other arguments to those methods. Thanks for the KIP! -John On Fri, 2020-08-14 at 01:37 +0100, Mohamed Chebbi wrote: > KIP updated with the comments of Bruno Cardona. > > Le 06/07/2020 à 22:36, Mohamed Chebbi a écrit : > > Thank Bruno for your review. > > > > Changes was added as you sugested. > > > > Le 06/07/2020 à 14:57, Bruno Cadonna a écrit : > > > Hi Mohamed, > > > > > > Thank you for the KIP. > > > > > > Comments regarding the KIP wiki: > > > > > > 1. In section "Public Interface", you should state what you want to > > > change in interface StreamsMetrics. In your case, you want to add two > > > methods. You can find a good example how to describe this in KIP-444 > > > (https://cwiki.apache.org/confluence/display/KAFKA/KIP-444%3A+Augment+metrics+for+Kafka+Streams). > > > > > > > > > > > > 2. In section "Compatibility, Deprecation, and Migration Plan" you > > > should state if anything is needed to keep backward compatibility. > > > Since you just want to add two methods to the interface, nothing is > > > needed. You should describe that under that section. > > > > > > Regarding the KIP content, I left some comments on the corresponding > > > Jira ticket. > > > > > > Best, > > > Bruno > > > > > > > > > On Sun, Jul 5, 2020 at 3:48 AM Mohamed Chebbi <mhmdche...@gmail.com> > > > wrote: > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-639%3A+Move+nodeLevelSensor+and+storeLevelSensor+methods+from+StreamsMetricsImpl+to+StreamsMetrics > > > > > > > > > > > >
