Merge branch 'rossetd/provisioning-api' into 'master'

tracing: Replace Provisioning mocks with actual API calls

Closes gitlab-org/opstrace/opstrace#2271

See merge request !129383



Merged-by: Doug Stull <dstull@gitlab.com>
Approved-by: Ross Byrne <robyrne@gitlab.com>
Approved-by: Miguel Rincon <mrincon@gitlab.com>
Approved-by: Doug Stull <dstull@gitlab.com>
Approved-by: Martin Čavoj <mcavoj@gitlab.com>
Reviewed-by: Miguel Rincon <mrincon@gitlab.com>
Reviewed-by: Doug Stull <dstull@gitlab.com>
Reviewed-by: Daniele Rossetti <drossetti@gitlab.com>
Reviewed-by: Martin Čavoj <mcavoj@gitlab.com>
Co-authored-by: Daniele Rossetti <drossetti@gitlab.com>

app/assets/javascripts/observability/client.js

+import * as Sentry from '@sentry/browser';
 import axios from '~/lib/utils/axios_utils';
-// import mockData from './mock_traces.json';
-
-function enableTraces() {
-  // TODO remove mocks https://gitlab.com/gitlab-org/opstrace/opstrace/-/issues/2271
-  return new Promise((resolve) => {
-    setTimeout(() => {
-      resolve();
-    }, 1000);
-  });
-}
 
-function isTracingEnabled() {
-  // TODO remove mocks https://gitlab.com/gitlab-org/opstrace/opstrace/-/issues/2271
-  return new Promise((resolve) => {
-    setTimeout(() => {
-      // Currently relying on manual provisioning, hence assuming tracing is enabled
-      resolve(true);
-    }, 1000);
-  });
+function reportErrorAndThrow(e) {
+  Sentry.captureException(e);
+  throw e;
+}
+// Provisioning API spec: https://gitlab.com/gitlab-org/opstrace/opstrace/-/blob/main/provisioning-api/pkg/provisioningapi/routes.go#L59
+async function enableTraces(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,
+    });
+  } catch (e) {
+    return reportErrorAndThrow(e);
+  }
 }
 
