This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 599616cb0de [fix](metrics) Escape label values in the Prometheus
output of FE (#69837)
599616cb0de is described below
commit 599616cb0de60049b72df3e8fcfd7f6b55883d1c
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Mon Nov 2 09:40:12 2026 +0000
[fix](metrics) Escape label values in the Prometheus output of FE (#69837)
### What problem does this PR solve?
Issue Number: #69907
Related PR: None
Problem Summary:
**In short:** the FE writes metric label values into its Prometheus
output without escaping them. A table name may contain a backslash, a
double quote or a line feed, and in cloud mode the Master FE labels four
per-table series with the table name, so a single such table leaves the
Master's whole `/metrics` response outside the text format, and
Prometheus keeps no sample of a scrape it cannot parse. This PR escapes
every label value that is not a literal through one function, as the
format requires. Ordinary names are written exactly as before.
#### Background
In the Prometheus text format a label value is written between double
quotes, and a backslash, a double quote or a line feed inside it must be
written as `\\`, `\"` and `\n`; no other character is escaped. A scrape
whose response does not parse fails as a whole.
With `enable_unicode_name_support`, which is on by default, a table name
may contain all three characters:
`FeNameFormat.UNICODE_TABLE_NAME_REGEX` only requires the last character
not to be white space. A local cluster with default settings accepts
tables named ``` `a"b` ```, `` `a\b` `` and one with a line feed in the
middle. The names of databases, users, workload groups, jobs and
catalogs keep their restricted patterns.
#### The problem, and what it cost
- `PrometheusMetricVisitor` concatenates label values as they are, in
every place that writes one: the labels of registered metrics (`visit`),
both kinds of histograms, the cloud per-table series
(`visitCloudTableStats`), the workload group series and the JVM GC
series.
- In cloud mode the Master FE exports `doris_fe_table_data_size`,
`doris_fe_table_rowset_count`, `doris_fe_table_segment_count` and
`doris_fe_table_row_count` for up to `prom_output_table_metrics_limit`
(10000) tables, labeled with `db_name` and `table_name`. One table whose
name holds one of the three characters makes the response invalid, and
every metric of the Master FE is lost while that table is exported.
- The coming monitoring of table streams and IVM materialized views
labels series with the names of materialized views and streams, which
would run into the same problem in both modes.
#### How this PR fixes it
- `escapeLabelValue` writes a backslash, a double quote and a line feed
as `\\`, `\"` and `\n` and leaves every other character alone, which is
the full set the format requires.
- Every label value that is not a literal goes through it: the two
histogram paths and `visit`, the database and table names of the
per-table series (escaped once per table, not once per series), the
workload group name and the GC name. The JVM GC and workload group names
cannot hold these characters today; they go through the same function so
that no label value in the class is written raw.
- A name without these characters comes back as the same string, so the
output of ordinary names does not change, byte for byte, and costs no
extra allocation (`String.replace` returns the string itself when
nothing matches).
- A null label value is written as `null`, as the concatenation without
escaping wrote it. Label values can be null: a table warm-up job has no
source cluster, so the series of its start and finish times carry a null
`src_cluster_name`.
#### Results
What the cloud per-table series is written as for each table name, and
what `promtool check metrics` (Prometheus v3.2.0) says about it:
| Table name | Before this PR | promtool | After this PR | promtool |
| --- | --- | --- | --- | --- |
| `t1` | `table_name="t1"` | OK | `table_name="t1"` | OK |
| `a"b` | `table_name="a"b"` | `unexpected end of label value "a"` |
`table_name="a\"b"` | OK |
| `a\b` | `table_name="a\b"` | `invalid escape sequence '\b'` |
`table_name="a\\b"` | OK |
| `a\` | `table_name="a\"` | `label value "a\"} 100" contains unescaped
new-line` | `table_name="a\\"` | OK |
| `a`, line feed, `b` | the sample split over two lines | `label value
"a" contains unescaped new-line` | `table_name="a\nb"` | OK |
A parser that follows the format turns each escaped value back into the
table name itself.
#### Classes, and how they call each other
- `PrometheusMetricVisitor`: the new private `escapeLabelValue`, called
wherever a label value is written.
- `MetricsTest`: `testPrometheusVisitorEscapesLabelValues` (a gauge with
an escaped and a plain label, and both kinds of histograms),
`testCloudTableStatsEscapeTableName` (the four per-table series, with a
mocked `CloudTabletStatMgr`) and
`testPrometheusVisitorWritesNullLabelValue` (a gauge and a labeled
histogram with a null label value).
```
MetricsAction (/metrics) -> MetricRepo.getMetric(PrometheusMetricVisitor)
|-- visitJvm GC name -> escapeLabelValue
|-- DORIS_METRIC_REGISTER.accept -> visit labels ->
escapeLabelValue
|-- visitHistograms -> visitHistogram (both) labels ->
escapeLabelValue
|-- visitCloudTableStats db / table name -> escapeLabelValue
`-- visitWorkloadGroup group name -> escapeLabelValue
```
Not changed: the JSON output (`type=json`), which builds its own strings
and would need JSON escaping instead; the core output (`type=core`),
which writes no labels; the metric names and the `# HELP` texts, which
are literals.
---
.../doris/metric/PrometheusMetricVisitor.java | 41 ++++++----
.../java/org/apache/doris/metric/MetricsTest.java | 88 ++++++++++++++++++++++
2 files changed, 113 insertions(+), 16 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/metric/PrometheusMetricVisitor.java
b/fe/fe-core/src/main/java/org/apache/doris/metric/PrometheusMetricVisitor.java
index e32097f54df..5bd299ad3ba 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/metric/PrometheusMetricVisitor.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/metric/PrometheusMetricVisitor.java
@@ -118,11 +118,11 @@ public class PrometheusMetricVisitor extends
MetricVisitor {
sb.append(Joiner.on(" ").join(TYPE, JVM_GC, "gauge\n"));
for (GarbageCollector gc : jvmStats.getGc()) {
sb.append(JVM_GC).append("{");
- sb.append("name=\"").append(gc.getName()).append("
Count").append("\", ").append("type=\"count\"} ")
- .append(gc.getCollectionCount()).append("\n");
+
sb.append("name=\"").append(escapeLabelValue(gc.getName())).append("
Count").append("\", ")
+ .append("type=\"count\"}
").append(gc.getCollectionCount()).append("\n");
sb.append(JVM_GC).append("{");
- sb.append("name=\"").append(gc.getName()).append("
Time").append("\", ").append("type=\"time\"} ")
- .append(gc.getCollectionTime().getMillis()).append("\n");
+
sb.append("name=\"").append(escapeLabelValue(gc.getName())).append("
Time").append("\", ")
+ .append("type=\"time\"}
").append(gc.getCollectionTime().getMillis()).append("\n");
}
// threads
@@ -164,7 +164,7 @@ public class PrometheusMetricVisitor extends MetricVisitor {
List<MetricLabel> labels = metric.getLabels();
if (!labels.isEmpty()) {
sb.append("{");
- List<String> labelStrs = labels.stream().map(l -> l.getKey() +
"=\"" + l.getValue()
+ List<String> labelStrs = labels.stream().map(l -> l.getKey() +
"=\"" + escapeLabelValue(l.getValue())
+ "\"").collect(Collectors.toList());
sb.append(Joiner.on(", ").join(labelStrs));
sb.append("}");
@@ -184,7 +184,7 @@ public class PrometheusMetricVisitor extends MetricVisitor {
if (kv.length == 1) {
names.add(kv[0]);
} else if (kv.length == 2) {
- tags.add(String.format("%s=\"%s\"", kv[0], kv[1]));
+ tags.add(String.format("%s=\"%s\"", kv[0],
escapeLabelValue(kv[1])));
}
}
final String fullName = prefix + String.join("_", names);
@@ -195,7 +195,7 @@ public class PrometheusMetricVisitor extends MetricVisitor {
public void visitHistogram(String prefix, String name, Histogram
histogram, List<MetricLabel> labels) {
final String fullName = prefix + name.replace('.', '_');
List<String> tags = labels.stream()
- .map(l -> l.getKey() + "=\"" + l.getValue() + "\"")
+ .map(l -> l.getKey() + "=\"" + escapeLabelValue(l.getValue())
+ "\"")
.collect(Collectors.toList());
appendHistogram(fullName, histogram, tags);
}
@@ -264,34 +264,36 @@ public class PrometheusMetricVisitor extends
MetricVisitor {
Collection<OlapTable.Statistics> values =
tabletStatMgr.getCloudTableStats();
long totalTableSize = tabletStatMgr.getTotalTableSize();
for (OlapTable.Statistics stats : values) {
+ String dbName = escapeLabelValue(stats.getDbName());
+ String tableName = escapeLabelValue(stats.getTableName());
dataSizeBuilder.append("doris_fe_table_data_size{db_name=\"");
- dataSizeBuilder.append(stats.getDbName());
+ dataSizeBuilder.append(dbName);
dataSizeBuilder.append("\", table_name=\"");
- dataSizeBuilder.append(stats.getTableName());
+ dataSizeBuilder.append(tableName);
dataSizeBuilder.append("\"} ");
dataSizeBuilder.append(stats.getDataSize());
dataSizeBuilder.append("\n");
rowsetCountBuilder.append("doris_fe_table_rowset_count{db_name=\"");
- rowsetCountBuilder.append(stats.getDbName());
+ rowsetCountBuilder.append(dbName);
rowsetCountBuilder.append("\", table_name=\"");
- rowsetCountBuilder.append(stats.getTableName());
+ rowsetCountBuilder.append(tableName);
rowsetCountBuilder.append("\"} ");
rowsetCountBuilder.append(stats.getRowsetCount());
rowsetCountBuilder.append("\n");
segmentCountBuilder.append("doris_fe_table_segment_count{db_name=\"");
- segmentCountBuilder.append(stats.getDbName());
+ segmentCountBuilder.append(dbName);
segmentCountBuilder.append("\", table_name=\"");
- segmentCountBuilder.append(stats.getTableName());
+ segmentCountBuilder.append(tableName);
segmentCountBuilder.append("\"} ");
segmentCountBuilder.append(stats.getSegmentCount());
segmentCountBuilder.append("\n");
tableRowCountBuilder.append("doris_fe_table_row_count{db_name=\"");
- tableRowCountBuilder.append(stats.getDbName());
+ tableRowCountBuilder.append(dbName);
tableRowCountBuilder.append("\", table_name=\"");
- tableRowCountBuilder.append(stats.getTableName());
+ tableRowCountBuilder.append(tableName);
tableRowCountBuilder.append("\"} ");
tableRowCountBuilder.append(stats.getRowCount());
tableRowCountBuilder.append("\n");
@@ -351,7 +353,7 @@ public class PrometheusMetricVisitor extends MetricVisitor {
Map<String, List<String>> workloadGroupMap =
Env.getCurrentEnv().getWorkloadGroupMgr()
.getWorkloadGroupQueryDetail();
for (Map.Entry<String, List<String>> entry :
workloadGroupMap.entrySet()) {
- String name = entry.getKey();
+ String name = escapeLabelValue(entry.getKey());
List<String> valList = entry.getValue();
tmpSb.append(String.format("%s{name=\"%s\", type=\"%s\"}
%s\n", counterTitle, name, "running_query_num",
valList.get(0)));
@@ -363,4 +365,11 @@ public class PrometheusMetricVisitor extends MetricVisitor
{
logger.warn("error happends when get workload group query detail
", e);
}
}
+
+ // The text format requires a backslash, a double quote and a line feed in
a label value to be escaped,
+ // and names such as a table name may contain any of them. A label value
may be null (a table warm-up job
+ // has no source cluster), which is written as null, as the concatenation
without escaping wrote it.
+ private static String escapeLabelValue(String value) {
+ return String.valueOf(value).replace("\\", "\\\\").replace("\"",
"\\\"").replace("\n", "\\n");
+ }
}
diff --git a/fe/fe-core/src/test/java/org/apache/doris/metric/MetricsTest.java
b/fe/fe-core/src/test/java/org/apache/doris/metric/MetricsTest.java
index 33fc70c2f90..10c729db9f7 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/metric/MetricsTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/metric/MetricsTest.java
@@ -18,7 +18,9 @@
package org.apache.doris.metric;
import org.apache.doris.analysis.UserIdentity;
+import org.apache.doris.catalog.CloudTabletStatMgr;
import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.OlapTable;
import org.apache.doris.cloud.CloudWarmUpJob;
import org.apache.doris.cloud.JobWarmUpStats;
import org.apache.doris.common.Config;
@@ -342,6 +344,92 @@ public class MetricsTest {
Assertions.assertFalse(prometheusResult.contains("doris_fe_disabled_latency_ms"));
}
+ @Test
+ public void testPrometheusVisitorEscapesLabelValues() {
+ // A table name may contain a backslash, a double quote and a line
feed, which the text format only
+ // accepts escaped in a label value; the plain label beside it is
written as is.
+ String name = "a\\b\"c\nd";
+ String escapedName = "a\\\\b\\\"c\\nd";
+ GaugeMetricImpl<Long> gauge = new GaugeMetricImpl<>("escape_test",
MetricUnit.NOUNIT, "escape test", 1L);
+ gauge.addLabel(new MetricLabel("table_name", name)).addLabel(new
MetricLabel("db_name", "db1"));
+ MetricVisitor visitor = new PrometheusMetricVisitor();
+ visitor.visit(MetricVisitor.FE_PREFIX, gauge);
+ Assertions.assertEquals("# HELP doris_fe_escape_test escape test\n"
+ + "# TYPE doris_fe_escape_test gauge\n"
+ + "doris_fe_escape_test{table_name=\"" + escapedName + "\",
db_name=\"db1\"} 1\n", visitor.finish());
+
+ HistogramMetric histogram = new HistogramMetric("escape.latency.ms",
+ Lists.newArrayList(new MetricLabel("table_name", name)));
+ histogram.update(10L);
+ visitor = new PrometheusMetricVisitor();
+ visitor.visitHistogram(MetricVisitor.FE_PREFIX, histogram.getName(),
histogram.getHistogram(),
+ histogram.getLabels());
+ // a histogram of the codahale registry carries its labels in the
name, as part.part.key=value
+ visitor.visitHistogram(MetricVisitor.FE_PREFIX,
"escape.name.ms.table_name=" + name,
+ histogram.getHistogram());
+ String result = visitor.finish();
+
Assertions.assertTrue(result.contains("doris_fe_escape_latency_ms{quantile=\"0.999\",table_name=\""
+ + escapedName + "\"} 10.0\n"), result);
+
Assertions.assertTrue(result.contains("doris_fe_escape_latency_ms_count{table_name=\""
+ + escapedName + "\"} 1\n"), result);
+
Assertions.assertTrue(result.contains("doris_fe_escape_name_ms{quantile=\"0.999\",table_name=\""
+ + escapedName + "\"} 10.0\n"), result);
+
Assertions.assertTrue(result.contains("doris_fe_escape_name_ms_count{table_name=\""
+ + escapedName + "\"} 1\n"), result);
+ }
+
+ @Test
+ public void testPrometheusVisitorWritesNullLabelValue() {
+ // A table warm-up job has no source cluster, so the series of its
start and finish times carry a null
+ // src_cluster_name; it is written as null, as before label values
were escaped.
+ GaugeMetricImpl<Long> gauge = new GaugeMetricImpl<>("null_label_test",
MetricUnit.NOUNIT, "null label test",
+ 1L);
+ gauge.addLabel(new MetricLabel("src_cluster_name", null)).addLabel(new
MetricLabel("dst_cluster_name", "cg1"));
+ MetricVisitor visitor = new PrometheusMetricVisitor();
+ visitor.visit(MetricVisitor.FE_PREFIX, gauge);
+ Assertions.assertEquals("# HELP doris_fe_null_label_test null label
test\n"
+ + "# TYPE doris_fe_null_label_test gauge\n"
+ + "doris_fe_null_label_test{src_cluster_name=\"null\",
dst_cluster_name=\"cg1\"} 1\n",
+ visitor.finish());
+
+ HistogramMetric histogram = new
HistogramMetric("null_label.latency.ms",
+ Lists.newArrayList(new MetricLabel("src_cluster_name", null)));
+ histogram.update(10L);
+ visitor = new PrometheusMetricVisitor();
+ visitor.visitHistogram(MetricVisitor.FE_PREFIX, histogram.getName(),
histogram.getHistogram(),
+ histogram.getLabels());
+ String result = visitor.finish();
+
Assertions.assertTrue(result.contains("doris_fe_null_label_latency_ms_count{src_cluster_name=\"null\"}
1\n"),
+ result);
+ }
+
+ @Test
+ public void testCloudTableStatsEscapeTableName() {
+ String originCloudUniqueId = Config.cloud_unique_id;
+ Env env = Env.getCurrentEnv();
+ Object originTabletStatMgr = Deencapsulation.getField(env,
"tabletStatMgr");
+ CloudTabletStatMgr tabletStatMgr =
Mockito.mock(CloudTabletStatMgr.class);
+
Mockito.when(tabletStatMgr.getCloudTableStats()).thenReturn(Collections.singletonList(
+ new OlapTable.Statistics("db1", "a\\b\"c\nd", 100L, 300L, 0L,
3L, 10L, 2L, 4L,
+ 0L, 0L, 0L, 0L, 0L, 0L)));
+ Mockito.when(tabletStatMgr.getTotalTableSize()).thenReturn(100L);
+ try {
+ Config.cloud_unique_id = "test_cloud_unique_id";
+ Deencapsulation.setField(env, "tabletStatMgr", tabletStatMgr);
+ MetricVisitor visitor = new PrometheusMetricVisitor();
+ visitor.visitCloudTableStats();
+ String result = visitor.finish();
+ String labels = "{db_name=\"db1\", table_name=\"a\\\\b\\\"c\\nd\"}
";
+ Assertions.assertTrue(result.contains("\ndoris_fe_table_data_size"
+ labels + "100\n"), result);
+
Assertions.assertTrue(result.contains("\ndoris_fe_table_rowset_count" + labels
+ "2\n"), result);
+
Assertions.assertTrue(result.contains("\ndoris_fe_table_segment_count" + labels
+ "4\n"), result);
+ Assertions.assertTrue(result.contains("\ndoris_fe_table_row_count"
+ labels + "10\n"), result);
+ } finally {
+ Deencapsulation.setField(env, "tabletStatMgr",
originTabletStatMgr);
+ Config.cloud_unique_id = originCloudUniqueId;
+ }
+ }
+
@Test
public void testVirtualComputeGroupSwitchMetricRename() {
String originCloudUniqueId = Config.cloud_unique_id;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
