ableegoldman commented on PR #12235:
URL: https://github.com/apache/kafka/pull/12235#issuecomment-1145785669

   @cadonna ok this is ready for another pass -- besides addressing your 
comments I made two changes worth noting:
   1. I moved the produced/consumed metrics to a new `TopicMetrics` class as 
they didn't exactly fit into ProcessorNodeMetrics since I realized that due to 
dynamic topic routing you could have more than one topic produced to by a given 
sink node.
   2. To address your (imo valid) concern about over-counting bytes that were 
sent to the producer but never made it to the topic, I moved the "-produced" 
sensor into `RecordCollectorImpl` and have it record inside the `send` callback 
after we've checked for errors
   
   The PR has definitely ballooned in size since you last reviewed it but 
that's almost all due to the new test coverage for the topic-level metrics. The 
actual logical changes are still relatively minor, so I'm hoping you can give 
this a +1 by now (and again, if so, please go ahead and merge it yourself)
   
   Thanks for the thorough reviews! :P 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to