From 7bd7a6ef70a61e5b81fa1ccccaf539a4b7270f14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt?= Date: Wed, 19 Apr 2023 11:11:18 +0200 Subject: [PATCH 1/7] =?UTF-8?q?=E2=9C=A8=20[RUMF-1534]=20allow=20(some)=20?= =?UTF-8?q?view=20updates=20after=20session=20expiration=20(#2167)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ✅ [RUMF-1534] consolidate trackViews specs Add tests around the view lifecycle (page exit, session renew and session keep alive) * ✨ [RUMF-1534] end the current view when the session expires * ✨ [RUMF-1534] stop session keep alive when session is expired Now that views are ended when the session expires, we can move the session keepalive logic within the view so it doesn't send view updates when no view is active anymore. * ♻️ [RUMF-1534] collocate view updates This commit moves all view updates within the "newView" function. Motivations: * This helps to track down when a view can be updated. * It makes sure that we don't send a view update on page unload when the session did expire before. One important functional change: a view update is now always sent when ending a view It was not the case before: we intentionally didn't send a view update when the session was renewed, because it could lead to imprecise measurements (ex: inaccurate view duration), especially when the page was frozen for a long time. But even if we didn't send a view update in this case, there was a good chance that a view update was still sent because of other events (ex: periodic session keepalive, counter updates...) We now always send a view update when the session is expired, which is a bit less problematic than when the session is renewed (because it happens sooner, without relying on arbitrary user interaction), but might still suffer from imprecise measurements in case of page freeze. We think this is an acceptable drawback and should not have a perceptible impact. This is an interesting change because next we'll want to send a view update to let the backend know that the session has expired. * ✨ [RUMF-1534] prevent initial view updates after session expiration * ✨ [RUMF-1534] do not accept new timings when view has ended * ✨ [RUMF-1534] allow view updates after session expiration * 👌 [RUMF-1534] schedule stop initial view timings after the view ends * 👌 simplify 'initial view timings' logic in trackViews This commit changes the way we track 'initial view timings' to make it similar to how we track 'view metrics' and 'event counts': the `trackInitialViewTimings` now takes a `scheduleViewUpdate` callback and returns a mutable `timings` object. This simplifies a bit the logic in `newView`. * 👌 update test title Co-authored-by: Yannick Adam --------- Co-authored-by: Yannick Adam --- packages/rum-core/src/domain/assembly.spec.ts | 13 - packages/rum-core/src/domain/assembly.ts | 5 +- .../view/setupViewTest.specHelper.ts | 7 + .../view/trackInitialViewTimings.spec.ts | 46 ++- .../view/trackInitialViewTimings.ts | 49 ++- .../view/trackViews.spec.ts | 303 +++++++++++++----- .../rumEventsCollection/view/trackViews.ts | 97 ++---- 7 files changed, 340 insertions(+), 180 deletions(-) diff --git a/packages/rum-core/src/domain/assembly.spec.ts b/packages/rum-core/src/domain/assembly.spec.ts index 75089fa5dd..8414c3dadd 100644 --- a/packages/rum-core/src/domain/assembly.spec.ts +++ b/packages/rum-core/src/domain/assembly.spec.ts @@ -534,19 +534,6 @@ describe('rum assembly', () => { expect(rumSessionManager.findTrackedSession).toHaveBeenCalledWith(123 as RelativeTime) }) - - it('should get current session state for view event', () => { - const rumSessionManager = createRumSessionManagerMock() - spyOn(rumSessionManager, 'findTrackedSession').and.callThrough() - - const { lifeCycle } = setupBuilder.withSessionManager(rumSessionManager).build() - notifyRawRumEvent(lifeCycle, { - rawRumEvent: createRawRumEvent(RumEventType.VIEW), - startTime: 123 as RelativeTime, - }) - - expect(rumSessionManager.findTrackedSession).toHaveBeenCalledWith(undefined) - }) }) describe('session context', () => { diff --git a/packages/rum-core/src/domain/assembly.ts b/packages/rum-core/src/domain/assembly.ts index bc5230c7ac..9fb7568155 100644 --- a/packages/rum-core/src/domain/assembly.ts +++ b/packages/rum-core/src/domain/assembly.ts @@ -89,10 +89,7 @@ export function startRumAssembly( ({ startTime, rawRumEvent, domainContext, savedCommonContext, customerContext }) => { const viewContext = viewContexts.findView(startTime) const urlContext = urlContexts.findUrl(startTime) - // allow to send events if the session was tracked when they start - // except for views which are continuously updated - // TODO: stop sending view updates when session is expired - const session = sessionManager.findTrackedSession(rawRumEvent.type !== RumEventType.VIEW ? startTime : undefined) + const session = sessionManager.findTrackedSession(startTime) if (session && viewContext && urlContext) { const commonContext = savedCommonContext || buildCommonContext() const actionId = actionContexts.findActionId(startTime) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/setupViewTest.specHelper.ts b/packages/rum-core/src/domain/rumEventsCollection/view/setupViewTest.specHelper.ts index f4fcc70b51..d374537fe0 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/setupViewTest.specHelper.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/setupViewTest.specHelper.ts @@ -15,12 +15,17 @@ export function setupViewTest( getHandledCount: getViewUpdateCount, } = spyOnViews('view update') lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, viewUpdateHandler) + const { handler: viewCreateHandler, getViewEvent: getViewCreate, getHandledCount: getViewCreateCount, } = spyOnViews('view create') lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, viewCreateHandler) + + const { handler: viewEndHandler, getViewEvent: getViewEnd, getHandledCount: getViewEndCount } = spyOnViews('view end') + lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, viewEndHandler) + const { stop, startView, addTiming } = trackViews( location, lifeCycle, @@ -38,6 +43,8 @@ export function setupViewTest( getViewUpdateCount, getViewCreate, getViewCreateCount, + getViewEnd, + getViewEndCount, getLatestViewContext: () => ({ id: getViewCreate(getViewCreateCount() - 1).id, }), diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackInitialViewTimings.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackInitialViewTimings.spec.ts index de369cf845..033b8e72f3 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackInitialViewTimings.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackInitialViewTimings.spec.ts @@ -13,6 +13,7 @@ import { LifeCycleEventType } from '../../lifeCycle' import { resetFirstHidden } from './trackFirstHidden' import type { Timings } from './trackInitialViewTimings' import { + KEEP_TRACKING_TIMINGS_AFTER_VIEW_DELAY, trackFirstContentfulPaintTiming, trackFirstInputTimings, trackLargestContentfulPaintTiming, @@ -48,13 +49,19 @@ const FAKE_FIRST_INPUT_ENTRY: RumFirstInputTiming = { startTime: 1000 as RelativeTime, } -describe('trackTimings', () => { +describe('trackInitialViewTimings', () => { let setupBuilder: TestSetupBuilder - let timingsCallback: jasmine.Spy<(value: Partial) => void> + let scheduleViewUpdateSpy: jasmine.Spy<() => void> + let trackInitialViewTimingsResult: ReturnType + let setLoadEventSpy: jasmine.Spy<(loadEvent: Duration) => void> beforeEach(() => { - timingsCallback = jasmine.createSpy() - setupBuilder = setup().beforeBuild(({ lifeCycle }) => trackInitialViewTimings(lifeCycle, timingsCallback)) + scheduleViewUpdateSpy = jasmine.createSpy() + setLoadEventSpy = jasmine.createSpy() + setupBuilder = setup().beforeBuild(({ lifeCycle }) => { + trackInitialViewTimingsResult = trackInitialViewTimings(lifeCycle, setLoadEventSpy, scheduleViewUpdateSpy) + return trackInitialViewTimingsResult + }) }) afterEach(() => { @@ -70,8 +77,8 @@ describe('trackTimings', () => { FAKE_FIRST_INPUT_ENTRY, ]) - expect(timingsCallback).toHaveBeenCalledTimes(3) - expect(timingsCallback.calls.mostRecent().args[0]).toEqual({ + expect(scheduleViewUpdateSpy).toHaveBeenCalledTimes(3) + expect(trackInitialViewTimingsResult.timings).toEqual({ firstByte: 123 as Duration, domComplete: 456 as Duration, domContentLoaded: 345 as Duration, @@ -82,6 +89,33 @@ describe('trackTimings', () => { loadEvent: 567 as Duration, }) }) + + it('allows delaying the stop logic', () => { + const { lifeCycle, clock } = setupBuilder.withFakeClock().build() + trackInitialViewTimingsResult.scheduleStop() + + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [FAKE_NAVIGATION_ENTRY]) + + expect(scheduleViewUpdateSpy).toHaveBeenCalledTimes(1) + + clock.tick(KEEP_TRACKING_TIMINGS_AFTER_VIEW_DELAY) + + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [FAKE_PAINT_ENTRY]) + + expect(scheduleViewUpdateSpy).toHaveBeenCalledTimes(1) + }) + + it('calls the `setLoadEvent` callback when the loadEvent timing is known', () => { + const { lifeCycle } = setupBuilder.build() + + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ + FAKE_NAVIGATION_ENTRY, + FAKE_PAINT_ENTRY, + FAKE_FIRST_INPUT_ENTRY, + ]) + + expect(setLoadEventSpy).toHaveBeenCalledOnceWith(567 as Duration) + }) }) describe('trackNavigationTimings', () => { diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackInitialViewTimings.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackInitialViewTimings.ts index 3d9f94b373..18eebd373e 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackInitialViewTimings.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackInitialViewTimings.ts @@ -1,5 +1,6 @@ import type { Duration, RelativeTime } from '@datadog/browser-core' import { + setTimeout, assign, addEventListeners, DOM_EVENT, @@ -23,6 +24,14 @@ import { trackFirstHidden } from './trackFirstHidden' // It happens in some cases like sleep mode or some browser implementations export const TIMING_MAXIMUM_DELAY = 10 * ONE_MINUTE +/** + * The initial view can finish quickly, before some metrics can be produced (ex: before the page load + * event, or the first input). Also, we don't want to trigger a view update indefinitely, to avoid + * updates on views that ended a long time ago. Keep watching for metrics after the view ends for a + * limited amount of time. + */ +export const KEEP_TRACKING_TIMINGS_AFTER_VIEW_DELAY = 5 * ONE_MINUTE + export interface Timings { firstContentfulPaint?: Duration firstByte?: Duration @@ -35,14 +44,22 @@ export interface Timings { firstInputTime?: Duration } -export function trackInitialViewTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void) { +export function trackInitialViewTimings( + lifeCycle: LifeCycle, + setLoadEvent: (loadEnd: Duration) => void, + scheduleViewUpdate: () => void +) { const timings: Timings = {} + function setTimings(newTimings: Partial) { assign(timings, newTimings) - callback(timings) + scheduleViewUpdate() } - const { stop: stopNavigationTracking } = trackNavigationTimings(lifeCycle, setTimings) + const { stop: stopNavigationTracking } = trackNavigationTimings(lifeCycle, (newTimings) => { + setLoadEvent(newTimings.loadEvent) + setTimings(newTimings) + }) const { stop: stopFCPTracking } = trackFirstContentfulPaintTiming(lifeCycle, (firstContentfulPaint) => setTimings({ firstContentfulPaint }) ) @@ -58,17 +75,31 @@ export function trackInitialViewTimings(lifeCycle: LifeCycle, callback: (timings }) }) + function stop() { + stopNavigationTracking() + stopFCPTracking() + stopLCPTracking() + stopFIDTracking() + } + return { - stop: () => { - stopNavigationTracking() - stopFCPTracking() - stopLCPTracking() - stopFIDTracking() + stop, + timings, + scheduleStop: () => { + setTimeout(stop, KEEP_TRACKING_TIMINGS_AFTER_VIEW_DELAY) }, } } -export function trackNavigationTimings(lifeCycle: LifeCycle, callback: (timings: Partial) => void) { +interface NavigationTimings { + domComplete: Duration + domContentLoaded: Duration + domInteractive: Duration + loadEvent: Duration + firstByte: Duration | undefined +} + +export function trackNavigationTimings(lifeCycle: LifeCycle, callback: (timings: NavigationTimings) => void) { const { unsubscribe: stop } = lifeCycle.subscribe(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, (entries) => { for (const entry of entries) { if (entry.entryType === 'navigation') { diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts index 99988faaea..02a4b514f8 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts @@ -18,9 +18,10 @@ import { RumEventType, ViewLoadingType } from '../../../rawRumEvent.types' import type { RumEvent } from '../../../rumEvent.types' import { LifeCycleEventType } from '../../lifeCycle' import type { ViewEvent } from './trackViews' -import { THROTTLE_VIEW_UPDATE_PERIOD } from './trackViews' +import { SESSION_KEEP_ALIVE_INTERVAL, THROTTLE_VIEW_UPDATE_PERIOD } from './trackViews' import type { ViewTest } from './setupViewTest.specHelper' import { setupViewTest } from './setupViewTest.specHelper' +import { KEEP_TRACKING_TIMINGS_AFTER_VIEW_DELAY } from './trackInitialViewTimings' const FAKE_PAINT_ENTRY: RumPerformancePaintTiming = { entryType: 'paint', @@ -255,10 +256,31 @@ describe('initial view', () => { expect(initialView.last.loadingTime).toBe(FAKE_NAVIGATION_ENTRY.loadEventEnd) }) }) + + it('should not update timings long after the view ended', () => { + const { lifeCycle, clock } = setupBuilder.withFakeClock().build() + const { getViewUpdateCount, startView } = viewTest + + startView() + + clock.tick(KEEP_TRACKING_TIMINGS_AFTER_VIEW_DELAY) + + expect(getViewUpdateCount()).toEqual(4) + + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ + FAKE_PAINT_ENTRY, + FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY, + FAKE_NAVIGATION_ENTRY, + ]) + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + expect(getViewUpdateCount()).toEqual(4) + }) }) }) -describe('renew session', () => { +describe('view lifecycle', () => { let setupBuilder: TestSetupBuilder let viewTest: ViewTest @@ -279,102 +301,202 @@ describe('renew session', () => { setupBuilder.cleanup() }) - it('should create new view on renew session', () => { - const { lifeCycle } = setupBuilder.build() - const { getViewCreateCount } = viewTest + describe('expire session', () => { + it('should end the view when the session expires', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewEndCount } = viewTest - expect(getViewCreateCount()).toBe(1) + expect(getViewEndCount()).toBe(0) + + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) - lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + expect(getViewEndCount()).toBe(1) + }) + + it('should send a final view update', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewUpdateCount } = viewTest + + expect(getViewUpdateCount()).toBe(1) + + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + + expect(getViewUpdateCount()).toBe(2) + }) - expect(getViewCreateCount()).toBe(2) + it('should not start a new view if the session expired', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewCreateCount } = viewTest + + expect(getViewCreateCount()).toBe(1) + + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + + expect(getViewCreateCount()).toBe(1) + }) + + it('should not end the view if the view already ended', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewEndCount, getViewUpdateCount } = viewTest + + lifeCycle.notify(LifeCycleEventType.PAGE_EXITED, { reason: PageExitReason.UNLOADING }) + + expect(getViewEndCount()).toBe(1) + expect(getViewUpdateCount()).toBe(2) + + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + + expect(getViewEndCount()).toBe(1) + expect(getViewUpdateCount()).toBe(2) + }) }) - it('should use the current view name, service and version for the new view', () => { - const { lifeCycle, changeLocation } = setupBuilder.build() - const { getViewCreateCount, getViewCreate, startView } = viewTest + describe('renew session', () => { + it('should create new view on renew session', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewCreateCount } = viewTest - lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + expect(getViewCreateCount()).toBe(1) - startView({ name: 'view 1', service: 'service 1', version: 'version 1' }) - startView({ name: 'view 2', service: 'service 2', version: 'version 2' }) - lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) - startView({ name: 'view 3', service: 'service 3', version: 'version 3' }) - changeLocation('/bar') - lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + expect(getViewCreateCount()).toBe(2) + }) - expect(getViewCreateCount()).toBe(8) + it('should use the current view name, service and version for the new view', () => { + const { lifeCycle, changeLocation } = setupBuilder.build() + const { getViewCreateCount, getViewCreate, startView } = viewTest - expect(getViewCreate(0)).toEqual( - jasmine.objectContaining({ - name: 'initial view name', - service: 'initial service', - version: 'initial version', - }) - ) - expect(getViewCreate(1)).toEqual( - jasmine.objectContaining({ - name: 'initial view name', - service: 'initial service', - version: 'initial version', - }) - ) - expect(getViewCreate(2)).toEqual( - jasmine.objectContaining({ - name: 'view 1', - service: 'service 1', - version: 'version 1', - }) - ) - expect(getViewCreate(3)).toEqual( - jasmine.objectContaining({ - name: 'view 2', - service: 'service 2', - version: 'version 2', - }) - ) - expect(getViewCreate(4)).toEqual( - jasmine.objectContaining({ - name: 'view 2', - service: 'service 2', - version: 'version 2', - }) - ) - expect(getViewCreate(5)).toEqual( - jasmine.objectContaining({ - name: 'view 3', - service: 'service 3', - version: 'version 3', - }) - ) - expect(getViewCreate(6)).toEqual( - jasmine.objectContaining({ - name: undefined, - service: undefined, - version: undefined, - }) - ) - expect(getViewCreate(7)).toEqual( - jasmine.objectContaining({ - name: undefined, - service: undefined, - version: undefined, - }) - ) - resetExperimentalFeatures() + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + + startView({ name: 'view 1', service: 'service 1', version: 'version 1' }) + startView({ name: 'view 2', service: 'service 2', version: 'version 2' }) + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + + startView({ name: 'view 3', service: 'service 3', version: 'version 3' }) + changeLocation('/bar') + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + + expect(getViewCreateCount()).toBe(8) + + expect(getViewCreate(0)).toEqual( + jasmine.objectContaining({ + name: 'initial view name', + service: 'initial service', + version: 'initial version', + }) + ) + expect(getViewCreate(1)).toEqual( + jasmine.objectContaining({ + name: 'initial view name', + service: 'initial service', + version: 'initial version', + }) + ) + expect(getViewCreate(2)).toEqual( + jasmine.objectContaining({ + name: 'view 1', + service: 'service 1', + version: 'version 1', + }) + ) + expect(getViewCreate(3)).toEqual( + jasmine.objectContaining({ + name: 'view 2', + service: 'service 2', + version: 'version 2', + }) + ) + expect(getViewCreate(4)).toEqual( + jasmine.objectContaining({ + name: 'view 2', + service: 'service 2', + version: 'version 2', + }) + ) + expect(getViewCreate(5)).toEqual( + jasmine.objectContaining({ + name: 'view 3', + service: 'service 3', + version: 'version 3', + }) + ) + expect(getViewCreate(6)).toEqual( + jasmine.objectContaining({ + name: undefined, + service: undefined, + version: undefined, + }) + ) + expect(getViewCreate(7)).toEqual( + jasmine.objectContaining({ + name: undefined, + service: undefined, + version: undefined, + }) + ) + resetExperimentalFeatures() + }) }) - it('should not update the current view when the session is renewed', () => { - const { lifeCycle } = setupBuilder.build() - const { getViewUpdateCount, getViewUpdate } = viewTest + describe('session keep alive', () => { + it('should emit a view update periodically', () => { + const { clock } = setupBuilder.withFakeClock().build() - expect(getViewUpdateCount()).toEqual(1) + const { getViewUpdateCount } = viewTest - lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + expect(getViewUpdateCount()).toEqual(1) + + clock.tick(SESSION_KEEP_ALIVE_INTERVAL) + + expect(getViewUpdateCount()).toEqual(2) + }) + + it('should not send periodical updates after the session has expired', () => { + const { lifeCycle, clock } = setupBuilder.withFakeClock().build() + const { getViewUpdateCount } = viewTest + + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + + expect(getViewUpdateCount()).toBe(2) + + clock.tick(SESSION_KEEP_ALIVE_INTERVAL) + + expect(getViewUpdateCount()).toBe(2) + }) + }) + + describe('page exit', () => { + ;[ + { exitReason: PageExitReason.UNLOADING, expectViewEnd: true }, + { exitReason: PageExitReason.PAGEHIDE, expectViewEnd: true }, + { exitReason: PageExitReason.FROZEN, expectViewEnd: false }, + { exitReason: PageExitReason.HIDDEN, expectViewEnd: false }, + ].forEach(({ exitReason, expectViewEnd }) => { + it(`should ${ + expectViewEnd ? '' : 'not ' + }end the current view when the page is exiting for reason ${exitReason}`, () => { + const { lifeCycle } = setupBuilder.build() + const { getViewEndCount } = viewTest + + expect(getViewEndCount()).toEqual(0) + + lifeCycle.notify(LifeCycleEventType.PAGE_EXITED, { reason: exitReason }) + + expect(getViewEndCount()).toEqual(expectViewEnd ? 1 : 0) + }) + }) + + it('should not create a new view when ending the view on page exit', () => { + const { lifeCycle } = setupBuilder.build() + const { getViewCreateCount } = viewTest - expect(getViewUpdateCount()).toEqual(2) - expect(getViewUpdate(0).id).not.toBe(getViewUpdate(1).id) + expect(getViewCreateCount()).toEqual(1) + + lifeCycle.notify(LifeCycleEventType.PAGE_EXITED, { reason: PageExitReason.UNLOADING }) + + expect(getViewCreateCount()).toEqual(1) + }) }) }) @@ -572,6 +694,21 @@ describe('view custom timings', () => { }) expect(displaySpy).toHaveBeenCalled() }) + + it('should not add custom timing when the session has expired', () => { + const { clock, lifeCycle } = setupBuilder.build() + const { getViewUpdateCount, addTiming } = viewTest + + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + + expect(getViewUpdateCount()).toBe(2) + + addTiming('foo', relativeNow()) + + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + expect(getViewUpdateCount()).toBe(2) + }) }) describe('start view', () => { diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts index bc530531c5..896641834e 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts @@ -1,5 +1,6 @@ import type { Duration, ClocksState, TimeStamp, Observable, Subscription, RelativeTime } from '@datadog/browser-core' import { + noop, PageExitReason, shallowClone, assign, @@ -77,84 +78,46 @@ export function trackViews( areViewsTrackedAutomatically: boolean, initialViewOptions?: ViewOptions ) { - const { stop: stopInitialViewTracking, initialView } = trackInitialView(initialViewOptions) - let currentView = initialView + let currentView = startNewView(ViewLoadingType.INITIAL_LOAD, clocksOrigin(), initialViewOptions) - const { stop: stopViewLifeCycle } = startViewLifeCycle() + startViewLifeCycle() let locationChangeSubscription: Subscription if (areViewsTrackedAutomatically) { locationChangeSubscription = renewViewOnLocationChange(locationChangeObservable) } - function trackInitialView(options?: ViewOptions) { - const initialView = newView( - lifeCycle, - domMutationObservable, - configuration, - location, - ViewLoadingType.INITIAL_LOAD, - clocksOrigin(), - options - ) - const { stop } = trackInitialViewTimings(lifeCycle, (timings) => { - initialView.updateTimings(timings) - initialView.scheduleUpdate() - }) - return { initialView, stop } - } - - function trackViewChange(startClocks?: ClocksState, viewOptions?: ViewOptions) { - return newView( - lifeCycle, - domMutationObservable, - configuration, - location, - ViewLoadingType.ROUTE_CHANGE, - startClocks, - viewOptions - ) + function startNewView(loadingType: ViewLoadingType, startClocks?: ClocksState, viewOptions?: ViewOptions) { + return newView(lifeCycle, domMutationObservable, configuration, location, loadingType, startClocks, viewOptions) } function startViewLifeCycle() { lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => { - // do not trigger view update to avoid wrong data - currentView.end() // Renew view on session renewal - currentView = trackViewChange(undefined, { + currentView = startNewView(ViewLoadingType.ROUTE_CHANGE, undefined, { name: currentView.name, service: currentView.service, version: currentView.version, }) }) + lifeCycle.subscribe(LifeCycleEventType.SESSION_EXPIRED, () => { + currentView.end() + }) + // End the current view on page unload lifeCycle.subscribe(LifeCycleEventType.PAGE_EXITED, (pageExitEvent) => { if (pageExitEvent.reason === PageExitReason.UNLOADING || pageExitEvent.reason === PageExitReason.PAGEHIDE) { currentView.end() - currentView.triggerUpdate() } }) - - // Session keep alive - const keepAliveInterval = setInterval(() => { - currentView.triggerUpdate() - }, SESSION_KEEP_ALIVE_INTERVAL) - - return { - stop: () => { - clearInterval(keepAliveInterval) - }, - } } function renewViewOnLocationChange(locationChangeObservable: Observable) { return locationChangeObservable.subscribe(({ oldLocation, newLocation }) => { if (areDifferentLocation(oldLocation, newLocation)) { currentView.end() - currentView.triggerUpdate() - currentView = trackViewChange() - return + currentView = startNewView(ViewLoadingType.ROUTE_CHANGE) } }) } @@ -162,17 +125,13 @@ export function trackViews( return { addTiming: (name: string, time: RelativeTime | TimeStamp = timeStampNow()) => { currentView.addTiming(name, time) - currentView.scheduleUpdate() }, startView: (options?: ViewOptions, startClocks?: ClocksState) => { currentView.end(startClocks) - currentView.triggerUpdate() - currentView = trackViewChange(startClocks, options) + currentView = startNewView(ViewLoadingType.ROUTE_CHANGE, startClocks, options) }, stop: () => { locationChangeSubscription?.unsubscribe() - stopInitialViewTracking() - stopViewLifeCycle() currentView.end() }, } @@ -189,7 +148,6 @@ function newView( ) { // Setup initial values const id = generateUUID() - let timings: Timings = {} const customTimings: ViewCustomTimings = {} let documentVersion = 0 let endClocks: ClocksState | undefined @@ -227,16 +185,26 @@ function newView( viewMetrics, } = trackViewMetrics(lifeCycle, domMutationObservable, configuration, scheduleViewUpdate, loadingType, startClocks) + const { scheduleStop: scheduleStopInitialViewTimingsTracking, timings } = + loadingType === ViewLoadingType.INITIAL_LOAD + ? trackInitialViewTimings(lifeCycle, setLoadEvent, scheduleViewUpdate) + : { scheduleStop: noop, timings: {} as Timings } + const { scheduleStop: scheduleStopEventCountsTracking, eventCounts } = trackViewEventCounts( lifeCycle, id, scheduleViewUpdate ) + // Session keep alive + const keepAliveIntervalId = setInterval(triggerViewUpdate, SESSION_KEEP_ALIVE_INTERVAL) + // Initial view update triggerViewUpdate() function triggerViewUpdate() { + cancelScheduleViewUpdate() + documentVersion += 1 const currentEnd = endClocks === undefined ? timeStampNow() : endClocks.timeStamp lifeCycle.notify( @@ -266,27 +234,26 @@ function newView( name, service, version, - scheduleUpdate: scheduleViewUpdate, end(clocks = clocksNow()) { + if (endClocks) { + // view already ended + return + } endClocks = clocks lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, { endClocks }) + clearInterval(keepAliveIntervalId) stopViewMetricsTracking() + scheduleStopInitialViewTimingsTracking() scheduleStopEventCountsTracking() - }, - triggerUpdate() { - // cancel any pending view updates execution - cancelScheduleViewUpdate() triggerViewUpdate() }, - updateTimings(newTimings: Timings) { - timings = newTimings - if (newTimings.loadEvent !== undefined) { - setLoadEvent(newTimings.loadEvent) - } - }, addTiming(name: string, time: RelativeTime | TimeStamp) { + if (endClocks) { + return + } const relativeTime = looksLikeRelativeTime(time) ? time : elapsed(startClocks.timeStamp, time) customTimings[sanitizeTiming(name)] = relativeTime + scheduleViewUpdate() }, } } From e4bb64da1845f3a86536a7fdf554a8176e99a81e Mon Sep 17 00:00:00 2001 From: Jeremy Comte Date: Wed, 19 Apr 2023 15:39:40 +0200 Subject: [PATCH 2/7] Create repository.datadog.yml (#2176) * Create repository.datadog.yml * Update repository.datadog.yml * Update repository.datadog.yml --- repository.datadog.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 repository.datadog.yml diff --git a/repository.datadog.yml b/repository.datadog.yml new file mode 100644 index 0000000000..0db33d0f85 --- /dev/null +++ b/repository.datadog.yml @@ -0,0 +1,6 @@ +--- +schema-version: v1 +kind: mergequeue +team: rum-browser +enable: true +to_staging_enable: true From cf429f8b5b1881d0df31fa217dd68889a998d4ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt?= Date: Thu, 20 Apr 2023 19:40:23 +0200 Subject: [PATCH 3/7] =?UTF-8?q?=E2=9C=A8=20[RUMF-1534]=20send=20a=20view?= =?UTF-8?q?=20update=20when=20session=20is=20expiring=20(#2166)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🏷️ [RUMF-1534] update `rum-events-format` * ✨ [RUMF-1534] define a session.is_active property on Views * ✨ [RUMF-1534] send a view update when session is expiring * ✅ [RUMF-1534] add e2e assertion --- .../domain/telemetry/telemetryEvent.types.ts | 12 ++++++++++++ packages/rum-core/src/domain/assembly.spec.ts | 1 + .../view/trackViews.spec.ts | 4 +++- .../rumEventsCollection/view/trackViews.ts | 13 +++++++++---- .../view/viewCollection.spec.ts | 8 ++++++++ .../view/viewCollection.ts | 1 + packages/rum-core/src/rawRumEvent.types.ts | 1 + packages/rum-core/src/rumEvent.types.ts | 19 +++++++++++++++++++ packages/rum-core/test/fixtures.ts | 1 + rum-events-format | 2 +- test/e2e/scenario/rum/sessions.scenario.ts | 1 + 11 files changed, 57 insertions(+), 6 deletions(-) diff --git a/packages/core/src/domain/telemetry/telemetryEvent.types.ts b/packages/core/src/domain/telemetry/telemetryEvent.types.ts index 1ffef1e012..c4877937cc 100644 --- a/packages/core/src/domain/telemetry/telemetryEvent.types.ts +++ b/packages/core/src/domain/telemetry/telemetryEvent.types.ts @@ -265,6 +265,18 @@ export type TelemetryConfigurationEvent = CommonTelemetryProperties & { * The upload frequency of batches (in milliseconds) */ batch_upload_frequency?: number + /** + * The version of React used in a ReactNative application + */ + react_version?: string + /** + * The version of ReactNative used in a ReactNative application + */ + react_native_version?: string + /** + * The version of Dart used in a Flutter application + */ + dart_version?: string [k: string]: unknown } [k: string]: unknown diff --git a/packages/rum-core/src/domain/assembly.spec.ts b/packages/rum-core/src/domain/assembly.spec.ts index 8414c3dadd..6f360fbcf9 100644 --- a/packages/rum-core/src/domain/assembly.spec.ts +++ b/packages/rum-core/src/domain/assembly.spec.ts @@ -544,6 +544,7 @@ describe('rum assembly', () => { }) expect(serverRumEvents[0].session).toEqual({ has_replay: undefined, + is_active: undefined, id: '1234', type: 'user', }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts index 02a4b514f8..5531b7f1fd 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts @@ -315,13 +315,15 @@ describe('view lifecycle', () => { it('should send a final view update', () => { const { lifeCycle } = setupBuilder.build() - const { getViewUpdateCount } = viewTest + const { getViewUpdateCount, getViewUpdate } = viewTest expect(getViewUpdateCount()).toBe(1) lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) expect(getViewUpdateCount()).toBe(2) + expect(getViewUpdate(0).sessionIsActive).toBe(true) + expect(getViewUpdate(1).sessionIsActive).toBe(false) }) it('should not start a new view if the session expired', () => { diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts index 896641834e..32713a5d03 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts @@ -43,6 +43,7 @@ export interface ViewEvent { startClocks: ClocksState duration: Duration isActive: boolean + sessionIsActive: boolean loadingTime?: Duration loadingType: ViewLoadingType cumulativeLayoutShift?: number @@ -102,7 +103,7 @@ export function trackViews( }) lifeCycle.subscribe(LifeCycleEventType.SESSION_EXPIRED, () => { - currentView.end() + currentView.end({ sessionIsActive: false }) }) // End the current view on page unload @@ -127,7 +128,7 @@ export function trackViews( currentView.addTiming(name, time) }, startView: (options?: ViewOptions, startClocks?: ClocksState) => { - currentView.end(startClocks) + currentView.end({ endClocks: startClocks }) currentView = startNewView(ViewLoadingType.ROUTE_CHANGE, startClocks, options) }, stop: () => { @@ -153,6 +154,7 @@ function newView( let endClocks: ClocksState | undefined const location = shallowClone(initialLocation) + let sessionIsActive = true let name: string | undefined let service: string | undefined let version: string | undefined @@ -223,6 +225,7 @@ function newView( timings, duration: elapsed(startClocks.timeStamp, currentEnd), isActive: endClocks === undefined, + sessionIsActive, eventCounts, }, viewMetrics @@ -234,12 +237,14 @@ function newView( name, service, version, - end(clocks = clocksNow()) { + end(options: { endClocks?: ClocksState; sessionIsActive?: boolean } = {}) { if (endClocks) { // view already ended return } - endClocks = clocks + endClocks = options.endClocks ?? clocksNow() + sessionIsActive = options.sessionIsActive ?? true + lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, { endClocks }) clearInterval(keepAliveIntervalId) stopViewMetricsTracking() diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts index 08370f2503..1abc8cd0e9 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts @@ -41,6 +41,7 @@ const VIEW: ViewEvent = { largestContentfulPaint: 10 as Duration, loadEvent: 10 as Duration, }, + sessionIsActive: true, } describe('viewCollection', () => { @@ -134,11 +135,18 @@ describe('viewCollection', () => { }, session: { has_replay: undefined, + is_active: undefined, }, feature_flags: undefined, }) }) + it('should set session.is_active to false if the session is inactive', () => { + const { lifeCycle, rawRumEvents } = setupBuilder.build() + lifeCycle.notify(LifeCycleEventType.VIEW_UPDATED, { ...VIEW, sessionIsActive: false }) + expect((rawRumEvents[rawRumEvents.length - 1].rawRumEvent as RawRumViewEvent).session.is_active).toBe(false) + }) + it('should include replay information if available', () => { const { lifeCycle, rawRumEvents } = setupBuilder.build() getReplayStatsSpy.and.returnValue({ segments_count: 4, records_count: 10, segments_total_raw_size: 1000 }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts index e38efbcc25..813fc8bd30 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts @@ -93,6 +93,7 @@ function processViewUpdate( feature_flags: featureFlagContext && !isEmptyObject(featureFlagContext) ? featureFlagContext : undefined, session: { has_replay: replayStats ? true : undefined, + is_active: view.sessionIsActive ? undefined : false, }, } if (!isEmptyObject(view.customTimings)) { diff --git a/packages/rum-core/src/rawRumEvent.types.ts b/packages/rum-core/src/rawRumEvent.types.ts index 8dd7ca4349..e2820bf66e 100644 --- a/packages/rum-core/src/rawRumEvent.types.ts +++ b/packages/rum-core/src/rawRumEvent.types.ts @@ -103,6 +103,7 @@ export interface RawRumViewEvent { } session: { has_replay: true | undefined + is_active: false | undefined } feature_flags?: Context _dd: { diff --git a/packages/rum-core/src/rumEvent.types.ts b/packages/rum-core/src/rumEvent.types.ts index 9d50019839..d4d342b01d 100644 --- a/packages/rum-core/src/rumEvent.types.ts +++ b/packages/rum-core/src/rumEvent.types.ts @@ -731,6 +731,25 @@ export type RumViewEvent = CommonProperties & { js_refresh_rate?: RumPerfMetric [k: string]: unknown } + /** + * Session properties + */ + readonly session?: { + /** + * The precondition that led to the creation of the session + */ + readonly start_precondition?: + | 'app_launch' + | 'inactivity_timeout' + | 'max_duration' + | 'explicit_stop' + | 'background_event' + /** + * Whether this session is currently active. Set to false to manually stop a session + */ + readonly is_active?: boolean + [k: string]: unknown + } /** * Feature flags properties */ diff --git a/packages/rum-core/test/fixtures.ts b/packages/rum-core/test/fixtures.ts index fb9eb2e119..deb22f9417 100644 --- a/packages/rum-core/test/fixtures.ts +++ b/packages/rum-core/test/fixtures.ts @@ -90,6 +90,7 @@ export function createRawRumEvent(type: RumEventType, overrides?: Context): RawR }, session: { has_replay: undefined, + is_active: undefined, }, }, overrides diff --git a/rum-events-format b/rum-events-format index de60449321..8d4141efa8 160000 --- a/rum-events-format +++ b/rum-events-format @@ -1 +1 @@ -Subproject commit de604493217a0acc08eb0da39ba02b603608e135 +Subproject commit 8d4141efa806ab2bc9ba910b2d1a396fa177381a diff --git a/test/e2e/scenario/rum/sessions.scenario.ts b/test/e2e/scenario/rum/sessions.scenario.ts index 3b4f8945ec..c912930dbf 100644 --- a/test/e2e/scenario/rum/sessions.scenario.ts +++ b/test/e2e/scenario/rum/sessions.scenario.ts @@ -112,6 +112,7 @@ describe('rum sessions', () => { await waitForRequests() expect(serverEvents.rumViews.length).toBe(1) + expect(serverEvents.rumViews[0].session.is_active).toBe(false) expect(serverEvents.logs.length).toBe(1) expect(serverEvents.sessionReplay.length).toBe(1) }) From fb1c78e7833c0acde3e74db0e869d124ea39e324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt?= Date: Fri, 21 Apr 2023 10:28:54 +0200 Subject: [PATCH 4/7] =?UTF-8?q?=E2=9C=A8=20[RUMF-1479]=20enable=20heatmaps?= =?UTF-8?q?=20collection=20(#2178)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../core/src/tools/experimentalFeatures.ts | 1 - .../domain/contexts/displayContext.spec.ts | 10 +---- .../src/domain/contexts/displayContext.ts | 5 --- .../action/trackClickActions.spec.ts | 44 +++---------------- .../action/trackClickActions.ts | 24 +++------- test/e2e/scenario/rum/actions.scenario.ts | 6 +-- 6 files changed, 18 insertions(+), 72 deletions(-) diff --git a/packages/core/src/tools/experimentalFeatures.ts b/packages/core/src/tools/experimentalFeatures.ts index fb6e666db6..76b12f532c 100644 --- a/packages/core/src/tools/experimentalFeatures.ts +++ b/packages/core/src/tools/experimentalFeatures.ts @@ -14,7 +14,6 @@ export enum ExperimentalFeature { PAGEHIDE = 'pagehide', FEATURE_FLAGS = 'feature_flags', RESOURCE_PAGE_STATES = 'resource_page_states', - CLICKMAP = 'clickmap', COLLECT_FLUSH_REASON = 'collect_flush_reason', SANITIZE_INPUTS = 'sanitize_inputs', REPLAY_JSON_PAYLOAD = 'replay_json_payload', diff --git a/packages/rum-core/src/domain/contexts/displayContext.spec.ts b/packages/rum-core/src/domain/contexts/displayContext.spec.ts index a600ff61b9..106844e04c 100644 --- a/packages/rum-core/src/domain/contexts/displayContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/displayContext.spec.ts @@ -1,15 +1,11 @@ -import { ExperimentalFeature, resetExperimentalFeatures, addExperimentalFeatures } from '@datadog/browser-core' import { getDisplayContext, resetDisplayContext } from './displayContext' describe('displayContext', () => { afterEach(() => { - resetExperimentalFeatures() resetDisplayContext() }) - it('should return current display context when ff enabled', () => { - addExperimentalFeatures([ExperimentalFeature.CLICKMAP]) - + it('should return current display context', () => { expect(getDisplayContext()).toEqual({ viewport: { width: jasmine.any(Number), @@ -17,8 +13,4 @@ describe('displayContext', () => { }, }) }) - - it('should not return current display context when ff disabled', () => { - expect(getDisplayContext()).not.toBeDefined() - }) }) diff --git a/packages/rum-core/src/domain/contexts/displayContext.ts b/packages/rum-core/src/domain/contexts/displayContext.ts index b5f4a5c9e5..8ecb43a179 100644 --- a/packages/rum-core/src/domain/contexts/displayContext.ts +++ b/packages/rum-core/src/domain/contexts/displayContext.ts @@ -1,14 +1,9 @@ -import { ExperimentalFeature, isExperimentalFeatureEnabled } from '@datadog/browser-core' import { getViewportDimension, initViewportObservable } from '../../browser/viewportObservable' let viewport: { width: number; height: number } | undefined let stopListeners: (() => void) | undefined export function getDisplayContext() { - if (!isExperimentalFeatureEnabled(ExperimentalFeature.CLICKMAP)) { - return - } - if (!viewport) { viewport = getViewportDimension() stopListeners = initViewportObservable().subscribe((viewportDimension) => { diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts index 378cfde5bd..198d89a428 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts @@ -1,13 +1,5 @@ import type { Context, Duration } from '@datadog/browser-core' -import { - addDuration, - addExperimentalFeatures, - resetExperimentalFeatures, - clocksNow, - timeStampNow, - relativeNow, - ExperimentalFeature, -} from '@datadog/browser-core' +import { addDuration, clocksNow, timeStampNow, relativeNow } from '@datadog/browser-core' import { createNewEvent } from '@datadog/browser-core/test' import type { TestSetupBuilder } from '../../../../test' import { setup, createFakeClick } from '../../../../test' @@ -105,39 +97,17 @@ describe('trackClickActions', () => { type: ActionType.CLICK, event: domEvent, frustrationTypes: [], - target: undefined, - position: undefined, + target: { + selector: '#button', + width: 100, + height: 100, + }, + position: { x: 50, y: 50 }, events: [domEvent], }, ]) }) - describe('when clickmap ff is enabled', () => { - beforeEach(() => { - addExperimentalFeatures([ExperimentalFeature.CLICKMAP]) - }) - - afterEach(() => { - resetExperimentalFeatures() - }) - - it('should set click position and target', () => { - const { clock } = setupBuilder.build() - emulateClick({ activity: {} }) - clock.tick(EXPIRE_DELAY) - expect(events[0]).toEqual( - jasmine.objectContaining({ - target: { - selector: '#button', - width: 100, - height: 100, - }, - position: { x: 50, y: 50 }, - }) - ) - }) - }) - it('should keep track of previously validated click actions', () => { const { clock } = setupBuilder.build() const pointerDownStart = relativeNow() diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts index b7712dd40e..357c8d6b4e 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts @@ -2,7 +2,6 @@ import type { Duration, ClocksState, RelativeTime, TimeStamp } from '@datadog/br import { includes, timeStampNow, - isExperimentalFeatureEnabled, Observable, assign, getRelativeTime, @@ -12,7 +11,6 @@ import { clocksNow, ONE_SECOND, elapsed, - ExperimentalFeature, } from '@datadog/browser-core' import type { FrustrationType } from '../../../rawRumEvent.types' import { ActionType } from '../../../rawRumEvent.types' @@ -240,27 +238,19 @@ function startClickAction( type ClickActionBase = Pick function computeClickActionBase(event: MouseEventOnElement, actionNameAttribute?: string): ClickActionBase { - let target: ClickAction['target'] - let position: ClickAction['position'] - - if (isExperimentalFeatureEnabled(ExperimentalFeature.CLICKMAP)) { - const rect = event.target.getBoundingClientRect() - target = { + const rect = event.target.getBoundingClientRect() + return { + type: ActionType.CLICK, + target: { width: Math.round(rect.width), height: Math.round(rect.height), selector: getSelectorFromElement(event.target, actionNameAttribute), - } - position = { + }, + position: { // Use clientX and Y because for SVG element offsetX and Y are relatives to the element x: Math.round(event.clientX - rect.left), y: Math.round(event.clientY - rect.top), - } - } - - return { - type: ActionType.CLICK, - target, - position, + }, name: getActionNameFromElement(event.target, actionNameAttribute), } } diff --git a/test/e2e/scenario/rum/actions.scenario.ts b/test/e2e/scenario/rum/actions.scenario.ts index 646482c96d..c3e8c288e6 100644 --- a/test/e2e/scenario/rum/actions.scenario.ts +++ b/test/e2e/scenario/rum/actions.scenario.ts @@ -3,7 +3,7 @@ import { createTest, flushEvents, html, waitForServersIdle } from '../../lib/fra describe('action collection', () => { createTest('track a click action') - .withRum({ trackUserInteractions: true, enableExperimentalFeatures: ['clickmap'] }) + .withRum({ trackUserInteractions: true }) .withBody( html` @@ -62,7 +62,7 @@ describe('action collection', () => { }) createTest('compute action target information before the UI changes') - .withRum({ trackFrustrations: true, enableExperimentalFeatures: ['clickmap'] }) + .withRum({ trackFrustrations: true }) .withBody( html` @@ -91,7 +91,7 @@ describe('action collection', () => { // click event. Skip this test. if (getBrowserName() !== 'firefox') { createTest('does not report a click on the body when the target element changes between mousedown and mouseup') - .withRum({ trackFrustrations: true, enableExperimentalFeatures: ['clickmap'] }) + .withRum({ trackFrustrations: true }) .withBody( html` From 469043325c862b8e51bc6efcc8d1cc63ff6e89b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt?= Date: Fri, 21 Apr 2023 10:29:01 +0200 Subject: [PATCH 5/7] =?UTF-8?q?=E2=9C=A8=20[RUMF-1530]=20enable=20sending?= =?UTF-8?q?=20replay=20metadata=20as=20json=20(#2177)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 6d439e56cc5f807e75096f41d65a2e2258cc3a8a. --- .../core/src/tools/experimentalFeatures.ts | 1 - packages/rum/src/boot/startRecording.spec.ts | 1 + .../buildReplayPayload.spec.ts | 77 +++---------------- .../segmentCollection/buildReplayPayload.ts | 35 +++------ packages/rum/test/readReplayPayload.ts | 22 +----- test/e2e/lib/framework/serverApps/intake.ts | 19 ++--- test/e2e/lib/types/serverEvents.ts | 5 +- .../scenario/recorder/recorder.scenario.ts | 71 +---------------- 8 files changed, 30 insertions(+), 201 deletions(-) diff --git a/packages/core/src/tools/experimentalFeatures.ts b/packages/core/src/tools/experimentalFeatures.ts index 76b12f532c..45cbb18250 100644 --- a/packages/core/src/tools/experimentalFeatures.ts +++ b/packages/core/src/tools/experimentalFeatures.ts @@ -16,7 +16,6 @@ export enum ExperimentalFeature { RESOURCE_PAGE_STATES = 'resource_page_states', COLLECT_FLUSH_REASON = 'collect_flush_reason', SANITIZE_INPUTS = 'sanitize_inputs', - REPLAY_JSON_PAYLOAD = 'replay_json_payload', } const enabledExperimentalFeatures: Set = new Set() diff --git a/packages/rum/src/boot/startRecording.spec.ts b/packages/rum/src/boot/startRecording.spec.ts index c92b55a301..ba0edfcf8d 100644 --- a/packages/rum/src/boot/startRecording.spec.ts +++ b/packages/rum/src/boot/startRecording.spec.ts @@ -91,6 +91,7 @@ describe('startRecording', () => { }, start: jasmine.any(Number), raw_segment_size: jasmine.any(Number), + compressed_segment_size: jasmine.any(Number), view: { id: 'view-id', }, diff --git a/packages/rum/src/domain/segmentCollection/buildReplayPayload.spec.ts b/packages/rum/src/domain/segmentCollection/buildReplayPayload.spec.ts index 1bbf57d06f..f5c630a700 100644 --- a/packages/rum/src/domain/segmentCollection/buildReplayPayload.spec.ts +++ b/packages/rum/src/domain/segmentCollection/buildReplayPayload.spec.ts @@ -1,8 +1,8 @@ import pako from 'pako' -import { addExperimentalFeatures, ExperimentalFeature, isIE, resetExperimentalFeatures } from '@datadog/browser-core' +import { isIE } from '@datadog/browser-core' import type { BrowserSegment, BrowserSegmentMetadata } from '../../types' import { readReplayPayload } from '../../../test' -import { buildReplayPayload, toFormEntries } from './buildReplayPayload' +import { buildReplayPayload } from './buildReplayPayload' describe('buildReplayPayload', () => { const SEGMENT = { foo: 'bar' } as unknown as BrowserSegment @@ -40,40 +40,14 @@ describe('buildReplayPayload', () => { expect(segment).toEqual(SEGMENT) }) - describe('with replay_json_payload experimental flag', () => { - beforeEach(() => { - addExperimentalFeatures([ExperimentalFeature.REPLAY_JSON_PAYLOAD]) - }) - - afterEach(() => { - resetExperimentalFeatures() - }) - - it('adds the metadata plus the segment sizes as the `event` entry', async () => { - const payload = buildReplayPayload(COMPRESSED_SEGMENT, METADATA, SERIALIZED_SEGMENT.length) - const eventEntry = (payload.data as FormData).get('event')! as File - expect(eventEntry.size).toBe(JSON.stringify(METADATA_AND_SEGMENT_SIZES).length) - expect(eventEntry.name).toBe('blob') - expect(eventEntry.type).toBe('application/json') - const { metadata } = await readReplayPayload(payload) - expect(metadata).toEqual(METADATA_AND_SEGMENT_SIZES) - }) - }) - - describe('without replay_json_payload experimental flag', () => { - it('adds the metadata plus the segment sizes as the `event` entry', () => { - const payload = buildReplayPayload(COMPRESSED_SEGMENT, METADATA, SERIALIZED_SEGMENT.length) - const formData = payload.data as FormData - expect(formData.get('application.id')).toBe(METADATA.application.id) - expect(formData.get('session.id')).toBe(METADATA.session.id) - expect(formData.get('view.id')).toBe(METADATA.view.id) - expect(formData.get('start')).toBe(String(METADATA.start)) - expect(formData.get('end')).toBe(String(METADATA.end)) - expect(formData.get('records_count')).toBe(String(METADATA.records_count)) - expect(formData.get('source')).toBe(METADATA.source) - expect(formData.get('creation_reason')).toBe(METADATA.creation_reason) - expect(formData.get('raw_segment_size')).toBe(String(SERIALIZED_SEGMENT.length)) - }) + it('adds the metadata plus the segment sizes as the `event` entry', async () => { + const payload = buildReplayPayload(COMPRESSED_SEGMENT, METADATA, SERIALIZED_SEGMENT.length) + const eventEntry = (payload.data as FormData).get('event')! as File + expect(eventEntry.size).toBe(JSON.stringify(METADATA_AND_SEGMENT_SIZES).length) + expect(eventEntry.name).toBe('blob') + expect(eventEntry.type).toBe('application/json') + const { metadata } = await readReplayPayload(payload) + expect(metadata).toEqual(METADATA_AND_SEGMENT_SIZES) }) it('returns the approximate byte counts of the request', () => { @@ -81,34 +55,3 @@ describe('buildReplayPayload', () => { expect(payload.bytesCount).toBe(COMPRESSED_SEGMENT.byteLength) }) }) - -describe('toFormEntries', () => { - let callbackSpy: jasmine.Spy<(key: string, value: string) => void> - beforeEach(() => { - callbackSpy = jasmine.createSpy() - }) - - it('handles top level properties', () => { - toFormEntries({ foo: 'bar', zig: 'zag' }, callbackSpy) - expect(callbackSpy.calls.allArgs()).toEqual([ - ['foo', 'bar'], - ['zig', 'zag'], - ]) - }) - - it('handles nested properties', () => { - toFormEntries({ foo: { bar: 'baz', zig: { zag: 'zug' } } }, callbackSpy) - expect(callbackSpy.calls.allArgs()).toEqual([ - ['foo.bar', 'baz'], - ['foo.zig.zag', 'zug'], - ]) - }) - - it('converts values to string', () => { - toFormEntries({ foo: 42, bar: null }, callbackSpy) - expect(callbackSpy.calls.allArgs()).toEqual([ - ['foo', '42'], - ['bar', 'null'], - ]) - }) -}) diff --git a/packages/rum/src/domain/segmentCollection/buildReplayPayload.ts b/packages/rum/src/domain/segmentCollection/buildReplayPayload.ts index daa0328bcf..0fa759fe5c 100644 --- a/packages/rum/src/domain/segmentCollection/buildReplayPayload.ts +++ b/packages/rum/src/domain/segmentCollection/buildReplayPayload.ts @@ -1,5 +1,5 @@ import type { Payload } from '@datadog/browser-core' -import { ExperimentalFeature, isExperimentalFeatureEnabled, objectEntries, assign } from '@datadog/browser-core' +import { assign } from '@datadog/browser-core' import type { BrowserSegmentMetadata } from '../../types' export type BrowserSegmentMetadataAndSegmentSizes = BrowserSegmentMetadata & { @@ -22,30 +22,15 @@ export function buildReplayPayload( `${metadata.session.id}-${metadata.start}` ) - if (isExperimentalFeatureEnabled(ExperimentalFeature.REPLAY_JSON_PAYLOAD)) { - const metadataAndSegmentSizes: BrowserSegmentMetadataAndSegmentSizes = assign( - { - raw_segment_size: rawSegmentBytesCount, - compressed_segment_size: data.byteLength, - }, - metadata - ) - const serializedMetadataAndSegmentSizes = JSON.stringify(metadataAndSegmentSizes) - formData.append('event', new Blob([serializedMetadataAndSegmentSizes], { type: 'application/json' })) - } else { - toFormEntries(metadata, (key, value) => formData.append(key, value)) - formData.append('raw_segment_size', rawSegmentBytesCount.toString()) - } + const metadataAndSegmentSizes: BrowserSegmentMetadataAndSegmentSizes = assign( + { + raw_segment_size: rawSegmentBytesCount, + compressed_segment_size: data.byteLength, + }, + metadata + ) + const serializedMetadataAndSegmentSizes = JSON.stringify(metadataAndSegmentSizes) + formData.append('event', new Blob([serializedMetadataAndSegmentSizes], { type: 'application/json' })) return { data: formData, bytesCount: data.byteLength } } - -export function toFormEntries(input: object, onEntry: (key: string, value: string) => void, prefix = '') { - objectEntries(input as { [key: string]: unknown }).forEach(([key, value]) => { - if (typeof value === 'object' && value !== null) { - toFormEntries(value, onEntry, `${prefix}${key}.`) - } else { - onEntry(`${prefix}${key}`, String(value)) - } - }) -} diff --git a/packages/rum/test/readReplayPayload.ts b/packages/rum/test/readReplayPayload.ts index 40ea969ffc..a60bc2234a 100644 --- a/packages/rum/test/readReplayPayload.ts +++ b/packages/rum/test/readReplayPayload.ts @@ -1,7 +1,7 @@ import pako from 'pako' import type { Payload } from '@datadog/browser-core' -import type { BrowserSegment, CreationReason } from '../src/types' +import type { BrowserSegment } from '../src/types' import type { BrowserSegmentMetadataAndSegmentSizes } from '../src/domain/segmentCollection' export async function readReplayPayload(payload: Payload) { @@ -18,26 +18,6 @@ function readSegmentFromReplayPayload(payload: Payload) { } export function readMetadataFromReplayPayload(payload: Payload) { - const formData = payload.data as FormData - if (!formData.has('event')) { - // TODO remove this when replay_json_payload is enabled - return { - application: { id: formData.get('application.id') as string }, - session: { id: formData.get('session.id') as string }, - view: { id: formData.get('view.id') as string }, - start: Number(formData.get('start')), - end: Number(formData.get('end')), - records_count: Number(formData.get('records_count')), - source: formData.get('source') as 'browser', - creation_reason: formData.get('creation_reason') as CreationReason, - raw_segment_size: Number(formData.get('raw_segment_size')), - index_in_view: Number(formData.get('index_in_view')), - has_full_snapshot: formData.get('has_full_snapshot') === 'true', - } satisfies Omit & { - compressed_segment_size?: number - } - } - return readJsonBlob((payload.data as FormData).get('event') as Blob) as Promise } diff --git a/test/e2e/lib/framework/serverApps/intake.ts b/test/e2e/lib/framework/serverApps/intake.ts index d16e967148..d48722f5c0 100644 --- a/test/e2e/lib/framework/serverApps/intake.ts +++ b/test/e2e/lib/framework/serverApps/intake.ts @@ -95,12 +95,7 @@ function forwardEventsToIntake(req: express.Request): Promise { function storeReplayData(req: express.Request, events: EventRegistry): Promise { return new Promise((resolve, reject) => { let segmentPromise: Promise - let metadataFromJsonPayloadPromise: Promise - - // TODO: remove this when enabling replay_json_payload - const metadataFromMultipartFields: { - [field: string]: string - } = {} + let metadataPromise: Promise req.busboy.on('file', (name, stream, info) => { const { filename, encoding, mimeType } = info @@ -112,20 +107,16 @@ function storeReplayData(req: express.Request, events: EventRegistry): Promise JSON.parse(data.toString()) as BrowserSegmentMetadataAndSegmentSizes ) } }) - req.busboy.on('field', (key: string, value: string) => { - metadataFromMultipartFields[key] = value - }) - req.busboy.on('finish', () => { - Promise.all([segmentPromise, metadataFromJsonPayloadPromise]) - .then(([segment, metadataFromJsonPayload]) => { - events.push('sessionReplay', { metadata: metadataFromJsonPayload || metadataFromMultipartFields, segment }) + Promise.all([segmentPromise, metadataPromise]) + .then(([segment, metadata]) => { + events.push('sessionReplay', { metadata, segment }) }) .then(resolve) .catch((e) => reject(e)) diff --git a/test/e2e/lib/types/serverEvents.ts b/test/e2e/lib/types/serverEvents.ts index 75ee26a288..f6a3cddd34 100644 --- a/test/e2e/lib/types/serverEvents.ts +++ b/test/e2e/lib/types/serverEvents.ts @@ -36,8 +36,5 @@ export interface SegmentFile { export interface SessionReplayCall { segment: SegmentFile - metadata: - | BrowserSegmentMetadataAndSegmentSizes - // TODO: remove this when enabling replay_json_payload - | Record + metadata: BrowserSegmentMetadataAndSegmentSizes } diff --git a/test/e2e/scenario/recorder/recorder.scenario.ts b/test/e2e/scenario/recorder/recorder.scenario.ts index bd2987020a..bead027797 100644 --- a/test/e2e/scenario/recorder/recorder.scenario.ts +++ b/test/e2e/scenario/recorder/recorder.scenario.ts @@ -1,10 +1,4 @@ -import type { - InputData, - StyleSheetRuleData, - BrowserSegment, - ScrollData, - CreationReason, -} from '@datadog/browser-rum/src/types' +import type { InputData, StyleSheetRuleData, BrowserSegment, ScrollData } from '@datadog/browser-rum/src/types' import { NodeType, IncrementalSource, MouseInteractionType } from '@datadog/browser-rum/src/types' // Import from src to have properties of const enums @@ -24,14 +18,10 @@ import { findMouseInteractionRecords, findElementWithTagName, } from '@datadog/browser-rum/test' -import { ExperimentalFeature } from '@datadog/browser-core/src/tools/experimentalFeatures' -import type { BrowserSegmentMetadataAndSegmentSizes } from '@datadog/browser-rum/src/domain/segmentCollection' import { flushEvents, createTest, bundleSetup, html } from '../../lib/framework' import { browserExecute, browserExecuteAsync } from '../../lib/helpers/browser' import { getFirstSegment, getLastSegment, initRumAndStartRecording } from '../../lib/helpers/replay' -import type { SegmentFile } from '../../lib/types/serverEvents' -const INTEGER_RE = /^\d+$/ const TIMESTAMP_RE = /^\d{13}$/ const UUID_RE = /^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$/ @@ -46,64 +36,7 @@ describe('recorder', () => { await flushEvents() expect(serverEvents.sessionReplay.length).toBe(1) - const { segment, metadata } = serverEvents.sessionReplay[0] as { - segment: SegmentFile - metadata: Record - } - expect(metadata).toEqual({ - 'application.id': jasmine.stringMatching(UUID_RE), - creation_reason: 'init', - end: jasmine.stringMatching(TIMESTAMP_RE), - has_full_snapshot: 'true', - records_count: jasmine.stringMatching(INTEGER_RE), - 'session.id': jasmine.stringMatching(UUID_RE), - start: jasmine.stringMatching(TIMESTAMP_RE), - 'view.id': jasmine.stringMatching(UUID_RE), - raw_segment_size: jasmine.stringMatching(INTEGER_RE), - index_in_view: '0', - source: 'browser', - }) - expect(segment).toEqual({ - data: { - application: { id: metadata['application.id'] }, - creation_reason: metadata.creation_reason as CreationReason, - end: Number(metadata.end), - has_full_snapshot: true, - records: jasmine.any(Array), - records_count: Number(metadata.records_count), - session: { id: metadata['session.id'] }, - start: Number(metadata.start), - view: { id: metadata['view.id'] }, - index_in_view: 0, - source: 'browser', - }, - encoding: jasmine.any(String), - filename: `${metadata['session.id']}-${metadata.start}`, - mimetype: 'application/octet-stream', - }) - expect(findMeta(segment.data)).toBeTruthy('have a Meta record') - expect(findFullSnapshot(segment.data)).toBeTruthy('have a FullSnapshot record') - expect(findIncrementalSnapshot(segment.data, IncrementalSource.MouseInteraction)).toBeTruthy( - 'have a IncrementalSnapshot/MouseInteraction record' - ) - }) - - createTest('record mouse move with replay_json_payload experimental feature') - .withRum({ - enableExperimentalFeatures: [ExperimentalFeature.REPLAY_JSON_PAYLOAD], - }) - .withRumInit(initRumAndStartRecording) - .run(async ({ serverEvents }) => { - await browserExecute(() => document.documentElement.outerHTML) - const html = await $('html') - await html.click() - await flushEvents() - - expect(serverEvents.sessionReplay.length).toBe(1) - const { segment, metadata } = serverEvents.sessionReplay[0] as { - segment: SegmentFile - metadata: BrowserSegmentMetadataAndSegmentSizes - } + const { segment, metadata } = serverEvents.sessionReplay[0] expect(metadata).toEqual({ application: { id: jasmine.stringMatching(UUID_RE) }, creation_reason: 'init', From 761f97910acde7fe1c291117fcba85acdc3ba83f Mon Sep 17 00:00:00 2001 From: Bastien Caudan <1331991+bcaudan@users.noreply.github.com> Date: Fri, 21 Apr 2023 14:16:45 +0200 Subject: [PATCH 6/7] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20[RUMF-1508]=20refactor?= =?UTF-8?q?=20error=20types=20/=20constants=20(#2179)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ♻️ extract error.types * ♻️ extract NonErrorPrefix * ♻️ increase no stack message usages --- packages/core/src/domain/error/error.spec.ts | 14 ++---- packages/core/src/domain/error/error.ts | 46 ++----------------- packages/core/src/domain/error/error.types.ts | 46 +++++++++++++++++++ .../domain/error/trackRuntimeError.spec.ts | 5 +- .../src/domain/error/trackRuntimeError.ts | 7 +-- .../createEventRateLimiter.spec.ts | 2 +- .../createEventRateLimiter.ts | 4 +- .../src/domain/telemetry/telemetry.spec.ts | 5 +- .../core/src/domain/telemetry/telemetry.ts | 7 +-- packages/core/src/index.ts | 12 ++--- packages/core/src/transport/httpRequest.ts | 2 +- .../transport/sendWithRetryStrategy.spec.ts | 2 +- .../src/transport/sendWithRetryStrategy.ts | 4 +- .../src/transport/startBatchWithReplica.ts | 2 +- packages/logs/src/domain/logger.ts | 4 +- .../error/errorCollection.ts | 4 +- 16 files changed, 85 insertions(+), 81 deletions(-) create mode 100644 packages/core/src/domain/error/error.types.ts diff --git a/packages/core/src/domain/error/error.spec.ts b/packages/core/src/domain/error/error.spec.ts index ced1829ffc..1283b663a3 100644 --- a/packages/core/src/domain/error/error.spec.ts +++ b/packages/core/src/domain/error/error.spec.ts @@ -1,20 +1,14 @@ import type { StackTrace } from '../tracekit' import { clocksNow } from '../../tools/utils/timeUtils' -import type { RawErrorCause, ErrorWithCause } from './error' -import { - createHandlingStack, - computeRawError, - getFileFromStackTraceString, - flattenErrorCauses, - ErrorSource, - ErrorHandling, -} from './error' +import { createHandlingStack, computeRawError, getFileFromStackTraceString, flattenErrorCauses } from './error' +import type { RawErrorCause, ErrorWithCause } from './error.types' +import { ErrorHandling, ErrorSource, NonErrorPrefix } from './error.types' describe('computeRawError', () => { const NOT_COMPUTED_STACK_TRACE: StackTrace = { name: undefined, message: undefined, stack: [] } as any const DEFAULT_RAW_ERROR_PARAMS = { startClocks: clocksNow(), - nonErrorPrefix: 'Uncaught', + nonErrorPrefix: NonErrorPrefix.UNCAUGHT, source: ErrorSource.CUSTOM, } diff --git a/packages/core/src/domain/error/error.ts b/packages/core/src/domain/error/error.ts index a112a8b58b..3bd11aaa6a 100644 --- a/packages/core/src/domain/error/error.ts +++ b/packages/core/src/domain/error/error.ts @@ -6,49 +6,9 @@ import { sanitize } from '../../tools/serialisation/sanitize' import type { ClocksState } from '../../tools/utils/timeUtils' import { noop } from '../../tools/utils/functionUtils' import { jsonStringify } from '../../tools/serialisation/jsonStringify' +import type { ErrorSource, ErrorHandling, RawError, RawErrorCause, ErrorWithCause, NonErrorPrefix } from './error.types' export const NO_ERROR_STACK_PRESENT_MESSAGE = 'No stack, consider using an instance of Error' -export const PROVIDED_ERROR_MESSAGE_PREFIX = 'Provided' - -export interface ErrorWithCause extends Error { - cause?: Error -} - -export type RawErrorCause = { - message: string - source: string - type?: string - stack?: string -} - -export interface RawError { - startClocks: ClocksState - message: string - type?: string - stack?: string - source: ErrorSource - originalError?: unknown - handling?: ErrorHandling - handlingStack?: string - causes?: RawErrorCause[] -} - -export const ErrorSource = { - AGENT: 'agent', - CONSOLE: 'console', - CUSTOM: 'custom', - LOGGER: 'logger', - NETWORK: 'network', - SOURCE: 'source', - REPORT: 'report', -} as const - -export const enum ErrorHandling { - HANDLED = 'handled', - UNHANDLED = 'unhandled', -} - -export type ErrorSource = (typeof ErrorSource)[keyof typeof ErrorSource] type RawErrorParams = { stackTrace?: StackTrace @@ -56,7 +16,7 @@ type RawErrorParams = { handlingStack?: string startClocks: ClocksState - nonErrorPrefix: string + nonErrorPrefix: NonErrorPrefix source: ErrorSource handling: ErrorHandling } @@ -80,7 +40,7 @@ export function computeRawError({ handling, originalError: sanitizedError, message: `${nonErrorPrefix} ${jsonStringify(sanitizedError)!}`, - stack: 'No stack, consider using an instance of Error', + stack: NO_ERROR_STACK_PRESENT_MESSAGE, handlingStack, type: stackTrace && stackTrace.name, } diff --git a/packages/core/src/domain/error/error.types.ts b/packages/core/src/domain/error/error.types.ts new file mode 100644 index 0000000000..d987c0ec86 --- /dev/null +++ b/packages/core/src/domain/error/error.types.ts @@ -0,0 +1,46 @@ +import type { ClocksState } from '../../tools/utils/timeUtils' + +export interface ErrorWithCause extends Error { + cause?: Error +} + +export type RawErrorCause = { + message: string + source: string + type?: string + stack?: string +} + +export interface RawError { + startClocks: ClocksState + message: string + type?: string + stack?: string + source: ErrorSource + originalError?: unknown + handling?: ErrorHandling + handlingStack?: string + causes?: RawErrorCause[] +} + +export const ErrorSource = { + AGENT: 'agent', + CONSOLE: 'console', + CUSTOM: 'custom', + LOGGER: 'logger', + NETWORK: 'network', + SOURCE: 'source', + REPORT: 'report', +} as const + +export const enum NonErrorPrefix { + UNCAUGHT = 'Uncaught', + PROVIDED = 'Provided', +} + +export const enum ErrorHandling { + HANDLED = 'handled', + UNHANDLED = 'unhandled', +} + +export type ErrorSource = (typeof ErrorSource)[keyof typeof ErrorSource] diff --git a/packages/core/src/domain/error/trackRuntimeError.spec.ts b/packages/core/src/domain/error/trackRuntimeError.spec.ts index 6f7c4ed2ff..14ef4ad8e5 100644 --- a/packages/core/src/domain/error/trackRuntimeError.spec.ts +++ b/packages/core/src/domain/error/trackRuntimeError.spec.ts @@ -1,6 +1,7 @@ import { Observable } from '../../tools/observable' -import type { RawError } from './error' +import { NO_ERROR_STACK_PRESENT_MESSAGE } from './error' import { trackRuntimeError } from './trackRuntimeError' +import type { RawError } from './error.types' describe('runtime error tracker', () => { const ERROR_MESSAGE = 'foo' @@ -73,7 +74,7 @@ describe('runtime error tracker', () => { setTimeout(() => { const collectedError = notifyError.calls.mostRecent().args[0] as RawError expect(collectedError.message).toEqual('Uncaught {"foo":"bar"}') - expect(collectedError.stack).toEqual('No stack, consider using an instance of Error') + expect(collectedError.stack).toEqual(NO_ERROR_STACK_PRESENT_MESSAGE) done() }, 100) }) diff --git a/packages/core/src/domain/error/trackRuntimeError.ts b/packages/core/src/domain/error/trackRuntimeError.ts index dac853b203..a7f9002cb1 100644 --- a/packages/core/src/domain/error/trackRuntimeError.ts +++ b/packages/core/src/domain/error/trackRuntimeError.ts @@ -1,8 +1,9 @@ import type { Observable } from '../../tools/observable' import { clocksNow } from '../../tools/utils/timeUtils' import { startUnhandledErrorCollection } from '../tracekit' -import { ErrorSource, computeRawError, ErrorHandling } from './error' -import type { RawError } from './error' +import { computeRawError } from './error' +import type { RawError } from './error.types' +import { ErrorHandling, ErrorSource, NonErrorPrefix } from './error.types' export function trackRuntimeError(errorObservable: Observable) { return startUnhandledErrorCollection((stackTrace, originalError) => { @@ -11,7 +12,7 @@ export function trackRuntimeError(errorObservable: Observable) { stackTrace, originalError, startClocks: clocksNow(), - nonErrorPrefix: 'Uncaught', + nonErrorPrefix: NonErrorPrefix.UNCAUGHT, source: ErrorSource.SOURCE, handling: ErrorHandling.UNHANDLED, }) diff --git a/packages/core/src/domain/eventRateLimiter/createEventRateLimiter.spec.ts b/packages/core/src/domain/eventRateLimiter/createEventRateLimiter.spec.ts index 849c659c4b..5ac41d8034 100644 --- a/packages/core/src/domain/eventRateLimiter/createEventRateLimiter.spec.ts +++ b/packages/core/src/domain/eventRateLimiter/createEventRateLimiter.spec.ts @@ -1,9 +1,9 @@ import type { Clock } from '../../../test' import { mockClock } from '../../../test' -import type { RawError } from '../error/error' import type { RelativeTime } from '../../tools/utils/timeUtils' import { relativeToClocks, resetNavigationStart, ONE_MINUTE } from '../../tools/utils/timeUtils' import { noop } from '../../tools/utils/functionUtils' +import type { RawError } from '../error/error.types' import { createEventRateLimiter } from './createEventRateLimiter' import type { EventRateLimiter } from './createEventRateLimiter' diff --git a/packages/core/src/domain/eventRateLimiter/createEventRateLimiter.ts b/packages/core/src/domain/eventRateLimiter/createEventRateLimiter.ts index 6031ed5a14..56de429f19 100644 --- a/packages/core/src/domain/eventRateLimiter/createEventRateLimiter.ts +++ b/packages/core/src/domain/eventRateLimiter/createEventRateLimiter.ts @@ -1,7 +1,7 @@ import { setTimeout } from '../../tools/timer' -import type { RawError } from '../error/error' -import { ErrorSource } from '../error/error' import { clocksNow, ONE_MINUTE } from '../../tools/utils/timeUtils' +import type { RawError } from '../error/error.types' +import { ErrorSource } from '../error/error.types' export type EventRateLimiter = ReturnType diff --git a/packages/core/src/domain/telemetry/telemetry.spec.ts b/packages/core/src/domain/telemetry/telemetry.spec.ts index cb0cd86d42..adad0c298d 100644 --- a/packages/core/src/domain/telemetry/telemetry.spec.ts +++ b/packages/core/src/domain/telemetry/telemetry.spec.ts @@ -1,4 +1,5 @@ import type { StackTrace } from '@datadog/browser-core' +import { NO_ERROR_STACK_PRESENT_MESSAGE } from '../error/error' import { callMonitored } from '../../tools/monitor' import type { ExperimentalFeature } from '../../tools/experimentalFeatures' import { resetExperimentalFeatures, addExperimentalFeatures } from '../../tools/experimentalFeatures' @@ -161,7 +162,7 @@ describe('formatError', () => { expect(formatError('message')).toEqual({ message: 'Uncaught "message"', error: { - stack: 'Not an instance of error', + stack: NO_ERROR_STACK_PRESENT_MESSAGE, }, }) }) @@ -170,7 +171,7 @@ describe('formatError', () => { expect(formatError({ foo: 'bar' })).toEqual({ message: 'Uncaught {"foo":"bar"}', error: { - stack: 'Not an instance of error', + stack: NO_ERROR_STACK_PRESENT_MESSAGE, }, }) }) diff --git a/packages/core/src/domain/telemetry/telemetry.ts b/packages/core/src/domain/telemetry/telemetry.ts index f52c612c04..4c16874c39 100644 --- a/packages/core/src/domain/telemetry/telemetry.ts +++ b/packages/core/src/domain/telemetry/telemetry.ts @@ -1,6 +1,6 @@ import type { Context } from '../../tools/serialisation/context' import { ConsoleApiName } from '../../tools/display' -import { toStackTraceString } from '../error/error' +import { toStackTraceString, NO_ERROR_STACK_PRESENT_MESSAGE } from '../error/error' import { getExperimentalFeatures } from '../../tools/experimentalFeatures' import type { Configuration } from '../configuration' import { INTAKE_SITE_STAGING, INTAKE_SITE_US1_FED } from '../configuration' @@ -14,6 +14,7 @@ import { startsWith, arrayFrom, includes, assign } from '../../tools/utils/polyf import { performDraw } from '../../tools/utils/numberUtils' import { jsonStringify } from '../../tools/serialisation/jsonStringify' import { combine } from '../../tools/mergeInto' +import { NonErrorPrefix } from '../error/error.types' import type { TelemetryEvent } from './telemetryEvent.types' import type { RawTelemetryConfiguration, RawTelemetryEvent } from './rawTelemetryEvent.types' import { StatusType, TelemetryType } from './rawTelemetryEvent.types' @@ -181,9 +182,9 @@ export function formatError(e: unknown) { } return { error: { - stack: 'Not an instance of error', + stack: NO_ERROR_STACK_PRESENT_MESSAGE, }, - message: `Uncaught ${jsonStringify(e)!}`, + message: `${NonErrorPrefix.UNCAUGHT} ${jsonStringify(e)!}`, } } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 3191ea1c50..543e5fe00c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -77,18 +77,13 @@ export { runOnReadyState } from './browser/runOnReadyState' export { getZoneJsOriginalValue } from './tools/getZoneJsOriginalValue' export { instrumentMethod, instrumentMethodAndCallOriginal, instrumentSetter } from './tools/instrumentMethod' export { - ErrorSource, - ErrorHandling, computeRawError, createHandlingStack, - RawError, - RawErrorCause, - ErrorWithCause, toStackTraceString, getFileFromStackTraceString, NO_ERROR_STACK_PRESENT_MESSAGE, - PROVIDED_ERROR_MESSAGE_PREFIX, } from './domain/error/error' +export { NonErrorPrefix } from './domain/error/error.types' export { Context, ContextArray, ContextValue } from './tools/serialisation/context' export { areCookiesAuthorized, getCookie, setCookie, deleteCookie, COOKIE_ACCESS_DELAY } from './browser/cookie' export { initXhrObservable, XhrCompleteContext, XhrStartContext } from './browser/xhrObservable' @@ -126,3 +121,8 @@ export * from './tools/utils/stringUtils' export * from './tools/matchOption' export * from './tools/utils/responseUtils' export * from './tools/utils/typeUtils' +export { ErrorHandling } from './domain/error/error.types' +export { ErrorSource } from './domain/error/error.types' +export { RawError } from './domain/error/error.types' +export { RawErrorCause } from './domain/error/error.types' +export { ErrorWithCause } from './domain/error/error.types' diff --git a/packages/core/src/transport/httpRequest.ts b/packages/core/src/transport/httpRequest.ts index a069ec1546..f796f49121 100644 --- a/packages/core/src/transport/httpRequest.ts +++ b/packages/core/src/transport/httpRequest.ts @@ -2,8 +2,8 @@ import type { EndpointBuilder } from '../domain/configuration' import { addTelemetryError } from '../domain/telemetry' import type { Context } from '../tools/serialisation/context' import { monitor } from '../tools/monitor' -import type { RawError } from '../domain/error/error' import { addEventListener } from '../browser/addEventListener' +import type { RawError } from '../domain/error/error.types' import { newRetryState, sendWithRetryStrategy } from './sendWithRetryStrategy' import type { FlushReason } from './flushController' diff --git a/packages/core/src/transport/sendWithRetryStrategy.spec.ts b/packages/core/src/transport/sendWithRetryStrategy.spec.ts index 1e6c1ca8d0..10be5d8625 100644 --- a/packages/core/src/transport/sendWithRetryStrategy.spec.ts +++ b/packages/core/src/transport/sendWithRetryStrategy.spec.ts @@ -1,6 +1,6 @@ import { mockClock, restoreNavigatorOnLine, setNavigatorOnLine } from '../../test' import type { Clock } from '../../test' -import { ErrorSource } from '../domain/error/error' +import { ErrorSource } from '../domain/error/error.types' import type { RetryState } from './sendWithRetryStrategy' import { newRetryState, diff --git a/packages/core/src/transport/sendWithRetryStrategy.ts b/packages/core/src/transport/sendWithRetryStrategy.ts index 5c75c89cd7..c294b0421e 100644 --- a/packages/core/src/transport/sendWithRetryStrategy.ts +++ b/packages/core/src/transport/sendWithRetryStrategy.ts @@ -1,10 +1,10 @@ import type { EndpointType } from '../domain/configuration' import { setTimeout } from '../tools/timer' -import type { RawError } from '../domain/error/error' import { clocksNow, ONE_MINUTE, ONE_SECOND } from '../tools/utils/timeUtils' -import { ErrorSource } from '../domain/error/error' import { ONE_MEBI_BYTE, ONE_KIBI_BYTE } from '../tools/utils/byteUtils' import { isServerError } from '../tools/utils/responseUtils' +import type { RawError } from '../domain/error/error.types' +import { ErrorSource } from '../domain/error/error.types' import type { Payload, HttpResponse } from './httpRequest' export const MAX_ONGOING_BYTES_COUNT = 80 * ONE_KIBI_BYTE diff --git a/packages/core/src/transport/startBatchWithReplica.ts b/packages/core/src/transport/startBatchWithReplica.ts index 012677537e..08ea9b61d8 100644 --- a/packages/core/src/transport/startBatchWithReplica.ts +++ b/packages/core/src/transport/startBatchWithReplica.ts @@ -1,8 +1,8 @@ import type { Configuration, EndpointBuilder } from '../domain/configuration' -import type { RawError } from '../domain/error/error' import type { Context } from '../tools/serialisation/context' import type { Observable } from '../tools/observable' import type { PageExitEvent } from '../browser/pageExitObservable' +import type { RawError } from '../domain/error/error.types' import { Batch } from './batch' import { createHttpRequest } from './httpRequest' import { createFlushController } from './flushController' diff --git a/packages/logs/src/domain/logger.ts b/packages/logs/src/domain/logger.ts index 85d9505b4a..cc3a35665c 100644 --- a/packages/logs/src/domain/logger.ts +++ b/packages/logs/src/domain/logger.ts @@ -5,7 +5,6 @@ import { clocksNow, computeRawError, ErrorHandling, - PROVIDED_ERROR_MESSAGE_PREFIX, computeStackTrace, CustomerDataType, deepClone, @@ -15,6 +14,7 @@ import { ErrorSource, monitored, sanitize, + NonErrorPrefix, } from '@datadog/browser-core' import type { LogsEvent } from '../logsEvent.types' @@ -70,7 +70,7 @@ export class Logger { const rawError = computeRawError({ stackTrace, originalError: error, - nonErrorPrefix: PROVIDED_ERROR_MESSAGE_PREFIX, + nonErrorPrefix: NonErrorPrefix.PROVIDED, source: ErrorSource.LOGGER, handling: ErrorHandling.HANDLED, startClocks: clocksNow(), diff --git a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts index 8be25615ef..d8b62cf6c6 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts @@ -1,6 +1,5 @@ import type { Context, RawError, ClocksState } from '@datadog/browser-core' import { - PROVIDED_ERROR_MESSAGE_PREFIX, isEmptyObject, assign, ErrorSource, @@ -10,6 +9,7 @@ import { computeStackTrace, Observable, trackRuntimeError, + NonErrorPrefix, } from '@datadog/browser-core' import type { RawRumErrorEvent } from '../../../rawRumEvent.types' import { RumEventType } from '../../../rawRumEvent.types' @@ -73,7 +73,7 @@ export function doStartErrorCollection( originalError: error, handlingStack, startClocks, - nonErrorPrefix: PROVIDED_ERROR_MESSAGE_PREFIX, + nonErrorPrefix: NonErrorPrefix.PROVIDED, source: ErrorSource.CUSTOM, handling: ErrorHandling.HANDLED, }) From fbcfb8a79b82bf2391422a9ff209e267e55007cc Mon Sep 17 00:00:00 2001 From: "ci.browser-sdk" Date: Mon, 24 Apr 2023 02:04:45 +0000 Subject: [PATCH 7/7] =?UTF-8?q?=F0=9F=91=B7=20Bump=20staging=20to=20stagin?= =?UTF-8?q?g-17?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 2154a4576c..dae77d6a36 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,5 +1,5 @@ variables: - CURRENT_STAGING: staging-16 + CURRENT_STAGING: staging-17 APP: 'browser-sdk' CURRENT_CI_IMAGE: 45 BUILD_STABLE_REGISTRY: '486234852809.dkr.ecr.us-east-1.amazonaws.com'