Browse Source

fix(spans): Factor any embedded transactions' trace bounds into the span view's trace bounds (#27977)

Alberto Leal 3 years ago
parent
commit
52239ce2ca

+ 4 - 10
static/app/components/events/interfaces/spans/header.tsx

@@ -53,6 +53,7 @@ type PropType = {
   trace: ParsedTraceType;
   trace: ParsedTraceType;
   event: EventTransaction;
   event: EventTransaction;
   operationNameFilters: ActiveOperationFilter;
   operationNameFilters: ActiveOperationFilter;
+  rootSpan: RawSpanType;
 };
 };
 
 
 type State = {
 type State = {
@@ -440,6 +441,7 @@ class TraceViewHeader extends React.Component<PropType, State> {
                 <ActualMinimap
                 <ActualMinimap
                   trace={this.props.trace}
                   trace={this.props.trace}
                   dividerPosition={dividerPosition}
                   dividerPosition={dividerPosition}
+                  rootSpan={this.props.rootSpan}
                 />
                 />
                 <CursorGuideHandler.Consumer>
                 <CursorGuideHandler.Consumer>
                   {({
                   {({
@@ -508,9 +510,10 @@ class TraceViewHeader extends React.Component<PropType, State> {
 class ActualMinimap extends React.PureComponent<{
 class ActualMinimap extends React.PureComponent<{
   trace: ParsedTraceType;
   trace: ParsedTraceType;
   dividerPosition: number;
   dividerPosition: number;
+  rootSpan: RawSpanType;
 }> {
 }> {
   renderRootSpan(): React.ReactNode {
   renderRootSpan(): React.ReactNode {
-    const {trace} = this.props;
+    const {trace, rootSpan} = this.props;
 
 
     const generateBounds = boundsGenerator({
     const generateBounds = boundsGenerator({
       traceStartTimestamp: trace.traceStartTimestamp,
       traceStartTimestamp: trace.traceStartTimestamp,
@@ -519,15 +522,6 @@ class ActualMinimap extends React.PureComponent<{
       viewEnd: 1,
       viewEnd: 1,
     });
     });
 
 
-    const rootSpan: RawSpanType = {
-      trace_id: trace.traceID,
-      span_id: trace.rootSpanID,
-      start_timestamp: trace.traceStartTimestamp,
-      timestamp: trace.traceEndTimestamp,
-      op: trace.op,
-      data: {},
-    };
-
     return this.renderSpan({
     return this.renderSpan({
       spanNumber: 0,
       spanNumber: 0,
       generateBounds,
       generateBounds,

+ 58 - 10
static/app/components/events/interfaces/spans/spanTreeModel.tsx

@@ -14,6 +14,7 @@ import {
   RawSpanType,
   RawSpanType,
   SpanChildrenLookupType,
   SpanChildrenLookupType,
   SpanType,
   SpanType,
+  TraceBound,
   TreeDepthType,
   TreeDepthType,
 } from './types';
 } from './types';
 import {
 import {
@@ -180,6 +181,8 @@ class SpanTreeModel {
     spanGrouping: EnhancedSpan[] | undefined;
     spanGrouping: EnhancedSpan[] | undefined;
     toggleSpanGroup: (() => void) | undefined;
     toggleSpanGroup: (() => void) | undefined;
     showSpanGroup: boolean;
     showSpanGroup: boolean;
+    addTraceBounds: (bounds: TraceBound) => void;
+    removeTraceBounds: (eventSlug: string) => void;
   }): EnhancedProcessedSpanType[] => {
   }): EnhancedProcessedSpanType[] => {
     const {
     const {
       operationNameFilters,
       operationNameFilters,
@@ -195,6 +198,8 @@ class SpanTreeModel {
       spanGrouping,
       spanGrouping,
       toggleSpanGroup,
       toggleSpanGroup,
       showSpanGroup,
       showSpanGroup,
+      addTraceBounds,
+      removeTraceBounds,
     } = props;
     } = props;
     let {treeDepth, continuingTreeDepths} = props;
     let {treeDepth, continuingTreeDepths} = props;
 
 
@@ -257,7 +262,10 @@ class SpanTreeModel {
       continuingTreeDepths,
       continuingTreeDepths,
       fetchEmbeddedChildrenState: this.fetchEmbeddedChildrenState,
       fetchEmbeddedChildrenState: this.fetchEmbeddedChildrenState,
       showEmbeddedChildren: this.showEmbeddedChildren,
       showEmbeddedChildren: this.showEmbeddedChildren,
-      toggleEmbeddedChildren: this.toggleEmbeddedChildren,
+      toggleEmbeddedChildren: this.toggleEmbeddedChildren({
+        addTraceBounds,
+        removeTraceBounds,
+      }),
       toggleSpanGroup:
       toggleSpanGroup:
         spanGroupingCriteria && toggleSpanGroup && !showSpanGroup
         spanGroupingCriteria && toggleSpanGroup && !showSpanGroup
           ? toggleSpanGroup
           ? toggleSpanGroup
@@ -323,6 +331,8 @@ class SpanTreeModel {
                 ? this.showSpanGroup
                 ? this.showSpanGroup
                 : showSpanGroup
                 : showSpanGroup
               : false,
               : false,
+            addTraceBounds,
+            removeTraceBounds,
           })
           })
         );
         );
 
 
@@ -428,18 +438,47 @@ class SpanTreeModel {
     return [wrappedSpan, ...descendants];
     return [wrappedSpan, ...descendants];
   };
   };
 
 
-  toggleEmbeddedChildren = (props: {orgSlug: string; eventSlug: string}) => {
-    this.showEmbeddedChildren = !this.showEmbeddedChildren;
-    this.fetchEmbeddedChildrenState = 'idle';
+  toggleEmbeddedChildren =
+    ({
+      addTraceBounds,
+      removeTraceBounds,
+    }: {
+      addTraceBounds: (bounds: TraceBound) => void;
+      removeTraceBounds: (eventSlug: string) => void;
+    }) =>
+    (props: {orgSlug: string; eventSlug: string}) => {
+      this.showEmbeddedChildren = !this.showEmbeddedChildren;
+      this.fetchEmbeddedChildrenState = 'idle';
+
+      if (!this.showEmbeddedChildren) {
+        if (this.embeddedChildren.length > 0) {
+          this.embeddedChildren.forEach(child => {
+            removeTraceBounds(child.generateTraceBounds().spanId);
+          });
+        }
+      }
 
 
-    if (this.showEmbeddedChildren && this.embeddedChildren.length === 0) {
-      return this.fetchEmbeddedTransactions(props);
-    }
+      if (this.showEmbeddedChildren) {
+        if (this.embeddedChildren.length === 0) {
+          return this.fetchEmbeddedTransactions({...props, addTraceBounds});
+        }
+        this.embeddedChildren.forEach(child => {
+          addTraceBounds(child.generateTraceBounds());
+        });
+      }
 
 
-    return Promise.resolve(undefined);
-  };
+      return Promise.resolve(undefined);
+    };
 
 
-  fetchEmbeddedTransactions({orgSlug, eventSlug}: {orgSlug: string; eventSlug: string}) {
+  fetchEmbeddedTransactions({
+    orgSlug,
+    eventSlug,
+    addTraceBounds,
+  }: {
+    orgSlug: string;
+    eventSlug: string;
+    addTraceBounds: (bounds: TraceBound) => void;
+  }) {
     const url = `/organizations/${orgSlug}/events/${eventSlug}/`;
     const url = `/organizations/${orgSlug}/events/${eventSlug}/`;
 
 
     this.fetchEmbeddedChildrenState = 'loading_embedded_transactions';
     this.fetchEmbeddedChildrenState = 'loading_embedded_transactions';
@@ -466,6 +505,7 @@ class SpanTreeModel {
 
 
           this.embeddedChildren = [parsedRootSpan];
           this.embeddedChildren = [parsedRootSpan];
           this.fetchEmbeddedChildrenState = 'idle';
           this.fetchEmbeddedChildrenState = 'idle';
+          addTraceBounds(parsedRootSpan.generateTraceBounds());
         })
         })
       )
       )
       .catch(
       .catch(
@@ -479,6 +519,14 @@ class SpanTreeModel {
   toggleSpanGroup = () => {
   toggleSpanGroup = () => {
     this.showSpanGroup = !this.showSpanGroup;
     this.showSpanGroup = !this.showSpanGroup;
   };
   };
+
+  generateTraceBounds = (): TraceBound => {
+    return {
+      spanId: this.span.span_id,
+      traceStartTimestamp: this.span.start_timestamp,
+      traceEndTimestamp: this.span.timestamp,
+    };
+  };
 }
 }
 
 
 export default SpanTreeModel;
 export default SpanTreeModel;

+ 1 - 0
static/app/components/events/interfaces/spans/traceView.tsx

@@ -38,6 +38,7 @@ class TraceView extends PureComponent<Props> {
             event={waterfallModel.event}
             event={waterfallModel.event}
             virtualScrollBarContainerRef={this.virtualScrollBarContainerRef}
             virtualScrollBarContainerRef={this.virtualScrollBarContainerRef}
             operationNameFilters={waterfallModel.operationNameFilters}
             operationNameFilters={waterfallModel.operationNameFilters}
+            rootSpan={waterfallModel.rootSpan.span}
           />
           />
         );
         );
       }}
       }}

+ 6 - 0
static/app/components/events/interfaces/spans/types.tsx

@@ -176,3 +176,9 @@ export type SpanFuseOptions = {
   distance: number;
   distance: number;
   maxPatternLength: number;
   maxPatternLength: number;
 };
 };
+
+export type TraceBound = {
+  spanId: string;
+  traceStartTimestamp: number;
+  traceEndTimestamp: number;
+};

+ 61 - 2
static/app/components/events/interfaces/spans/waterfallModel.tsx

@@ -15,6 +15,7 @@ import {
   ParsedTraceType,
   ParsedTraceType,
   RawSpanType,
   RawSpanType,
   SpanFuseOptions,
   SpanFuseOptions,
+  TraceBound,
 } from './types';
 } from './types';
 import {boundsGenerator, generateRootSpan, getSpanID, parseTrace} from './utils';
 import {boundsGenerator, generateRootSpan, getSpanID, parseTrace} from './utils';
 
 
@@ -32,6 +33,7 @@ class WaterfallModel {
   filterSpans: FilterSpans | undefined = undefined;
   filterSpans: FilterSpans | undefined = undefined;
   searchQuery: string | undefined = undefined;
   searchQuery: string | undefined = undefined;
   hiddenSpanGroups: Set<string>;
   hiddenSpanGroups: Set<string>;
+  traceBounds: Array<TraceBound>;
 
 
   constructor(event: Readonly<EventTransaction>) {
   constructor(event: Readonly<EventTransaction>) {
     this.event = event;
     this.event = event;
@@ -45,6 +47,10 @@ class WaterfallModel {
       true
       true
     );
     );
 
 
+    // Track the trace bounds of the current transaction and the trace bounds of
+    // any embedded transactions
+    this.traceBounds = [this.rootSpan.generateTraceBounds()];
+
     this.indexSearch(this.parsedTrace, rootSpan);
     this.indexSearch(this.parsedTrace, rootSpan);
 
 
     // Set of span IDs whose sub-trees should be hidden. This is used for the
     // Set of span IDs whose sub-trees should be hidden. This is used for the
@@ -52,6 +58,7 @@ class WaterfallModel {
     this.hiddenSpanGroups = new Set();
     this.hiddenSpanGroups = new Set();
 
 
     makeObservable(this, {
     makeObservable(this, {
+      parsedTrace: observable,
       rootSpan: observable,
       rootSpan: observable,
 
 
       // operation names filtering
       // operation names filtering
@@ -68,6 +75,11 @@ class WaterfallModel {
       // span group toggling
       // span group toggling
       hiddenSpanGroups: observable,
       hiddenSpanGroups: observable,
       toggleSpanGroup: action,
       toggleSpanGroup: action,
+
+      // trace bounds
+      traceBounds: observable,
+      addTraceBounds: action,
+      removeTraceBounds: action,
     });
     });
   }
   }
 
 
@@ -205,6 +217,52 @@ class WaterfallModel {
     this.hiddenSpanGroups.add(spanID);
     this.hiddenSpanGroups.add(spanID);
   };
   };
 
 
+  addTraceBounds = (traceBound: TraceBound) => {
+    this.traceBounds.push(traceBound);
+
+    this.parsedTrace = {
+      ...this.parsedTrace,
+      ...this.getTraceBounds(),
+    };
+  };
+
+  removeTraceBounds = (spanId: string) => {
+    this.traceBounds = this.traceBounds.filter(bound => bound.spanId !== spanId);
+
+    // traceBounds must always be non-empty
+    if (this.traceBounds.length === 0) {
+      this.traceBounds = [this.rootSpan.generateTraceBounds()];
+    }
+
+    this.parsedTrace = {
+      ...this.parsedTrace,
+      ...this.getTraceBounds(),
+    };
+  };
+
+  getTraceBounds = () => {
+    // traceBounds must always be non-empty
+    if (this.traceBounds.length === 0) {
+      this.traceBounds = [this.rootSpan.generateTraceBounds()];
+    }
+
+    return this.traceBounds.reduce(
+      (acc, bounds) => {
+        return {
+          traceStartTimestamp: Math.min(
+            acc.traceStartTimestamp,
+            bounds.traceStartTimestamp
+          ),
+          traceEndTimestamp: Math.max(acc.traceEndTimestamp, bounds.traceEndTimestamp),
+        };
+      },
+      {
+        traceStartTimestamp: this.traceBounds[0].traceStartTimestamp,
+        traceEndTimestamp: this.traceBounds[0].traceEndTimestamp,
+      }
+    );
+  };
+
   generateBounds = ({
   generateBounds = ({
     viewStart,
     viewStart,
     viewEnd,
     viewEnd,
@@ -213,8 +271,7 @@ class WaterfallModel {
     viewEnd: number; // in [0, 1]
     viewEnd: number; // in [0, 1]
   }) => {
   }) => {
     return boundsGenerator({
     return boundsGenerator({
-      traceStartTimestamp: this.parsedTrace.traceStartTimestamp,
-      traceEndTimestamp: this.parsedTrace.traceEndTimestamp,
+      ...this.getTraceBounds(),
       viewStart,
       viewStart,
       viewEnd,
       viewEnd,
     });
     });
@@ -247,6 +304,8 @@ class WaterfallModel {
       spanGrouping: undefined,
       spanGrouping: undefined,
       toggleSpanGroup: undefined,
       toggleSpanGroup: undefined,
       showSpanGroup: false,
       showSpanGroup: false,
+      addTraceBounds: this.addTraceBounds,
+      removeTraceBounds: this.removeTraceBounds,
     });
     });
   };
   };
 }
 }

+ 62 - 1
tests/js/spec/components/events/interfaces/spans/spanTreeModel.spec.tsx

@@ -331,11 +331,20 @@ describe('SpanTreeModel', () => {
       spanGrouping: undefined,
       spanGrouping: undefined,
       toggleSpanGroup: undefined,
       toggleSpanGroup: undefined,
       showSpanGroup: false,
       showSpanGroup: false,
+      addTraceBounds: () => {},
+      removeTraceBounds: () => {},
     });
     });
 
 
     expect(spans).toEqual(fullWaterfall);
     expect(spans).toEqual(fullWaterfall);
 
 
-    const promise = spanTreeModel.toggleEmbeddedChildren({
+    let mockAddTraceBounds = jest.fn();
+    let mockRemoveTraceBounds = jest.fn();
+
+    // embed a child transaction
+    let promise = spanTreeModel.toggleEmbeddedChildren({
+      addTraceBounds: mockAddTraceBounds,
+      removeTraceBounds: mockRemoveTraceBounds,
+    })({
       orgSlug: 'sentry',
       orgSlug: 'sentry',
       eventSlug: 'project:19c403a10af34db2b7d93ad669bb51ed',
       eventSlug: 'project:19c403a10af34db2b7d93ad669bb51ed',
     });
     });
@@ -345,6 +354,8 @@ describe('SpanTreeModel', () => {
 
 
     await promise;
     await promise;
 
 
+    expect(mockAddTraceBounds).toHaveBeenCalled();
+    expect(mockRemoveTraceBounds).not.toHaveBeenCalled();
     expect(spanTreeModel.fetchEmbeddedChildrenState).toBe('idle');
     expect(spanTreeModel.fetchEmbeddedChildrenState).toBe('idle');
 
 
     spans = spanTreeModel.getSpansList({
     spans = spanTreeModel.getSpansList({
@@ -364,6 +375,8 @@ describe('SpanTreeModel', () => {
       spanGrouping: undefined,
       spanGrouping: undefined,
       toggleSpanGroup: undefined,
       toggleSpanGroup: undefined,
       showSpanGroup: false,
       showSpanGroup: false,
+      addTraceBounds: () => {},
+      removeTraceBounds: () => {},
     });
     });
 
 
     const fullWaterfallExpected: EnhancedProcessedSpanType[] = [...fullWaterfall];
     const fullWaterfallExpected: EnhancedProcessedSpanType[] = [...fullWaterfall];
@@ -418,11 +431,56 @@ describe('SpanTreeModel', () => {
       }
       }
     );
     );
 
 
+    fullWaterfallExpected[0] = {
+      ...fullWaterfallExpected[0],
+    };
     assert(fullWaterfallExpected[0].type === 'span');
     assert(fullWaterfallExpected[0].type === 'span');
     fullWaterfallExpected[0].numOfSpanChildren += 1;
     fullWaterfallExpected[0].numOfSpanChildren += 1;
     fullWaterfallExpected[0].showEmbeddedChildren = true;
     fullWaterfallExpected[0].showEmbeddedChildren = true;
 
 
     expect(spans).toEqual(fullWaterfallExpected);
     expect(spans).toEqual(fullWaterfallExpected);
+
+    mockAddTraceBounds = jest.fn();
+    mockRemoveTraceBounds = jest.fn();
+
+    // un-embed a child transaction
+    promise = spanTreeModel.toggleEmbeddedChildren({
+      addTraceBounds: mockAddTraceBounds,
+      removeTraceBounds: mockRemoveTraceBounds,
+    })({
+      orgSlug: 'sentry',
+      eventSlug: 'project:19c403a10af34db2b7d93ad669bb51ed',
+    });
+    expect(spanTreeModel.fetchEmbeddedChildrenState).toBe('idle');
+
+    await promise;
+
+    expect(mockAddTraceBounds).not.toHaveBeenCalled();
+    expect(mockRemoveTraceBounds).toHaveBeenCalled();
+    expect(spanTreeModel.fetchEmbeddedChildrenState).toBe('idle');
+
+    spans = spanTreeModel.getSpansList({
+      operationNameFilters: {
+        type: 'no_filter',
+      },
+      generateBounds,
+      treeDepth: 0,
+      isLastSibling: true,
+      continuingTreeDepths: [],
+      hiddenSpanGroups: new Set(),
+      spanGroups: new Set(),
+      filterSpans: undefined,
+      previousSiblingEndTimestamp: undefined,
+      event,
+      isOnlySibling: true,
+      spanGrouping: undefined,
+      toggleSpanGroup: undefined,
+      showSpanGroup: false,
+      addTraceBounds: () => {},
+      removeTraceBounds: () => {},
+    });
+
+    expect(spans).toEqual(fullWaterfall);
   });
   });
 
 
   it('toggleEmbeddedChildren - error state', async () => {
   it('toggleEmbeddedChildren - error state', async () => {
@@ -432,6 +490,9 @@ describe('SpanTreeModel', () => {
     const spanTreeModel = new SpanTreeModel(rootSpan, parsedTrace.childSpans, api);
     const spanTreeModel = new SpanTreeModel(rootSpan, parsedTrace.childSpans, api);
 
 
     const promise = spanTreeModel.toggleEmbeddedChildren({
     const promise = spanTreeModel.toggleEmbeddedChildren({
+      addTraceBounds: () => {},
+      removeTraceBounds: () => {},
+    })({
       orgSlug: 'sentry',
       orgSlug: 'sentry',
       eventSlug: 'project:broken',
       eventSlug: 'project:broken',
     });
     });