-async function fetchTrace(tracingUrl, traceId) {
-  if (!traceId) {
-    throw new Error('traceId is required.');
+// Provisioning API spec: https://gitlab.com/gitlab-org/opstrace/opstrace/-/blob/main/provisioning-api/pkg/provisioningapi/routes.go#L37
+async function isTracingEnabled(provisioningUrl) {
+  try {
+    const { data } = await axios.get(provisioningUrl, { withCredentials: true });
+    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;
+    }
+  } catch (e) {
+    if (e.response.status === 404) {
+      return false;
+    }
+    return reportErrorAndThrow(e);
   }
+  return reportErrorAndThrow(new Error('Failed to check provisioning')); // eslint-disable-line @gitlab/require-i18n-strings
+}
 
-  const { data } = await axios.get(tracingUrl, {
-    withCredentials: true,
-    params: {
-      trace_id: traceId,
-    },
-  });
+async function fetchTrace(tracingUrl, traceId) {
+  try {
+    if (!traceId) {
+      throw new Error('traceId is required.');
+    }
+
+    const { data } = await axios.get(tracingUrl, {
+      withCredentials: true,
+      params: {
+        trace_id: traceId,
+      },
+    });
 
-  // TODO: Improve local GDK dev experience with tracing https://gitlab.com/gitlab-org/opstrace/opstrace/-/issues/2308
-  // const data = { traces: [mockData.traces.find((t) => t.trace_id === traceId)] };
+    if (!Array.isArray(data.traces) || data.traces.length === 0) {
+      throw new Error('traces are missing/invalid in the response'); // eslint-disable-line @gitlab/require-i18n-strings
+    }
 
-  if (!Array.isArray(data.traces) || data.traces.length === 0) {
-    throw new Error('traces are missing/invalid in the response.'); // eslint-disable-line @gitlab/require-i18n-strings
+    return data.traces[0];
+  } catch (e) {
+    return reportErrorAndThrow(e);
   }
-
-  return data.traces[0];
 }
 
 /**
@@ -152,18 +169,18 @@ function filterObjToQueryParams(filterObj) {
 async function fetchTraces(tracingUrl, filters = {}) {
   const filterParams = filterObjToQueryParams(filters);
 
-  const { data } = await axios.get(tracingUrl, {
-    withCredentials: true,
-    params: filterParams,
-  });
-  // TODO: Improve local GDK dev experience with tracing https://gitlab.com/gitlab-org/opstrace/opstrace/-/issues/2308
-  // Uncomment the line below to test this locally
-  // const data = mockData;
-
-  if (!Array.isArray(data.traces)) {
-    throw new Error('traces are missing/invalid in the response.'); // eslint-disable-line @gitlab/require-i18n-strings
+  try {
+    const { data } = await axios.get(tracingUrl, {
+      withCredentials: true,
+      params: filterParams,
+    });
+    if (!Array.isArray(data.traces)) {
+      throw new Error('traces are missing/invalid in the response'); // eslint-disable-line @gitlab/require-i18n-strings
+    }
+    return data.traces;
+  } catch (e) {
+    return reportErrorAndThrow(e);
   }
-  return data.traces;
 }
 
 export function buildClient({ provisioningUrl, tracingUrl }) {

lib/gitlab/observability.rb

@@ -31,9 +31,8 @@ def tracing_url(project)
       "#{Gitlab::Observability.observability_url}/query/#{project.group.id}/#{project.id}/v1/traces"
     end
 
-    def provisioning_url(_project)
-      # TODO Change to correct endpoint when API is ready
-      Gitlab::Observability.observability_url.to_s
+    def provisioning_url(project)
+      "#{Gitlab::Observability.observability_url}/v3/tenant/#{project.id}"
     end
 
     # Returns true if the GitLab Observability UI (GOUI) feature flag is enabled

spec/frontend/observability/client_spec.js

 import MockAdapter from 'axios-mock-adapter';
+import * as Sentry from '@sentry/browser';
 import { buildClient } from '~/observability/client';
 import axios from '~/lib/utils/axios_utils';
 
 jest.mock('~/lib/utils/axios_utils');
+jest.mock('@sentry/browser');
 
 describe('buildClient', () => {
   let client;
   let axiosMock;
 
   const tracingUrl = 'https://example.com/tracing';
-  const EXPECTED_ERROR_MESSAGE = 'traces are missing/invalid in the response';
+  const provisioningUrl = 'https://example.com/provisioning';
+
+  const FETCHING_TRACES_ERROR = 'traces are missing/invalid in the response';
 
   beforeEach(() => {
     axiosMock = new MockAdapter(axios);
@@ -17,7 +21,7 @@ describe('buildClient', () => {
 
     client = buildClient({
       tracingUrl,
-      provisioningUrl: 'https://example.com/provisioning',
+      provisioningUrl,
     });
   });
 
@@ -25,6 +29,77 @@ describe('buildClient', () => {
     axiosMock.restore();
   });
 
+  describe('isTracingEnabled', () => {
+    it('returns true if requests succeedes', async () => {
+      axiosMock.onGet(provisioningUrl).reply(200, {
+        status: 'ready',
+      });
+
+      const enabled = await client.isTracingEnabled();
+
+      expect(enabled).toBe(true);
+    });
+
+    it('returns false if response is 404', async () => {
+      axiosMock.onGet(provisioningUrl).reply(404);
+
+      const enabled = await client.isTracingEnabled();
+
+      expect(enabled).toBe(false);
+    });
+
+    // 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
+    it('returns true for any status', async () => {
+      axiosMock.onGet(provisioningUrl).reply(200, {
+        status: 'not ready',
+      });
+
+      const enabled = await client.isTracingEnabled();
+
+      expect(enabled).toBe(true);
+    });
+
+    it('throws in case of any non-404 error', async () => {
+      axiosMock.onGet(provisioningUrl).reply(500);
+
+      const e = 'Request failed with status code 500';
+      await expect(client.isTracingEnabled()).rejects.toThrow(e);
+      expect(Sentry.captureException).toHaveBeenCalledWith(new Error(e));
+    });
+
+    it('throws in case of unexpected response', async () => {
+      axiosMock.onGet(provisioningUrl).reply(200, {});
+
+      const e = 'Failed to check provisioning';
+      await expect(client.isTracingEnabled()).rejects.toThrow(e);
+      expect(Sentry.captureException).toHaveBeenCalledWith(new Error(e));
+    });
+  });
+
+  describe('enableTraces', () => {
+    it('makes a PUT request to the provisioning URL', async () => {
+      let putConfig;
+      axiosMock.onPut(provisioningUrl).reply((config) => {
+        putConfig = config;
+        return [200];
+      });
+
+      await client.enableTraces();
+
+      expect(putConfig.withCredentials).toBe(true);
+    });
+
+    it('reports an error if the req fails', async () => {
+      axiosMock.onPut(provisioningUrl).reply(401);
+
+      const e = 'Request failed with status code 401';
+
+      await expect(client.enableTraces()).rejects.toThrow(e);
+      expect(Sentry.captureException).toHaveBeenCalledWith(new Error(e));
+    });
+  });
+
   describe('fetchTrace', () => {
     it('fetches the trace from the tracing URL', async () => {
       const mockTraces = [
@@ -53,16 +128,18 @@ describe('buildClient', () => {
       return expect(client.fetchTrace()).rejects.toThrow('traceId is required.');
     });
 
-    it('rejects if traces are empty', () => {
+    it('rejects if traces are empty', async () => {
       axiosMock.onGet(tracingUrl).reply(200, { traces: [] });
 
-      return expect(client.fetchTrace('trace-1')).rejects.toThrow(EXPECTED_ERROR_MESSAGE);
+      await expect(client.fetchTrace('trace-1')).rejects.toThrow(FETCHING_TRACES_ERROR);
+      expect(Sentry.captureException).toHaveBeenCalledWith(new Error(FETCHING_TRACES_ERROR));
     });
 
-    it('rejects if traces are invalid', () => {
+    it('rejects if traces are invalid', async () => {
       axiosMock.onGet(tracingUrl).reply(200, { traces: 'invalid' });
 
-      return expect(client.fetchTraces()).rejects.toThrow(EXPECTED_ERROR_MESSAGE);
+      await expect(client.fetchTraces()).rejects.toThrow(FETCHING_TRACES_ERROR);
+      expect(Sentry.captureException).toHaveBeenCalledWith(new Error(FETCHING_TRACES_ERROR));
     });
   });
 
@@ -91,16 +168,18 @@ describe('buildClient', () => {
       expect(result).toEqual(mockTraces);
     });
 
-    it('rejects if traces are missing', () => {
+    it('rejects if traces are missing', async () => {
       axiosMock.onGet(tracingUrl).reply(200, {});
 
-      return expect(client.fetchTraces()).rejects.toThrow(EXPECTED_ERROR_MESSAGE);
+      await expect(client.fetchTraces()).rejects.toThrow(FETCHING_TRACES_ERROR);
+      expect(Sentry.captureException).toHaveBeenCalledWith(new Error(FETCHING_TRACES_ERROR));
     });
 
-    it('rejects if traces are invalid', () => {
+    it('rejects if traces are invalid', async () => {
       axiosMock.onGet(tracingUrl).reply(200, { traces: 'invalid' });
 
-      return expect(client.fetchTraces()).rejects.toThrow(EXPECTED_ERROR_MESSAGE);
+      await expect(client.fetchTraces()).rejects.toThrow(FETCHING_TRACES_ERROR);
+      expect(Sentry.captureException).toHaveBeenCalledWith(new Error(FETCHING_TRACES_ERROR));
     });
 
     describe('query filter', () => {

spec/lib/gitlab/observability_spec.rb

@@ -49,7 +49,7 @@
   describe '.provisioning_url' do
     subject { described_class.provisioning_url(project) }
 
-    it { is_expected.to eq(described_class.observability_url.to_s) }
+    it { is_expected.to eq("#{described_class.observability_url}/v3/tenant/#{project.id}") }
   end
 
   describe '.build_full_url' do