Merge pull request #33 from zendesk/ej/fix-interrupting-spans-capture

Fix interrupting spans capture

Author: yojenkins

src/v3/Trace.ts

@@ -265,14 +265,14 @@ export class TraceStateMachine<
    * if we have long tasks before FMP, we want to use them as a potential grouping post FMP.
    */
   debouncingSpanBuffer: SpanAndAnnotation<RelationSchemasT>[] = []
-  #provisionalBuffer: SpanAndAnnotation<RelationSchemasT>[] = []
+  #draftBuffer: SpanAndAnnotation<RelationSchemasT>[] = []
 
   // eslint-disable-next-line consistent-return
-  #processProvisionalBuffer(): Transition<RelationSchemasT> | void {
+  #processDraftBuffer(): Transition<RelationSchemasT> | void {
     // process items in the buffer (stick the relatedTo in the entries) (if its empty, well we can skip this!)
     let span: SpanAndAnnotation<RelationSchemasT> | undefined
     // eslint-disable-next-line no-cond-assign
-    while ((span = this.#provisionalBuffer.shift())) {
+    while ((span = this.#draftBuffer.shift())) {
       const transition = this.emit('onProcessSpan', span)
       if (transition) return transition
     }
@@ -316,6 +316,7 @@ export class TraceStateMachine<
           for (const doesSpanMatch of this.#context.definition
             .interruptOnSpans) {
             if (doesSpanMatch(spanAndAnnotation, this.#context)) {
+              // TODO: do we want to record this span? Nothing else is being recorded at this point and is instead going into the buffer
               return {
                 transitionToState: 'complete',
                 interruptionReason: doesSpanMatch.requiredSpan
@@ -329,7 +330,7 @@ export class TraceStateMachine<
         }
 
         // else, add into span buffer
-        this.#provisionalBuffer.push(spanAndAnnotation)
+        this.#draftBuffer.push(spanAndAnnotation)
         return undefined
       },
 
@@ -352,7 +353,7 @@ export class TraceStateMachine<
 
     active: {
       onEnterState: (_transition: OnEnterActive) => {
-        const nextTransition = this.#processProvisionalBuffer()
+        const nextTransition = this.#processDraftBuffer()
         if (nextTransition) return nextTransition
 
         return undefined
@@ -381,6 +382,8 @@ export class TraceStateMachine<
           for (const doesSpanMatch of this.#context.definition
             .interruptOnSpans) {
             if (doesSpanMatch(spanAndAnnotation, this.#context)) {
+              // still record the span that interrupted the trace
+              this.sideEffectFns.addSpanToRecording(spanAndAnnotation)
               return {
                 transitionToState: 'interrupted',
                 interruptionReason: doesSpanMatch.requiredSpan
@@ -509,11 +512,13 @@ export class TraceStateMachine<
           }
         }
 
+        // The debouncing buffer will be used to correctly group the spans into clusters when calculating the cpu idle in the waiting-for-interactive state
+        // We record the spans here as well, so that they are included even if we never make it out of the debouncing state
         this.debouncingSpanBuffer.push(spanAndAnnotation)
+        this.sideEffectFns.addSpanToRecording(spanAndAnnotation)
 
         if (spanEndTimeEpoch > this.#debounceDeadline) {
           // done debouncing
-          this.sideEffectFns.addSpanToRecording(spanAndAnnotation)
           return { transitionToState: 'waiting-for-interactive' }
         }
 
@@ -544,8 +549,6 @@ export class TraceStateMachine<
           }
         }
 
-        this.sideEffectFns.addSpanToRecording(spanAndAnnotation)
-
         // does span satisfy any of the "debouncedOn" and if so, restart our debounce timer
         if (this.#context.definition.debounceOnSpans) {
           for (const doesSpanMatch of this.#context.definition
@@ -807,14 +810,14 @@ export class TraceStateMachine<
     // terminal states:
     interrupted: {
       onEnterState: (transition: OnEnterInterrupted) => {
-        // depending on the reason, if we're coming from draft, we want to flush the provisional buffer:
+        // depending on the reason, if we're coming from draft, we want to flush the buffer:
         if (
           transition.transitionFromState === 'draft' &&
           !isInvalidTraceInterruptionReason(transition.interruptionReason)
         ) {
           let span: SpanAndAnnotation<RelationSchemasT> | undefined
           // eslint-disable-next-line no-cond-assign
-          while ((span = this.#provisionalBuffer.shift())) {
+          while ((span = this.#draftBuffer.shift())) {
             this.sideEffectFns.addSpanToRecording(span)
           }
         }

src/v3/TraceManager.test.ts

@@ -472,9 +472,9 @@ describe('TraceManager', () => {
           (spanAndAnnotation) => spanAndAnnotation.span.performanceEntry,
         ),
       ).toMatchInlineSnapshot(`
-        events    | start
-        timeline  | |
-        time (ms) | 0
+        events    | start        interrupt
+        timeline  | |-<⋯ +100 ⋯>-|
+        time (ms) | 0            100
       `)
       expect(report.name).toBe('ticket.interrupt-on-basic-operation')
       expect(report.duration).toBeNull()
@@ -554,8 +554,8 @@ describe('TraceManager', () => {
         name: 'ticket.interrupt-on-basic-operation',
         type: 'operation',
         relationSchemaName: 'ticket',
-        requiredSpans: [{ name: 'end', isIdle: true }],
-        debounceOnSpans: [{ name: 'end' }],
+        requiredSpans: [{ name: 'component', isIdle: true }],
+        debounceOnSpans: [{ name: 'component' }],
         variants: {
           cold_boot: { timeout: DEFAULT_COLDBOOT_TIMEOUT_DURATION },
         },
@@ -569,7 +569,7 @@ describe('TraceManager', () => {
 
       // prettier-ignore
       const { spans } = getSpansFromTimeline<TicketIdRelationSchemasFixture>`
-      Events: ${Render('start', 0)}-----${Render('end', 50, {isIdle: true})}-----${Render('end', 50, {isIdle: false})}
+      Events: ${Render('start', 0)}-----${Render('component', 50, {isIdle: true})}-----${Render('component', 50, {isIdle: false})}
       Time:   ${0}                      ${100}                                   ${200}
       `
 
@@ -585,9 +585,9 @@ describe('TraceManager', () => {
           (spanAndAnnotation) => spanAndAnnotation.span.performanceEntry,
         ),
       ).toMatchInlineSnapshot(`
-        events    | start        end(50)
-        timeline  | |-<⋯ +100 ⋯>-[++++++++++++++++++++++++++++++++++++++++++++++++]
-        time (ms) | 0            100
+        events    | start        component(50)                  component(50)
+        timeline  | |-<⋯ +100 ⋯>-[++++++++++++++]---------------[++++++++++++++]-
+        time (ms) | 0            100                            200
       `)
       expect(report.name).toBe('ticket.interrupt-on-basic-operation')
       expect(report.interruptionReason).toBe('idle-component-no-longer-idle')