morningman commented on code in PR #69837:
URL: https://github.com/apache/doris/pull/69837#discussion_r4430480375
##########
fe/fe-core/src/main/java/org/apache/doris/metric/PrometheusMetricVisitor.java:
##########
@@ -164,7 +164,7 @@ public void visit(String prefix,
@SuppressWarnings("rawtypes") Metric metric) {
List<MetricLabel> labels = metric.getLabels();
if (!labels.isEmpty()) {
sb.append("{");
- List<String> labelStrs = labels.stream().map(l -> l.getKey() +
"=\"" + l.getValue()
Review Comment:
Fixed in 4ddeac2fee7: `escapeLabelValue` now writes a null value as `null`,
as the concatenation without escaping did, so every value without a backslash,
a double quote or a line feed is written exactly as before.
`MetricsTest#testPrometheusVisitorWritesNullLabelValue` writes a gauge and a
labeled histogram with a null label value; without the fix it fails with a
NullPointerException.
The trigger is wider than a cancellation. `runPendingJob` calls
`updateClusterWarmUpJobLatestStartTime` with the null `srcClusterName` each
time a table warm-up job starts, and neither per-job time series is ever
removed from the registry. So in cloud mode the first table warm-up job to
start would have made every scrape of the Master FE's `/metrics` fail until the
FE restarted. The other producers are covered too: the registry already accepts
a null label value (`computeLabelId` maps it to an empty string), and the fix
sits in the one function every label value goes through.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
