Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22509 )
Change subject: [util] fix MetricsTest.TableAndTabletPrometheusTest ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/22509/1/src/kudu/util/metrics-test.cc File src/kudu/util/metrics-test.cc: http://gerrit.cloudera.org:8080/#/c/22509/1/src/kudu/util/metrics-test.cc@158 PS1, Line 158: 1111111111 > Is there a reason why the tablet lengths are different in this line and in The reason of this change is two-fold: * Have more reasonable formatting in the code * Have length variation in the entity names * Use identifiers different from the ones that Kudu master uses on its own Do you have any particular corner case in mind that might be caught by the original code but not the modified code? It seems the author of the original code copy-pasted the output from kudu-master metrics for the system tablet as-is when putting together this test scenario, but it doesn't bring any extra coverage, only making it's harder to format the strings in the code. Even more: it would hide some issues when using the exact UUIDs that master uses for its system tablet if the metrics registry was shared between the test and the master's runtime. As for catching corner cases: that's not the way to provide the desired coverage, IMO. But I agree that there should be a test for various corner cases, along with a test to make sure the set of characters produced by our metrics is acceptable by Prometheus specs: https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels -- To view, visit http://gerrit.cloudera.org:8080/22509 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I15ee5fc8974f5df931a33cdcbc09c3da4da691a7 Gerrit-Change-Number: 22509 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Mon, 24 Feb 2025 20:23:15 +0000 Gerrit-HasComments: Yes
