🔥[RUMF-430] stop maintaining old cookies (#342)

cf #337
DataDog · Apr 7, 2020 · d198f64 · d198f64
1 parent d1907d8
commit d198f64
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 77 deletions.
diff --git 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
@@ -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<T> {
@@ -23,24 +22,16 @@ 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<Type extends string>(
   sessionTypeKey: string,
-  oldCookieName: string,
   computeSessionState: (rawType?: string) => { type: Type; isTracked: boolean }
 ): Session<Type> {
   const sessionCookie = cacheCookieAccess(SESSION_COOKIE_NAME)
   tryOldCookiesMigration(sessionCookie)
   const renewObservable = new Observable<void>()
   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<Type extends string>(
     // 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
@@ -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
@@ -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<unknown>, 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<unknown>) {
     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<unknown>) {
     expect(session.getId()).toBeUndefined()
     expect(getCookie(SESSION_COOKIE_NAME)).not.toContain('id=')
-    expect(getCookie(OLD_SESSION_COOKIE_NAME)).toBeUndefined()
   }
 
-  function expectSessionTypeToBe(
-    session: Session<unknown>,
-    sessionTypeKey: string,
-    oldTypeCookieName: string,
-    sessionTypeValue: string
-  ) {
+  function expectSessionTypeToBe(session: Session<unknown>, 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<unknown>,
-    sessionTypeKey: string,
-    oldTypeCookieName: string
-  ) {
+  function expectSessionTypeToNotBeDefined(session: Session<unknown>, 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
@@ -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
@@ -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)