Skip to content

Fix metrics query builder not handling * wildcard correctly

Daniele Rossetti requested to merge drosse/metrics-fix-groupby-wildcard into master

What does this MR do and why?

Fix an issue where the metrics group-by filter did not handle * wildcard correctly when all values are selected.

The API could indeed return default_group_by_attributes: ['*'], when all attributes are selected, but this is currently treated as a normal value. This changes adds logic to interpret the wildcard correctly.

As part of this, I've also made a small refactor and decided not to pass the full searchmetadata config to the group-by component, but only the relevant part. This will allow to add some sanitisation and validation at the parent level if needed, rather than having the child component access the raw config directly.

Closes gitlab-org/opstrace/opstrace#2654 (closed)

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Before After

image.png

image

image.png

How to set up and validate locally

  • Prerequisites: be logged in and running GDK with Ultimate license
  • Enable :observability_metrics feature flag

Apply patch to load mocks ( copy the patch content below and run in your terminal: pbpaste | git apply --allow-empty )

 
diff --git a/app/assets/javascripts/observability/client.js b/app/assets/javascripts/observability/client.js
index 335c70d220c1..7fcd9516b9ec 100644
--- a/app/assets/javascripts/observability/client.js
+++ b/app/assets/javascripts/observability/client.js
@@ -1,22 +1,52 @@
+/* eslint-disable @gitlab/require-i18n-strings */
 import { isValidDate } from '~/lib/utils/datetime_utility';
 import * as Sentry from '~/sentry/sentry_browser_wrapper';
 import axios from '~/lib/utils/axios_utils';
 import { logError } from '~/lib/logger';
 import { DEFAULT_SORTING_OPTION, SORTING_OPTIONS, CUSTOM_DATE_RANGE_OPTION } from './constants';

+const MOCK_METRICS = {
+  metrics: [
+    {
+      name: 'app.ads.ad_requests',
+      description: 'Counts ad requests by request and response type',
+      type: 'Sum',
+    },
+    {
+      name: 'http.client.duration',
+      description: 'measures the duration of the outbound HTTP request',
+      type: 'Histogram',
+    },
+    {
+      name: 'http.server.duration',
+      description: 'Measures the duration of inbound HTTP requests.',
+      type: 'ExponentialHistogram',
+    },
+    {
+      name: 'kafka.consumer.assigned_partitions',
+      description: 'The number of partitions currently assigned to this consumer',
+      type: 'Gauge',
+    },
+  ],
+};
+
 function reportErrorAndThrow(e) {
   logError(e);
   Sentry.captureException(e);
   throw e;
 }
