Grant Henke 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 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/17607/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17607/2//COMMIT_MSG@16 PS2, Line 16: "hostname": "MacBook-Pro.local", Can you update this message to indicate that you also added the hostname? Is it worth adding the IP too? http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc File src/kudu/master/master-test.cc: http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc@1387 PS2, Line 1387: ){ Add a space before the brace http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc@1396 PS2, Line 1396: ASSERT_TRUE(GetHostname(&server_hostname).ok()); ASSERT_OK(GetHostname(&server_hostname)); http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc File src/kudu/server/server_base.cc: http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc@854 PS2, Line 854: e{ Add a space before brace http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc@855 PS2, Line 855: metric_entity_->SetAttribute("hostname", "Could not be determined" ); I think I would prefer an empty string given the presence of a value could be interpreted to be the hostname. http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/tserver/tablet_server-test.cc@3819 PS2, Line 3819: ){ Add a space before braces http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/tserver/tablet_server-test.cc@3828 PS2, Line 3828: ASSERT_TRUE(GetHostname(&server_hostname).ok()); ASSERT_OK(GetHostname(&server_hostname)); -- 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: 2 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 22 Jun 2021 13:49:57 +0000 Gerrit-HasComments: Yes
