[BUGFIX] Prometheus: ignore empty series selectors in label variables

docs/prometheus/model.md

@@ -121,6 +121,8 @@ spec:
   # `datasource` is a datasource selector. If not provided, the default PrometheusDatasource is used.
   # See the documentation about the datasources to understand how it is selected.
   datasource: <Prometheus Datasource selector> # Optional
+  # `matchers` are series selectors that filter the label names (Prometheus `match[]` parameter).
+  # Entries that are empty, contain only whitespace, or become empty after variable interpolation are ignored.
   matchers:
     - <string> # Optional
 ```
@@ -174,6 +176,8 @@ spec:
   # See the documentation about the datasources to understand how it is selected.
   datasource: <Prometheus Datasource selector> # Optional
   labelName: <string>
+  # `matchers` are series selectors that filter the label values (Prometheus `match[]` parameter).
+  # Entries that are empty, contain only whitespace, or become empty after variable interpolation are ignored.
   matchers:
     - <string> # Optional
 ```

prometheus/src/plugins/PrometheusLabelNamesVariable.test.ts

@@ -0,0 +1,88 @@
+// Copyright The Perses Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import type { VariableStateMap } from '@perses-dev/components';
+import type { GetVariableOptionsContext } from '@perses-dev/plugin-system';
+import type { DatasourceSpec } from '@perses-dev/spec';
+import type { Mock } from 'vitest';
+
+import { PrometheusDatasource } from './prometheus-datasource';
+import { PrometheusLabelNamesVariable } from './PrometheusLabelNamesVariable';
+import type { PrometheusDatasourceSpec } from './types';
+
+const datasource: PrometheusDatasourceSpec = { directUrl: 'http://localhost:9090' };
+
+const fetchJson = vi.fn();
+const client = PrometheusDatasource.createClient(datasource, { fetchJson });
+
+const getDatasource: Mock = vi.fn(async (): Promise<DatasourceSpec<PrometheusDatasourceSpec>> => {
+  return {
+    default: false,
+    plugin: { kind: 'PrometheusDatasource', spec: datasource },
+  };
+});
+const getDatasourceClient: Mock = vi.fn(async () => client);
+
+function createContext(variables: VariableStateMap = {}): GetVariableOptionsContext {
+  return {
+    datasourceStore: {
+      getDatasource,
+      getDatasourceClient,
+      listDatasourceSelectItems: vi.fn(async () => []),
+      getLocalDatasources: vi.fn(),
+      setLocalDatasources: vi.fn(),
+      getSavedDatasources: vi.fn(),
+      setSavedDatasources: vi.fn(),
+    },
+    variables,
+    timeRange: { start: new Date('2023-01-01T00:00:00Z'), end: new Date('2023-01-01T01:00:00Z') },
+  };
+}
+
+// The `match[]` values of the only label names request sent to Prometheus. Label names are requested with a POST, so
+// the selectors are in the form body.
+function sentMatchers(): string[] {
+  expect(fetchJson).toHaveBeenCalledTimes(1);
+  const [, init] = fetchJson.mock.lastCall as [string, { body: URLSearchParams }];
+  return init.body.getAll('match[]');
+}
+
+describe('PrometheusLabelNamesVariable', () => {
+  beforeEach(() => {
+    fetchJson.mockReset();
+    fetchJson.mockResolvedValue({ status: 'success', data: ['instance', 'job'] });
+  });
+
+  it('should not send match[] when the variable has no series selector', async () => {
+    const { data } = await PrometheusLabelNamesVariable.getVariableOptions({}, createContext());
+
+    expect(sentMatchers()).toEqual([]);
+    expect(data).toEqual([
+      { value: 'instance', label: 'instance' },
+      { value: 'job', label: 'job' },
+    ]);
+  });
+
+  it('should interpolate the series selectors and not send the empty ones', async () => {
+    const { data } = await PrometheusLabelNamesVariable.getVariableOptions(
+      { matchers: ['', ' up{job="$job"} ', ' ', '$selector', 'go_info'] },
+      createContext({ job: { value: 'node', loading: false }, selector: { value: ' ', loading: false } }),
+    );
+
+    expect(sentMatchers()).toEqual([' up{job="node"} ', 'go_info']);
+    expect(data).toEqual([
+      { value: 'instance', label: 'instance' },
+      { value: 'job', label: 'job' },
+    ]);
+  });
+});

