Fix metrics attributes filtering (!148092) · Merge requests · GitLab.org / GitLab

Daniele Rossetti requested to merge drosse/fix-metrics-filtering into master Mar 27, 2024

What does this MR do and why?

Fixes issues reported after testing metrics filtering gitlab-org/opstrace/opstrace#2559 (comment 1833923775)

Metrics chart is not updating with new data. refreshing the page shows the correct chart, meaning the chart is just not updating data correctly
- Fixes:
  - Upon investigation, I've realised that gl-chart ( and underlying echart ) merges new data with old data, rather than replacing it. There is an API to change this behaviour by forcing it to replace new data (https://echarts.apache.org/en/api.html#echartsInstance.setOption), but gl-chart does not make use of that. Changing it for all gl-chart seems risky as it might break other charts.
    
    Instead I've found a workaround which sets the data to empty object first, so that it kind of stop from merging it. Alternatively, I guess we could provide a prop in gl-chart to configure its mergingBehaviour.
    
    As I need a quick fix for now and the feature is still in Experiment anyway, we can start with the workaround and then make gl-chart configurable in a follow up.
Currently, in order for filters so be processed, the user needs to select the search 🔍 button first, and the "Search" button. We need to remove the 🔍 and make it so that the user doesn't need too many interactions to submit a query
- Fixes:
  - the showSearchButton was not forwarded to the gitlab-ui component, so it's still showing.
  - We also need to listen for input changes, not just submit, so that the user doesn't need to explicity submit the search bar

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
Screen_Recording_2024-03-28_at_10.23.46	Screen_Recording_2024-03-28_at_10.22.23

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 )

diff --git a/app/assets/javascripts/observability/client.js b/app/assets/javascripts/observability/client.js
index 9510efb5953b..bf0631a3eab2 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), 1000);
+  });
+}
+
 // 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) {
@@ -327,14 +358,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);
   }
@@ -387,6 +416,7 @@ function addGroupByFilterToQueryParams(groupByFilter, params) {
   }
 }

+let called = false;
 async function fetchMetric(searchUrl, name, type, options = {}) {
   try {
     if (!name) {
@@ -415,16 +445,143 @@ async function fetchMetric(searchUrl, name, type, options = {}) {
       addGroupByFilterToQueryParams(groupBy, params);
     }

-    const { data } = await axios.get(searchUrl, {
-      params,
-      signal: options.abortController?.signal,
-      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;
+    if (called) {
+      called = false;
+      return mockReturnDataWithDelay([
+        {
+          name: 'app.ads.ad_requests',
+          description: 'Counts ad requests by request and response type',
+          unit: '',
+          type: 'Sum',
+          attributes: {
+            foo: 'bar',
+          },
+          values: [
+            [1705247947518101200, 1],
+            [1705253554585113900, 2],
+            [1705258930136038700, 3],
+            [1705266254790955000, 4],
+            [1705274618400971300, 5],
+          ],
+        },
+      ]);
+    }
+    called = true;
+    return mockReturnDataWithDelay(data.results);
   } catch (e) {
     return reportErrorAndThrow(e);
   }
@@ -443,11 +600,20 @@ async function fetchMetricSearchMetadata(searchMetadataUrl, name, type) {
       mname: name,
       mtype: type,
     });
-    const { data } = await axios.get(searchMetadataUrl, {
-      params,
-      withCredentials: true,
+
+    console.log(`[DEBUG] Fetching metrics search metadata with params: ${params.toString()}`);
+
+    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);

Go to https://local.gitlab.com:3443/flightjs/Flight/-/metrics/app.ads.ad_requests?type=Sum

Edited Mar 28, 2024 by Daniele Rossetti

Fix metrics attributes filtering

What does this MR do and why?

MR acceptance checklist

Screenshots or screen recordings

How to set up and validate locally

Merge request reports