Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions packages/core/src/oldCookiesMigration.ts
Original file line number Diff line number Diff line change
@@ -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'

Expand Down
15 changes: 0 additions & 15 deletions packages/core/src/sessionManagement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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])
Expand All @@ -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
Expand Down
9 changes: 7 additions & 2 deletions packages/core/test/oldCookiesMigration.spec.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down
73 changes: 24 additions & 49 deletions packages/core/test/sessionManagement.spec.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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(() => {
Expand All @@ -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', () => {
Expand All @@ -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,
}))
Expand All @@ -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,
}))
Expand All @@ -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,
}))
Expand All @@ -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,
}))
Expand Down
5 changes: 1 addition & 4 deletions packages/logs/src/loggerSession.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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,
Expand Down
5 changes: 1 addition & 4 deletions packages/rum/src/rumSession.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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)
Expand Down