Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14280 )
Change subject: IMPALA-8946: Fix histogram rendering to Prometheus ...................................................................... Patch Set 1: (1 comment) Thank you for noticing and fixing this! I think this looks good, just one question about whether we want to fix an additional issue as part of this. http://gerrit.cloudera.org:8080/#/c/14280/1/be/src/util/histogram-metric.cc File be/src/util/histogram-metric.cc: http://gerrit.cloudera.org:8080/#/c/14280/1/be/src/util/histogram-metric.cc@131 PS1, Line 131: *value << name << "_count " << histogram_->TotalCount(); It looks like prometheus also wants a _sum member for summary metrics? I'm not sure how gracefully it handles it being missing. Obviously your change makes this much closer to the spec. It seems like we could extend HistogramMetric/HdrHistogram to track the sum as well. -- To view, visit http://gerrit.cloudera.org:8080/14280 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8849586964140c040ca4ea14b777c2d98d126b59 Gerrit-Change-Number: 14280 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Mon, 23 Sep 2019 18:41:18 +0000 Gerrit-HasComments: Yes
