Skip to content

Commit 31e7495

Browse files
niieaniniieani
committed
fix: fix some TODOs
1 parent be98080 commit 31e7495

File tree

2 files changed

+82
-58
lines changed

2 files changed

+82
-58
lines changed

‎src/v3/ActiveTrace.ts

Lines changed: 81 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import type {
2626
StateHandlerPayloads,
2727
} from './typeUtils'
2828

29-
interface CreateTraceRecordingConfig<ScopeT extends ScopeBase> {
29+
interface FinalState<ScopeT extends ScopeBase> {
3030
transitionFromState: NonTerminalTraceStates
3131
interruptionReason?: TraceInterruptionReason
3232
cpuIdleSpanAndAnnotation?: SpanAndAnnotation<ScopeT>
@@ -47,8 +47,7 @@ interface OnEnterInterrupted {
4747
interruptionReason: TraceInterruptionReason
4848
}
4949

50-
interface OnEnterComplete<ScopeT extends ScopeBase>
51-
extends CreateTraceRecordingConfig<ScopeT> {
50+
interface OnEnterComplete<ScopeT extends ScopeBase> extends FinalState<ScopeT> {
5251
transitionToState: 'complete'
5352
}
5453

@@ -73,9 +72,7 @@ export type Transition<ScopeT extends ScopeBase> = DistributiveOmit<
7372
'transitionFromState'
7473
>
7574

76-
type FinalizeFn<ScopeT extends ScopeBase> = (
77-
config: CreateTraceRecordingConfig<ScopeT>,
78-
) => void
75+
type FinalizeFn<ScopeT extends ScopeBase> = (config: FinalState<ScopeT>) => void
7976

8077
export type States<ScopeT extends ScopeBase> =
8178
TraceStateMachine<ScopeT>['states']
@@ -108,7 +105,7 @@ export class TraceStateMachine<ScopeT extends ScopeBase> {
108105
readonly requiredToEndIndexChecklist: Set<number>
109106
}
110107
readonly sideEffectFns: {
111-
readonly finalize: FinalizeFn<ScopeT>
108+
readonly storeFinalizeState: FinalizeFn<ScopeT>
112109
}
113110
currentState: TraceStates = 'recording'
114111
/** the span that ended at the furthest point in time */
@@ -165,6 +162,12 @@ export class TraceStateMachine<ScopeT extends ScopeBase> {
165162
}
166163

167164
for (let i = 0; i < this.context.definition.requiredToEnd.length; i++) {
165+
if (!this.context.requiredToEndIndexChecklist.has(i)) {
166+
// we previously checked off this index
167+
// eslint-disable-next-line no-continue
168+
continue
169+
}
170+
168171
const definition = this.context.definition.requiredToEnd[i]!
169172
if (
170173
doesEntryMatchDefinition(
@@ -173,24 +176,29 @@ export class TraceStateMachine<ScopeT extends ScopeBase> {
173176
this.context.input.scope,
174177
)
175178
) {
179+
console.log(
180+
'# got a match!',
181+
'span',
182+
spanAndAnnotation,
183+
'matches',
184+
definition,
185+
'remaining items',
186+
this.context.requiredToEndIndexChecklist,
187+
)
176188
// remove the index of this definition from the list of requiredToEnd
177189
this.context.requiredToEndIndexChecklist.delete(i)
178190

179191
// Sometimes spans are processed out of order, we update the lastRelevant if this span ends later
180192
if (
193+
!this.lastRelevant ||
181194
spanAndAnnotation.annotation.operationRelativeEndTime >
182-
(this.lastRelevant?.annotation.operationRelativeEndTime ?? 0)
195+
(this.lastRelevant?.annotation.operationRelativeEndTime ?? 0)
183196
) {
184197
this.lastRelevant = spanAndAnnotation
185198
}
186199
}
187200
}
188201

189-
console.log(
190-
'# requiredToEndIndexChecklist',
191-
this.context.requiredToEndIndexChecklist,
192-
spanAndAnnotation,
193-
)
194202
if (this.context.requiredToEndIndexChecklist.size === 0) {
195203
return { transitionToState: 'debouncing' }
196204
}
@@ -212,17 +220,23 @@ export class TraceStateMachine<ScopeT extends ScopeBase> {
212220
// the final, settled state of the component
213221
debouncing: {
214222
onEnterState: (payload: OnEnterDebouncing) => {
223+
if (!this.lastRelevant) {
224+
// this should never happen
225+
return {
226+
transitionToState: 'interrupted',
227+
interruptionReason: 'invalid-state-transition',
228+
}
229+
}
215230
if (!this.context.definition.debounceOn) {
216231
return { transitionToState: 'waiting-for-interactive' }
217232
}
218-
if (this.lastRelevant) {
219-
// set the first debounce deadline
220-
this.debounceDeadline =
221-
this.lastRelevant.span.startTime.epoch +
222-
this.lastRelevant.span.duration +
223-
(this.context.definition.debounceDuration ??
224-
DEFAULT_DEBOUNCE_DURATION)
225-
}
233+
// set the first debounce deadline
234+
this.debounceDeadline =
235+
this.lastRelevant.span.startTime.epoch +
236+
this.lastRelevant.span.duration +
237+
(this.context.definition.debounceDuration ??
238+
DEFAULT_DEBOUNCE_DURATION)
239+
226240
return undefined
227241
},
228242

@@ -306,6 +320,14 @@ export class TraceStateMachine<ScopeT extends ScopeBase> {
306320

307321
'waiting-for-interactive': {
308322
onEnterState: (payload: OnEnterWaitingForInteractive) => {
323+
if (!this.lastRelevant) {
324+
// this should never happen
325+
return {
326+
transitionToState: 'interrupted',
327+
interruptionReason: 'invalid-state-transition',
328+
}
329+
}
330+
309331
this.lastRequiredSpan = this.lastRelevant
310332
const interactiveConfig = this.context.definition.captureInteractive
311333
if (!interactiveConfig) {
@@ -316,17 +338,12 @@ export class TraceStateMachine<ScopeT extends ScopeBase> {
316338
}
317339
}
318340

319-
if (this.lastRequiredSpan) {
320-
this.interactiveDeadline =
321-
this.lastRequiredSpan.span.startTime.epoch +
322-
this.lastRequiredSpan.span.duration +
323-
((typeof interactiveConfig === 'object' &&
324-
interactiveConfig.timeout) ||
325-
DEFAULT_INTERACTIVE_TIMEOUT_DURATION)
326-
} else {
327-
// TODO: do we want an error state? will this condition every be true?
328-
return { transitionToState: 'complete' }
329-
}
341+
this.interactiveDeadline =
342+
this.lastRequiredSpan.span.startTime.epoch +
343+
this.lastRequiredSpan.span.duration +
344+
((typeof interactiveConfig === 'object' &&
345+
interactiveConfig.timeout) ||
346+
DEFAULT_INTERACTIVE_TIMEOUT_DURATION)
330347

331348
this.cpuIdleLongTaskProcessor = createCPUIdleProcessor<
332349
EntryType<ScopeT>
@@ -340,7 +357,6 @@ export class TraceStateMachine<ScopeT extends ScopeBase> {
340357
typeof interactiveConfig === 'object' ? interactiveConfig : {},
341358
)
342359

343-
// TODO: start the timer for tti debouncing
344360
return undefined
345361
},
346362

@@ -419,15 +435,14 @@ export class TraceStateMachine<ScopeT extends ScopeBase> {
419435

420436
// terminal states:
421437
interrupted: {
422-
onEnterState: (payload: OnEnterInterrupted) => {
423-
this.sideEffectFns.finalize(payload)
438+
onEnterState: (_payload: OnEnterInterrupted) => {
439+
// terminal state, but we reuse the payload for generating the report in ActiveTrace
424440
},
425441
},
426442

427443
complete: {
428-
onEnterState: (payload: OnEnterComplete<ScopeT>) => {
429-
console.log('# complete payload', payload)
430-
this.sideEffectFns.finalize(payload)
444+
onEnterState: (_payload: OnEnterComplete<ScopeT>) => {
445+
// terminal state, but we reuse the payload for generating the report in ActiveTrace
431446
},
432447
},
433448
} satisfies StatesBase<ScopeT>
@@ -490,6 +505,8 @@ export class ActiveTrace<ScopeT extends ScopeBase> {
490505
SpanAndAnnotation<ScopeT>
491506
> = new WeakMap()
492507

508+
finalState: FinalState<ScopeT> | undefined
509+
493510
constructor(
494511
definition: CompleteTraceDefinition<ScopeT>,
495512
input: ActiveTraceConfig<ScopeT>,
@@ -501,15 +518,13 @@ export class ActiveTrace<ScopeT extends ScopeBase> {
501518
definition,
502519
input,
503520
sideEffectFns: {
504-
finalize: this.finalize,
521+
storeFinalizeState: this.storeFinalizeState,
505522
},
506523
})
507524
}
508525

509-
finalize = (config: CreateTraceRecordingConfig<ScopeT>) => {
510-
const traceRecording = this.createTraceRecording(config)
511-
console.log('# recording?', traceRecording)
512-
this.input.onEnd(traceRecording)
526+
storeFinalizeState = (config: FinalState<ScopeT>) => {
527+
this.finalState = config
513528
}
514529

515530
// this is public API only and should not be called internally
@@ -564,23 +579,29 @@ export class ActiveTrace<ScopeT extends ScopeBase> {
564579
spanAndAnnotation.span = span
565580
}
566581

567-
const transitionPayload = this.stateMachine.emit(
582+
const transition = this.stateMachine.emit(
568583
'onProcessSpan',
569584
spanAndAnnotation,
570585
)
571-
// console.log('transitionPayload', transitionPayload)
572-
// if the final state is interrupted,
573-
// we decided that we should not record the entry nor annotate it externally
574-
// TODO: this if statement needs to be validated/rethought
575-
if (
586+
587+
const shouldRecord =
576588
!existingAnnotation &&
577-
(!transitionPayload ||
578-
transitionPayload.transitionToState === 'complete' ||
579-
// TODO: this condition doesn't make sense
580-
transitionPayload.transitionToState !== 'interrupted')
581-
) {
582-
console.log('# PUSH INTO RECORDED ITEMS', spanAndAnnotation)
589+
(!transition || transition.transitionToState !== 'interrupted')
590+
591+
// DECISION: if the final state is interrupted, we should not record the entry nor annotate it externally
592+
if (shouldRecord) {
583593
this.recordedItems.push(spanAndAnnotation)
594+
}
595+
596+
if (
597+
transition?.transitionToState === 'interrupted' ||
598+
transition?.transitionToState === 'complete'
599+
) {
600+
const traceRecording = this.createTraceRecording(transition)
601+
this.input.onEnd(traceRecording)
602+
}
603+
604+
if (shouldRecord) {
584605
return {
585606
[this.definition.name]: spanAndAnnotation.annotation,
586607
}
@@ -673,7 +694,9 @@ export class ActiveTrace<ScopeT extends ScopeBase> {
673694
private createTraceRecording = ({
674695
transitionFromState,
675696
interruptionReason,
676-
}: CreateTraceRecordingConfig<ScopeT>): TraceRecording<ScopeT> => {
697+
cpuIdleSpanAndAnnotation,
698+
lastRequiredSpanAndAnnotation,
699+
}: FinalState<ScopeT>): TraceRecording<ScopeT> => {
677700
const { id, scope } = this.input
678701
const { name } = this.definition
679702
const { computedSpans, computedValues, spanAttributes, attributes } = this
@@ -701,8 +724,8 @@ export class ActiveTrace<ScopeT extends ScopeBase> {
701724
interruptionReason && transitionFromState !== 'waiting-for-interactive'
702725
? 'interrupted'
703726
: anyErrors
704-
? 'error'
705-
: 'ok',
727+
? 'error'
728+
: 'ok',
706729
computedSpans,
707730
computedValues,
708731
attributes,

‎src/v3/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export type TraceInterruptionReason =
3737
| 'manually-aborted'
3838
| 'idle-component-no-longer-idle'
3939
| 'matched-on-interrupt'
40+
| 'invalid-state-transition'
4041

4142
export type ReportFn<ScopeT extends ScopeBase> = (
4243
trace: TraceRecording<ScopeT>,

0 commit comments

Comments
 (0)