From 2f1978372b2e26e07533e9d767b96095fe3c947a Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Mon, 6 Apr 2020 17:16:45 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A5[RUMF-430]=20stop=20maintaining=20o?= =?UTF-8?q?ld=20cookies?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cf #337 --- packages/core/src/oldCookiesMigration.ts | 7 +- packages/core/src/sessionManagement.ts | 15 ---- .../core/test/oldCookiesMigration.spec.ts | 9 ++- packages/core/test/sessionManagement.spec.ts | 73 ++++++------------- packages/logs/src/loggerSession.ts | 5 +- packages/rum/src/rumSession.ts | 5 +- 6 files changed, 37 insertions(+), 77 deletions(-) diff --git a/packages/core/src/oldCookiesMigration.ts b/packages/core/src/oldCookiesMigration.ts index fa796cd249..2dee902bdc 100644 --- a/packages/core/src/oldCookiesMigration.ts +++ b/packages/core/src/oldCookiesMigration.ts @@ -1,10 +1,11 @@ -import { CookieCache, getCookie, setCookie } from './cookie' -import { isValidSessionString, OLD_SESSION_COOKIE_NAME, persistSession, SessionState } from './sessionManagement' +import { CookieCache, getCookie } from './cookie' +import { persistSession, SessionState } from './sessionManagement' -// duplicate values to avoid dependency issues +export const OLD_SESSION_COOKIE_NAME = '_dd' export const OLD_RUM_COOKIE_NAME = '_dd_r' export const OLD_LOGS_COOKIE_NAME = '_dd_l' +// duplicate values to avoid dependency issues export const RUM_SESSION_KEY = 'rum' export const LOGS_SESSION_KEY = 'logs' diff --git a/packages/core/src/sessionManagement.ts b/packages/core/src/sessionManagement.ts index 584c5f5f7f..1c3d1c73b4 100644 --- a/packages/core/src/sessionManagement.ts +++ b/packages/core/src/sessionManagement.ts @@ -4,7 +4,6 @@ import { tryOldCookiesMigration } from './oldCookiesMigration' import * as utils from './utils' export const SESSION_COOKIE_NAME = '_dd_s' -export const OLD_SESSION_COOKIE_NAME = '_dd' export const EXPIRATION_DELAY = 15 * utils.ONE_MINUTE export interface Session { @@ -23,14 +22,9 @@ export interface SessionState { /** * Limit access to cookie to avoid performance issues - * - * Old cookies are maintained only to allow rollback without compatibility issue - * Since the new version does not update the old cookies, - * pages in both old and new format should live together without issue */ export function startSessionManagement( sessionTypeKey: string, - oldCookieName: string, computeSessionState: (rawType?: string) => { type: Type; isTracked: boolean } ): Session { const sessionCookie = cacheCookieAccess(SESSION_COOKIE_NAME) @@ -38,9 +32,6 @@ export function startSessionManagement( const renewObservable = new Observable() let currentSessionId = retrieveSession(sessionCookie).id - const oldSessionId = cacheCookieAccess(OLD_SESSION_COOKIE_NAME) - const oldSessionType = cacheCookieAccess(oldCookieName) - const expandOrRenewSession = utils.throttle(() => { const session = retrieveSession(sessionCookie) const { type, isTracked } = computeSessionState(session[sessionTypeKey]) @@ -51,12 +42,6 @@ export function startSessionManagement( // save changes and expand session duration persistSession(session, sessionCookie) - // update old cookies - oldSessionType.set(type, EXPIRATION_DELAY) - if (isTracked) { - oldSessionId.set(session.id!, EXPIRATION_DELAY) - } - // If the session id has changed, notify that the session has been renewed if (isTracked && currentSessionId !== session.id) { currentSessionId = session.id diff --git a/packages/core/test/oldCookiesMigration.spec.ts b/packages/core/test/oldCookiesMigration.spec.ts index da2e8c25f5..b540f6f8b6 100644 --- a/packages/core/test/oldCookiesMigration.spec.ts +++ b/packages/core/test/oldCookiesMigration.spec.ts @@ -1,7 +1,12 @@ import { getCookie, SESSION_COOKIE_NAME, setCookie } from '../src' import { cacheCookieAccess } from '../src/cookie' -import { OLD_LOGS_COOKIE_NAME, OLD_RUM_COOKIE_NAME, tryOldCookiesMigration } from '../src/oldCookiesMigration' -import { EXPIRATION_DELAY, OLD_SESSION_COOKIE_NAME } from '../src/sessionManagement' +import { + OLD_LOGS_COOKIE_NAME, + OLD_RUM_COOKIE_NAME, + OLD_SESSION_COOKIE_NAME, + tryOldCookiesMigration, +} from '../src/oldCookiesMigration' +import { EXPIRATION_DELAY } from '../src/sessionManagement' describe('old cookies migration', () => { it('should not touch current cookie', () => { diff --git a/packages/core/test/sessionManagement.spec.ts b/packages/core/test/sessionManagement.spec.ts index c5db5910b3..d407b77d31 100644 --- a/packages/core/test/sessionManagement.spec.ts +++ b/packages/core/test/sessionManagement.spec.ts @@ -1,11 +1,5 @@ import { cacheCookieAccess, COOKIE_ACCESS_DELAY, CookieCache, getCookie, setCookie } from '../src/cookie' -import { - OLD_SESSION_COOKIE_NAME, - Session, - SESSION_COOKIE_NAME, - startSessionManagement, - stopSessionManagement, -} from '../src/sessionManagement' +import { Session, SESSION_COOKIE_NAME, startSessionManagement, stopSessionManagement } from '../src/sessionManagement' import { isIE } from '../src/specHelper' describe('cacheCookieAccess', () => { @@ -48,56 +42,37 @@ enum FakeSessionType { } describe('startSessionManagement', () => { const DURATION = 123456 - const FIRST_SESSION_TYPE_OLD_COOKIE = 'foo' - const SECOND_SESSION_TYPE_OLD_COOKIE = 'bar' const FIRST_SESSION_TYPE_KEY = 'first' const SECOND_SESSION_TYPE_KEY = 'second' function expireSession() { setCookie(SESSION_COOKIE_NAME, '', DURATION) - setCookie(OLD_SESSION_COOKIE_NAME, '', DURATION) - setCookie(FIRST_SESSION_TYPE_OLD_COOKIE, '', DURATION) - setCookie(SECOND_SESSION_TYPE_OLD_COOKIE, '', DURATION) jasmine.clock().tick(COOKIE_ACCESS_DELAY) } function expectSessionIdToBe(session: Session, sessionId: string) { expect(session.getId()).toBe(sessionId) expect(getCookie(SESSION_COOKIE_NAME)).toContain(`id=${sessionId}`) - expect(getCookie(OLD_SESSION_COOKIE_NAME)).toContain(sessionId) } function expectSessionIdToBeDefined(session: Session) { expect(session.getId()).toMatch(/^[a-f0-9-]+$/) expect(getCookie(SESSION_COOKIE_NAME)).toMatch(/id=[a-f0-9-]+/) - expect(getCookie(OLD_SESSION_COOKIE_NAME)).toMatch(/[a-f0-9-]+/) } function expectSessionIdToNotBeDefined(session: Session) { expect(session.getId()).toBeUndefined() expect(getCookie(SESSION_COOKIE_NAME)).not.toContain('id=') - expect(getCookie(OLD_SESSION_COOKIE_NAME)).toBeUndefined() } - function expectSessionTypeToBe( - session: Session, - sessionTypeKey: string, - oldTypeCookieName: string, - sessionTypeValue: string - ) { + function expectSessionTypeToBe(session: Session, sessionTypeKey: string, sessionTypeValue: string) { expect(session.getType()).toEqual(sessionTypeValue) expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${sessionTypeKey}=${sessionTypeValue}`) - expect(getCookie(oldTypeCookieName)).toEqual(sessionTypeValue) } - function expectSessionTypeToNotBeDefined( - session: Session, - sessionTypeKey: string, - oldTypeCookieName: string - ) { + function expectSessionTypeToNotBeDefined(session: Session, sessionTypeKey: string) { expect(session.getType()).toBeUndefined() expect(getCookie(SESSION_COOKIE_NAME)).not.toContain(`${sessionTypeKey}=`) - expect(getCookie(oldTypeCookieName)).toBeUndefined() } beforeEach(() => { @@ -116,47 +91,47 @@ describe('startSessionManagement', () => { }) it('when tracked, should store session type and id', () => { - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, () => ({ + const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ isTracked: true, type: FakeSessionType.TRACKED, })) expectSessionIdToBeDefined(session) - expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, FakeSessionType.TRACKED) + expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FakeSessionType.TRACKED) }) it('when not tracked should store session type', () => { - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, () => ({ + const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ isTracked: false, type: FakeSessionType.NOT_TRACKED, })) expectSessionIdToNotBeDefined(session) - expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, FakeSessionType.NOT_TRACKED) + expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FakeSessionType.NOT_TRACKED) }) it('when tracked should keep existing session type and id', () => { setCookie(SESSION_COOKIE_NAME, 'id=abcdef&first=tracked', DURATION) - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, () => ({ + const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ isTracked: true, type: FakeSessionType.TRACKED, })) expectSessionIdToBe(session, 'abcdef') - expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, FakeSessionType.TRACKED) + expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FakeSessionType.TRACKED) }) it('when not tracked should keep existing session type', () => { setCookie(SESSION_COOKIE_NAME, 'first=not-tracked', DURATION) - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, () => ({ + const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ isTracked: false, type: FakeSessionType.NOT_TRACKED, })) expectSessionIdToNotBeDefined(session) - expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, FakeSessionType.NOT_TRACKED) + expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FakeSessionType.NOT_TRACKED) }) describe('computeSessionState', () => { @@ -167,31 +142,31 @@ describe('startSessionManagement', () => { }) it('should be called with an empty value if the cookie is not defined', () => { - startSessionManagement(FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, spy) + startSessionManagement(FIRST_SESSION_TYPE_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, FIRST_SESSION_TYPE_OLD_COOKIE, spy) + startSessionManagement(FIRST_SESSION_TYPE_KEY, spy) expect(spy).toHaveBeenCalledWith('invalid') }) it('should be called with the TRACKED type', () => { setCookie(SESSION_COOKIE_NAME, 'first=tracked', DURATION) - startSessionManagement(FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, spy) + startSessionManagement(FIRST_SESSION_TYPE_KEY, spy) expect(spy).toHaveBeenCalledWith(FakeSessionType.TRACKED) }) it('should be called with the NOT_TRACKED type', () => { setCookie(SESSION_COOKIE_NAME, 'first=not-tracked', DURATION) - startSessionManagement(FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, spy) + startSessionManagement(FIRST_SESSION_TYPE_KEY, spy) expect(spy).toHaveBeenCalledWith(FakeSessionType.NOT_TRACKED) }) }) it('should renew on activity after expiration', () => { - const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, () => ({ + const session = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ isTracked: true, type: FakeSessionType.TRACKED, })) @@ -202,24 +177,24 @@ describe('startSessionManagement', () => { expect(renewSessionSpy).not.toHaveBeenCalled() expectSessionIdToNotBeDefined(session) - expectSessionTypeToNotBeDefined(session, FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE) + expectSessionTypeToNotBeDefined(session, FIRST_SESSION_TYPE_KEY) document.dispatchEvent(new CustomEvent('click')) expect(renewSessionSpy).toHaveBeenCalled() expectSessionIdToBeDefined(session) - expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, FakeSessionType.TRACKED) + expectSessionTypeToBe(session, FIRST_SESSION_TYPE_KEY, FakeSessionType.TRACKED) }) describe('multiple startSessionManagement calls', () => { it('should re-use the same session id', () => { - const firstSession = startSessionManagement(FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, () => ({ + const firstSession = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ isTracked: true, type: FakeSessionType.TRACKED, })) const idA = firstSession.getId() - const secondSession = startSessionManagement(SECOND_SESSION_TYPE_KEY, SECOND_SESSION_TYPE_OLD_COOKIE, () => ({ + const secondSession = startSessionManagement(SECOND_SESSION_TYPE_KEY, () => ({ isTracked: true, type: FakeSessionType.TRACKED, })) @@ -229,11 +204,11 @@ describe('startSessionManagement', () => { }) it('should have independent types', () => { - const firstSession = startSessionManagement(FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, () => ({ + const firstSession = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ isTracked: true, type: FakeSessionType.TRACKED, })) - const secondSession = startSessionManagement(SECOND_SESSION_TYPE_KEY, SECOND_SESSION_TYPE_OLD_COOKIE, () => ({ + const secondSession = startSessionManagement(SECOND_SESSION_TYPE_KEY, () => ({ isTracked: false, type: FakeSessionType.NOT_TRACKED, })) @@ -243,14 +218,14 @@ describe('startSessionManagement', () => { }) it('should notify each renew observables', () => { - const firstSession = startSessionManagement(FIRST_SESSION_TYPE_KEY, FIRST_SESSION_TYPE_OLD_COOKIE, () => ({ + const firstSession = startSessionManagement(FIRST_SESSION_TYPE_KEY, () => ({ isTracked: true, type: FakeSessionType.TRACKED, })) const renewSessionASpy = jasmine.createSpy() firstSession.renewObservable.subscribe(renewSessionASpy) - const secondSession = startSessionManagement(SECOND_SESSION_TYPE_KEY, SECOND_SESSION_TYPE_OLD_COOKIE, () => ({ + const secondSession = startSessionManagement(SECOND_SESSION_TYPE_KEY, () => ({ isTracked: true, type: FakeSessionType.TRACKED, })) diff --git a/packages/logs/src/loggerSession.ts b/packages/logs/src/loggerSession.ts index 1ad7877018..f6401aa272 100644 --- a/packages/logs/src/loggerSession.ts +++ b/packages/logs/src/loggerSession.ts @@ -1,6 +1,5 @@ import { Configuration, performDraw, startSessionManagement } from '@datadog/browser-core' -export const LOGGER_OLD_COOKIE_NAME = '_dd_l' export const LOGGER_SESSION_KEY = 'logs' export interface LoggerSession { @@ -21,9 +20,7 @@ export function startLoggerSession(configuration: Configuration, areCookieAuthor isTracked: () => isTracked, } } - const session = startSessionManagement(LOGGER_SESSION_KEY, LOGGER_OLD_COOKIE_NAME, (rawType) => - computeSessionState(configuration, rawType) - ) + const session = startSessionManagement(LOGGER_SESSION_KEY, (rawType) => computeSessionState(configuration, rawType)) return { getId: session.getId, isTracked: () => session.getType() === LoggerSessionType.TRACKED, diff --git a/packages/rum/src/rumSession.ts b/packages/rum/src/rumSession.ts index b488e76b19..6861258b22 100644 --- a/packages/rum/src/rumSession.ts +++ b/packages/rum/src/rumSession.ts @@ -1,7 +1,6 @@ import { Configuration, performDraw, startSessionManagement } from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType } from './lifeCycle' -export const RUM_OLD_COOKIE_NAME = '_dd_r' export const RUM_SESSION_KEY = 'rum' export interface RumSession { @@ -17,9 +16,7 @@ export enum RumSessionType { } export function startRumSession(configuration: Configuration, lifeCycle: LifeCycle): RumSession { - const session = startSessionManagement(RUM_SESSION_KEY, RUM_OLD_COOKIE_NAME, (rawType) => - computeSessionState(configuration, rawType) - ) + const session = startSessionManagement(RUM_SESSION_KEY, (rawType) => computeSessionState(configuration, rawType)) session.renewObservable.subscribe(() => { lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED)