gortiz commented on code in PR #15387:
URL: https://github.com/apache/pinot/pull/15387#discussion_r2018026339
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/BrokerMetrics.java:
##########
@@ -84,4 +91,23 @@ protected BrokerMeter[] getMeters() {
protected BrokerGauge[] getGauges() {
return BrokerGauge.values();
}
+
+ // Emits metrics based on BrokerResponse. Currently only emits metrics for
exceptions.
+ // If a broker response has multiple exceptions, we will emit metrics for
all of them.
+ // Thus, the sum total of all exceptions is >= total number of queries
impacted.
+ // Additionally, some parts of code might already be emitting metrics for
individual error codes.
+ // But that list isn't accurate with a many-to-many relationship (or no
metrics) between error codes and metrics.
+ // This method ensures we emit metrics for all queries that have exceptions
with a one-to-one mapping.
+ public void emitBrokerResponseMetrics(BrokerResponse brokerResponse) {
Review Comment:
Please change the simple comment (`//`) to one of the javadoc formats
(either the new `///` or the good old classic `/**/)
Thinking about the place... maybe it could be done in the opposite way. We
can add a emitMetrics(BrokerMetrics) in BrokerResponse. WDYT?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
