Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17607 )
Change subject: [metrics] KUDU-3269: Add UUID of the server into the metrics output ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/17607/1/src/kudu/server/server_base.cc File src/kudu/server/server_base.cc: http://gerrit.cloudera.org:8080/#/c/17607/1/src/kudu/server/server_base.cc@852 PS1, Line 852: if (GetHostname(&server_hostname).ok()) { : metric_entity_->SetAttribute("hostname", server_hostname ); : } Nit: In case of error, instead of omitting the attribute entirely should the value be empty/null? One minor benefit is that the caller doesn't have to check explicitly for the presence of the hostname key if uuid is present. -- To view, visit http://gerrit.cloudera.org:8080/17607 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016 Gerrit-Change-Number: 17607 Gerrit-PatchSet: 1 Gerrit-Owner: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 21 Jun 2021 16:01:03 +0000 Gerrit-HasComments: Yes
