krishan1390 commented on code in PR #15387:
URL: https://github.com/apache/pinot/pull/15387#discussion_r2016312059


##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java:
##########
@@ -560,6 +568,24 @@ public static Response 
getPinotQueryResponse(BrokerResponse brokerResponse)
         .build();
   }
 
+  // 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.
+  private void emitBrokerResponseMetrics(BrokerResponse brokerResponse) {

Review Comment:
   This method should be moved to a common place so that it can be reused in 
BrokerGrpcServer and other places. 
   
   what would be the right class for it ? BrokerResponse or BrokerMetrics or 
something else ? 



-- 
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: commits-unsubscr...@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to