Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24096 )
Change subject: KUDU-3692 Export entity attributes as Prometheus labels ...................................................................... Patch Set 2: Code-Review+1 (3 comments) Overall I think the patch is in a good state, some nits and a suggestion about the flags default value. Thank you! http://gerrit.cloudera.org:8080/#/c/24096/2/src/kudu/util/metrics.cc File src/kudu/util/metrics.cc: http://gerrit.cloudera.org:8080/#/c/24096/2/src/kudu/util/metrics.cc@42 PS2, Line 42: true For now I think we should set this to default false. This way on upgrade, users existing setup could break 'out of the blue'. On a later release we can change this to be the default with migration steps. What do you think? http://gerrit.cloudera.org:8080/#/c/24096/2/src/kudu/util/prometheus_writer.h File src/kudu/util/prometheus_writer.h: http://gerrit.cloudera.org:8080/#/c/24096/2/src/kudu/util/prometheus_writer.h@62 PS2, Line 62: if (!labels.empty() && labels.back() == ',') { nit: !labels.empty() is redundant here as above we already have early return. http://gerrit.cloudera.org:8080/#/c/24096/2/src/kudu/util/test_util.cc File src/kudu/util/test_util.cc: http://gerrit.cloudera.org:8080/#/c/24096/2/src/kudu/util/test_util.cc@747 PS2, Line 747: // Every HELP should have a corresponding TYPE and vice versa. The part verifies that every value line has a HELP entry, but not that HELP precedes the value in the output. It could technically pass even if HELP appeared after the value. -- To view, visit http://gerrit.cloudera.org:8080/24096 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I690918d06f19c393369b8fc43c3ec90dd2231d3d Gerrit-Change-Number: 24096 Gerrit-PatchSet: 2 Gerrit-Owner: Yan-Daojiang <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Yan-Daojiang <[email protected]> Gerrit-Comment-Date: Wed, 18 Mar 2026 11:46:04 +0000 Gerrit-HasComments: Yes
