Defined a new broker metric for total query processing time (#8941)

* Defined a new broker metric for total query processing time and added metric to broker.yaml * change metric type to Timer and verified that it works * reverting package-lock updates
apache · Jun 24, 2022 · 6582750 · 6582750
1 parent 47c637c
commit 6582750
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 0 deletions.
diff --git a/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml b/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml
@@ -123,6 +123,11 @@ rules:
   cache: true
   labels:
     table: "$1"
+- pattern: "\"org.apache.pinot.common.metrics\"<type=\"BrokerMetrics\", name=\"pinot.broker.(\\w+)\\.queryTotalTimeMs\"><>(\\w+)"
+  name: "pinot_broker_queryTotalTimeMs_$2"
+  cache: true
+  labels:
+    table: "$1"
 - pattern: "\"org.apache.pinot.common.metrics\"<type=\"BrokerMetrics\", name=\"pinot.broker.(\\w+)_(\\w+).serverMissingForRouting\"><>(\\w+)"
   name: "pinot_broker_serverMissingForRouting_$3"
   cache: true

diff --git a/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/pinot.yml b/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/pinot.yml
@@ -186,6 +186,11 @@ rules:
   cache: true
   labels:
     table: "$1"
+- pattern: "\"org.apache.pinot.common.metrics\"<type=\"BrokerMetrics\", name=\"pinot.broker.(\\w+)\\.queryTotalTimeMs\"><>(\\w+)"
+  name: "pinot_broker_queryTotalTimeMs_$2"
+  cache: true
+  labels:
+    table: "$1"
 - pattern: "\"org.apache.pinot.common.metrics\"<type=\"BrokerMetrics\", name=\"pinot.broker.(\\w+)\\.reduce\"><>(\\w+)"
   name: "pinot_broker_reduce_$2"
   cache: true

diff --git a/...broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java b/...broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
@@ -50,6 +50,7 @@
 import org.apache.pinot.common.metrics.BrokerMeter;
 import org.apache.pinot.common.metrics.BrokerMetrics;
 import org.apache.pinot.common.metrics.BrokerQueryPhase;
+import org.apache.pinot.common.metrics.BrokerTimer;
 import org.apache.pinot.common.request.BrokerRequest;
 import org.apache.pinot.common.request.Expression;
 import org.apache.pinot.common.request.ExpressionType;
@@ -585,6 +586,8 @@ private BrokerResponseNative handleRequest(long requestId, String query, JsonNod
     brokerResponse.setTimeUsedMs(totalTimeMs);
     requestContext.setQueryProcessingTime(totalTimeMs);
     augmentStatistics(requestContext, brokerResponse);
+    _brokerMetrics.addTimedTableValue(rawTableName, BrokerTimer.QUERY_TOTAL_TIME_MS, totalTimeMs,
+        TimeUnit.MILLISECONDS);
 
     logBrokerResponse(requestId, query, requestContext, tableName, numUnavailableSegments, serverStats, brokerResponse,
         totalTimeMs);

diff --git a/pinot-common/src/main/java/org/apache/pinot/common/metrics/BrokerTimer.java b/pinot-common/src/main/java/org/apache/pinot/common/metrics/BrokerTimer.java
@@ -29,6 +29,7 @@ public enum BrokerTimer implements AbstractMetrics.Timer {
   ROUTING_TABLE_UPDATE_TIME(true),
   CLUSTER_CHANGE_QUEUE_TIME(true), // metric tracking the freshness lag for consuming segments
   FRESHNESS_LAG_MS(false),
+  QUERY_TOTAL_TIME_MS(false),
 
   // The latency of sending the request from broker to server
   NETTY_CONNECTION_SEND_REQUEST_LATENCY(false),