prometheus/src/plugins/PrometheusLabelNamesVariable.tsx

@@ -12,15 +12,10 @@
 // limitations under the License.
 
 import type { VariablePlugin, GetVariableOptionsContext } from '@perses-dev/plugin-system';
-import {
-  replaceVariables,
-  parseVariables,
-  datasourceSelectValueToSelector,
-  isVariableDatasource,
-} from '@perses-dev/plugin-system';
+import { parseVariables, datasourceSelectValueToSelector, isVariableDatasource } from '@perses-dev/plugin-system';
 
 import { DEFAULT_PROM, getPrometheusTimeRange, PROM_DATASOURCE_KIND } from '../model';
-import { resolvePrometheusDatasource } from './interpolation';
+import { interpolateMatchers, resolvePrometheusDatasource } from './interpolation';
 import { stringArrayToVariableOptions, PrometheusLabelNamesVariableEditor } from './prometheus-variables';
 import type { PrometheusLabelNamesVariableOptions } from './types';
 
@@ -37,7 +32,7 @@ export const PrometheusLabelNamesVariable: VariablePlugin<PrometheusLabelNamesVa
       datasourceSelector,
       ctx.variables,
     );
-    const match = spec.matchers ? spec.matchers.map((m) => replaceVariables(m, ctx.variables)) : undefined;
+    const match = interpolateMatchers(spec.matchers, ctx.variables);
     const timeRange = getPrometheusTimeRange(ctx.timeRange);
 
     const { data: options } = await client.labelNames({ 'match[]': match, ...timeRange }, requestOptions);

prometheus/src/plugins/PrometheusLabelValuesVariable.test.ts

@@ -0,0 +1,106 @@
+// Copyright The Perses Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import type { VariableStateMap } from '@perses-dev/components';
+import type { GetVariableOptionsContext } from '@perses-dev/plugin-system';
+import type { DatasourceSpec } from '@perses-dev/spec';
+import type { Mock } from 'vitest';
+
+import { PrometheusDatasource } from './prometheus-datasource';
+import { PrometheusLabelValuesVariable } from './PrometheusLabelValuesVariable';
+import type { PrometheusDatasourceSpec } from './types';
+
+const datasource: PrometheusDatasourceSpec = { directUrl: 'http://localhost:9090' };
+
+const fetchJson = vi.fn();
+const client = PrometheusDatasource.createClient(datasource, { fetchJson });
+
+const getDatasource: Mock = vi.fn(async (): Promise<DatasourceSpec<PrometheusDatasourceSpec>> => {
+  return {
+    default: false,
+    plugin: { kind: 'PrometheusDatasource', spec: datasource },
+  };
+});
+const getDatasourceClient: Mock = vi.fn(async () => client);
+
+function createContext(variables: VariableStateMap = {}): GetVariableOptionsContext {
+  return {
+    datasourceStore: {
+      getDatasource,
+      getDatasourceClient,
+      listDatasourceSelectItems: vi.fn(async () => []),
+      getLocalDatasources: vi.fn(),
+      setLocalDatasources: vi.fn(),
+      getSavedDatasources: vi.fn(),
+      setSavedDatasources: vi.fn(),
+    },
+    variables,
+    timeRange: { start: new Date('2023-01-01T00:00:00Z'), end: new Date('2023-01-01T01:00:00Z') },
+  };
+}
+
+// The `match[]` values of the only label values request sent to Prometheus.
+function sentMatchers(): string[] {
+  expect(fetchJson).toHaveBeenCalledTimes(1);
+  const [url] = fetchJson.mock.lastCall as [string];
+  return new URL(url).searchParams.getAll('match[]');
+}
+
+describe('PrometheusLabelValuesVariable', () => {
+  beforeEach(() => {
+    fetchJson.mockReset();
+    fetchJson.mockResolvedValue({ status: 'success', data: ['node', 'prometheus'] });
+  });
+
+  it('should not send match[] when the variable has no series selector', async () => {
+    const { data } = await PrometheusLabelValuesVariable.getVariableOptions({ labelName: 'job' }, createContext());
+
+    expect(sentMatchers()).toEqual([]);
+    expect(data).toEqual([
+      { value: 'node', label: 'node' },
+      { value: 'prometheus', label: 'prometheus' },
+    ]);
+  });
+
+  it('should not send empty series selectors to Prometheus', async () => {
+    const { data } = await PrometheusLabelValuesVariable.getVariableOptions(
+      { labelName: 'job', matchers: ['', '  ', '\t\n'] },
+      createContext(),
+    );
+
+    expect(sentMatchers()).toEqual([]);
+    expect(data).toEqual([
+      { value: 'node', label: 'node' },
+      { value: 'prometheus', label: 'prometheus' },
+    ]);
+  });
+
+  it('should interpolate the series selectors and send the non-empty ones unchanged', async () => {
+    // Prometheus accepts whitespace around a selector, so the non-empty selectors are not trimmed.
+    await PrometheusLabelValuesVariable.getVariableOptions(
+      { labelName: 'job', matchers: [' up{job="$job"} ', '', 'go_info'] },
+      createContext({ job: { value: 'node', loading: false } }),
+    );
+
+    expect(sentMatchers()).toEqual([' up{job="node"} ', 'go_info']);
+  });
+
+  it('should not send a series selector that interpolates to an empty string', async () => {
+    await PrometheusLabelValuesVariable.getVariableOptions(
+      { labelName: 'job', matchers: ['$selector'] },
+      createContext({ selector: { value: '', loading: false } }),
+    );
+
+    expect(sentMatchers()).toEqual([]);
+  });
+});

