Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
KKcorps merged PR #13290: URL: https://github.com/apache/pinot/pull/13290 -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
shounakmk219 commented on PR #13290: URL: https://github.com/apache/pinot/pull/13290#issuecomment-2150188180 ## Controller metrics validations: 1. https://github.com/apache/pinot/assets/25409127/cb1a6438-8797-4de0-99f1-cff12ec7bd51";> 2. https://github.com/apache/pinot/assets/25409127/9f2570be-1ecb-487f-bf2b-943fd3ffcb01";> ## Broker metrics validations 1. https://github.com/apache/pinot/assets/25409127/85fbb98d-e27e-4341-bd30-0d7a3d72facf";> 2. https://github.com/apache/pinot/assets/25409127/8f23d296-54e5-43d6-8da2-a987d8d476bb";> ## Server metrics validation 1. https://github.com/apache/pinot/assets/25409127/15aa2187-aedb-414b-8537-387ef5b32d87";> 2. https://github.com/apache/pinot/assets/25409127/9abad935-6f1e-42ac-859b-9be7342138ef";> -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
shounakmk219 commented on code in PR #13290: URL: https://github.com/apache/pinot/pull/13290#discussion_r1627830416 ## docker/images/pinot/etc/jmx_prometheus_javaagent/configs/server.yml: ## @@ -73,6 +66,14 @@ rules: table: "$1$3" tableType: "$4" partition: "$5" +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" Review Comment: Yeah. `realtimeIngestionDelayMs` is handled by moving the metric rule earlier present on line 16 to line 175. Will add screenshots on validation as well ## docker/images/pinot/etc/jmx_prometheus_javaagent/configs/server.yml: ## @@ -171,6 +172,14 @@ rules: name: "pinot_server_grpc$1_$2" cache: true +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" + name: "pinot_server_$5_$6" + cache: true + labels: +database: "$2" +table: "$1$3" Review Comment: replied above -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
shounakmk219 commented on code in PR #13290: URL: https://github.com/apache/pinot/pull/13290#discussion_r1627823043 ## docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml: ## @@ -112,6 +118,12 @@ rules: - pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" name: "pinot_broker_routingTableUpdateTime_$1" cache: true +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" + name: "pinot_broker_adaptiveServerSelectorType_$1" Review Comment: Its does not have any extracted info to add as a label. More of a static config info metric ## docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml: ## @@ -112,6 +118,12 @@ rules: - pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" name: "pinot_broker_routingTableUpdateTime_$1" cache: true +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" + name: "pinot_broker_adaptiveServerSelectorType_$1" + cache: true +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" + name: "pinot_broker_adaptiveServerSelectorType_$1_$2" Review Comment: replied above -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
shounakmk219 commented on code in PR #13290: URL: https://github.com/apache/pinot/pull/13290#discussion_r1627809416 ## docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml: ## @@ -59,6 +59,12 @@ rules: labels: database: "$2" table: "$1$3" +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" + name: "pinot_broker_requestSize_$4" + cache: true + labels: +database: "$2" +table: "$1$3" Review Comment: $1 matches the whole `databaseName.` part hence no need to delimit explicitly. -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
suddendust commented on code in PR #13290: URL: https://github.com/apache/pinot/pull/13290#discussion_r1624064432 ## docker/images/pinot/etc/jmx_prometheus_javaagent/configs/server.yml: ## @@ -171,6 +172,14 @@ rules: name: "pinot_server_grpc$1_$2" cache: true +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" + name: "pinot_server_$5_$6" + cache: true + labels: +database: "$2" +table: "$1$3" Review Comment: Same as above -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
suddendust commented on code in PR #13290: URL: https://github.com/apache/pinot/pull/13290#discussion_r1624061378 ## docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml: ## @@ -59,6 +59,12 @@ rules: labels: database: "$2" table: "$1$3" +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" + name: "pinot_broker_requestSize_$4" + cache: true + labels: +database: "$2" +table: "$1$3" Review Comment: Do we want to delimit `$1` from `$3`? -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
suddendust commented on code in PR #13290: URL: https://github.com/apache/pinot/pull/13290#discussion_r1624060430 ## docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml: ## @@ -112,6 +118,12 @@ rules: - pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" name: "pinot_broker_routingTableUpdateTime_$1" cache: true +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" + name: "pinot_broker_adaptiveServerSelectorType_$1" Review Comment: Can we please add lables here? ## docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml: ## @@ -112,6 +118,12 @@ rules: - pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" name: "pinot_broker_routingTableUpdateTime_$1" cache: true +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" + name: "pinot_broker_adaptiveServerSelectorType_$1" + cache: true +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" + name: "pinot_broker_adaptiveServerSelectorType_$1_$2" Review Comment: Same as above -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
suddendust commented on code in PR #13290: URL: https://github.com/apache/pinot/pull/13290#discussion_r1624057967 ## docker/images/pinot/etc/jmx_prometheus_javaagent/configs/server.yml: ## @@ -73,6 +66,14 @@ rules: table: "$1$3" tableType: "$4" partition: "$5" +- pattern: "\"org\\.apache\\.pinot\\.common\\.metrics\"<>(\\w+)" Review Comment: `realtimeIngestionDelayMs` is a different metric than this. Can you validate if that one is being successfully 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-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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
npawar commented on PR #13290: URL: https://github.com/apache/pinot/pull/13290#issuecomment-2142618653 Please do add testing done and screenshots of validated rules as they appear in jconsole / prometheus explore -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
shounakmk219 commented on PR #13290: URL: https://github.com/apache/pinot/pull/13290#issuecomment-2142485592 > How shall we test it out? One possibility is to get the list of available metrics on broker/controller/server using `curl localhost:8080` running 0.8.0, then deploy this image, do the same and compare. This is not trivial to do though. Yeah it's a pain to test these changes. Right now just making sure no random string appear in `table` label. -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
soumitra-st commented on PR #13290: URL: https://github.com/apache/pinot/pull/13290#issuecomment-2142480075 Looks good, deferring to @suddendust to double check. -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
soumitra-st commented on PR #13290: URL: https://github.com/apache/pinot/pull/13290#issuecomment-2142478946 How shall we test it out? One possibility is to get the list of available metrics on broker/controller/server using `curl localhost:8080` running 0.8.0, then deploy this image, do the same and compare. This is not trivial to do though. -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
shounakmk219 commented on PR #13290: URL: https://github.com/apache/pinot/pull/13290#issuecomment-2142284257 > Can you list the broken metrics and how they are fixed in this PR? @soumitra-st Updated the description with metric details. -- 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
Re: [PR] Fix few metric rules which were affected by the database prefix handling [pinot]
soumitra-st commented on PR #13290: URL: https://github.com/apache/pinot/pull/13290#issuecomment-2142166914 Can you list the broken metrics and how they are fixed in this PR? -- 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
