[GitHub] [kafka] ableegoldman commented on pull request #12235: KAFKA-13945: add bytes/records consumed and produced metrics

[GitBox <gi...@apache.org>]

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-unsubscribe@kafka.apache.org

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