+
+function mockReturnDataWithDelay(data) {
+  return new Promise((resolve) => {
+    setTimeout(() => resolve(data), 500);
+  });
+}
+
 // Provisioning API spec: https://gitlab.com/gitlab-org/opstrace/opstrace/-/blob/main/provisioning-api/pkg/provisioningapi/routes.go#L59
 async function enableObservability(provisioningUrl) {
   try {
-    // Note: axios.put(url, undefined, {withCredentials: true}) does not send cookies properly, so need to use the API below for the correct behaviour
-    return await axios(provisioningUrl, {
-      method: 'put',
-      withCredentials: true,
-    });
+    console.log('[DEBUG] Enabling Observability');
+    return mockReturnDataWithDelay();
   } catch (e) {
     return reportErrorAndThrow(e);
   }
@@ -25,11 +55,12 @@ async function enableObservability(provisioningUrl) {
 // Provisioning API spec: https://gitlab.com/gitlab-org/opstrace/opstrace/-/blob/main/provisioning-api/pkg/provisioningapi/routes.go#L37
 async function isObservabilityEnabled(provisioningUrl) {
   try {
-    const { data } = await axios.get(provisioningUrl, { withCredentials: true });
+    console.log('[DEBUG] Checking Observability Enabled');
+    const data = { status: 'ready' };
     if (data && data.status) {
       // we currently ignore the 'status' payload and just check if the request was successful
       // We might improve this as part of https://gitlab.com/gitlab-org/opstrace/opstrace/-/issues/2315
-      return true;
+      return mockReturnDataWithDelay(true);
     }
   } catch (e) {
     if (e.response.status === 404) {
@@ -322,14 +353,12 @@ async function fetchMetrics(metricsUrl, { filters = {}, limit } = {}) {
         }
       }
     }
-    const { data } = await axios.get(metricsUrl, {
-      withCredentials: true,
-      params,
-    });
+    console.log(`[DEBUG] Fetching metrics with params: ${params.toString()}`);
+    const data = MOCK_METRICS;
     if (!Array.isArray(data.metrics)) {
       throw new Error('metrics are missing/invalid in the response'); // eslint-disable-line @gitlab/require-i18n-strings
     }
-    return data;
+    return mockReturnDataWithDelay(data);
   } catch (e) {
     return reportErrorAndThrow(e);
   }
@@ -408,15 +437,121 @@ async function fetchMetric(searchUrl, name, type, options = {}) {
       addGroupByFilterToQueryParams(groupBy, params);
     }

-    const { data } = await axios.get(searchUrl, {
-      params,
-      withCredentials: true,
-    });
-
+    console.log(`[DEBUG] Fetching metric with params: ${params.toString()}`);
+
+    const data = {
+      start_ts: 1705243529862125800,
+      end_ts: 1705329929862125800,
+      results: [
+        {
+          name: 'app.ads.ad_requests',
+          description: 'Counts ad requests by request and response type',
+          unit: '',
+          type: 'Sum',
+          attributes: {
+            'app.ads.ad_request_type': 'NOT_TARGETED',
+            'app.ads.ad_response_type': 'RANDOM',
+          },
+          values: [
+            [1705247947518101200, 2252],
+            [1705253554585113900, 2252],
+            [1705258930136038700, 2252],
+            [1705266254790955000, 2252],
+            [1705274618400971300, 2252],
+            [1705274618400971300, 2252],
+            [1705281863109378800, 2252],
+            [1705288569114644700, 2252],
+            [1705288790896014800, 2252],
+            [1705289034855967500, 2252],
+            [1705289034855967500, 2252],
+            [1705289512918064400, 2252],
+            [1705290250183041800, 2252],
+            [1705290310185114400, 2252],
+            [1705290736882757600, 2252],
+            [1705290796883791400, 2252],
+            [1705290856883483600, 2252],
+            [1705290987064807700, 2252],
+            [1705291047065788400, 2252],
+            [1705291107064986400, 2252],
+            [1705291167065962000, 2252],
+            [1705291227066536200, 2252],
+          ],
+        },
+        {
+          name: 'app.ads.ad_requests',
+          description: 'Counts ad requests by request and response type',
+          unit: '',
+          type: 'Sum',
+          attributes: {
+            'app.ads.ad_request_type': 'TARGETED',
+            'app.ads.ad_response_type': 'RANDOM',
+          },
+          values: [
+            [1705247947518101200, 2278],
+            [1705253554585113900, 2278],
+            [1705258930136038700, 2278],
+            [1705266254790955000, 2278],
+            [1705274618400971300, 2278],
+            [1705274618400971300, 2278],
+            [1705281863109378800, 2278],
+            [1705288569114644700, 2278],
+            [1705288790896014800, 2278],
+            [1705289034855967500, 2278],
+            [1705289034855967500, 2278],
+            [1705289512918064400, 2278],
+            [1705290250183041800, 2278],
+            [1705290310185114400, 2278],
+            [1705290736882757600, 2278],
+            [1705290796883791400, 2278],
+            [1705290856883483600, 2278],
+            [1705290987064807700, 2278],
+            [1705291047065788400, 2278],
+            [1705291107064986400, 2278],
+            [1705291167065962000, 2278],
+            [1705291227066536200, 2278],
+            [1705291633437533400, 2278],
+            [1705291693438488300, 2278],
+            [1705291753436971000, 2278],
+            [1705291813440958500, 2278],
+          ],
+        },
+        {
+          name: 'app.ads.ad_requests',
+          description: 'Counts ad requests by request and response type',
+          unit: '',
+          type: 'Sum',
+          attributes: {
+            'app.ads.ad_request_type': 'TARGETED',
+            'app.ads.ad_response_type': 'TARGETED',
+          },
+          values: [
+            [1705247947518101200, 11436],
+            [1705253554585113900, 11436],
+            [1705258930136038700, 11436],
+            [1705266254790955000, 11436],
+            [1705274618400971300, 11436],
+            [1705274618400971300, 11436],
+            [1705281863109378800, 11436],
+            [1705288569114644700, 11436],
+            [1705288790896014800, 11436],
+            [1705289034855967500, 11436],
+            [1705289034855967500, 11436],
+            [1705289512918064400, 11436],
+            [1705290250183041800, 11436],
+            [1705290310185114400, 11436],
+            [1705290736882757600, 11436],
+            [1705290796883791400, 11436],
+            [1705290856883483600, 11436],
+            [1705290987064807700, 11436],
+            [1705291047065788400, 11436],
+          ],
+        },
+      ],
+    };
     if (!Array.isArray(data.results)) {
       throw new Error('metrics are missing/invalid in the response'); // eslint-disable-line @gitlab/require-i18n-strings
     }
-    return data.results;
+    return mockReturnDataWithDelay(data.results);
   } catch (e) {
     return reportErrorAndThrow(e);
   }
@@ -431,15 +566,17 @@ async function fetchMetricSearchMetadata(searchMetadataUrl, name, type) {
       throw new Error('fetchMetric() - metric type is required.');
     }

-    const params = new URLSearchParams({
-      mname: name,
-      mtype: type,
-    });
-    const { data } = await axios.get(searchMetadataUrl, {
-      params,
-      withCredentials: true,
+    return mockReturnDataWithDelay({
+      name: 'app.ads.ad_requests',
+      type: 'Sum',
+      description: 'Counts ad requests by request and response type',
+      attribute_keys: ['app.ads.ad_request_type', 'app.ads.ad_response_type'],
+      last_ingested_at: 1707489287944422100,
+      supported_aggregations: ['1m', '1h', '1d'],
+      supported_functions: ['avg', 'sum', 'min', 'max', 'count'],
+      default_group_by_attributes: ['*'],
+      default_group_by_function: 'sum',
     });
-    return data;
   } catch (e) {
     return reportErrorAndThrow(e);
   }
diff --git a/app/assets/javascripts/observability/components/observability_container.vue b/app/assets/javascripts/observability/components/observability_container.vue
index b89c2624f81c..f6cbf7ee771f 100644
--- a/app/assets/javascripts/observability/components/observability_container.vue
+++ b/app/assets/javascripts/observability/components/observability_container.vue
@@ -27,12 +27,12 @@ export default {

     // TODO: Improve local GDK dev experience with tracing https://gitlab.com/gitlab-org/opstrace/opstrace/-/issues/2308
     // Uncomment the lines below to to test this locally
-    // setTimeout(() => {
-    //   this.messageHandler({
-    //     data: { type: 'AUTH_COMPLETION', status: 'success' },
-    //     origin: new URL(this.oauthUrl).origin,
-    //   });
-    // }, 2000);
+    setTimeout(() => {
+      this.messageHandler({
+        data: { type: 'AUTH_COMPLETION', status: 'success' },
+        origin: new URL(this.apiConfig.oauthUrl).origin,
+      });
+    }, 2000);
   },
   destroyed() {
     window.removeEventListener('message', this.messageHandler);
Edited by Daniele Rossetti

Merge request reports