You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "suddendust (via GitHub)" <gi...@apache.org> on 2023/11/30 07:11:02 UTC

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

suddendust opened a new pull request, #12073:
URL: https://github.com/apache/pinot/pull/12073

   Recently, we detected that the catch-all label was incorrect - It failed to capture a metric for which a specific regex had not been added. It also does not capture metrics that contain the kafka topic in the name. This PR fixes these problems. Commented regexes are captured with catch-all, I have kept them as they can come in handy later.
   
   Commented metrics are exported successfully:
   
   <img width="1294" alt="Screenshot 2023-11-30 at 12 37 27 PM" src="https://github.com/apache/pinot/assets/84911643/c367107a-dc9e-4e18-ae52-3823ed237186">
   
   <img width="1220" alt="Screenshot 2023-11-30 at 12 38 46 PM" src="https://github.com/apache/pinot/assets/84911643/e13c6471-7c90-444b-9034-e5b5fc9a53e5">
   
   <img width="1300" alt="Screenshot 2023-11-30 at 12 39 44 PM" src="https://github.com/apache/pinot/assets/84911643/f3e0d740-a59e-48bc-a4f5-cf3ffb8303f3">
   <img width="1660" alt="Screenshot 2023-11-30 at 12 40 03 PM" src="https://github.com/apache/pinot/assets/84911643/9b16fda9-1324-42e5-92bc-9107e2b7eec4">
   
   
   <img width="1153" alt="Screenshot 2023-11-30 at 12 40 19 PM" src="https://github.com/apache/pinot/assets/84911643/2928fc82-5a7a-4856-a616-bf4c8bc917b2">
   
   <img width="1660" alt="Screenshot 2023-11-30 at 12 40 32 PM" src="https://github.com/apache/pinot/assets/84911643/4d18883f-dbb3-48db-9bd5-7d2b06fea846">
   
   <img width="1383" alt="Screenshot 2023-11-30 at 12 40 47 PM" src="https://github.com/apache/pinot/assets/84911643/ac388342-57ed-411b-8cb6-041c7c0d966f">
   


-- 
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


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

Posted by "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


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

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #12073:
URL: https://github.com/apache/pinot/pull/12073#issuecomment-1834401945

   Out of the scope of this PR, will these regex increase the memory footprint of the metrics? I've been observing saw-shape memory usage on servers without query load, and suspecting that is caused by the metrics. #8232 listed some previous work to reduce the memory footprint, which might provide some insights.


-- 
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


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

Posted by "KKcorps (via GitHub)" <gi...@apache.org>.
KKcorps commented on code in PR #12073:
URL: https://github.com/apache/pinot/pull/12073#discussion_r1410453242


##########
docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml:
##########
@@ -157,15 +157,24 @@ 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.
-- pattern: "\"?org\\.apache\\.pinot\\.common\\.metrics\"?<type=\"?\\w+\"?, name=\"?pinot\\.(\\w+)\\.(\\w+)_(OFFLINE|REALTIME)\\.(\\w+)\"?><>(\\w+)"
-  name: "pinot_$1_$4_$5"
+- pattern: "\"?org\\.apache\\.pinot\\.common\\.metrics\"?<type=\"?\\w+\"?, name=\"?pinot\\.(\\w+)\\.(\\w+)\\.(\\w+)_(OFFLINE|REALTIME)\\.(\\w+)\"?><>(\\w+)"

Review Comment:
   Should we change the existing catch all? I feel like the old one might be capturing something which may get lost with this new
   
   It's better imo to include this one as a seperate 



##########
docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml:
##########
@@ -157,15 +157,24 @@ 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.
-- pattern: "\"?org\\.apache\\.pinot\\.common\\.metrics\"?<type=\"?\\w+\"?, name=\"?pinot\\.(\\w+)\\.(\\w+)_(OFFLINE|REALTIME)\\.(\\w+)\"?><>(\\w+)"
-  name: "pinot_$1_$4_$5"
+- pattern: "\"?org\\.apache\\.pinot\\.common\\.metrics\"?<type=\"?\\w+\"?, name=\"?pinot\\.(\\w+)\\.(\\w+)\\.(\\w+)_(OFFLINE|REALTIME)\\.(\\w+)\"?><>(\\w+)"

