Re: [PR] Catch-all Regex for JXM -> Prom Exporter [pinot]

["zhtaoxiang (via GitHub)" <gi...@apache.org>]

zhtaoxiang commented on code in PR #12073:
URL: https://github.com/apache/pinot/pull/12073#discussion_r1414264165


##########
docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml:
##########
@@ -157,15 +157,37 @@ rules:
 
   ## Metrics that fit the catch-all patterns above should not be added to this file.
   ## In case a metric does not fit the catch-all patterns, add them before this comment
-
-  # This is a catch-all pattern for pinot table metrics with offline/realtime suffix.
+  # This is a catch-all pattern for pinot table metrics with offline/realtime suffix without kafka topic
   # Patterns after this line may be skipped.

Review Comment:
   nit: I feel that we should add this comment for each catch-all pattern so that people know the potential risks?



##########
docker/images/pinot/etc/jmx_prometheus_javaagent/configs/server.yml:
##########
@@ -50,21 +50,21 @@ rules:
 - pattern: "\"org.apache.pinot.common.metrics\"<type=\"ServerMetrics\", name=\"pinot.server.llcControllerResponse(\\w+)\"><>(\\w+)"
   name: "pinot_server_llcControllerResponse_$1_$2"
   cache: true
-- pattern: "\"org.apache.pinot.common.metrics\"<type=\"ServerMetrics\", name=\"pinot.server.llcPartitionConsuming.([^\\.]*?)_(OFFLINE|REALTIME)\\-(.+)\\-(\\w+)\"><>(\\w+)"
-  name: "pinot_server_llcPartitionConsuming_$5"
-  cache: true
-  labels:
-    table: "$1"
-    tableType: "$2"
-    topic: "$3"
-    partition: "$4"
-- pattern: "\"org.apache.pinot.common.metrics\"<type=\"ServerMetrics\", name=\"pinot.server.realtimeIngestionDelayMs.([^\\.]*?)_(OFFLINE|REALTIME)\\.(\\w+)\"><>(\\w+)"
-  name: "pinot_server_realtimeIngestionDelayMs_$4"
-  cache: true
-  labels:
-    table: "$1"
-    tableType: "$2"
-    partition: "$3"
+#- pattern: "\"org.apache.pinot.common.metrics\"<type=\"ServerMetrics\", name=\"pinot.server.llcPartitionConsuming.([^\\.]*?)_(OFFLINE|REALTIME)\\-(.+)\\-(\\w+)\"><>(\\w+)"

Review Comment:
   just want to confirm: with those newly-added patterns, those commented out ones will remain the same, right? We don't want to introduce breaking changes 😛 



##########
docker/images/pinot/etc/jmx_prometheus_javaagent/configs/server.yml:
##########
@@ -50,21 +50,21 @@ rules:
 - pattern: "\"org.apache.pinot.common.metrics\"<type=\"ServerMetrics\", name=\"pinot.server.llcControllerResponse(\\w+)\"><>(\\w+)"
   name: "pinot_server_llcControllerResponse_$1_$2"
   cache: true
-- pattern: "\"org.apache.pinot.common.metrics\"<type=\"ServerMetrics\", name=\"pinot.server.llcPartitionConsuming.([^\\.]*?)_(OFFLINE|REALTIME)\\-(.+)\\-(\\w+)\"><>(\\w+)"
-  name: "pinot_server_llcPartitionConsuming_$5"
-  cache: true
-  labels:
-    table: "$1"
-    tableType: "$2"
-    topic: "$3"
-    partition: "$4"
-- pattern: "\"org.apache.pinot.common.metrics\"<type=\"ServerMetrics\", name=\"pinot.server.realtimeIngestionDelayMs.([^\\.]*?)_(OFFLINE|REALTIME)\\.(\\w+)\"><>(\\w+)"
-  name: "pinot_server_realtimeIngestionDelayMs_$4"
-  cache: true
-  labels:
-    table: "$1"
-    tableType: "$2"
-    partition: "$3"
+#- pattern: "\"org.apache.pinot.common.metrics\"<type=\"ServerMetrics\", name=\"pinot.server.llcPartitionConsuming.([^\\.]*?)_(OFFLINE|REALTIME)\\-(.+)\\-(\\w+)\"><>(\\w+)"

Review Comment:
   just want to confirm: with those newly-added patterns, those commented out ones will remain the same, right? We don't want to introduce breaking changes 😛 



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

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


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