From 68c385e9e92dfd9b86af78209071ac2ab25ef9b7 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Thu, 23 Apr 2020 17:03:27 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20fix=20session=20type?= =?UTF-8?q?=20naming=20collision?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - cookie session type -> tracking type - cookie session type key -> product key --- packages/core/src/sessionManagement.ts | 18 +-- packages/core/test/sessionManagement.spec.ts | 132 +++++++++---------- packages/logs/src/loggerSession.ts | 26 ++-- packages/logs/test/loggerSession.spec.ts | 20 +-- packages/rum/src/rumSession.ts | 42 +++--- packages/rum/test/rumSession.spec.ts | 14 +- 6 files changed, 128 insertions(+), 124 deletions(-) diff --git a/packages/core/src/sessionManagement.ts b/packages/core/src/sessionManagement.ts index 360b003f08..39e0814945 100644 --- a/packages/core/src/sessionManagement.ts +++ b/packages/core/src/sessionManagement.ts @@ -12,7 +12,7 @@ export const VISIBILITY_CHECK_DELAY = utils.ONE_MINUTE export interface Session { renewObservable: Observable getId(): string | undefined - getType(): T | undefined + getTrackingType(): T | undefined } export interface SessionState { @@ -25,10 +25,10 @@ export interface SessionState { /** * Limit access to cookie to avoid performance issues */ -export function startSessionManagement( - sessionTypeKey: string, - computeSessionState: (rawType?: string) => { type: Type; isTracked: boolean } -): Session { +export function startSessionManagement( + productKey: string, + computeSessionState: (rawTrackingType?: string) => { trackingType: TrackingType; isTracked: boolean } +): Session { const sessionCookie = cacheCookieAccess(SESSION_COOKIE_NAME) tryOldCookiesMigration(sessionCookie) const renewObservable = new Observable() @@ -36,8 +36,8 @@ export function startSessionManagement( const expandOrRenewSession = utils.throttle(() => { const session = retrieveActiveSession(sessionCookie) - const { type, isTracked } = computeSessionState(session[sessionTypeKey]) - session[sessionTypeKey] = type + const { trackingType, isTracked } = computeSessionState(session[productKey]) + session[productKey] = trackingType if (isTracked && !session.id) { session.id = utils.generateUUID() session.created = String(Date.now()) @@ -65,8 +65,8 @@ export function startSessionManagement( getId() { return retrieveActiveSession(sessionCookie).id }, - getType() { - return retrieveActiveSession(sessionCookie)[sessionTypeKey] as Type | undefined + getTrackingType() { + return retrieveActiveSession(sessionCookie)[productKey] as TrackingType | undefined }, renewObservable, } diff --git a/packages/core/test/sessionManagement.spec.ts b/packages/core/test/sessionManagement.spec.ts index 42e4adc29c..d2a00193b3 100644 --- a/packages/core/test/sessionManagement.spec.ts +++ b/packages/core/test/sessionManagement.spec.ts @@ -45,14 +45,14 @@ describe('cacheCookieAccess', () => { }) }) -enum FakeSessionType { +enum FakeTrackingType { NOT_TRACKED = 'not-tracked', TRACKED = 'tracked', } describe('startSessionManagement', () => { const DURATION = 123456 - const FIRST_SESSION_TYPE_KEY = 'first' - const SECOND_SESSION_TYPE_KEY = 'second' + const FIRST_PRODUCT_KEY = 'first' + const SECOND_PRODUCT_KEY = 'second' function expireSession() { setCookie(SESSION_COOKIE_NAME, '', DURATION) @@ -74,14 +74,14 @@ describe('startSessionManagement', () => { expect(getCookie(SESSION_COOKIE_NAME)).not.toContain('id=') } - function expectSessionTypeToBe(session: Session, sessionTypeKey: string, sessionTypeValue: string) { - expect(session.getType()).toEqual(sessionTypeValue) - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${sessionTypeKey}=${sessionTypeValue}`) + function expectTrackingTypeToBe(session: Session, productKey: string, trackingType: string) { + expect(session.getTrackingType()).toEqual(trackingType) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${productKey}=${trackingType}`) } - function expectSessionTypeToNotBeDefined(session: Session, sessionTypeKey: string) { - expect(session.getType()).toBeUndefined() - expect(getCookie(SESSION_COOKIE_NAME)).not.toContain(`${sessionTypeKey}=`) + function expectTrackingTypeToNotBeDefined(session: Session, productKey: string) { + expect(session.getTrackingType()).toBeUndefined() + expect(getCookie(SESSION_COOKIE_NAME)).not.toContain(`${productKey}=`) } beforeEach(() => { @@ -101,87 +101,87 @@ describe('startSessionManagement', () => { }) describe('cookie management', () => { - it('when tracked, should store session type and id', () => { - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + it('when tracked, should store tracking type and session id', () => { + const session = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) expectSessionIdToBeDefined(session) - expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FakeSessionType.TRACKED) + expectTrackingTypeToBe(session, FIRST_PRODUCT_KEY, FakeTrackingType.TRACKED) }) - it('when not tracked should store session type', () => { - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + it('when not tracked should store tracking type', () => { + const session = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: false, - type: FakeSessionType.NOT_TRACKED, + trackingType: FakeTrackingType.NOT_TRACKED, })) expectSessionIdToNotBeDefined(session) - expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FakeSessionType.NOT_TRACKED) + expectTrackingTypeToBe(session, FIRST_PRODUCT_KEY, FakeTrackingType.NOT_TRACKED) }) - it('when tracked should keep existing session type and id', () => { + it('when tracked should keep existing tracking type and session id', () => { setCookie(SESSION_COOKIE_NAME, 'id=abcdef&first=tracked', DURATION) - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + const session = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) expectSessionIdToBe(session, 'abcdef') - expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FakeSessionType.TRACKED) + expectTrackingTypeToBe(session, FIRST_PRODUCT_KEY, FakeTrackingType.TRACKED) }) - it('when not tracked should keep existing session type', () => { + it('when not tracked should keep existing tracking type', () => { setCookie(SESSION_COOKIE_NAME, 'first=not-tracked', DURATION) - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + const session = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: false, - type: FakeSessionType.NOT_TRACKED, + trackingType: FakeTrackingType.NOT_TRACKED, })) expectSessionIdToNotBeDefined(session) - expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FakeSessionType.NOT_TRACKED) + expectTrackingTypeToBe(session, FIRST_PRODUCT_KEY, FakeTrackingType.NOT_TRACKED) }) }) describe('computeSessionState', () => { - let spy: (rawType?: string) => { type: FakeSessionType; isTracked: boolean } + let spy: (rawTrackingType?: string) => { trackingType: FakeTrackingType; isTracked: boolean } beforeEach(() => { - spy = jasmine.createSpy().and.returnValue({ isTracked: true, type: FakeSessionType.TRACKED }) + spy = jasmine.createSpy().and.returnValue({ isTracked: true, trackingType: FakeTrackingType.TRACKED }) }) it('should be called with an empty value if the cookie is not defined', () => { - startSessionManagement(FIRST_SESSION_TYPE_KEY, spy) + startSessionManagement(FIRST_PRODUCT_KEY, spy) expect(spy).toHaveBeenCalledWith(undefined) }) it('should be called with an invalid value if the cookie has an invalid value', () => { setCookie(SESSION_COOKIE_NAME, 'first=invalid', DURATION) - startSessionManagement(FIRST_SESSION_TYPE_KEY, spy) + startSessionManagement(FIRST_PRODUCT_KEY, spy) expect(spy).toHaveBeenCalledWith('invalid') }) - it('should be called with the TRACKED type', () => { + it('should be called with TRACKED', () => { setCookie(SESSION_COOKIE_NAME, 'first=tracked', DURATION) - startSessionManagement(FIRST_SESSION_TYPE_KEY, spy) - expect(spy).toHaveBeenCalledWith(FakeSessionType.TRACKED) + startSessionManagement(FIRST_PRODUCT_KEY, spy) + expect(spy).toHaveBeenCalledWith(FakeTrackingType.TRACKED) }) - it('should be called with the NOT_TRACKED type', () => { + it('should be called with NOT_TRACKED', () => { setCookie(SESSION_COOKIE_NAME, 'first=not-tracked', DURATION) - startSessionManagement(FIRST_SESSION_TYPE_KEY, spy) - expect(spy).toHaveBeenCalledWith(FakeSessionType.NOT_TRACKED) + startSessionManagement(FIRST_PRODUCT_KEY, spy) + expect(spy).toHaveBeenCalledWith(FakeTrackingType.NOT_TRACKED) }) }) describe('session renewal', () => { it('should renew on activity after expiration', () => { - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + const session = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) const renewSessionSpy = jasmine.createSpy() session.renewObservable.subscribe(renewSessionSpy) @@ -190,58 +190,58 @@ describe('startSessionManagement', () => { expect(renewSessionSpy).not.toHaveBeenCalled() expectSessionIdToNotBeDefined(session) - expectSessionTypeToNotBeDefined(session, FIRST_SESSION_TYPE_KEY) + expectTrackingTypeToNotBeDefined(session, FIRST_PRODUCT_KEY) document.dispatchEvent(new CustomEvent('click')) expect(renewSessionSpy).toHaveBeenCalled() expectSessionIdToBeDefined(session) - expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FakeSessionType.TRACKED) + expectTrackingTypeToBe(session, FIRST_PRODUCT_KEY, FakeTrackingType.TRACKED) }) }) describe('multiple startSessionManagement calls', () => { it('should re-use the same session id', () => { - const firstSession = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + const firstSession = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) const idA = firstSession.getId() - const secondSession = startSessionManagement(SECOND_SESSION_TYPE_KEY, () => ({ + const secondSession = startSessionManagement(SECOND_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) const idB = secondSession.getId() expect(idA).toBe(idB) }) - it('should have independent types', () => { - const firstSession = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + it('should have independent tracking types', () => { + const firstSession = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) - const secondSession = startSessionManagement(SECOND_SESSION_TYPE_KEY, () => ({ + const secondSession = startSessionManagement(SECOND_PRODUCT_KEY, () => ({ isTracked: false, - type: FakeSessionType.NOT_TRACKED, + trackingType: FakeTrackingType.NOT_TRACKED, })) - expect(firstSession.getType()).toEqual(FakeSessionType.TRACKED) - expect(secondSession.getType()).toEqual(FakeSessionType.NOT_TRACKED) + expect(firstSession.getTrackingType()).toEqual(FakeTrackingType.TRACKED) + expect(secondSession.getTrackingType()).toEqual(FakeTrackingType.NOT_TRACKED) }) it('should notify each renew observables', () => { - const firstSession = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + const firstSession = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) const renewSessionASpy = jasmine.createSpy() firstSession.renewObservable.subscribe(renewSessionASpy) - const secondSession = startSessionManagement(SECOND_SESSION_TYPE_KEY, () => ({ + const secondSession = startSessionManagement(SECOND_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) const renewSessionBSpy = jasmine.createSpy() secondSession.renewObservable.subscribe(renewSessionBSpy) @@ -260,9 +260,9 @@ describe('startSessionManagement', () => { describe('session timeout', () => { it('should expire the session when the time out delay is reached', () => { - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + const session = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) expect(session.getId()).toBeDefined() expect(getCookie(SESSION_COOKIE_NAME)).toBeDefined() @@ -275,9 +275,9 @@ describe('startSessionManagement', () => { it('should renew an existing timed out session', () => { setCookie(SESSION_COOKIE_NAME, `id=abcde&first=tracked&created=${Date.now() - SESSION_TIME_OUT_DELAY}`, DURATION) - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + const session = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) expect(session.getId()).not.toBe('abcde') @@ -287,9 +287,9 @@ describe('startSessionManagement', () => { it('should not add created date to an existing session from an older versions', () => { setCookie(SESSION_COOKIE_NAME, `id=abcde&first=tracked`, DURATION) - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + const session = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) expect(session.getId()).toBe('abcde') @@ -320,9 +320,9 @@ describe('startSessionManagement', () => { }) it('should expire the session after expiration delay', () => { - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + const session = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) expectSessionIdToBeDefined(session) @@ -331,9 +331,9 @@ describe('startSessionManagement', () => { }) it('should expand duration on activity', () => { - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + const session = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) expectSessionIdToBeDefined(session) @@ -350,9 +350,9 @@ describe('startSessionManagement', () => { it('should expand session on visibility', () => { setPageVisibility('visible') - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ + const session = startSessionManagement(FIRST_PRODUCT_KEY, () => ({ isTracked: true, - type: FakeSessionType.TRACKED, + trackingType: FakeTrackingType.TRACKED, })) jasmine.clock().tick(3 * VISIBILITY_CHECK_DELAY) diff --git a/packages/logs/src/loggerSession.ts b/packages/logs/src/loggerSession.ts index f6401aa272..93654506b3 100644 --- a/packages/logs/src/loggerSession.ts +++ b/packages/logs/src/loggerSession.ts @@ -7,41 +7,43 @@ export interface LoggerSession { isTracked: () => boolean } -export enum LoggerSessionType { +export enum LoggerTrackingType { NOT_TRACKED = '0', TRACKED = '1', } export function startLoggerSession(configuration: Configuration, areCookieAuthorized: boolean): LoggerSession { if (!areCookieAuthorized) { - const isTracked = computeSessionType(configuration) === LoggerSessionType.TRACKED + const isTracked = computeTrackingType(configuration) === LoggerTrackingType.TRACKED return { getId: () => undefined, isTracked: () => isTracked, } } - const session = startSessionManagement(LOGGER_SESSION_KEY, (rawType) => computeSessionState(configuration, rawType)) + const session = startSessionManagement(LOGGER_SESSION_KEY, (rawTrackingType) => + computeSessionState(configuration, rawTrackingType) + ) return { getId: session.getId, - isTracked: () => session.getType() === LoggerSessionType.TRACKED, + isTracked: () => session.getTrackingType() === LoggerTrackingType.TRACKED, } } -function computeSessionType(configuration: Configuration): string { +function computeTrackingType(configuration: Configuration): string { if (!performDraw(configuration.sampleRate)) { - return LoggerSessionType.NOT_TRACKED + return LoggerTrackingType.NOT_TRACKED } - return LoggerSessionType.TRACKED + return LoggerTrackingType.TRACKED } function computeSessionState(configuration: Configuration, rawSessionType?: string) { - const sessionType = hasValidLoggerSession(rawSessionType) ? rawSessionType : computeSessionType(configuration) + const trackingType = hasValidLoggerSession(rawSessionType) ? rawSessionType : computeTrackingType(configuration) return { - isTracked: sessionType === LoggerSessionType.TRACKED, - type: sessionType, + trackingType, + isTracked: trackingType === LoggerTrackingType.TRACKED, } } -function hasValidLoggerSession(type?: string): type is LoggerSessionType { - return type === LoggerSessionType.NOT_TRACKED || type === LoggerSessionType.TRACKED +function hasValidLoggerSession(trackingType?: string): trackingType is LoggerTrackingType { + return trackingType === LoggerTrackingType.NOT_TRACKED || trackingType === LoggerTrackingType.TRACKED } diff --git a/packages/logs/test/loggerSession.spec.ts b/packages/logs/test/loggerSession.spec.ts index d8ded11cd1..22c95f2e67 100644 --- a/packages/logs/test/loggerSession.spec.ts +++ b/packages/logs/test/loggerSession.spec.ts @@ -7,7 +7,7 @@ import { stopSessionManagement, } from '@datadog/browser-core' -import { LOGGER_SESSION_KEY, LoggerSessionType, startLoggerSession } from '../src/loggerSession' +import { LOGGER_SESSION_KEY, LoggerTrackingType, startLoggerSession } from '../src/loggerSession' describe('logger session', () => { const DURATION = 123456 @@ -28,39 +28,39 @@ describe('logger session', () => { jasmine.clock().uninstall() }) - it('when tracked should store session type and id', () => { + it('when tracked should store tracking type and session id', () => { tracked = true startLoggerSession(configuration as Configuration, true) - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerSessionType.TRACKED}`) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerTrackingType.TRACKED}`) expect(getCookie(SESSION_COOKIE_NAME)).toMatch(/id=[a-f0-9-]+/) }) - it('when not tracked should store session type', () => { + it('when not tracked should store tracking type', () => { tracked = false startLoggerSession(configuration as Configuration, true) - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerSessionType.NOT_TRACKED}`) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerTrackingType.NOT_TRACKED}`) expect(getCookie(SESSION_COOKIE_NAME)).not.toContain('id=') }) - it('when tracked should keep existing session type and id', () => { + it('when tracked should keep existing tracking type and session id', () => { setCookie(SESSION_COOKIE_NAME, 'id=abcdef&logs=1', DURATION) startLoggerSession(configuration as Configuration, true) - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerSessionType.TRACKED}`) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerTrackingType.TRACKED}`) expect(getCookie(SESSION_COOKIE_NAME)).toContain('id=abcdef') }) - it('when not tracked should keep existing session type', () => { + it('when not tracked should keep existing tracking type', () => { setCookie(SESSION_COOKIE_NAME, 'logs=0', DURATION) startLoggerSession(configuration as Configuration, true) - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerSessionType.NOT_TRACKED}`) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerTrackingType.NOT_TRACKED}`) }) it('should renew on activity after expiration', () => { @@ -74,7 +74,7 @@ describe('logger session', () => { document.body.click() expect(getCookie(SESSION_COOKIE_NAME)).toMatch(/id=[a-f0-9-]+/) - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerSessionType.TRACKED}`) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerTrackingType.TRACKED}`) }) it('when no cookies available, isTracked is computed at each call and getId is undefined', () => { diff --git a/packages/rum/src/rumSession.ts b/packages/rum/src/rumSession.ts index 6861258b22..ed334d67c7 100644 --- a/packages/rum/src/rumSession.ts +++ b/packages/rum/src/rumSession.ts @@ -9,14 +9,16 @@ export interface RumSession { isTrackedWithResource: () => boolean } -export enum RumSessionType { +export enum RumTrackingType { NOT_TRACKED = '0', TRACKED_WITH_RESOURCES = '1', TRACKED_WITHOUT_RESOURCES = '2', } export function startRumSession(configuration: Configuration, lifeCycle: LifeCycle): RumSession { - const session = startSessionManagement(RUM_SESSION_KEY, (rawType) => computeSessionState(configuration, rawType)) + const session = startSessionManagement(RUM_SESSION_KEY, (rawTrackingType) => + computeSessionState(configuration, rawTrackingType) + ) session.renewObservable.subscribe(() => { lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) @@ -24,40 +26,40 @@ export function startRumSession(configuration: Configuration, lifeCycle: LifeCyc return { getId: session.getId, - isTracked: () => session.getId() !== undefined && isTracked(session.getType()), + isTracked: () => session.getId() !== undefined && isTracked(session.getTrackingType()), isTrackedWithResource: () => - session.getId() !== undefined && session.getType() === RumSessionType.TRACKED_WITH_RESOURCES, + session.getId() !== undefined && session.getTrackingType() === RumTrackingType.TRACKED_WITH_RESOURCES, } } -function computeSessionState(configuration: Configuration, rawSessionType?: string) { - let sessionType: RumSessionType - if (hasValidRumSession(rawSessionType)) { - sessionType = rawSessionType +function computeSessionState(configuration: Configuration, rawTrackingType?: string) { + let trackingType: RumTrackingType + if (hasValidRumSession(rawTrackingType)) { + trackingType = rawTrackingType } else if (!performDraw(configuration.sampleRate)) { - sessionType = RumSessionType.NOT_TRACKED + trackingType = RumTrackingType.NOT_TRACKED } else if (!performDraw(configuration.resourceSampleRate)) { - sessionType = RumSessionType.TRACKED_WITHOUT_RESOURCES + trackingType = RumTrackingType.TRACKED_WITHOUT_RESOURCES } else { - sessionType = RumSessionType.TRACKED_WITH_RESOURCES + trackingType = RumTrackingType.TRACKED_WITH_RESOURCES } return { - isTracked: isTracked(sessionType), - type: sessionType, + trackingType, + isTracked: isTracked(trackingType), } } -function hasValidRumSession(type?: string): type is RumSessionType { +function hasValidRumSession(trackingType?: string): trackingType is RumTrackingType { return ( - type === RumSessionType.NOT_TRACKED || - type === RumSessionType.TRACKED_WITH_RESOURCES || - type === RumSessionType.TRACKED_WITHOUT_RESOURCES + trackingType === RumTrackingType.NOT_TRACKED || + trackingType === RumTrackingType.TRACKED_WITH_RESOURCES || + trackingType === RumTrackingType.TRACKED_WITHOUT_RESOURCES ) } -function isTracked(rumSessionType: RumSessionType | undefined) { +function isTracked(rumSessionType: RumTrackingType | undefined) { return ( - rumSessionType === RumSessionType.TRACKED_WITH_RESOURCES || - rumSessionType === RumSessionType.TRACKED_WITHOUT_RESOURCES + rumSessionType === RumTrackingType.TRACKED_WITH_RESOURCES || + rumSessionType === RumTrackingType.TRACKED_WITHOUT_RESOURCES ) } diff --git a/packages/rum/test/rumSession.spec.ts b/packages/rum/test/rumSession.spec.ts index 47d3a9a267..b7bf89651a 100644 --- a/packages/rum/test/rumSession.spec.ts +++ b/packages/rum/test/rumSession.spec.ts @@ -10,7 +10,7 @@ import { } from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' -import { RUM_SESSION_KEY, RumSessionType, startRumSession } from '../src/rumSession' +import { RUM_SESSION_KEY, RumTrackingType, startRumSession } from '../src/rumSession' function setupDraws({ tracked, trackedWithResources }: { tracked?: boolean; trackedWithResources?: boolean }) { spyOn(Math, 'random').and.returnValues(tracked ? 0 : 1, trackedWithResources ? 0 : 1) @@ -52,7 +52,7 @@ describe('rum session', () => { startRumSession(configuration as Configuration, lifeCycle) expect(renewSessionSpy).not.toHaveBeenCalled() - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumSessionType.TRACKED_WITH_RESOURCES}`) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumTrackingType.TRACKED_WITH_RESOURCES}`) expect(getCookie(SESSION_COOKIE_NAME)).toMatch(/id=[a-f0-9-]/) }) @@ -62,7 +62,7 @@ describe('rum session', () => { startRumSession(configuration as Configuration, lifeCycle) expect(renewSessionSpy).not.toHaveBeenCalled() - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumSessionType.TRACKED_WITHOUT_RESOURCES}`) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumTrackingType.TRACKED_WITHOUT_RESOURCES}`) expect(getCookie(SESSION_COOKIE_NAME)).toMatch(/id=[a-f0-9-]/) }) @@ -72,7 +72,7 @@ describe('rum session', () => { startRumSession(configuration as Configuration, lifeCycle) expect(renewSessionSpy).not.toHaveBeenCalled() - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumSessionType.NOT_TRACKED}`) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumTrackingType.NOT_TRACKED}`) expect(getCookie(SESSION_COOKIE_NAME)).not.toContain('id=') }) @@ -82,7 +82,7 @@ describe('rum session', () => { startRumSession(configuration as Configuration, lifeCycle) expect(renewSessionSpy).not.toHaveBeenCalled() - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumSessionType.TRACKED_WITH_RESOURCES}`) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumTrackingType.TRACKED_WITH_RESOURCES}`) expect(getCookie(SESSION_COOKIE_NAME)).toContain('id=abcdef') }) @@ -92,7 +92,7 @@ describe('rum session', () => { startRumSession(configuration as Configuration, lifeCycle) expect(renewSessionSpy).not.toHaveBeenCalled() - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumSessionType.NOT_TRACKED}`) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumTrackingType.NOT_TRACKED}`) }) it('should renew on activity after expiration', () => { @@ -107,7 +107,7 @@ describe('rum session', () => { document.dispatchEvent(new CustomEvent('click')) expect(renewSessionSpy).toHaveBeenCalled() - expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumSessionType.TRACKED_WITH_RESOURCES}`) + expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${RUM_SESSION_KEY}=${RumTrackingType.TRACKED_WITH_RESOURCES}`) expect(getCookie(SESSION_COOKIE_NAME)).toMatch(/id=[a-f0-9-]/) }) }) From 37127f76b366a10ecdb7dccde88b15acd7205c14 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Thu, 23 Apr 2020 17:14:38 +0200 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=90=9B=20compute=20session=20type=20f?= =?UTF-8?q?or=20each=20event?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit synthetics instrumentation is delayed on new page creation avoid to mess the session type of all the events of the page --- packages/rum/src/rum.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/rum/src/rum.ts b/packages/rum/src/rum.ts index fbba5a994f..0e7de977ab 100644 --- a/packages/rum/src/rum.ts +++ b/packages/rum/src/rum.ts @@ -164,8 +164,6 @@ export function startRum( ) ) - const sessionTpe = getSessionType() - const batch = startRumBatch( configuration, session, @@ -173,7 +171,9 @@ export function startRum( applicationId, date: new Date().getTime(), session: { - type: sessionTpe, + // must be computed on each event because synthetics instrumentation can be done after sdk execution + // cf https://github.com/puppeteer/puppeteer/issues/3667 + type: getSessionType(), }, sessionId: viewContext.sessionId, view: {