Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19133 )
Change subject: [metrics] Expose Prometheus Metrics in Kudu ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/19133/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19133/6//COMMIT_MSG@25 PS6, Line 25: ‘kudu-master’ nit: kudu-master http://gerrit.cloudera.org:8080/#/c/19133/6/src/kudu/util/metrics.h File src/kudu/util/metrics.h: http://gerrit.cloudera.org:8080/#/c/19133/6/src/kudu/util/metrics.h@766 PS6, Line 766: // All metrics must be able to render themselves as Prometheus. : // virtual Status WriteAsPrometheus(PrometheusWriter* writer) const = 0; : nit: remove them if not used. http://gerrit.cloudera.org:8080/#/c/19133/6/src/kudu/util/metrics.h@1275 PS6, Line 1275: output << prefix->str() << prototype_->name() << : "{unit_type=\"" << MetricUnit::Name(prototype_->unit()) << "\"" << "}" : << " " << value() << "\n"; nit: how about using Substitute to build the string? http://gerrit.cloudera.org:8080/#/c/19133/6/src/kudu/util/metrics.cc File src/kudu/util/metrics.cc: http://gerrit.cloudera.org:8080/#/c/19133/6/src/kudu/util/metrics.cc@735 PS6, Line 735: output << "# HELP " << name() << " " << description() << "\n"; : output << "# TYPE " << name() << " " << MetricType::Name(type()) << "\n"; There are many metrics with the same prototype, each of them will print HELP and TYPE? -- To view, visit http://gerrit.cloudera.org:8080/19133 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b2d01bd56f6f0f1b6f31cbce15e671d16521739 Gerrit-Change-Number: 19133 Gerrit-PatchSet: 6 Gerrit-Owner: Khazar Mammadli <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Sat, 15 Oct 2022 13:52:51 +0000 Gerrit-HasComments: Yes
