Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24790 )
Change subject: [metrics] Support quantile selection in /metrics_prometheus ...................................................................... Patch Set 2: Code-Review+2 (3 comments) LGTM, just a few nits -- consider addressing these if it makes sense. Aside from that, I'm fine with submitting this as-is in PS2. Thank you for working on this! http://gerrit.cloudera.org:8080/#/c/24790/2/src/kudu/util/metrics.h File src/kudu/util/metrics.h: http://gerrit.cloudera.org:8080/#/c/24790/2/src/kudu/util/metrics.h@597 PS2, Line 597: std::array<std::string, kNumHistogramQuantiles> nit: could the code benefit from a typedef for this (e.g., HistogramQuantiles and alike)? http://gerrit.cloudera.org:8080/#/c/24790/2/src/kudu/util/metrics.cc File src/kudu/util/metrics.cc: http://gerrit.cloudera.org:8080/#/c/24790/2/src/kudu/util/metrics.cc@170 PS2, Line 170: '0', '0.75', '0.95', '0.99', '0.999', '0.9999', '1' nit: could this string be declared as a constexpr character/string literal and used here, in the description of the 'metrics_prometheus_default_quantiles' flag above, and in the warning message below at line 300 (as of PS2)? http://gerrit.cloudera.org:8080/#/c/24790/2/src/kudu/util/metrics.cc@298 PS2, Line 298: LOG(WARNING) nit: since ParseQuantiles() is being called for every request that has quantiles query parameter specified, maybe it makes sense to use KLOG_EVERY_N_SECS(WARNING, 60) or similar here to avoid flooding the logs in case of scrapper's misconfiguration? -- To view, visit http://gerrit.cloudera.org:8080/24790 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01b48aa30f26f2a7efcd842bf6512a023cbf90ff Gerrit-Change-Number: 24790 Gerrit-PatchSet: 2 Gerrit-Owner: Yan-Daojiang <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Yan-Daojiang <[email protected]> Gerrit-Comment-Date: Tue, 08 Sep 2026 18:33:07 +0000 Gerrit-HasComments: Yes
