Browse Source

fix(trace) match event_id by error (#81370)

When linking by eventId, the id can belong to an error that is
associated to any node. This PR changes the find functionality so that
it also considers events and performance issues associated with any node
in the tree.
Jonas 3 months ago
parent
commit
4d946ce567

+ 33 - 0
static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx

@@ -15,6 +15,7 @@ import {
 import type {ParentAutogroupNode} from './parentAutogroupNode';
 import {TraceTree} from './traceTree';
 import {
+  assertTransactionNode,
   makeEventTransaction,
   makeSpan,
   makeTrace,
@@ -993,6 +994,38 @@ describe('TraceTree', () => {
     });
   });
 
+  describe('FindByID', () => {
+    it('finds transaction by event_id', () => {
+      const traceWithError = makeTrace({
+        transactions: [
+          makeTransaction({transaction: 'first', event_id: 'first-event-id'}),
+        ],
+      });
+      const tree = TraceTree.FromTrace(traceWithError, traceMetadata);
+      const node = TraceTree.FindByID(tree.root, 'first-event-id');
+
+      assertTransactionNode(node);
+      expect(node.value.transaction).toBe('first');
+    });
+
+    it('matches by error event_id', () => {
+      const traceWithError = makeTrace({
+        transactions: [
+          makeTransaction({
+            transaction: 'first',
+            event_id: 'txn-event-id',
+            errors: [makeTraceError({event_id: 'error-event-id'})],
+          }),
+        ],
+      });
+      const tree = TraceTree.FromTrace(traceWithError, traceMetadata);
+      const node = TraceTree.FindByID(tree.root, 'error-event-id');
+
+      assertTransactionNode(node);
+      expect(node.value.transaction).toBe('first');
+    });
+  });
+
   describe('FindAll', () => {
     it('finds all nodes by predicate', () => {
       const tree = TraceTree.FromTrace(trace, traceMetadata);

+ 30 - 14
static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx

@@ -1222,10 +1222,38 @@ export class TraceTree extends TraceTreeEventDispatcher {
       if (isTransactionNode(n)) {
         // A transaction itself is a span and we are starting to treat it as such.
         // Hence we check for both event_id and span_id.
-        return n.value.event_id === eventId || n.value.span_id === eventId;
+        if (n.value.event_id === eventId || n.value.span_id === eventId) {
+          return true;
+        }
+
+        // If we dont have an exact match, then look for an event_id in the errors or performance issues
+        for (const e of n.errors) {
+          if (e.event_id === eventId) {
+            return true;
+          }
+        }
+        for (const p of n.performance_issues) {
+          if (p.event_id === eventId) {
+            return true;
+          }
+        }
       }
       if (isSpanNode(n)) {
-        return n.value.span_id === eventId;
+        if (n.value.span_id === eventId) {
+          return true;
+        }
+
+        // If we dont have an exact match, then look for an event_id in the errors or performance issues
+        for (const e of n.errors) {
+          if (e.event_id === eventId) {
+            return true;
+          }
+        }
+        for (const p of n.performance_issues) {
+          if (p.event_id === eventId) {
+            return true;
+          }
+        }
       }
       if (isTraceErrorNode(n)) {
         return n.value.event_id === eventId;
@@ -1255,18 +1283,6 @@ export class TraceTree extends TraceTreeEventDispatcher {
         return true;
       }
 
-      // If we dont have an exact match, then look for an event_id in the errors or performance issues
-      for (const e of n.errors) {
-        if (e.event_id === eventId) {
-          return true;
-        }
-      }
-      for (const p of n.performance_issues) {
-        if (p.event_id === eventId) {
-          return true;
-        }
-      }
-
       return false;
     });
   }