-
Notifications
You must be signed in to change notification settings - Fork 21
Add unit/integration testing for @opencensus/web-instrumentation-zone #104
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,12 +30,16 @@ import { | |
declare const Zone: ZoneType & { prototype: Zone }; | ||
|
||
// Delay of 50 ms to reset currentEventTracingZone. | ||
const RESET_TRACING_ZONE_DELAY = 50; | ||
export const RESET_TRACING_ZONE_DELAY = 50; | ||
|
||
export class InteractionTracker { | ||
// Allows to track several events triggered by the same user interaction in the right Zone. | ||
private currentEventTracingZone?: Zone; | ||
|
||
// Used to reset the currentEventTracingZone when the interaction | ||
// finishes before it is reset automatically. | ||
private currentResetTracingZoneTimeout?: number; | ||
|
||
// Map of interaction Ids to stopwatches. | ||
private readonly interactions: { | ||
[index: string]: OnPageInteractionStopwatch; | ||
|
@@ -46,23 +50,26 @@ export class InteractionTracker { | |
[index: string]: number; | ||
} = {}; | ||
|
||
constructor() { | ||
private static singletonInstance: InteractionTracker; | ||
|
||
private constructor() { | ||
this.patchZoneRunTask(); | ||
this.patchZoneScheduleTask(); | ||
this.patchZoneCancelTask(); | ||
this.patchHistoryApi(); | ||
} | ||
|
||
static startTracking(): InteractionTracker { | ||
return this.singletonInstance || (this.singletonInstance = new this()); | ||
} | ||
|
||
private patchZoneRunTask() { | ||
const runTask = Zone.prototype.runTask; | ||
Zone.prototype.runTask = ( | ||
task: AsyncTask, | ||
applyThis: unknown, | ||
applyArgs: unknown | ||
) => { | ||
console.warn('Running task'); | ||
console.log(task); | ||
|
||
const interceptingElement = getTrackedElement(task); | ||
const interactionName = resolveInteractionName( | ||
interceptingElement, | ||
|
@@ -87,7 +94,6 @@ export class InteractionTracker { | |
try { | ||
return runTask.call(task.zone as {}, task, applyThis, applyArgs); | ||
} finally { | ||
console.log('Run task finished.'); | ||
if ( | ||
interceptingElement || | ||
(shouldCountTask(task) && isTrackedTask(task)) | ||
|
@@ -155,7 +161,7 @@ export class InteractionTracker { | |
tracing.tracer.startRootSpan(spanOptions, root => { | ||
// As startRootSpan creates the zone and Zone.current corresponds to the | ||
// new zone, we have to set the currentEventTracingZone with the Zone.current | ||
// to capture the new zone, also, start the `OnPageInteraction` to capture the | ||
// to capture the new zone, also, start the `OnPageInteraction` to capture the | ||
// new root span. | ||
this.currentEventTracingZone = Zone.current; | ||
this.interactions[traceId] = startOnPageInteraction({ | ||
|
@@ -170,14 +176,19 @@ export class InteractionTracker { | |
|
||
// Timeout to reset currentEventTracingZone to allow the creation of a new | ||
// zone for a new user interaction. | ||
Zone.root.run(() => | ||
setTimeout( | ||
() => (this.currentEventTracingZone = undefined), | ||
Zone.root.run(() => { | ||
// Store the timeout in case the interaction finishes before | ||
// this callback runs. | ||
this.currentResetTracingZoneTimeout = window.setTimeout( | ||
() => this.resetCurrentTracingZone(), | ||
RESET_TRACING_ZONE_DELAY | ||
) | ||
); | ||
console.log('New zone:'); | ||
console.log(this.currentEventTracingZone); | ||
); | ||
}); | ||
} | ||
|
||
private resetCurrentTracingZone() { | ||
this.currentEventTracingZone = undefined; | ||
this.currentResetTracingZoneTimeout = undefined; | ||
} | ||
|
||
/** Increments the count of outstanding tasks for a given interaction id. */ | ||
|
@@ -233,6 +244,14 @@ export class InteractionTracker { | |
this.interactionCompletionTimeouts[interactionId] = setTimeout(() => { | ||
this.completeInteraction(interactionId); | ||
delete this.interactionCompletionTimeouts[interactionId]; | ||
// In case the interaction finished beforeresetCurrentTracingZone is called, | ||
// this in order to allow the creating of a new interaction. | ||
if (this.currentResetTracingZoneTimeout) { | ||
Zone.root.run(() => { | ||
clearTimeout(this.currentResetTracingZoneTimeout); | ||
this.resetCurrentTracingZone(); | ||
}); | ||
} | ||
}); | ||
}); | ||
} | ||
|
@@ -375,10 +394,15 @@ function resolveInteractionName( | |
function shouldCountTask(task: Task): boolean { | ||
if (!task.data) return false; | ||
|
||
// Don't count periodic tasks with a delay greater than 1 s. | ||
if (task.data.isPeriodic && (task.data.delay && task.data.delay >= 1000)) { | ||
return false; | ||
} | ||
// Don't count periodic tasks like setInterval as they will be repeatedly | ||
// called. This will cause that the interaction never finishes, then would be | ||
// imposible to measure the stability of the interaction. | ||
// This case only applies for `setInterval` as we support `setTimeout`. | ||
// TODO: ideally OpenCensus Web can manage this kind of tasks, so for example | ||
// if a periodic task ends up doing some work in the future it will still | ||
// be associated with that same older tracing zone. This is something we have to | ||
// think of. | ||
if (task.data.isPeriodic) return false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you change this to ignore all periodic tasks? I would think that calls to One other thing that should be a TODO for later I think - if you have a periodic task with > 1 second of time, then I think it should be executed outside of the tracing zone. Otherwise, if it ends up doing some work in the future it will still be associated with that same older tracing zone. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found out that if we track periodic tasks, the interaction might not finish as they are called repeatedly, so we won't be able to determine the stability of the interaction. |
||
|
||
// We're only interested in macroTasks and microTasks. | ||
return task.type === 'macroTask' || task.type === 'microTask'; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you do this in the tests rather than here in the production code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done