Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24836 )
Change subject: [metrics] add --metrics_prometheus_default_metrics for a server-side allowlist ...................................................................... Patch Set 1: Code-Review+1 (2 comments) Just two questions from my side, otherwise looks good to me. Thank you for working on this! http://gerrit.cloudera.org:8080/#/c/24836/1/src/kudu/util/metrics.cc File src/kudu/util/metrics.cc: http://gerrit.cloudera.org:8080/#/c/24836/1/src/kudu/util/metrics.cc@212 PS1, Line 212: SplitStringUsing(value, ",", &tokens); : if (tokens.empty()) { : LOG(WARNING) << Substitute( : "--$0 is set to '$1' but contains no usable metric name; no metric " : "name filtering will be applied", flag_name, value); The validator warns when the value collapses to zero tokens (,,), which is the "filtering silently disabled -> exports everything" case the commit message calls out. It won't warn on a value that yields a whitespace-only token (e.g. " " or "a, "): that survives SplitStringUsing, matches no metric name, and yields the opposite silent failure - an empty scrape. Same latent issue exists for the per-request metrics param and for the sibling flags, so I'd leave the code as-is; worth a one-line acknowledgement in the comment that only the zero-token case is guarded, if you want the asymmetry documented. http://gerrit.cloudera.org:8080/#/c/24836/1/src/kudu/util/metrics.cc@381 PS1, Line 381: void GetPrometheusMetricsFilter(vector<string>* entity_metrics) { ParseArray on a present-but-empty ?metrics= returns an empty vector, so this function treats "caller explicitly asked for no filter" identically to "caller passed no param" and applies the flag default. That means once the flag is set, a scrape can no longer request the full metric set via ?metrics=. This exactly matches the quantiles/merge_rules siblings, so I read it as intentional - just confirming that's the accepted family behavior and not a gap worth a doc note. -- To view, visit http://gerrit.cloudera.org:8080/24836 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I70ef54eef456108f8503a4edfdfb68fab3173353 Gerrit-Change-Number: 24836 Gerrit-PatchSet: 1 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: Fri, 18 Sep 2026 10:49:35 +0000 Gerrit-HasComments: Yes
