From c861b31b72b6716d166b74b2701a22a528342c2a Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Tue, 29 Oct 2024 19:40:12 +0100 Subject: [PATCH 1/2] Support UTF-8 metric names and labels in web UI Fixes most of https://github.com/prometheus/prometheus/issues/15202 This should address all areas of the UI except for the autocompletion in the codemirror-promql text editor. The strategy here is that any time we print or internally serialize (like for the PromLens tree view) either a metric name or a label name as part of a selector or in other relevant parts of PromQL, we check whether it contains characters beyond what was previously supported, and if so, quote and escape it. In the case of metric names, we also have to move them from the beginning of the selector into the curly braces. Signed-off-by: Julius Volz --- .../mantine-ui/src/components/LabelBadges.tsx | 3 +- web/ui/mantine-ui/src/lib/formatSeries.ts | 14 ++- .../mantine-ui/src/pages/query/SeriesName.tsx | 39 +++++-- web/ui/mantine-ui/src/promql/format.tsx | 63 +++++++--- web/ui/mantine-ui/src/promql/serialize.ts | 44 ++++--- .../src/promql/serializeAndFormat.test.ts | 109 +++++++++++++++++- web/ui/mantine-ui/src/promql/utils.ts | 15 +++ 7 files changed, 242 insertions(+), 45 deletions(-) diff --git a/web/ui/mantine-ui/src/components/LabelBadges.tsx b/web/ui/mantine-ui/src/components/LabelBadges.tsx index f60a37f03..8aa713556 100644 --- a/web/ui/mantine-ui/src/components/LabelBadges.tsx +++ b/web/ui/mantine-ui/src/components/LabelBadges.tsx @@ -2,6 +2,7 @@ import { Badge, BadgeVariant, Group, MantineColor, Stack } from "@mantine/core"; import { FC } from "react"; import { escapeString } from "../lib/escapeString"; import badgeClasses from "../Badge.module.css"; +import { maybeQuoteLabelName } from "../promql/utils"; export interface LabelBadgesProps { labels: Record; @@ -30,7 +31,7 @@ export const LabelBadges: FC = ({ }} key={k} > - {k}="{escapeString(v)}" + {maybeQuoteLabelName(k)}="{escapeString(v)}" ); })} diff --git a/web/ui/mantine-ui/src/lib/formatSeries.ts b/web/ui/mantine-ui/src/lib/formatSeries.ts index b79c40076..007659070 100644 --- a/web/ui/mantine-ui/src/lib/formatSeries.ts +++ b/web/ui/mantine-ui/src/lib/formatSeries.ts @@ -1,12 +1,24 @@ +import { + maybeQuoteLabelName, + metricContainsExtendedCharset, +} from "../promql/utils"; import { escapeString } from "./escapeString"; +// TODO: Maybe replace this with the new PromLens-derived serialization code in src/promql/serialize.ts? export const formatSeries = (labels: { [key: string]: string }): string => { if (labels === null) { return "scalar"; } + if (metricContainsExtendedCharset(labels.__name__ || "")) { + return `{"${escapeString(labels.__name__)}",${Object.entries(labels) + .filter(([k]) => k !== "__name__") + .map(([k, v]) => `${maybeQuoteLabelName(k)}="${escapeString(v)}"`) + .join(", ")}}`; + } + return `${labels.__name__ || ""}{${Object.entries(labels) .filter(([k]) => k !== "__name__") - .map(([k, v]) => `${k}="${escapeString(v)}"`) + .map(([k, v]) => `${maybeQuoteLabelName(k)}="${escapeString(v)}"`) .join(", ")}}`; }; diff --git a/web/ui/mantine-ui/src/pages/query/SeriesName.tsx b/web/ui/mantine-ui/src/pages/query/SeriesName.tsx index 66a7856f5..61bc62eee 100644 --- a/web/ui/mantine-ui/src/pages/query/SeriesName.tsx +++ b/web/ui/mantine-ui/src/pages/query/SeriesName.tsx @@ -5,6 +5,10 @@ import classes from "./SeriesName.module.css"; import { escapeString } from "../../lib/escapeString"; import { useClipboard } from "@mantine/hooks"; import { notifications } from "@mantine/notifications"; +import { + maybeQuoteLabelName, + metricContainsExtendedCharset, +} from "../../promql/utils"; interface SeriesNameProps { labels: { [key: string]: string } | null; @@ -15,8 +19,26 @@ const SeriesName: FC = ({ labels, format }) => { const clipboard = useClipboard(); const renderFormatted = (): React.ReactElement => { + const metricExtendedCharset = + labels && metricContainsExtendedCharset(labels.__name__ || ""); + const labelNodes: React.ReactElement[] = []; let first = true; + + // If the metric name uses the extended new charset, we need to escape it, + // put it into the label matcher list, and make sure it's the first item. + if (metricExtendedCharset) { + labelNodes.push( + + + "{escapeString(labels.__name__)}" + + + ); + + first = false; + } + for (const label in labels) { if (label === "__name__") { continue; @@ -37,7 +59,10 @@ const SeriesName: FC = ({ labels, format }) => { }} title="Click to copy label matcher" > - {label}= + + {maybeQuoteLabelName(label)} + + = "{escapeString(labels[label])}" @@ -52,9 +77,11 @@ const SeriesName: FC = ({ labels, format }) => { return ( - - {labels ? labels.__name__ : ""} - + {!metricExtendedCharset && ( + + {labels ? labels.__name__ : ""} + + )} {"{"} {labelNodes} {"}"} @@ -62,10 +89,6 @@ const SeriesName: FC = ({ labels, format }) => { ); }; - if (labels === null) { - return <>scalar; - } - if (format) { return renderFormatted(); } diff --git a/web/ui/mantine-ui/src/promql/format.tsx b/web/ui/mantine-ui/src/promql/format.tsx index 05dd7d410..399644408 100644 --- a/web/ui/mantine-ui/src/promql/format.tsx +++ b/web/ui/mantine-ui/src/promql/format.tsx @@ -8,14 +8,21 @@ import ASTNode, { MatrixSelector, } from "./ast"; import { formatPrometheusDuration } from "../lib/formatTime"; -import { maybeParenthesizeBinopChild, escapeString } from "./utils"; +import { + maybeParenthesizeBinopChild, + escapeString, + maybeQuoteLabelName, + metricContainsExtendedCharset, +} from "./utils"; export const labelNameList = (labels: string[]): React.ReactNode[] => { return labels.map((l, i) => { return ( {i !== 0 && ", "} - {l} + + {maybeQuoteLabelName(l)} + ); }); @@ -69,27 +76,45 @@ const formatAtAndOffset = ( const formatSelector = ( node: VectorSelector | MatrixSelector ): ReactElement => { - const matchLabels = node.matchers - .filter( - (m) => - !( - m.name === "__name__" && - m.type === matchType.equal && - m.value === node.name - ) - ) - .map((m, i) => ( - - {i !== 0 && ","} - {m.name} - {m.type} - "{escapeString(m.value)}" + const matchLabels: JSX.Element[] = []; + + // If the metric name contains the new extended charset, we need to escape it + // and add it at the beginning of the matchers list in the curly braces. + const metricName = + node.name || + node.matchers.find( + (m) => m.name === "__name__" && m.type === matchType.equal + )?.value || + ""; + const metricExtendedCharset = metricContainsExtendedCharset(metricName); + if (metricExtendedCharset) { + matchLabels.push( + + "{escapeString(metricName)}" - )); + ); + } + + matchLabels.push( + ...node.matchers + .filter((m) => !(m.name === "__name__" && m.type === matchType.equal)) + .map((m, i) => ( + + {(i !== 0 || metricExtendedCharset) && ","} + + {maybeQuoteLabelName(m.name)} + + {m.type} + "{escapeString(m.value)}" + + )) + ); return ( <> - {node.name} + {!metricExtendedCharset && ( + {metricName} + )} {matchLabels.length > 0 && ( <> {"{"} diff --git a/web/ui/mantine-ui/src/promql/serialize.ts b/web/ui/mantine-ui/src/promql/serialize.ts index af9c6ef15..1d2c63f4f 100644 --- a/web/ui/mantine-ui/src/promql/serialize.ts +++ b/web/ui/mantine-ui/src/promql/serialize.ts @@ -11,8 +11,14 @@ import { aggregatorsWithParam, maybeParenthesizeBinopChild, escapeString, + metricContainsExtendedCharset, + maybeQuoteLabelName, } from "./utils"; +const labelNameList = (labels: string[]): string => { + return labels.map((ln) => maybeQuoteLabelName(ln)).join(", "); +}; + const serializeAtAndOffset = ( timestamp: number | null, startOrEnd: StartOrEnd, @@ -28,15 +34,23 @@ const serializeAtAndOffset = ( const serializeSelector = (node: VectorSelector | MatrixSelector): string => { const matchers = node.matchers - .filter( - (m) => - !( - m.name === "__name__" && - m.type === matchType.equal && - m.value === node.name - ) - ) - .map((m) => `${m.name}${m.type}"${escapeString(m.value)}"`); + .filter((m) => !(m.name === "__name__" && m.type === matchType.equal)) + .map( + (m) => `${maybeQuoteLabelName(m.name)}${m.type}"${escapeString(m.value)}"` + ); + + // If the metric name contains the new extended charset, we need to escape it + // and add it at the beginning of the matchers list in the curly braces. + const metricName = + node.name || + node.matchers.find( + (m) => m.name === "__name__" && m.type === matchType.equal + )?.value || + ""; + const metricExtendedCharset = metricContainsExtendedCharset(metricName); + if (metricExtendedCharset) { + matchers.unshift(`"${escapeString(metricName)}"`); + } const range = node.type === nodeType.matrixSelector @@ -48,7 +62,7 @@ const serializeSelector = (node: VectorSelector | MatrixSelector): string => { node.offset ); - return `${node.name}${matchers.length > 0 ? `{${matchers.join(",")}}` : ""}${range}${atAndOffset}`; + return `${!metricExtendedCharset ? metricName : ""}${matchers.length > 0 ? `{${matchers.join(",")}}` : ""}${range}${atAndOffset}`; }; const serializeNode = ( @@ -68,9 +82,9 @@ const serializeNode = ( case nodeType.aggregation: return `${initialInd}${node.op}${ node.without - ? ` without(${node.grouping.join(", ")}) ` + ? ` without(${labelNameList(node.grouping)}) ` : node.grouping.length > 0 - ? ` by(${node.grouping.join(", ")}) ` + ? ` by(${labelNameList(node.grouping)}) ` : "" }(${childListSeparator}${ aggregatorsWithParam.includes(node.op) && node.param !== null @@ -119,16 +133,16 @@ const serializeNode = ( const vm = node.matching; if (vm !== null && (vm.labels.length > 0 || vm.on)) { if (vm.on) { - matching = ` on(${vm.labels.join(", ")})`; + matching = ` on(${labelNameList(vm.labels)})`; } else { - matching = ` ignoring(${vm.labels.join(", ")})`; + matching = ` ignoring(${labelNameList(vm.labels)})`; } if ( vm.card === vectorMatchCardinality.manyToOne || vm.card === vectorMatchCardinality.oneToMany ) { - grouping = ` group_${vm.card === vectorMatchCardinality.manyToOne ? "left" : "right"}(${vm.include.join(",")})`; + grouping = ` group_${vm.card === vectorMatchCardinality.manyToOne ? "left" : "right"}(${labelNameList(vm.include)})`; } } diff --git a/web/ui/mantine-ui/src/promql/serializeAndFormat.test.ts b/web/ui/mantine-ui/src/promql/serializeAndFormat.test.ts index ea045612c..a2b97ec90 100644 --- a/web/ui/mantine-ui/src/promql/serializeAndFormat.test.ts +++ b/web/ui/mantine-ui/src/promql/serializeAndFormat.test.ts @@ -99,7 +99,7 @@ describe("serializeNode and formatNode", () => { timestamp: null, startOrEnd: null, }, - output: '{__name__="metric_name"} offset 1m', + output: "metric_name offset 1m", }, { // Escaping in label values. @@ -642,6 +642,113 @@ describe("serializeNode and formatNode", () => { == bool on(label1, label2) group_right(label3) …`, }, + // Test new Prometheus 3.0 UTF-8 support. + { + node: { + bool: false, + lhs: { + bool: false, + lhs: { + expr: { + matchers: [ + { + name: "__name__", + type: matchType.equal, + value: "metric_ä", + }, + { + name: "foo", + type: matchType.equal, + value: "bar", + }, + ], + name: "", + offset: 0, + startOrEnd: null, + timestamp: null, + type: nodeType.vectorSelector, + }, + grouping: ["a", "ä"], + op: aggregationType.sum, + param: null, + type: nodeType.aggregation, + without: false, + }, + matching: { + card: vectorMatchCardinality.manyToOne, + include: ["c", "ü"], + labels: ["b", "ö"], + on: true, + }, + op: binaryOperatorType.div, + rhs: { + expr: { + matchers: [ + { + name: "__name__", + type: matchType.equal, + value: "metric_ö", + }, + { + name: "bar", + type: matchType.equal, + value: "foo", + }, + ], + name: "", + offset: 0, + startOrEnd: null, + timestamp: null, + type: nodeType.vectorSelector, + }, + grouping: ["d", "ä"], + op: aggregationType.sum, + param: null, + type: nodeType.aggregation, + without: true, + }, + type: nodeType.binaryExpr, + }, + matching: { + card: vectorMatchCardinality.oneToOne, + include: [], + labels: ["e", "ö"], + on: false, + }, + op: binaryOperatorType.add, + rhs: { + expr: { + matchers: [ + { + name: "__name__", + type: matchType.equal, + value: "metric_ü", + }, + ], + name: "", + offset: 0, + startOrEnd: null, + timestamp: null, + type: nodeType.vectorSelector, + }, + type: nodeType.parenExpr, + }, + type: nodeType.binaryExpr, + }, + output: + 'sum by(a, "ä") ({"metric_ä",foo="bar"}) / on(b, "ö") group_left(c, "ü") sum without(d, "ä") ({"metric_ö",bar="foo"}) + ignoring(e, "ö") ({"metric_ü"})', + prettyOutput: ` sum by(a, "ä") ( + {"metric_ä",foo="bar"} + ) + / on(b, "ö") group_left(c, "ü") + sum without(d, "ä") ( + {"metric_ö",bar="foo"} + ) ++ ignoring(e, "ö") + ( + {"metric_ü"} + )`, + }, ]; tests.forEach((t) => { diff --git a/web/ui/mantine-ui/src/promql/utils.ts b/web/ui/mantine-ui/src/promql/utils.ts index 5477ee596..2f1cc11d2 100644 --- a/web/ui/mantine-ui/src/promql/utils.ts +++ b/web/ui/mantine-ui/src/promql/utils.ts @@ -267,6 +267,21 @@ export const humanizedValueType: Record = { [valueType.matrix]: "range vector", }; +const metricNameRe = /^[a-zA-Z_:][a-zA-Z0-9_:]*$/; +const labelNameCharsetRe = /^[a-zA-Z_][a-zA-Z0-9_]*$/; + +export const metricContainsExtendedCharset = (str: string) => { + return !metricNameRe.test(str); +}; + +export const labelNameContainsExtendedCharset = (str: string) => { + return !labelNameCharsetRe.test(str); +}; + export const escapeString = (str: string) => { return str.replace(/([\\"])/g, "\\$1"); }; + +export const maybeQuoteLabelName = (str: string) => { + return labelNameContainsExtendedCharset(str) ? `"${escapeString(str)}"` : str; +}; From 76ca7d08d9ae9cb23ea0a498f87370a26f16dc1e Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Wed, 30 Oct 2024 16:43:10 +0100 Subject: [PATCH 2/2] Fixup: re-add erroneously removed lines Signed-off-by: Julius Volz --- web/ui/mantine-ui/src/pages/query/SeriesName.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/ui/mantine-ui/src/pages/query/SeriesName.tsx b/web/ui/mantine-ui/src/pages/query/SeriesName.tsx index 61bc62eee..d03b530f0 100644 --- a/web/ui/mantine-ui/src/pages/query/SeriesName.tsx +++ b/web/ui/mantine-ui/src/pages/query/SeriesName.tsx @@ -89,6 +89,10 @@ const SeriesName: FC = ({ labels, format }) => { ); }; + if (labels === null) { + return <>scalar; + } + if (format) { return renderFormatted(); }