Loki/Prometheus: Improve label browser performance (#34244)

* Loki/Prometheus: Improve label browser performance - raise limits - introduce debounce for Metrics browser filter fields (not done for Loki) - discard facetting responses when selector was already changed again by user * Clean up comments and imports
2025-02-25 18:55:37 -06:00 · 2021-05-18 18:57:38 +02:00 · 2021-05-18 18:57:38 +02:00 · 0683692509
commit 0683692509
parent 0b44182bbf
4 changed files with 49 additions and 23 deletions
--- a/public/app/plugins/datasource/loki/components/LokiLabelBrowser.tsx
+++ b/public/app/plugins/datasource/loki/components/LokiLabelBrowser.tsx
@ -10,7 +10,7 @@ import { GrafanaTheme } from '@grafana/data';
 import { LokiLabel } from './LokiLabel';

 // Hard limit on labels to render
-const MAX_LABEL_COUNT = 100;
+const MAX_LABEL_COUNT = 1000;
 const MAX_VALUE_COUNT = 10000;
 const MAX_AUTO_SELECT = 4;
 const EMPTY_SELECTOR = '{}';
@ -161,7 +161,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => ({
 }));

 export class UnthemedLokiLabelBrowser extends React.Component<BrowserProps, BrowserState> {
-  state = {
+  state: BrowserState = {
    labels: [] as SelectableLabel[],
    searchTerm: '',
    status: 'Ready',
@ -269,7 +269,7 @@ export class UnthemedLokiLabelBrowser extends React.Component<BrowserProps, Brow
        this.setState({ labels }, () => {
          this.state.labels.forEach((label) => {
            if (label.selected) {
-              this.fetchValues(label.name);
+              this.fetchValues(label.name, EMPTY_SELECTOR);
            }
          });
        });
@ -287,7 +287,7 @@ export class UnthemedLokiLabelBrowser extends React.Component<BrowserProps, Brow
    if (label.selected) {
      // Refetch values for newly selected label...
      if (!label.values) {
-        this.fetchValues(name);
+        this.fetchValues(name, buildSelector(this.state.labels));
      }
    } else {
      // Only need to facet when deselecting labels
@ -304,7 +304,7 @@ export class UnthemedLokiLabelBrowser extends React.Component<BrowserProps, Brow
      });
      this.setState({ labels }, () => {
        // Get fresh set of values
-        this.state.labels.forEach((label) => label.selected && this.fetchValues(label.name));
+        this.state.labels.forEach((label) => label.selected && this.fetchValues(label.name, selector));
      });
    } else {
      // Do facetting
@ -312,18 +312,23 @@ export class UnthemedLokiLabelBrowser extends React.Component<BrowserProps, Brow
    }
  };

