Skip to content

Tracing - Fix spans collapsing + Improve spans tree building logic

Daniele Rossetti requested to merge rossetd/tracing-more-fixes into master

What does this MR do and why?

  • Improve spans tree building logic by handling unsorted list: Currently the tree building logic expects the root node to be in the first position, and all parent nodes to appear before their children. I've improved the logic by handling unsorted lists as well. No user-facing change here. (Closes gitlab-org/opstrace/opstrace#2392 (closed))

  • Fix expanded/collapsed state in children spans: Because children spans are currently added/removed (v-if) from the DOM upon expanding/collapsing, we don't preserve their expanded/collapsed state. I've changed it so that the children spans are made hidden/visible instead (v-show) so that their state is preserved I'm aware this might be less performant, but we don't expect tree with such a big number of children that it could have a big impact. Nevertheless will keep an eye out and consider improving this if the necessity arises (Closes gitlab-org/opstrace/opstrace#2399 (closed))

  • Update time range filter options. (Closes gitlab-org/opstrace/opstrace#2402 (closed))

Screenshots or screen recordings

  • Collapsed/Expanded state fix
Before (collapsed/expanded state not maintained) collapsing-before
After (collapsed/expanded state maintained) collapsing-after
  • New time filters

image

How to set up and validate locally

  • Enable FF :observability_tracing
Apply this patch to load mocked data
diff --git a/app/assets/javascripts/observability/client.js b/app/assets/javascripts/observability/client.js
index 718001e98fe5..d8c8f8a78d11 100644
--- a/app/assets/javascripts/observability/client.js
+++ b/app/assets/javascripts/observability/client.js
@@ -1,5 +1,6 @@
 import * as Sentry from '@sentry/browser';
 import axios from '~/lib/utils/axios_utils';
+import mockData from './mock_traces.json';
 
 function reportErrorAndThrow(e) {
   Sentry.captureException(e);
@@ -9,10 +10,11 @@ function reportErrorAndThrow(e) {
 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,
-    });
+    // return await axios(provisioningUrl, {
+    //   method: 'put',
+    //   withCredentials: true,
+    // });
+    return true;
   } catch (e) {
     return reportErrorAndThrow(e);
   }
@@ -21,7 +23,9 @@ async function enableTraces(provisioningUrl) {
 // 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 });
+    // const { data } = await axios.get(provisioningUrl, { withCredentials: true });
+    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
@@ -42,12 +46,13 @@ async function fetchTrace(tracingUrl, traceId) {
       throw new Error('traceId is required.');
     }
 
-    const { data } = await axios.get(tracingUrl, {
-      withCredentials: true,
-      params: {
-        trace_id: traceId,
-      },
-    });
+    // const { data } = await axios.get(tracingUrl, {
+    //   withCredentials: true,
+    //   params: {
+    //     trace_id: traceId,
+    //   },
+    // });
+    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
@@ -170,10 +175,11 @@ async function fetchTraces(tracingUrl, filters = {}) {
   const filterParams = filterObjToQueryParams(filters);
 
   try {
-    const { data } = await axios.get(tracingUrl, {
-      withCredentials: true,
-      params: filterParams,
-    });
+    // const { data } = await axios.get(tracingUrl, {
+    //   withCredentials: true,
+    //   params: filterParams,
+    // });
+    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
     }
diff --git a/app/assets/javascripts/observability/components/observability_container.vue b/app/assets/javascripts/observability/components/observability_container.vue
index b7697cea2996..26f6998d5969 100644
--- a/app/assets/javascripts/observability/components/observability_container.vue
+++ b/app/assets/javascripts/observability/components/observability_container.vue
@@ -33,12 +33,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.oauthUrl).origin,
+      });
+    }, 2000);
   },
   destroyed() {
     window.removeEventListener('message', this.messageHandler);

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Daniele Rossetti

Merge request reports