prometheus/src/plugins/PrometheusLabelValuesVariable.tsx

@@ -21,7 +21,7 @@ import {
 import type { DatasourceSelector, DatasourceSpec } from '@perses-dev/spec';
 
 import { DEFAULT_PROM, getPrometheusTimeRange, PROM_DATASOURCE_KIND } from '../model';
-import { resolvePrometheusDatasource } from './interpolation';
+import { interpolateMatchers, resolvePrometheusDatasource } from './interpolation';
 import { stringArrayToVariableOptions, PrometheusLabelValuesVariableEditor } from './prometheus-variables';
 import type { PrometheusLabelValuesVariableOptions, PrometheusDatasourceSpec } from './types';
 
@@ -109,7 +109,7 @@ export const PrometheusLabelValuesVariable: VariablePlugin<PrometheusLabelValues
       datasourceSelector,
       ctx.variables,
     );
-    const match = pluginDef.matchers ? pluginDef.matchers.map((m) => replaceVariables(m, ctx.variables)) : undefined;
+    const match = interpolateMatchers(pluginDef.matchers, ctx.variables);
 
     const timeRange = getPrometheusTimeRange(ctx.timeRange);
 

prometheus/src/plugins/interpolation.ts

@@ -14,6 +14,7 @@
 import type { VariableStateMap } from '@perses-dev/components';
 import { interpolateHeaders, interpolateQueryParams } from '@perses-dev/components';
 import type { DatasourceStore } from '@perses-dev/plugin-system';
+import { replaceVariables } from '@perses-dev/plugin-system';
 import type { DatasourceSelector, DatasourceSpec } from '@perses-dev/spec';
 
 import type { ClientRequestOptions, PrometheusClient } from '../model';
@@ -51,6 +52,21 @@ export function interpolateDatasourceProxyParams(
   };
 }
 
+/**
+ * Interpolates the series selectors of a label names or label values variable into `match[]` values. Selectors that
+ * are empty or only whitespace after interpolation, like the one added by "Add Series Selector" before it is filled,
+ * are left out: Prometheus rejects an empty `match[]` ("parse error: unexpected end of input"), and sending no
+ * `match[]` at all means no filter.
+ */
+export function interpolateMatchers(
+  matchers: string[] | undefined,
+  variableState: VariableStateMap,
+): string[] | undefined {
+  return matchers
+    ?.map((matcher) => replaceVariables(matcher, variableState))
+    .filter((matcher) => matcher.trim() !== '');
+}
+
 export async function getInterpolatedRequestOptions(
   datasourceStore: DatasourceStore,
   datasource: DatasourceSelector,