Yan-Daojiang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24096 )

Change subject: KUDU-3692 Export entity attributes as Prometheus labels
......................................................................


Patch Set 3:

(3 comments)

> 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!

All comments addressed. PTAL when you have time.

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: fals
> For now I think we should set this to default false. This way on upgrade, u
Thank you for the reminder. I overlooked this point earlier.
Enabling it by default may impact the current environment, so it has now been 
set to false.


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.back() == ',') {
> nit: !labels.empty() is redundant here as above we already have early retur
Done


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:       // This is a value line. Verify that HELP and TYPE have 
already been seen.
> The part verifies that every value line has a HELP entry, but not that HELP
Good catch, thanks for pointing this out.

I've reworked this into a single-pass approach. The help_lines and type_lines 
maps are now populated incrementally during the same loop that checks value 
lines. When a value line is encountered, help_lines.count(metric_name) only 
finds entries that were inserted by earlier lines in the iteration, so it 
effectively enforces that HELP/TYPE must precede value lines.



--
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: 3
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 14:03:47 +0000
Gerrit-HasComments: Yes

Reply via email to