diff --git a/.changeset/label-lookup-metric-name.md b/.changeset/label-lookup-metric-name.md new file mode 100644 index 000000000..09102779c --- /dev/null +++ b/.changeset/label-lookup-metric-name.md @@ -0,0 +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 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({ diff --git a/packages/api/src/controllers/timeseriesEngine.ts b/packages/api/src/controllers/timeseriesEngine.ts index 3f63e8dc1..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, @@ -121,7 +122,16 @@ 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(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}`; } /** 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; + } + } +}