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 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/24790/1/src/kudu/util/metrics.h File src/kudu/util/metrics.h: http://gerrit.cloudera.org:8080/#/c/24790/1/src/kudu/util/metrics.h@586 PS1, Line 586: std::vector<std::string> quantiles; Since only preset quantile label are ever added into this container and the content is de-duplicated, maybe switch to std::array<std::string, ...> here? http://gerrit.cloudera.org:8080/#/c/24790/1/src/kudu/util/metrics.cc File src/kudu/util/metrics.cc: http://gerrit.cloudera.org:8080/#/c/24790/1/src/kudu/util/metrics.cc@43 PS1, Line 43: const char* const kHistogramQuantileTags[] = { : "0", "0.75", "0.95", "0.99", "0.999", "0.9999", "1" : }; : : bool IsKnownQuantileTag(const std::string& tag) { : for (const char* known : kHistogramQuantileTags) { : if (tag == known) { : return true; : } : } : return false; : } Does it makes sense to make these contstexpr? http://gerrit.cloudera.org:8080/#/c/24790/1/src/kudu/util/metrics.cc@153 PS1, Line 153: using std::string; : using std::unordered_set; : using std::vector; : using strings::Substitute; : using strings::SubstituteAndAppend; nit: maybe, move this block to come before flag validators, and omit the 'std::' prefix in the validator's code? http://gerrit.cloudera.org:8080/#/c/24790/1/src/kudu/util/metrics.cc@1413 PS1, Line 1413: static constexpr struct QuantileInfo { : const char* const tag; : const double quantile; : } kQuantiles[] = { : { "0.75", 75.0 }, : { "0.95", 95.0 }, : { "0.99", 99.0 }, : { "0.999", 99.9 }, : { "0.9999", 99.99 }, : }; Is there a chance to unify this and the newly introduced kHistogramQuantileTags in the beginning of this file, so we don't duplicate/scatter the information on the preset quantiles? -- 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: 1 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: Fri, 04 Sep 2026 15:24:20 +0000 Gerrit-HasComments: Yes