-  async fetchValues(name: string) {
+  async fetchValues(name: string, selector: string) {
    const { languageProvider } = this.props;
    this.updateLabelState(name, { loading: true }, `Fetching values for ${name}`);
    try {
      let rawValues = await languageProvider.getLabelValues(name);
+      // If selector changed, clear loading state and discard result by returning early
+      if (selector !== buildSelector(this.state.labels)) {
+        this.updateLabelState(name, { loading: false }, '');
+        return;
+      }
      if (rawValues.length > MAX_VALUE_COUNT) {
        const error = `Too many values for ${name} (showing only ${MAX_VALUE_COUNT} of ${rawValues.length})`;
        rawValues = rawValues.slice(0, MAX_VALUE_COUNT);
        this.setState({ error });
      }
      const values: FacettableValue[] = rawValues.map((value) => ({ name: value }));
-      this.updateLabelState(name, { values, loading: false }, '');
+      this.updateLabelState(name, { values, loading: false });
    } catch (error) {
      console.error(error);
    }
@ -336,6 +341,13 @@ export class UnthemedLokiLabelBrowser extends React.Component<BrowserProps, Brow
    }
    try {
      const possibleLabels = await languageProvider.fetchSeriesLabels(selector, true);
+      // If selector changed, clear loading state and discard result by returning early
+      if (selector !== buildSelector(this.state.labels)) {
+        if (lastFacetted) {
+          this.updateLabelState(lastFacetted, { loading: false });
+        }
+        return;
+      }
      if (Object.keys(possibleLabels).length === 0) {
        // Sometimes the backend does not return a valid set
        console.error('No results for label combination, but should not occur.');
@ -368,7 +380,6 @@ export class UnthemedLokiLabelBrowser extends React.Component<BrowserProps, Brow
    const styles = getStyles(theme);
    let selectedLabels = labels.filter((label) => label.selected && label.values);
    if (searchTerm) {
-      // TODO extract from render() and debounce
      selectedLabels = selectedLabels.map((label) => ({
        ...label,
        values: label.values?.filter((value) => value.selected || value.name.includes(searchTerm)),
--- a/public/app/plugins/datasource/prometheus/components/Label.tsx
+++ b/public/app/plugins/datasource/prometheus/components/Label.tsx
@ -59,7 +59,12 @@ export const Label = forwardRef<HTMLElement, Props>(
        )}
        {...rest}
      >
-        <Highlighter textToHighlight={text} searchWords={searchWords} highlightClassName={styles.matchHighLight} />
+        <Highlighter
+          textToHighlight={text}
+          searchWords={searchWords}
+          autoEscape
+          highlightClassName={styles.matchHighLight}
+        />
      </span>
    );
  }
--- a/public/app/plugins/datasource/prometheus/components/PrometheusMetricsBrowser.test.tsx
+++ b/public/app/plugins/datasource/prometheus/components/PrometheusMetricsBrowser.test.tsx
@ -270,10 +270,10 @@ describe('PrometheusMetricsBrowser', () => {
    await screen.findByLabelText('Values for label2');
    expect(await screen.findAllByRole('option', { name: /value/ })).toHaveLength(4);
    // Typing '1' to filter for values
-    userEvent.type(screen.getByLabelText('Filter expression for label values'), '1');
+    await userEvent.type(screen.getByLabelText('Filter expression for label values'), '1');
    expect(screen.getByLabelText('Filter expression for label values')).toHaveValue('1');
-    expect(screen.queryByRole('option', { name: 'value2-2' })).not.toBeInTheDocument();
    expect(await screen.findAllByRole('option', { name: /value/ })).toHaveLength(3);
+    expect(screen.queryByRole('option', { name: 'value2-2' })).not.toBeInTheDocument();
  });

  it('facets labels', async () => {
@ -299,6 +299,7 @@ describe('PrometheusMetricsBrowser', () => {
    const value12 = await screen.findByRole('option', { name: 'value1-2', selected: false });
    userEvent.click(value12);
    await screen.findByRole('option', { name: 'value1-2', selected: true });
+    await screen.findByRole('option', { name: /label3/, selected: false });
    userEvent.click(screen.getByRole('option', { name: /label3/ }));
    await screen.findByLabelText('Values for label3');
    expect(screen.queryByRole('option', { name: 'value1-1', selected: true })).toBeInTheDocument();
--- a/public/app/plugins/datasource/prometheus/components/PrometheusMetricsBrowser.tsx
+++ b/public/app/plugins/datasource/prometheus/components/PrometheusMetricsBrowser.tsx
@ -10,7 +10,7 @@ import { Label as PromLabel } from './Label';

 // Hard limit on labels to render
 const MAX_LABEL_COUNT = 10000;
-const MAX_VALUE_COUNT = 10000;
+const MAX_VALUE_COUNT = 50000;
 const EMPTY_SELECTOR = '{}';
 const METRIC_LABEL = '__name__';
 export const LAST_USED_LABELS_KEY = 'grafana.datasources.prometheus.browser.labels';
@ -176,7 +176,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => ({
 *              to create a single, generic component.
 */
 export class UnthemedPrometheusMetricsBrowser extends React.Component<BrowserProps, BrowserState> {
-  state = {
+  state: BrowserState = {
    labels: [] as SelectableLabel[],
    labelSearchTerm: '',
    metricSearchTerm: '',
@ -231,7 +231,7 @@ export class UnthemedPrometheusMetricsBrowser extends React.Component<BrowserPro
    });
    store.delete(LAST_USED_LABELS_KEY);
    // Get metrics
-    this.fetchValues(METRIC_LABEL);
+    this.fetchValues(METRIC_LABEL, EMPTY_SELECTOR);
  };

  onClickLabel = (name: string, value: string | undefined, event: React.MouseEvent<HTMLElement>) => {
@ -314,7 +314,7 @@ export class UnthemedPrometheusMetricsBrowser extends React.Component<BrowserPro
          this.setState({ error });
        }
        // Get metrics
-        this.fetchValues(METRIC_LABEL);
+        this.fetchValues(METRIC_LABEL, EMPTY_SELECTOR);
        // Auto-select previously selected labels
        const labels: SelectableLabel[] = rawLabels.map((label, i, arr) => ({
          name: label,
@ -325,7 +325,7 @@ export class UnthemedPrometheusMetricsBrowser extends React.Component<BrowserPro
        this.setState({ labels }, () => {
          this.state.labels.forEach((label) => {
            if (label.selected) {
-              this.fetchValues(label.name);
+              this.fetchValues(label.name, EMPTY_SELECTOR);
            }
          });
        });
@ -343,7 +343,7 @@ export class UnthemedPrometheusMetricsBrowser extends React.Component<BrowserPro
    if (label.selected) {
      // Refetch values for newly selected label...
      if (!label.values) {
-        this.fetchValues(name);
+        this.fetchValues(name, buildSelector(this.state.labels));
      }
    } else {
      // Only need to facet when deselecting labels
@ -361,7 +361,7 @@ export class UnthemedPrometheusMetricsBrowser extends React.Component<BrowserPro
      this.setState({ labels }, () => {
        // Get fresh set of values
        this.state.labels.forEach(
-          (label) => (label.selected || label.name === METRIC_LABEL) && this.fetchValues(label.name)
+          (label) => (label.selected || label.name === METRIC_LABEL) && this.fetchValues(label.name, selector)
        );
      });
    } else {
@ -370,18 +370,23 @@ export class UnthemedPrometheusMetricsBrowser extends React.Component<BrowserPro
    }
  };

-  async fetchValues(name: string) {
+  async fetchValues(name: string, selector: string) {
    const { languageProvider } = this.props;
    this.updateLabelState(name, { loading: true }, `Fetching values for ${name}`);
    try {
      let rawValues = await languageProvider.getLabelValues(name);
+      // If selector changed, clear loading state and discard result by returning early
+      if (selector !== buildSelector(this.state.labels)) {
+        this.updateLabelState(name, { loading: false });
+        return;
+      }
      if (rawValues.length > MAX_VALUE_COUNT) {
        const error = `Too many values for ${name} (showing only ${MAX_VALUE_COUNT} of ${rawValues.length})`;
        rawValues = rawValues.slice(0, MAX_VALUE_COUNT);
        this.setState({ error });
      }
      const values: FacettableValue[] = rawValues.map((value) => ({ name: value }));
-      this.updateLabelState(name, { values, loading: false }, '');
+      this.updateLabelState(name, { values, loading: false });
    } catch (error) {
      console.error(error);
    }
@ -394,6 +399,13 @@ export class UnthemedPrometheusMetricsBrowser extends React.Component<BrowserPro
    }
    try {
      const possibleLabels = await languageProvider.fetchSeriesLabels(selector, true);
+      // If selector changed, clear loading state and discard result by returning early
+      if (selector !== buildSelector(this.state.labels)) {
+        if (lastFacetted) {
+          this.updateLabelState(lastFacetted, { loading: false });
+        }
+        return;
+      }
      if (Object.keys(possibleLabels).length === 0) {
        // Sometimes the backend does not return a valid set
        console.error('No results for label combination, but should not occur.');
@ -432,7 +444,6 @@ export class UnthemedPrometheusMetricsBrowser extends React.Component<BrowserPro
    // Filter metrics
    let metrics = labels.find((label) => label.name === METRIC_LABEL);
    if (metrics && metricSearchTerm) {
-      // TODO extract from render() and debounce
      metrics = {
        ...metrics,
        values: metrics.values?.filter((value) => value.selected || value.name.includes(metricSearchTerm)),
@ -442,14 +453,12 @@ export class UnthemedPrometheusMetricsBrowser extends React.Component<BrowserPro
    // Filter labels
    let nonMetricLabels = labels.filter((label) => !label.hidden && label.name !== METRIC_LABEL);
    if (labelSearchTerm) {
-      // TODO extract from render() and debounce
      nonMetricLabels = nonMetricLabels.filter((label) => label.selected || label.name.includes(labelSearchTerm));
    }

    // Filter non-metric label values
    let selectedLabels = nonMetricLabels.filter((label) => label.selected && label.values);
    if (valueSearchTerm) {
-      // TODO extract from render() and debounce
      selectedLabels = selectedLabels.map((label) => ({
        ...label,
        values: label.values?.filter((value) => value.selected || value.name.includes(valueSearchTerm)),