Explore metrics: OTel breakdown get request header too large with job and instance bug (#97136)

* don't duplicate job or instance in match terms for OTel * truncate job and instance list if exceeding 2000 chars for GET request * test for warning when truncating * fix check for duplicated job and instance, make easier to read * add trans and remove console.log * report when a metric has too many labels for job and info * remove console.warn * fix tests for not logging warning * fix tests * remove unused import * make i18n-extract * Update public/app/features/trails/interactions.ts Co-authored-by: ismail simsek <ismailsimsek09@gmail.com> * allow to dismiss warning --------- Co-authored-by: ismail simsek <ismailsimsek09@gmail.com>
2024-12-12 11:31:55 -06:00
parent 482af91782
commit 3266d62d6d
10 changed files with 98 additions and 16 deletions
@@ -1,7 +1,7 @@
 import init from '@bsull/augurs/outlier';
 import { css } from '@emotion/css';
 import { isNumber, max, min, throttle } from 'lodash';
-import { useEffect } from 'react';
+import { useEffect, useState } from 'react';

 import { DataFrame, FieldType, GrafanaTheme2, PanelData, SelectableValue } from '@grafana/data';
 import { config } from '@grafana/runtime';
@@ -26,7 +26,7 @@ import {
  VizPanel,
 } from '@grafana/scenes';
 import { DataQuery, SortOrder, TooltipDisplayMode } from '@grafana/schema';
-import { Button, Field, LoadingPlaceholder, useStyles2 } from '@grafana/ui';
+import { Alert, Button, Field, LoadingPlaceholder, useStyles2 } from '@grafana/ui';
 import { Trans } from 'app/core/internationalization';

 import { getAutoQueriesForMetric } from '../AutomaticMetricQueries/AutoQueryEngine';
@@ -47,6 +47,7 @@ import {
  VAR_FILTERS,
  VAR_GROUP_BY,
  VAR_GROUP_BY_EXP,
+  VAR_MISSING_OTEL_TARGETS,
  VAR_OTEL_GROUP_LEFT,
 } from '../shared';
 import { getColorByIndex, getTrailFor } from '../utils';
@@ -338,6 +339,14 @@ export class LabelBreakdownScene extends SceneObjectBase<LabelBreakdownSceneStat
      allLabelOptions.filter((option) => option.value !== ALL_VARIABLE_VALUE).unshift(all);
    }

+    const [dismissOtelWarning, updateDismissOtelWarning] = useState(false);
+    const missingOtelTargets = sceneGraph.lookupVariable(VAR_MISSING_OTEL_TARGETS, trail)?.getValue();
+    if (missingOtelTargets && !dismissOtelWarning) {
+      reportExploreMetrics('missing_otel_labels_by_truncating_job_and_instance', {
+        metric: trail.state.metric,
+      });
+    }
+
    useEffect(() => {
      if (useOtelExperience) {
        // this will update the group left variable
@@ -369,6 +378,22 @@ export class LabelBreakdownScene extends SceneObjectBase<LabelBreakdownSceneStat
              </Field>
            )}
          </div>
+          {missingOtelTargets && !dismissOtelWarning && (
+            <Alert
+              title={`Warning: There may be missing Open Telemetry resource attributes.`}
+              severity={'warning'}
+              key={'warning'}
+              onRemove={() => updateDismissOtelWarning(true)}
+              className={styles.truncatedOTelResources}
+            >
+              <Trans i18nKey={'explore-metrics.breakdown.missing-otel-labels'}>
+                This metric has too many job and instance label values to call the Prometheus label_values endpoint with
+                the match[] parameter. These label values are used to join the metric with target_info, which contains
+                the resource attributes. Please include more resource attributes filters.
+              </Trans>
+            </Alert>
+          )}
+
          <div className={styles.content}>{body && <body.Component model={body} />}</div>
        </StatusWrapper>
      </div>
@@ -400,6 +425,10 @@ function getStyles(theme: GrafanaTheme2) {
      gap: theme.spacing(2),
      justifyContent: 'space-between',
    }),
+    truncatedOTelResources: css({
+      minWidth: '30vw',
+      flexGrow: 0,
+    }),
  };
 }

