From abab16e826605e95f867d62f7a77ce1887bebd4c Mon Sep 17 00:00:00 2001 From: Cynthia Ma Date: Mon, 16 Dec 2024 17:01:53 -0800 Subject: [PATCH] feat: update reference to matchers --- src/v3/ActiveTrace.ts | 23 +++++++++++----------- src/v3/recordingComputeUtils.ts | 34 +++++++++++++++++++++------------ 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/v3/ActiveTrace.ts b/src/v3/ActiveTrace.ts index cbb670fe..cdb65cc9 100644 --- a/src/v3/ActiveTrace.ts +++ b/src/v3/ActiveTrace.ts @@ -259,8 +259,8 @@ export class TraceStateMachine< continue } - const matcher = this.context.definition.requiredSpans[i]! - if (matcher(spanAndAnnotation, this.context)) { + const doesSpanMatch = this.context.definition.requiredSpans[i]! + if (doesSpanMatch(spanAndAnnotation, this.context)) { // remove the index of this definition from the list of requiredToEnd this.context.requiredToEndIndexChecklist.delete(i) @@ -389,11 +389,10 @@ export class TraceStateMachine< span: { ...span, isIdle: true }, } if (idleRegressionCheckSpan) { - for (const matcher of this.context.definition.requiredSpans) { + for (const doesSpanMatch of this.context.definition.requiredSpans) { if ( - // v3 TODO: rename matcher in the whole file to 'doesSpanMatch' -> Cynthia - matcher(idleRegressionCheckSpan, this.context) && - matcher.isIdle + doesSpanMatch(idleRegressionCheckSpan, this.context) && + doesSpanMatch.isIdle ) { // check if we regressed on "isIdle", and if so, transition to interrupted with reason return { @@ -408,8 +407,8 @@ export class TraceStateMachine< // does span satisfy any of the "debouncedOn" and if so, restart our debounce timer if (this.context.definition.debounceOn) { - for (const matcher of this.context.definition.debounceOn) { - if (matcher(spanAndAnnotation, this.context)) { + for (const doesSpanMatch of this.context.definition.debounceOn) { + if (doesSpanMatch(spanAndAnnotation, this.context)) { // Sometimes spans are processed out of order, we update the lastRelevant if this span ends later if ( spanAndAnnotation.annotation.operationRelativeEndTime > @@ -629,8 +628,8 @@ export class TraceStateMachine< // if the entry matches any of the interruptOn criteria, // transition to complete state with the 'matched-on-interrupt' interruptionReason if (this.context.definition.interruptOn) { - for (const matcher of this.context.definition.interruptOn) { - if (matcher(spanAndAnnotation, this.context)) { + for (const doesSpanMatch of this.context.definition.interruptOn) { + if (doesSpanMatch(spanAndAnnotation, this.context)) { return { transitionToState: 'complete', interruptionReason: 'matched-on-interrupt', @@ -907,8 +906,8 @@ export class ActiveTrace< if (!this.definition.labelMatching) return labels Object.entries(this.definition.labelMatching).forEach( - ([label, matcher]) => { - if (matcher(span, context)) { + ([label, doesSpanMatch]) => { + if (doesSpanMatch(span, context)) { labels.push(label) } }, diff --git a/src/v3/recordingComputeUtils.ts b/src/v3/recordingComputeUtils.ts index d66c6dba..21afb5ac 100644 --- a/src/v3/recordingComputeUtils.ts +++ b/src/v3/recordingComputeUtils.ts @@ -56,14 +56,14 @@ export function getComputedValues< for (const definition of traceDefinition.computedValueDefinitions) { const { name, matches, computeValueFromMatches } = definition - // Initialize arrays to hold matches for each matcher + // Initialize arrays to hold matches for each doesSpanMatch matcher const matchingEntriesByMatcher: SpanAndAnnotation[][] = Array.from({ length: matches.length }, () => []) // Single pass through recordedItems for (const item of recordedItems) { - matches.forEach((matcher, index) => { - if (matcher(item, { input, definition: traceDefinition })) { + matches.forEach((doesSpanMatch, index) => { + if (doesSpanMatch(item, { input, definition: traceDefinition })) { matchingEntriesByMatcher[index]!.push(item) } }) @@ -153,7 +153,6 @@ export function getComputedSpans< return computedSpans } - function getComputedRenderBeaconSpans< TracerScopeKeysT extends KeysOfUnion, AllPossibleScopesT, @@ -175,6 +174,7 @@ function getComputedRenderBeaconSpans< firstContentStart: number | undefined renderCount: number sumOfDurations: number + lastRenderStartTime: number | undefined // Track the last render start time } >() @@ -186,12 +186,10 @@ function getComputedRenderBeaconSpans< entry.span.type !== 'component-render' && entry.span.type !== 'component-render-start' ) { - // need to look at component-render-start too, because react might discard some renders as optimization - // ratio of component-render-start to component-render isn't always 1:1 continue } const { name, startTime, duration, scope, renderedOutput } = entry.span - // accept any span that either matches scope or doesn't share any of the scope values + const scopeMatch = scopeKeys.every( (key) => (scope as PossibleScopeObject | undefined)?.[key] === undefined || @@ -199,6 +197,7 @@ function getComputedRenderBeaconSpans< (scope as PossibleScopeObject)[key], ) if (!scopeMatch) continue + const start = startTime.now const contentfulRenderEnd = entry.span.type === 'component-render' && renderedOutput === 'content' @@ -207,8 +206,6 @@ function getComputedRenderBeaconSpans< const spanTimes = renderSpansByBeacon.get(name) - // v3 TODO: make sure that sumOfRenderDurations takes into account that mismatch between render-start and full render - might be discarded and re-render - should extend the first render duration from the first render start to the first end - if (!spanTimes) { renderSpansByBeacon.set(name, { firstStart: start, @@ -220,6 +217,8 @@ function getComputedRenderBeaconSpans< entry.span.type === 'component-render' && renderedOutput === 'loading' ? start + duration : undefined, + lastRenderStartTime: + entry.span.type === 'component-render-start' ? start : undefined, }) } else { spanTimes.firstStart = Math.min(spanTimes.firstStart, start) @@ -227,10 +226,21 @@ function getComputedRenderBeaconSpans< contentfulRenderEnd && spanTimes.firstContentfulRenderEnd ? Math.min(spanTimes.firstContentfulRenderEnd, contentfulRenderEnd) : contentfulRenderEnd ?? spanTimes.firstContentfulRenderEnd + if (entry.span.type === 'component-render') { spanTimes.renderCount += 1 + // If there was a pending render start, include the time from that start to this render + if (spanTimes.lastRenderStartTime !== undefined) { + spanTimes.sumOfDurations += + start + duration - spanTimes.lastRenderStartTime + spanTimes.lastRenderStartTime = undefined + } else { + spanTimes.sumOfDurations += duration + } + } else if (entry.span.type === 'component-render-start') { + spanTimes.lastRenderStartTime = start } - spanTimes.sumOfDurations += duration + if ( spanTimes.firstContentStart === undefined && renderedOutput === 'content' @@ -301,8 +311,8 @@ export function createTraceRecording< const anyNonSuppressedErrors = recordedItems.some( (spanAndAnnotation) => spanAndAnnotation.span.status === 'error' && - !definition.suppressErrorStatusPropagationOn?.some((matcher) => - matcher(spanAndAnnotation, data), + !definition.suppressErrorStatusPropagationOn?.some((doesSpanMatch) => + doesSpanMatch(spanAndAnnotation, data), ), )