From 35580e359812008edc750d239218678c1310dcb6 Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Mon, 14 Dec 2020 18:01:16 +0100 Subject: [PATCH 1/4] =?UTF-8?q?=E2=9C=A8=20Add=20isActive=20attribute=20to?= =?UTF-8?q?=20view=20events?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../view/trackViews.spec.ts | 40 +++++++++++++++++++ .../rumEventsCollection/view/trackViews.ts | 6 ++- .../view/viewCollection.spec.ts | 2 + .../view/viewCollection.ts | 1 + packages/rum/src/types.ts | 1 + packages/rum/test/fixtures.ts | 1 + 6 files changed, 49 insertions(+), 2 deletions(-) diff --git a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts index 18fd829ec6..7dee67596b 100644 --- a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts @@ -356,6 +356,46 @@ describe('rum track loading type', () => { }) }) +describe('rum track view is active', () => { + let setupBuilder: TestSetupBuilder + let handler: jasmine.Spy + let getViewEvent: (index: number) => View + + beforeEach(() => { + ;({ handler, getViewEvent } = spyOnViews()) + + setupBuilder = setup() + .withFakeClock() + .withFakeLocation('/foo') + .beforeBuild(({ location, lifeCycle }) => { + lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler) + return trackViews(location, lifeCycle) + }) + }) + + afterEach(() => { + setupBuilder.cleanup() + }) + + it('should collect initial view as active', () => { + setupBuilder.build() + expect(getViewEvent(0).isActive).toBe(true) + }) + + it('should collect old view as inactive and new one as active after a route change', () => { + setupBuilder.build() + history.pushState({}, '', '/bar') + expect(getViewEvent(1).isActive).toBe(false) + expect(getViewEvent(2).isActive).toBe(true) + }) + + it('should collect view as active after a search change', () => { + setupBuilder.build() + history.pushState({}, '', '/foo?bar=qux') + expect(getViewEvent(1).isActive).toBe(true) + }) +}) + describe('rum track loading time', () => { let setupBuilder: TestSetupBuilder let handler: jasmine.Spy diff --git a/packages/rum/src/domain/rumEventsCollection/view/trackViews.ts b/packages/rum/src/domain/rumEventsCollection/view/trackViews.ts index e6f28fee3d..660de6aa97 100644 --- a/packages/rum/src/domain/rumEventsCollection/view/trackViews.ts +++ b/packages/rum/src/domain/rumEventsCollection/view/trackViews.ts @@ -15,6 +15,7 @@ export interface View { documentVersion: number startTime: number duration: number + isActive: boolean loadingTime?: number | undefined loadingType: ViewLoadingType cumulativeLayoutShift?: number @@ -51,8 +52,8 @@ export function trackViews(location: Location, lifeCycle: LifeCycle) { function onLocationChange() { if (currentView.isDifferentView(location)) { // Renew view on location changes - currentView.triggerUpdate() currentView.end() + currentView.triggerUpdate() currentView = newView(lifeCycle, location, ViewLoadingType.ROUTE_CHANGE, currentView.url) } else { currentView.updateLocation(location) @@ -69,8 +70,8 @@ export function trackViews(location: Location, lifeCycle: LifeCycle) { // End the current view on page unload lifeCycle.subscribe(LifeCycleEventType.BEFORE_UNLOAD, () => { - currentView.triggerUpdate() currentView.end() + currentView.triggerUpdate() }) // Session keep alive @@ -163,6 +164,7 @@ function newView( startTime, timings, duration: (endTime === undefined ? performance.now() : endTime) - startTime, + isActive: endTime === undefined, }) } diff --git a/packages/rum/src/domain/rumEventsCollection/view/viewCollection.spec.ts b/packages/rum/src/domain/rumEventsCollection/view/viewCollection.spec.ts index d4d978ed26..265b81d473 100644 --- a/packages/rum/src/domain/rumEventsCollection/view/viewCollection.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/view/viewCollection.spec.ts @@ -34,6 +34,7 @@ describe('viewCollection', () => { userActionCount: 10, }, id: 'xxx', + isActive: false, loadingTime: 20, loadingType: ViewLoadingType.INITIAL_LOAD, location: {}, @@ -71,6 +72,7 @@ describe('viewCollection', () => { }, firstContentfulPaint: 10 * 1e6, firstInputDelay: 12 * 1e6, + isActive: false, largestContentfulPaint: 10 * 1e6, loadEvent: 10 * 1e6, loadingTime: 20 * 1e6, diff --git a/packages/rum/src/domain/rumEventsCollection/view/viewCollection.ts b/packages/rum/src/domain/rumEventsCollection/view/viewCollection.ts index 944e113beb..7a9cbe2fe4 100644 --- a/packages/rum/src/domain/rumEventsCollection/view/viewCollection.ts +++ b/packages/rum/src/domain/rumEventsCollection/view/viewCollection.ts @@ -31,6 +31,7 @@ function processViewUpdate(view: View) { }, firstContentfulPaint: msToNs(view.timings.firstContentfulPaint), firstInputDelay: msToNs(view.timings.firstInputDelay), + isActive: view.isActive, largestContentfulPaint: msToNs(view.timings.largestContentfulPaint), loadEvent: msToNs(view.timings.loadEvent), loadingTime: msToNs(view.loadingTime), diff --git a/packages/rum/src/types.ts b/packages/rum/src/types.ts index 2f32389d91..3695ac02dc 100644 --- a/packages/rum/src/types.ts +++ b/packages/rum/src/types.ts @@ -66,6 +66,7 @@ export interface RumViewEvent { loadEvent?: number loadingTime?: number timeSpent: number + isActive: boolean error: Count action: Count longTask: Count diff --git a/packages/rum/test/fixtures.ts b/packages/rum/test/fixtures.ts index 6b906b1376..4ed2eaa1a7 100644 --- a/packages/rum/test/fixtures.ts +++ b/packages/rum/test/fixtures.ts @@ -67,6 +67,7 @@ export function createRawRumEvent(type: RumEventType, overrides?: Context): RawR view: { action: { count: 0 }, error: { count: 0 }, + isActive: true, loadingType: ViewLoadingType.INITIAL_LOAD, longTask: { count: 0 }, resource: { count: 0 }, From 3aced340c5da79bd4599ec8c539c89cbe3a8194c Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Tue, 15 Dec 2020 11:34:09 +0100 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=91=8C=20Clarify=20test=20intent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rum/src/domain/rumEventsCollection/view/trackViews.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts index 7dee67596b..2a692537d3 100644 --- a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts @@ -389,7 +389,7 @@ describe('rum track view is active', () => { expect(getViewEvent(2).isActive).toBe(true) }) - it('should collect view as active after a search change', () => { + it('should not collect view as inactive after any location change', () => { setupBuilder.build() history.pushState({}, '', '/foo?bar=qux') expect(getViewEvent(1).isActive).toBe(true) From b777ed06b9bf5aecfbc8e77243277299a843f378 Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Tue, 15 Dec 2020 16:29:07 +0100 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=91=8C=20=20Better=20tests=20descript?= =?UTF-8?q?ions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastien Caudan --- .../src/domain/rumEventsCollection/view/trackViews.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts index 2a692537d3..51eb76e31a 100644 --- a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts @@ -377,19 +377,19 @@ describe('rum track view is active', () => { setupBuilder.cleanup() }) - it('should collect initial view as active', () => { + it('should set initial view as active', () => { setupBuilder.build() expect(getViewEvent(0).isActive).toBe(true) }) - it('should collect old view as inactive and new one as active after a route change', () => { + it('should set old view as inactive and new one as active after a route change', () => { setupBuilder.build() history.pushState({}, '', '/bar') expect(getViewEvent(1).isActive).toBe(false) expect(getViewEvent(2).isActive).toBe(true) }) - it('should not collect view as inactive after any location change', () => { + it('should keep view as active after a search change', () => { setupBuilder.build() history.pushState({}, '', '/foo?bar=qux') expect(getViewEvent(1).isActive).toBe(true) From 8e37872dead77183584b8e028a529ecaecb0649e Mon Sep 17 00:00:00 2001 From: Amine Ben hammou Date: Tue, 15 Dec 2020 17:13:08 +0100 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=91=8C=20Remove=20unused=20fake=20clo?= =?UTF-8?q?ck?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rum/src/domain/rumEventsCollection/view/trackViews.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts index 51eb76e31a..0178816e1b 100644 --- a/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts +++ b/packages/rum/src/domain/rumEventsCollection/view/trackViews.spec.ts @@ -365,7 +365,6 @@ describe('rum track view is active', () => { ;({ handler, getViewEvent } = spyOnViews()) setupBuilder = setup() - .withFakeClock() .withFakeLocation('/foo') .beforeBuild(({ location, lifeCycle }) => { lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler)