Skip to content

Commit 61e9c7c

Browse files
niieanibbrzoska
authored andcommitted
fix: correctly interrupt from draft trace
1 parent 5bfcd36 commit 61e9c7c

5 files changed

+55
-39
lines changed

src/v3/Trace.ts

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,14 @@ interface OnEnterComplete<RelationSchemasT>
9494
transitionToState: 'complete'
9595
}
9696

97+
export type FinalTransition<RelationSchemasT> = (
98+
| (OnEnterInterrupted &
99+
Omit<FinalState<RelationSchemasT>, 'transitionFromState'>)
100+
| OnEnterComplete<RelationSchemasT>
101+
) & {
102+
lastRelevantSpanAndAnnotation: SpanAndAnnotation<RelationSchemasT> | undefined
103+
}
104+
97105
interface OnEnterWaitingForInteractive {
98106
transitionToState: 'waiting-for-interactive'
99107
transitionFromState: NonTerminalTraceStates
@@ -325,27 +333,25 @@ export class TraceStateMachine<
325333
}
326334
}
327335

336+
// add into span buffer
337+
this.#draftBuffer.push(spanAndAnnotation)
338+
328339
// if the entry matches any of the interruptOnSpans criteria,
329-
// transition to complete state with the 'matched-on-interrupt' interruptionReason
340+
// transition to interrupted state with the correct interruptionReason
330341
if (this.#context.definition.interruptOnSpans) {
331342
for (const doesSpanMatch of this.#context.definition
332343
.interruptOnSpans) {
333344
if (doesSpanMatch(spanAndAnnotation, this.#context)) {
334-
// TODO: do we want to record this span? Nothing else is being recorded at this point and is instead going into the buffer
335345
return {
336-
transitionToState: 'complete',
346+
transitionToState: 'interrupted',
337347
interruptionReason: doesSpanMatch.requiredSpan
338348
? 'matched-on-required-span-with-error'
339349
: 'matched-on-interrupt',
340-
lastRequiredSpanAndAnnotation: this.lastRequiredSpan,
341-
completeSpanAndAnnotation: this.completeSpan,
342350
}
343351
}
344352
}
345353
}
346354

347-
// else, add into span buffer
348-
this.#draftBuffer.push(spanAndAnnotation)
349355
return undefined
350356
},
351357

