Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_" ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics-test.cc File be/src/util/metrics-test.cc: http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics-test.cc@467 PS2, Line 467: TEST_F(MetricsTest, PrometheusMetricNames) { nit: could add a test for "an_impala_metric" and one with uppercase characters http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.h File be/src/util/metrics.h: http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.h@472 PS2, Line 472: impala I wonder if we should actually test for "impala_" here to make sure that all impala metrics start with "impala_"? http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.cc File be/src/util/metrics.cc: http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.cc@310 PS2, Line 310: LOG(INFO) Should this be something like VLOG(3)? Otherwise I think we log this every time, no? -- To view, visit http://gerrit.cloudera.org:8080/13941 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f Gerrit-Change-Number: 13941 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Comment-Date: Sun, 28 Jul 2019 19:24:17 +0000 Gerrit-HasComments: Yes
