From c83953eb76cfb261c8131576e8aa3494c4adfd6a Mon Sep 17 00:00:00 2001 From: Baris Can Vural Date: Tue, 10 Nov 2026 16:04:39 +0300 Subject: [PATCH 1/4] perf(api): read only the matched metrics' tags rows in PromQL label lookups A label lookup with match[] filtered the tags table by `id IN (timeSeriesSelector(...))`. The tags table is ordered by metric name, so that predicate alone reads every tags row. When every selector names its metric (`name{...}`, `name` or `{__name__="name"}`), also filter on metric_name: the same rows, but only those metrics' part of the table is read. --- .changeset/label-lookup-metric-name.md | 5 ++++ .../__tests__/timeseriesEngine.test.ts | 19 +++++++++++++++ .../api/src/controllers/timeseriesEngine.ts | 24 ++++++++++++++++++- 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 .changeset/label-lookup-metric-name.md diff --git a/.changeset/label-lookup-metric-name.md b/.changeset/label-lookup-metric-name.md new file mode 100644 index 000000000..39470e004 --- /dev/null +++ b/.changeset/label-lookup-metric-name.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/api': patch +--- + +perf: PromQL label lookups with `match[]` read only the matched metrics' tags rows when every selector names its metric diff --git a/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts b/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts index f1f0d2b36..769e3e42c 100644 --- a/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts +++ b/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts @@ -1,6 +1,7 @@ import { formatMatrixResponse, formatVectorResponse, + selectorMetricName, } from '@/controllers/timeseriesEngine'; describe('formatMatrixResponse', () => { @@ -45,3 +46,21 @@ describe('formatVectorResponse', () => { ]); }); }); + +describe('selectorMetricName', () => { + it('reads the metric name of a bare or braced selector', () => { + expect(selectorMetricName('up')).toBe('up'); + expect(selectorMetricName(' node_cpu:rate5m{job="x"}')).toBe( + 'node_cpu:rate5m', + ); + expect(selectorMetricName('{job="x", __name__="up"}')).toBe('up'); + expect(selectorMetricName('{__name__="up"}')).toBe('up'); + }); + + it('returns undefined when the selector may match several metrics', () => { + expect(selectorMetricName('{job="x"}')).toBeUndefined(); + expect(selectorMetricName('{__name__=~"up|down"}')).toBeUndefined(); + expect(selectorMetricName('{__name__!="up"}')).toBeUndefined(); + expect(selectorMetricName('{job="__name__=\\"up\\""}')).toBeUndefined(); + }); +}); diff --git a/packages/api/src/controllers/timeseriesEngine.ts b/packages/api/src/controllers/timeseriesEngine.ts index 3f63e8dc1..5470acf6b 100644 --- a/packages/api/src/controllers/timeseriesEngine.ts +++ b/packages/api/src/controllers/timeseriesEngine.ts @@ -121,7 +121,29 @@ function getSeriesSelectorCondition({ )`, ); - return chSql`id IN (${concatChSql(' UNION DISTINCT ', idsPerSelector)})`; + const ids = chSql`id IN (${concatChSql(' UNION DISTINCT ', idsPerSelector)})`; + // The tags table is sorted by metric name, so `id IN (...)` alone reads every + // tags row. When each selector names its metric, say so: same rows, but only + // those metrics' part of the table is read. + const names = match.map(selectorMetricName); + if (names.some(n => n == null)) return ids; + const list = [...new Set(names as string[])].map( + n => chSql`${{ String: n }}`, + ); + return chSql`metric_name IN (${concatChSql(', ', list)}) AND ${ids}`; +} + +/** + * The metric a series selector is limited to: `name{...}`, `name`, or + * `{__name__="name", ...}`. Undefined when it may match more than one metric. + */ +export function selectorMetricName(selector: string): string | undefined { + const bare = /^\s*([a-zA-Z_:][a-zA-Z0-9_:]*)\s*(\{|$)/.exec(selector); + if (bare) return bare[1]; + const label = /^\s*\{(?:[^}]*,)?\s*__name__\s*=\s*"((?:[^"\\]|\\.)*)"/.exec( + selector, + ); + return label ? label[1] : undefined; } /** From b7c7613ff49abed20df033a20240fdb7e7529557 Mon Sep 17 00:00:00 2001 From: Baris Can Vural Date: Tue, 10 Nov 2026 17:14:56 +0300 Subject: [PATCH 2/4] fix(api): don't read __name__ from inside another label's value The metric name was found by skipping everything before `__name__`, which could land inside a single-quoted or backtick value and narrow the lookup to the wrong metric. Match whole label matchers before it, and only take a plain metric name. --- .../__tests__/timeseriesEngine.test.ts | 6 ++++++ .../api/src/controllers/timeseriesEngine.ts | 20 ++++++++++--------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts b/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts index 769e3e42c..58ff2ca41 100644 --- a/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts +++ b/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts @@ -55,6 +55,9 @@ describe('selectorMetricName', () => { ); expect(selectorMetricName('{job="x", __name__="up"}')).toBe('up'); expect(selectorMetricName('{__name__="up"}')).toBe('up'); + expect(selectorMetricName(`{job=~'a|b', env!=\`x\`, __name__='up'}`)).toBe( + 'up', + ); }); it('returns undefined when the selector may match several metrics', () => { @@ -62,5 +65,8 @@ describe('selectorMetricName', () => { expect(selectorMetricName('{__name__=~"up|down"}')).toBeUndefined(); expect(selectorMetricName('{__name__!="up"}')).toBeUndefined(); expect(selectorMetricName('{job="__name__=\\"up\\""}')).toBeUndefined(); + expect(selectorMetricName(`{job='a,__name__="up"'}`)).toBeUndefined(); + expect(selectorMetricName('{job=`a,__name__="up"`}')).toBeUndefined(); + expect(selectorMetricName('{__name__="a\\"b"}')).toBeUndefined(); }); }); diff --git a/packages/api/src/controllers/timeseriesEngine.ts b/packages/api/src/controllers/timeseriesEngine.ts index 5470acf6b..d1b752470 100644 --- a/packages/api/src/controllers/timeseriesEngine.ts +++ b/packages/api/src/controllers/timeseriesEngine.ts @@ -125,14 +125,19 @@ function getSeriesSelectorCondition({ // The tags table is sorted by metric name, so `id IN (...)` alone reads every // tags row. When each selector names its metric, say so: same rows, but only // those metrics' part of the table is read. - const names = match.map(selectorMetricName); - if (names.some(n => n == null)) return ids; - const list = [...new Set(names as string[])].map( - n => chSql`${{ String: n }}`, - ); + const names = match + .map(selectorMetricName) + .filter((n): n is string => n != null); + if (names.length < match.length) return ids; + const list = [...new Set(names)].map(n => chSql`${{ String: n }}`); return chSql`metric_name IN (${concatChSql(', ', list)}) AND ${ids}`; } +// Whole matchers before `__name__`, so a quoted value containing +// `__name__="x"` isn't read as one. Names that need escaping fall back. +const NAME_MATCHER = + /^\s*\{(?:\s*[a-zA-Z_]\w*\s*(?:=~|!~|!=|=)\s*(?:"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'|`[^`]*`)\s*,)*\s*__name__\s*=\s*(["'`])([a-zA-Z_:][a-zA-Z0-9_:]*)\1/; + /** * The metric a series selector is limited to: `name{...}`, `name`, or * `{__name__="name", ...}`. Undefined when it may match more than one metric. @@ -140,10 +145,7 @@ function getSeriesSelectorCondition({ export function selectorMetricName(selector: string): string | undefined { const bare = /^\s*([a-zA-Z_:][a-zA-Z0-9_:]*)\s*(\{|$)/.exec(selector); if (bare) return bare[1]; - const label = /^\s*\{(?:[^}]*,)?\s*__name__\s*=\s*"((?:[^"\\]|\\.)*)"/.exec( - selector, - ); - return label ? label[1] : undefined; + return NAME_MATCHER.exec(selector)?.[2]; } /** From a34014d88a26e47a1fa830ba56516486dbd8703f Mon Sep 17 00:00:00 2001 From: Baris Can Vural Date: Tue, 10 Nov 2026 20:02:59 +0300 Subject: [PATCH 3/4] refactor: read the selector's metric name with the PromQL parser Use @prometheus-io/lezer-promql in a common-utils module instead of a regex in the API, as asked in review. --- .changeset/label-lookup-metric-name.md | 4 +- .../__tests__/timeseriesEngine.test.ts | 25 ----------- .../api/src/controllers/timeseriesEngine.ts | 18 +------- .../src/core/__tests__/promqlSelector.test.ts | 27 ++++++++++++ .../common-utils/src/core/promqlSelector.ts | 41 +++++++++++++++++++ 5 files changed, 73 insertions(+), 42 deletions(-) create mode 100644 packages/common-utils/src/core/__tests__/promqlSelector.test.ts create mode 100644 packages/common-utils/src/core/promqlSelector.ts diff --git a/.changeset/label-lookup-metric-name.md b/.changeset/label-lookup-metric-name.md index 39470e004..09102779c 100644 --- a/.changeset/label-lookup-metric-name.md +++ b/.changeset/label-lookup-metric-name.md @@ -1,5 +1,7 @@ --- '@hyperdx/api': patch +'@hyperdx/common-utils': patch --- -perf: PromQL label lookups with `match[]` read only the matched metrics' tags rows when every selector names its metric +perf: PromQL label lookups with `match[]` read only the matched metrics' tags +rows when every selector names its metric diff --git a/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts b/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts index 58ff2ca41..f1f0d2b36 100644 --- a/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts +++ b/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts @@ -1,7 +1,6 @@ import { formatMatrixResponse, formatVectorResponse, - selectorMetricName, } from '@/controllers/timeseriesEngine'; describe('formatMatrixResponse', () => { @@ -46,27 +45,3 @@ describe('formatVectorResponse', () => { ]); }); }); - -describe('selectorMetricName', () => { - it('reads the metric name of a bare or braced selector', () => { - expect(selectorMetricName('up')).toBe('up'); - expect(selectorMetricName(' node_cpu:rate5m{job="x"}')).toBe( - 'node_cpu:rate5m', - ); - expect(selectorMetricName('{job="x", __name__="up"}')).toBe('up'); - expect(selectorMetricName('{__name__="up"}')).toBe('up'); - expect(selectorMetricName(`{job=~'a|b', env!=\`x\`, __name__='up'}`)).toBe( - 'up', - ); - }); - - it('returns undefined when the selector may match several metrics', () => { - expect(selectorMetricName('{job="x"}')).toBeUndefined(); - expect(selectorMetricName('{__name__=~"up|down"}')).toBeUndefined(); - expect(selectorMetricName('{__name__!="up"}')).toBeUndefined(); - expect(selectorMetricName('{job="__name__=\\"up\\""}')).toBeUndefined(); - expect(selectorMetricName(`{job='a,__name__="up"'}`)).toBeUndefined(); - expect(selectorMetricName('{job=`a,__name__="up"`}')).toBeUndefined(); - expect(selectorMetricName('{__name__="a\\"b"}')).toBeUndefined(); - }); -}); diff --git a/packages/api/src/controllers/timeseriesEngine.ts b/packages/api/src/controllers/timeseriesEngine.ts index d1b752470..2f51aec67 100644 --- a/packages/api/src/controllers/timeseriesEngine.ts +++ b/packages/api/src/controllers/timeseriesEngine.ts @@ -8,6 +8,7 @@ import { Metadata, MetadataCache, } from '@hyperdx/common-utils/dist/core/metadata'; +import { promqlSelectorMetricName } from '@hyperdx/common-utils/dist/core/promqlSelector'; import { PrometheusMatrixResult, PrometheusVectorResult, @@ -126,28 +127,13 @@ function getSeriesSelectorCondition({ // tags row. When each selector names its metric, say so: same rows, but only // those metrics' part of the table is read. const names = match - .map(selectorMetricName) + .map(promqlSelectorMetricName) .filter((n): n is string => n != null); if (names.length < match.length) return ids; const list = [...new Set(names)].map(n => chSql`${{ String: n }}`); return chSql`metric_name IN (${concatChSql(', ', list)}) AND ${ids}`; } -// Whole matchers before `__name__`, so a quoted value containing -// `__name__="x"` isn't read as one. Names that need escaping fall back. -const NAME_MATCHER = - /^\s*\{(?:\s*[a-zA-Z_]\w*\s*(?:=~|!~|!=|=)\s*(?:"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'|`[^`]*`)\s*,)*\s*__name__\s*=\s*(["'`])([a-zA-Z_:][a-zA-Z0-9_:]*)\1/; - -/** - * The metric a series selector is limited to: `name{...}`, `name`, or - * `{__name__="name", ...}`. Undefined when it may match more than one metric. - */ -export function selectorMetricName(selector: string): string | undefined { - const bare = /^\s*([a-zA-Z_:][a-zA-Z0-9_:]*)\s*(\{|$)/.exec(selector); - if (bare) return bare[1]; - return NAME_MATCHER.exec(selector)?.[2]; -} - /** * Returns SQL predicates restricting tags rows to the series a label lookup * should consider: those overlapping [startMs, endMs], and those matched by diff --git a/packages/common-utils/src/core/__tests__/promqlSelector.test.ts b/packages/common-utils/src/core/__tests__/promqlSelector.test.ts new file mode 100644 index 000000000..1bcd285aa --- /dev/null +++ b/packages/common-utils/src/core/__tests__/promqlSelector.test.ts @@ -0,0 +1,27 @@ +import { promqlSelectorMetricName } from '@/core/promqlSelector'; + +describe('promqlSelectorMetricName', () => { + it.each([ + ['up', 'up'], + [' node_cpu:rate5m{job="x"}', 'node_cpu:rate5m'], + ['{__name__="up"}', 'up'], + ['{job="x", __name__="up"}', 'up'], + ["{job=~'a|b', env!=`x`, __name__='up'}", 'up'], + ])('reads the metric of %s', (selector, name) => { + expect(promqlSelectorMetricName(selector)).toBe(name); + }); + + it.each([ + '{job="x"}', + '{__name__=~"up|down"}', + '{job="x", __name__!="up"}', + '{job="__name__=\\"up\\""}', + `{job='a,__name__="up"'}`, + '{job=`a,__name__="up"`}', + '{__name__="a\\"b"}', + 'sum(up)', + 'up{', + ])('returns undefined for %s', selector => { + expect(promqlSelectorMetricName(selector)).toBeUndefined(); + }); +}); diff --git a/packages/common-utils/src/core/promqlSelector.ts b/packages/common-utils/src/core/promqlSelector.ts new file mode 100644 index 000000000..a2536757a --- /dev/null +++ b/packages/common-utils/src/core/promqlSelector.ts @@ -0,0 +1,41 @@ +import { parser } from '@prometheus-io/lezer-promql'; + +/** + * The metric a series selector is limited to: `name`, `name{...}`, or + * `{__name__="name", ...}`. Undefined when the selector may match more than + * one metric, isn't a single plain selector, or doesn't parse. + */ +export function promqlSelectorMetricName(selector: string): string | undefined { + const tree = parser.parse(selector); + let parseError = false; + tree.iterate({ + enter: node => { + if (node.type.isError) parseError = true; + }, + }); + const vectorSelector = tree.topNode.firstChild; + if (parseError || vectorSelector?.name !== 'VectorSelector') return; + + const identifier = vectorSelector.getChild('Identifier'); + if (identifier) return selector.slice(identifier.from, identifier.to); + + const matchers = + vectorSelector + .getChild('LabelMatchers') + ?.getChildren('UnquotedLabelMatcher') ?? []; + for (const matcher of matchers) { + const name = matcher.getChild('LabelName'); + const op = matcher.getChild('MatchOp')?.firstChild; + const value = matcher.getChild('StringLiteral'); + if ( + name && + value && + op?.name === 'EqlSingle' && + selector.slice(name.from, name.to) === '__name__' + ) { + // Values with escapes would need unquoting; fall back instead. + const text = selector.slice(value.from + 1, value.to - 1); + return text.includes('\\') ? undefined : text; + } + } +} From 71e2aa66b8773d6d599500db8ede60920c35554f Mon Sep 17 00:00:00 2001 From: Baris Can Vural Date: Tue, 10 Nov 2026 20:11:35 +0300 Subject: [PATCH 4/4] test(api): cover a match[] list mixing named and unnamed selectors --- .../__tests__/timeseriesEngine.int.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/api/src/controllers/__tests__/timeseriesEngine.int.test.ts b/packages/api/src/controllers/__tests__/timeseriesEngine.int.test.ts index 32653e22f..74c0185d4 100644 --- a/packages/api/src/controllers/__tests__/timeseriesEngine.int.test.ts +++ b/packages/api/src/controllers/__tests__/timeseriesEngine.int.test.ts @@ -202,6 +202,17 @@ describe('timeseriesEngine controller', () => { ).toEqual(['api', 'batch']); }); + // A selector without a metric name can match any metric, so naming + // `old_metric` in another selector mustn't narrow the lookup to it. + it('keeps the series of a selector without a metric name', async () => { + expect( + await labelValues({ + labelName: 'job', + match: ['{job="web"}', 'old_metric'], + }), + ).toEqual(['batch', 'web']); + }); + it('scopes metric names to the selector', async () => { expect( await labelValues({