@@ -1241,30 +1247,14 @@ export class Trace<
12411247
transition.transitionToState === 'interrupted' ||
12421248
transition.transitionToState === 'complete'
12431249
) {
1244-
const endOfOperationSpan =
1245-
(transition.transitionToState === 'complete' &&
1246-
(transition.cpuIdleSpanAndAnnotation ??
1247-
transition.completeSpanAndAnnotation)) ||
1248-
lastRelevantSpanAndAnnotation
1249-
12501250
const traceRecording = createTraceRecording(
12511251
{
12521252
definition: this.definition,
1253-
// only keep items captured until the endOfOperationSpan
1254-
recordedItems: endOfOperationSpan
1255-
? new Set(
1256-
[...this.recordedItems].filter(
1257-
(item) =>
1258-
item.span.startTime.now + item.span.duration <=
1259-
endOfOperationSpan.span.startTime.now +
1260-
endOfOperationSpan.span.duration,
1261-
),
1262-
)
1263-
: this.recordedItems,
1253+
recordedItems: this.recordedItems,
12641254
input: this.input,
12651255
recordedItemsByLabel: this.recordedItemsByLabel,
12661256
},
1267-
transition,
1257+
{ ...transition, lastRelevantSpanAndAnnotation },
12681258
)
12691259
this.onEnd(traceRecording)
12701260

src/v3/TraceManagerWithDraft.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ describe('TraceManager', () => {
396396
// prettier-ignore
397397
const { spans } = getSpansFromTimeline<TicketIdRelationSchemasFixture>`
398398
Events: ${Render('start', 0)}-----${Render('interrupt', 0)}-----${Render('end', 0)}
399-
Time: ${0} ${100} ${200}
399+
Time: ${0} ${100} ${200}
400400
`
401401
processSpans(spans, traceManager)
402402

@@ -406,12 +406,12 @@ describe('TraceManager', () => {
406406
AnyPossibleReportFn<TicketIdRelationSchemasFixture>
407407
>[0] = reportFn.mock.calls[0][0]
408408

409-
// there are NO entries in the report because this trace was interrupted before transitioning from draft to active
410-
expect(report.entries).toHaveLength(0)
409+
// this trace was interrupted before transitioning from draft to active, so the only available entries are up to the interrupt
410+
expect(report.entries).toHaveLength(2)
411411
expect(report.name).toBe('ticket.interrupt-on-basic-operation')
412412
expect(report.duration).toBeNull()
413-
expect(report.status).toBe('interrupted')
414413
expect(report.interruptionReason).toBe('matched-on-interrupt')
414+
expect(report.status).toBe('interrupted')
415415
})
416416

417417
it('interrupts a draft trace when interrupt() is called with error', () => {

src/v3/recordingComputeUtils.test.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ describe('recordingComputeUtils', () => {
155155
{
156156
transitionFromState: 'active',
157157
interruptionReason: 'timeout',
158+
transitionToState: 'interrupted',
159+
lastRelevantSpanAndAnnotation: undefined,
158160
},
159161
)
160162

@@ -675,7 +677,11 @@ describe('recordingComputeUtils', () => {
675677
},
676678
recordedItemsByLabel: {},
677679
},
678-
{ transitionFromState: 'active' },
680+
{
681+
transitionFromState: 'active',
682+
transitionToState: 'complete',
683+
lastRelevantSpanAndAnnotation: undefined,
684+
},
679685
)
680686

681687
expect(recording.computedRenderBeaconSpans['test-component']).toEqual({
@@ -708,7 +714,11 @@ describe('recordingComputeUtils', () => {
708714
},
709715
recordedItemsByLabel: {},
710716
},
711-
{ transitionFromState: 'active' },
717+
{
718+
transitionFromState: 'active',
719+
transitionToState: 'complete',
720+
lastRelevantSpanAndAnnotation: undefined,
721+
},
712722
)
713723

714724
// Verify the variant is included in the recording

src/v3/recordingComputeUtils.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { ensureMatcherFnOrSpecialToken } from './ensureMatcherFn'
33
import { type SpanMatcherFn } from './matchSpan'
44
import type { SpanAndAnnotation } from './spanAnnotationTypes'
55
import type { ActiveTraceInput, DraftTraceInput } from './spanTypes'
6-
import type { FinalState } from './Trace'
6+
import type { FinalState, FinalTransition } from './Trace'
77
import type { TraceRecording } from './traceRecordingTypes'
88
import type { SpecialEndToken, SpecialStartToken, TraceContext } from './types'
99

@@ -223,7 +223,7 @@ function getComputedRenderBeaconSpans<
223223
RelationSchemasT,
224224
const VariantsT extends string,
225225
>(
226-
recordedItems: ReadonlySet<SpanAndAnnotation<RelationSchemasT>>,
226+
recordedItems: readonly SpanAndAnnotation<RelationSchemasT>[],
227227
input: ActiveTraceInput<RelationSchemasT[SelectedRelationNameT], VariantsT>,
228228
): TraceRecording<
229229
SelectedRelationNameT,
@@ -380,18 +380,35 @@ export function createTraceRecording<
380380
context: TraceContext<SelectedRelationNameT, RelationSchemasT, VariantsT>,
381381
{
382382
transitionFromState,
383+
transitionToState,
383384
interruptionReason,
384385
cpuIdleSpanAndAnnotation,
385386
completeSpanAndAnnotation,
386387
lastRequiredSpanAndAnnotation,
387-
}: FinalState<RelationSchemasT>,
388+
lastRelevantSpanAndAnnotation,
389+
}: FinalTransition<RelationSchemasT>,
388390
): TraceRecording<SelectedRelationNameT, RelationSchemasT> {
389391
const { definition, recordedItems, input } = context
390392
const { id, relatedTo, variant } = input
391393
const { name } = definition
394+
395+
const endOfOperationSpan =
396+
(transitionToState === 'complete' &&
397+
(cpuIdleSpanAndAnnotation ?? completeSpanAndAnnotation)) ||
398+
lastRelevantSpanAndAnnotation
399+
400+
// only keep items captured until the endOfOperationSpan or if not available, the lastRelevantSpan
401+
const recordedItemsArray = endOfOperationSpan
402+
? [...recordedItems].filter(
403+
(item) =>
404+
item.span.startTime.now + item.span.duration <=
405+
endOfOperationSpan.span.startTime.now +
406+
endOfOperationSpan.span.duration,
407+
)
408+
: [...recordedItems.values()]
409+
392410
// CODE CLEAN UP TODO: let's get this information (wasInterrupted) from up top (in FinalState)
393-
const wasInterrupted =
394-
interruptionReason && transitionFromState !== 'waiting-for-interactive'
411+
const wasInterrupted = transitionToState === 'interrupted'
395412
const computedSpans = !wasInterrupted
396413
? getComputedSpans(context, {
397414
completeSpanAndAnnotation,
@@ -401,11 +418,9 @@ export function createTraceRecording<
401418
const computedValues = !wasInterrupted ? getComputedValues(context) : {}
402419
const computedRenderBeaconSpans =
403420
!wasInterrupted && isActiveTraceInput(input)
404-
? getComputedRenderBeaconSpans(recordedItems, input)
421+
? getComputedRenderBeaconSpans(recordedItemsArray, input)
405422
: {}
406423

407-
const recordedItemsArray = [...recordedItems.values()]
408-
409424
const anyNonSuppressedErrors = recordedItemsArray.some(
410425
(spanAndAnnotation) =>
411426
spanAndAnnotation.span.status === 'error' &&

src/v3/requiredSpanWithErrorStatus.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { SpanMatcherFn, SpanMatcherTags } from './matchSpan'
22

3+
// tag matcher with a special, internal matcher tag, and match on span.status === 'error'
34
export function requiredSpanWithErrorStatus<
45
const SelectedRelationNameT extends keyof RelationSchemasT,
56
const RelationSchemasT,

0 commit comments

Comments
 (0)