Review Comment:
   Should we change the existing catch all? I feel like the old one might be capturing something which may get lost with this new
   
   It's better imo to include this one as a seperate rule



-- 
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


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

Posted by "suddendust (via GitHub)" <gi...@apache.org>.
suddendust commented on code in PR #12073:
URL: https://github.com/apache/pinot/pull/12073#discussion_r1414290730


##########
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:
   Yes, that has been tested. Have explained in the PR description.



-- 
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


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

Posted by "zhtaoxiang (via GitHub)" <gi...@apache.org>.
zhtaoxiang commented on code in PR #12073:
URL: https://github.com/apache/pinot/pull/12073#discussion_r1440714361


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

Review Comment:
   +1



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

Review Comment:
   and also can we remove other commented code?



-- 
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


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

Posted by "suddendust (via GitHub)" <gi...@apache.org>.
suddendust commented on code in PR #12073:
URL: https://github.com/apache/pinot/pull/12073#discussion_r1410715104


##########
docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml:
##########
@@ -157,15 +157,24 @@ 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.
-- pattern: "\"?org\\.apache\\.pinot\\.common\\.metrics\"?<type=\"?\\w+\"?, name=\"?pinot\\.(\\w+)\\.(\\w+)_(OFFLINE|REALTIME)\\.(\\w+)\"?><>(\\w+)"
-  name: "pinot_$1_$4_$5"
+- pattern: "\"?org\\.apache\\.pinot\\.common\\.metrics\"?<type=\"?\\w+\"?, name=\"?pinot\\.(\\w+)\\.(\\w+)\\.(\\w+)_(OFFLINE|REALTIME)\\.(\\w+)\"?><>(\\w+)"

Review Comment:
   Good catch, fixed.



-- 
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


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

Posted by "suddendust (via GitHub)" <gi...@apache.org>.
suddendust commented on code in PR #12073:
URL: https://github.com/apache/pinot/pull/12073#discussion_r1452651274


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

Review Comment:
   Done



-- 
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


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

Posted by "suddendust (via GitHub)" <gi...@apache.org>.
suddendust commented on PR #12073:
URL: https://github.com/apache/pinot/pull/12073#issuecomment-1853392693

   - Added these catch-all regexes in pinot.yml.
   - Have tested all commented regexes by running the exporter locally, all such metrics are being exported.


-- 
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


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

Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr merged PR #12073:
URL: https://github.com/apache/pinot/pull/12073


-- 
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


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

Posted by "suddendust (via GitHub)" <gi...@apache.org>.
suddendust commented on code in PR #12073:
URL: https://github.com/apache/pinot/pull/12073#discussion_r1414501894


##########
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:
   >Commented metrics are exported successfully



-- 
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


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

Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12073:
URL: https://github.com/apache/pinot/pull/12073#discussion_r1440609057


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

Review Comment:
   do we still need this? can we delete?



-- 
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


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

Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #12073:
URL: https://github.com/apache/pinot/pull/12073#issuecomment-1893219156

   > Out of the scope of this PR, will these regex increase the memory footprint of the metrics? I've been observing saw-shape memory usage on servers without query load, and suspecting that is caused by the metrics. #8232 listed some previous work to reduce the memory footprint, which might provide some insights.
   
   It is difficult to analyze why #8232 increased efficiency given it:
   1. Doesn't seem to include the older rules (which seem to be hardcoded before #8232.
   2. Adds one rule per metric but also adds cache. In the 
   
   I would say it is quite more expensive to process hundreds of very large regexps with the same prefix than a couple of them with correct quantifiers. And it is obviously easier for us to write.


-- 
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