@@ -65,6 +65,7 @@ import {
  VAR_DATASOURCE,
  VAR_DATASOURCE_EXPR,
  VAR_FILTERS,
+  VAR_MISSING_OTEL_TARGETS,
  VAR_OTEL_DEPLOYMENT_ENV,
  VAR_OTEL_GROUP_LEFT,
  VAR_OTEL_JOIN_QUERY,
@@ -732,6 +733,11 @@ function getVariableSet(
        value: undefined,
        hide: VariableHide.hideVariable,
      }),
+      new ConstantVariable({
+        name: VAR_MISSING_OTEL_TARGETS,
+        hide: VariableHide.hideVariable,
+        value: false,
+      }),
    ],
  });
 }
@@ -123,6 +123,9 @@ type Interactions = {
      sortBy: string
  };
  wasm_not_supported: {},
+  missing_otel_labels_by_truncating_job_and_instance: {
+    metric?: string;
+  },
 };

 const PREFIX = 'grafana_explore_metrics_';
@@ -9,6 +9,19 @@ import {
  getFilteredResourceAttributes,
 } from './api';

+jest.mock('./util', () => ({
+  ...jest.requireActual('./util'),
+  limitOtelMatchTerms: jest.fn().mockImplementation(() => {
+    return {
+      jobsRegex: 'job=~"job1|job2"',
+      instancesRegex: 'instance=~"instance1|instance2"',
+      // this flag is used when the values exceed 2000 characters
+      // in this mock we are not including more, just flipping the flag
+      missingOtelTargets: true,
+    };
+  }),
+}));
+
 jest.mock('@grafana/runtime', () => ({
  ...jest.requireActual('@grafana/runtime'),
  config: {
@@ -72,7 +85,7 @@ describe('OTEL API', () => {
    to: 'now',
  };

-  afterAll(() => {
+  afterEach(() => {
    jest.clearAllMocks();
  });

@@ -117,11 +130,16 @@ describe('OTEL API', () => {

  describe('getFilteredResourceAttributes', () => {
    it('should fetch and filter OTEL resources with excluded filters', async () => {
-      const resources = await getFilteredResourceAttributes(dataSourceUid, timeRange, 'metric', ['job']);
+      const { attributes } = await getFilteredResourceAttributes(dataSourceUid, timeRange, 'metric', ['job']);
      // promotedResourceAttribute will be filtered out because even though it is a resource attribute, it is also a metric label and wee prioritize metric labels
-      expect(resources).not.toEqual(['promotedResourceAttribute', 'resourceAttribute']);
+      expect(attributes).not.toEqual(['promotedResourceAttribute', 'resourceAttribute']);
      // the resource attributes returned are the ones only present on target_info
-      expect(resources).toEqual(['resourceAttribute']);
+      expect(attributes).toEqual(['resourceAttribute']);
+    });
+
+    it('should return a boolean true if the job and instance list for matching is truncated', async () => {
+      const { missingOtelTargets } = await getFilteredResourceAttributes(dataSourceUid, timeRange, 'metric', ['job']);
+      expect(missingOtelTargets).toBe(true);
    });
  });
 });
@@ -5,7 +5,7 @@ import { config, getBackendSrv } from '@grafana/runtime';
 import { callSuggestionsApi } from '../utils';

 import { OtelResponse, LabelResponse, OtelTargetType } from './types';
-import { sortResources } from './util';
+import { limitOtelMatchTerms, sortResources } from './util';

 const OTEL_RESOURCE_EXCLUDED_FILTERS = ['__name__', 'deployment_environment']; // name is handled by metric search metrics bar
 /**
@@ -242,14 +242,16 @@ export async function getFilteredResourceAttributes(
  // OTel metrics require unique identifies for the resource. Job+instance is the unique identifier.
  // If there are none, we cannot join on a target_info resource
  if (metricResources.jobs.length === 0 || metricResources.instances.length === 0) {
-    return [];
+    return { attributes: [], missingOtelTargets: false };
  }

  // The URL for the labels endpoint
  const url = `/api/datasources/uid/${datasourceUid}/resources/api/v1/labels`;

  // The match param for the metric to get all possible labels for this metric
-  const metricMatchParam = `${metric}{job=~"${metricResources.jobs.join('|')}",instance=~"${metricResources.instances.join('|')}"}`;
+  const metricMatchTerms = limitOtelMatchTerms([], metricResources.jobs, metricResources.instances);
+
+  let metricMatchParam = `${metric}{${metricMatchTerms.jobsRegex},${metricMatchTerms.instancesRegex}}`;

  const start = getPrometheusTime(timeRange.from, false);
  const end = getPrometheusTime(timeRange.to, true);
@@ -272,7 +274,7 @@ export async function getFilteredResourceAttributes(
  const metricLabels = metricResponse.data ?? [];

  // only get the resource attributes filtered by job and instance values present on the metric
-  const targetInfoMatchParam = `target_info{job=~"${metricResources.jobs.join('|')}",instance=~"${metricResources.instances.join('|')}"}`;
+  let targetInfoMatchParam = `target_info{${metricMatchTerms.jobsRegex},${metricMatchTerms.instancesRegex}}`;

  const targetInfoParams: Record<string, string | number> = {
    start,
@@ -303,5 +305,5 @@ export async function getFilteredResourceAttributes(
  // return a string array
  const resourceAttributes = sortedResourceAttributes.map((el) => el.text);

-  return resourceAttributes;
+  return { attributes: resourceAttributes, missingOtelTargets: metricMatchTerms.missingOtelTargets };
 }
@@ -5,6 +5,7 @@ import { DataTrail } from '../DataTrail';
 import {
  VAR_DATASOURCE_EXPR,
  VAR_FILTERS,
+  VAR_MISSING_OTEL_TARGETS,
  VAR_OTEL_DEPLOYMENT_ENV,
  VAR_OTEL_GROUP_LEFT,
  VAR_OTEL_JOIN_QUERY,
@@ -174,6 +175,8 @@ export function limitOtelMatchTerms(
  // stop before the total count reaches 2000
  // show a warning that there are missing OTel targets and
  // the user must select more OTel resource attributes
+  const jobCheck: { [key: string]: boolean } = {};
+  const instanceCheck: { [key: string]: boolean } = {};
  for (let i = 0; i < jobsList.length; i++) {
    // use or character for the count
    const orChars = i === 0 ? 0 : 2;
@@ -191,9 +194,12 @@ export function limitOtelMatchTerms(
        jobsRegex += `${jobsList[i]}`;
        instancesRegex += `${instancesList[i]}`;
      } else {
-        jobsRegex += `|${jobsList[i]}`;
-        instancesRegex += `|${instancesList[i]}`;
+        // check to make sure we aren't duplicating job or instance
+        jobsRegex += jobCheck[jobsList[i]] ? '' : `|${jobsList[i]}`;
+        instancesRegex += instanceCheck[instancesList[i]] ? '' : `|${instancesList[i]}`;
      }
+      jobCheck[jobsList[i]] = true;
+      instanceCheck[instancesList[i]] = true;
    } else {
      missingOtelTargets = true;
      break;
@@ -239,7 +245,12 @@ export async function updateOtelJoinWithGroupLeft(trail: DataTrail, metric: stri
  }
  const otelGroupLeft = sceneGraph.lookupVariable(VAR_OTEL_GROUP_LEFT, trail);
  const otelJoinQueryVariable = sceneGraph.lookupVariable(VAR_OTEL_JOIN_QUERY, trail);
-  if (!(otelGroupLeft instanceof ConstantVariable) || !(otelJoinQueryVariable instanceof ConstantVariable)) {
+  const missingOtelTargetsVariable = sceneGraph.lookupVariable(VAR_MISSING_OTEL_TARGETS, trail);
+  if (
+    !(otelGroupLeft instanceof ConstantVariable) ||
+    !(otelJoinQueryVariable instanceof ConstantVariable) ||
+    !(missingOtelTargetsVariable instanceof ConstantVariable)
+  ) {
    return;
  }
  // Remove the group left
@@ -272,7 +283,12 @@ export async function updateOtelJoinWithGroupLeft(trail: DataTrail, metric: stri
    excludeFilterKeys = excludeFilterKeys.concat(['job', 'instance']);
  }
  const datasourceUid = sceneGraph.interpolate(trail, VAR_DATASOURCE_EXPR);
-  const attributes = await getFilteredResourceAttributes(datasourceUid, timeRange, metric, excludeFilterKeys);
+  const { attributes, missingOtelTargets } = await getFilteredResourceAttributes(
+    datasourceUid,
+    timeRange,
+    metric,
+    excludeFilterKeys
+  );
  // here we start to add the attributes to the group left
  if (attributes.length > 0) {
    // update the group left variable that contains all the filtered resource attributes
@@ -283,6 +299,8 @@ export async function updateOtelJoinWithGroupLeft(trail: DataTrail, metric: stri
    // update the join query that is interpolated in all queries
    otelJoinQueryVariable.setState({ value: otelJoinQuery });
  }
+  // used to show a warning in label breakdown that the user must select more OTel resource attributes
+  missingOtelTargetsVariable.setState({ value: missingOtelTargets });
 }

 /**
@@ -21,7 +21,9 @@ jest.mock('./api', () => ({
  totalOtelResources: jest.fn(() => ({ job: 'oteldemo', instance: 'instance' })),
  getDeploymentEnvironments: jest.fn(() => ['production', 'staging']),
  isOtelStandardization: jest.fn(() => true),
-  getFilteredResourceAttributes: jest.fn().mockResolvedValue(['resourceAttribute']),
+  getFilteredResourceAttributes: jest
+    .fn()
+    .mockResolvedValue({ attributes: ['resourceAttribute'], missingOtelTargets: false }),
 }));

 describe('sortResources', () => {
@@ -34,6 +34,8 @@ export const VAR_OTEL_GROUP_BY = 'otel_groupby';
 export const VAR_OTEL_GROUP_BY_EXPR = '${otel_groupby}';
 export const VAR_OTEL_GROUP_LEFT = 'otel_group_left';
 export const VAR_OTEL_GROUP_LEFT_EXPR = '${otel_group_left}';
+export const VAR_MISSING_OTEL_TARGETS = 'missing_otel_targets';
+export const VAR_MISSING_OTEL_TARGETS_EXPR = '${missing_otel_targets}';

 export const LOGS_METRIC = '$__logs__';
 export const KEY_SQR_METRIC_VIZ_QUERY = 'sqr-metric-viz-query';
@@ -1290,6 +1290,7 @@
    "breakdown": {
      "clearFilter": "Clear filter",
      "labelSelect": "Select",
+      "missing-otel-labels": "This metric has too many job and instance label values to call the Prometheus label_values endpoint with the match[] parameter. These label values are used to join the metric with target_info, which contains the resource attributes. Please include more resource attributes filters.",
      "noMatchingValue": "No values found matching; {{filter}}",
      "sortBy": "Sort by"
    },
@@ -1290,6 +1290,7 @@
    "breakdown": {
      "clearFilter": "Cľęäř ƒįľŧęř",
      "labelSelect": "Ŝęľęčŧ",
+      "missing-otel-labels": "Ŧĥįş męŧřįč ĥäş ŧőő mäŉy ĵőþ äŉđ įŉşŧäŉčę ľäþęľ väľūęş ŧő čäľľ ŧĥę Přőmęŧĥęūş ľäþęľ_väľūęş ęŉđpőįŉŧ ŵįŧĥ ŧĥę mäŧčĥ[] päřämęŧęř. Ŧĥęşę ľäþęľ väľūęş äřę ūşęđ ŧő ĵőįŉ ŧĥę męŧřįč ŵįŧĥ ŧäřģęŧ_įŉƒő, ŵĥįčĥ čőŉŧäįŉş ŧĥę řęşőūřčę äŧŧřįþūŧęş. Pľęäşę įŉčľūđę mőřę řęşőūřčę äŧŧřįþūŧęş ƒįľŧęřş.",
      "noMatchingValue": "Ńő väľūęş ƒőūŉđ mäŧčĥįŉģ; {{filter}}",
      "sortBy": "Ŝőřŧ þy"
    },