chore: merge update and get session functions

.gitignore

@@ -22,3 +22,5 @@ tests_output/
 
 vitest-report
 __screenshots__
+
+html

eslint.config.mjs

@@ -253,6 +253,7 @@ export default [
 			'.vscode/',
 			'**/.next/',
 			'vitest-report',
+			'html',
 		],
 	},
 ]

packages/web/src/SessionBasedSampler.ts

@@ -18,7 +18,7 @@
 
 import { Context, Link, Sampler, SamplingResult, SpanAttributes, SpanKind } from '@opentelemetry/api'
 import { AlwaysOffSampler, AlwaysOnSampler } from '@opentelemetry/core'
-import { getRumSessionId } from './session'
+import { getOrCreateSessionIdAndUpdateExpirationIfNecessary } from './session'
 
 export interface SessionBasedSamplerConfig {
 	/**
@@ -78,7 +78,10 @@ export class SessionBasedSampler implements Sampler {
 		// Implementation based on @opentelemetry/core TraceIdRatioBasedSampler
 		// but replacing deciding based on traceId with sessionId
 		// (not extended from due to private methods)
-		const currentSession = getRumSessionId({ useLocalStorage: this.useLocalStorageForSessionMetadata })
+		const currentSession = getOrCreateSessionIdAndUpdateExpirationIfNecessary({
+			useLocalStorage: this.useLocalStorageForSessionMetadata,
+			forceStore: true,
+		})
 		if (this._currentSession !== currentSession) {
 			this._currentSessionSampled = this._accumulate(currentSession) < this._upperBound
 			this._currentSession = currentSession

packages/web/src/SplunkSpanAttributesProcessor.ts

@@ -18,7 +18,9 @@
 
 import { Attributes } from '@opentelemetry/api'
 import { Span, SpanProcessor } from '@opentelemetry/sdk-trace-base'
-import { getRumSessionId } from './session'
+import { getOrCreateSessionIdAndUpdateExpirationIfNecessary } from './session'
+import { suppressTracing } from '@opentelemetry/core'
+import { context } from '@opentelemetry/api'
 
 export class SplunkSpanAttributesProcessor implements SpanProcessor {
 	private readonly _globalAttributes: Attributes
@@ -43,13 +45,18 @@ export class SplunkSpanAttributesProcessor implements SpanProcessor {
 	}
 
 	onStart(span: Span): void {
-		span.setAttribute('location.href', location.href)
-		span.setAttributes(this._globalAttributes)
-		span.setAttribute(
-			'splunk.rumSessionId',
-			getRumSessionId({ useLocalStorage: this.useLocalStorageForSessionMetadata }),
-		)
-		span.setAttribute('browser.instance.visibility_state', document.visibilityState)
+		context.with(suppressTracing(context.active()), () => {
+			span.setAttribute('location.href', location.href)
+			span.setAttributes(this._globalAttributes)
+			span.setAttribute(
+				'splunk.rumSessionId',
+				getOrCreateSessionIdAndUpdateExpirationIfNecessary({
+					useLocalStorage: this.useLocalStorageForSessionMetadata,
+					forceStore: true,
+				}),
+			)
+			span.setAttribute('browser.instance.visibility_state', document.visibilityState)
+		})
 	}
 
 	setGlobalAttributes(attributes?: Attributes): void {

packages/web/src/index.ts

@@ -43,7 +43,7 @@ import { type SplunkExporterConfig } from './exporters/common'
 import { SplunkZipkinExporter } from './exporters/zipkin'
 import { ERROR_INSTRUMENTATION_NAME, SplunkErrorInstrumentation } from './SplunkErrorInstrumentation'
 import { generateId, getPluginConfig } from './utils'
-import { getRumSessionId, initSessionTracking, updateSessionStatus } from './session'
+import { getOrCreateSessionIdAndUpdateExpirationIfNecessary, getRumSessionId, initSessionTracking } from './session'
 import { SplunkWebSocketInstrumentation } from './SplunkWebSocketInstrumentation'
 import { initWebVitals } from './webvitals'
 import { SplunkLongTaskInstrumentation } from './SplunkLongTaskInstrumentation'
@@ -387,7 +387,6 @@ export const SplunkRum: SplunkOtelWebType = {
 		// TODO
 		_deinitSessionTracking = initSessionTracking(
 			provider,
-			instanceId,
 			eventTarget,
 			processedOptions.cookieDomain,
 			!!options._experimental_allSpansExtendSession,
@@ -560,7 +559,7 @@ export const SplunkRum: SplunkOtelWebType = {
 			return
 		}
 
-		updateSessionStatus({
+		getOrCreateSessionIdAndUpdateExpirationIfNecessary({
 			forceStore: false,
 			useLocalStorage: this._processedOptions.persistence === 'localStorage',
 			forceActivity: this._processedOptions._experimental_allSpansExtendSession,

packages/web/src/session/session.ts

@@ -23,8 +23,6 @@ import { parseCookieToSessionState, renewCookieTimeout } from './cookie-session'
 import { SessionState, SessionId } from './types'
 import { getSessionStateFromLocalStorage, setSessionStateToLocalStorage } from './local-storage-session'
 import { SESSION_INACTIVITY_TIMEOUT_MS } from './constants'
-import { suppressTracing } from '@opentelemetry/core'
-import { context } from '@opentelemetry/api'
 
 /*
     The basic idea is to let the browser expire cookies for us "naturally" once
@@ -71,15 +69,22 @@ export function getCurrentSessionState({ useLocalStorage = false, forceStoreRead
 // 1) Check if the cookie has been expired by the browser; if so, create a new one
 // 2) If activity has occurred since the last periodic invocation, renew the cookie timeout
 // (Only exported for testing purposes.)
-export function updateSessionStatus({
-	forceStore,
-	useLocalStorage = false,
-	forceActivity,
-}: {
-	forceActivity?: boolean
-	forceStore: boolean
-	useLocalStorage: boolean
-}): void {
+export function getOrCreateSessionIdAndUpdateExpirationIfNecessary(
+	{
+		forceStore,
+		useLocalStorage,
+		forceActivity,
+	}: {
+		forceActivity?: boolean
+		forceStore: boolean
+		useLocalStorage: boolean
+	},
+	level = 0,
+): string {
+	if (hasNativeSessionId()) {
+		return window['SplunkRumNative'].getNativeSessionId()
+	}
+
 	let sessionState = getCurrentSessionState({ useLocalStorage, forceStoreRead: forceStore })
 	let shouldForceWrite = false
 	if (!sessionState) {
@@ -104,6 +109,29 @@ export function updateSessionStatus({
 	}
 
 	recentActivity = false
+
+	// New session created, check if another tab has created a new session at the same time
+	if (shouldForceWrite && level < 1) {
+		return getOrCreateSessionIdAndUpdateExpirationIfNecessary(
+			{
+				forceStore: true,
+				useLocalStorage,
+			},
+			level + 1,
+		)
+	}
+
+	return sessionState.id
+}
+
+export function getCurrentSessionId({
+	forceStore,
+	useLocalStorage,
+}: {
+	forceStore: boolean
+	useLocalStorage: boolean
+}): string | undefined {
+	return getCurrentSessionState({ useLocalStorage, forceStoreRead: forceStore })?.id
 }
 
 function hasNativeSessionId(): boolean {
@@ -128,13 +156,6 @@ class SessionSpanProcessor implements SpanProcessor {
 		if (this.options.allSpansAreActivity) {
 			markActivity()
 		}
-
-		context.with(suppressTracing(context.active()), () => {
-			updateSessionStatus({
-				forceStore: false,
-				useLocalStorage: this.options.useLocalStorage,
-			})
-		})
 	}
 
 	shutdown(): Promise<void> {
@@ -146,7 +167,6 @@ const ACTIVITY_EVENTS = ['click', 'scroll', 'mousedown', 'keydown', 'touchend',
 
 export function initSessionTracking(
 	provider: WebTracerProvider,
-	instanceId: SessionId,
 	newEventTarget: InternalEventTarget,
 	domain?: string,
 	allSpansAreActivity = false,
@@ -169,8 +189,6 @@ export function initSessionTracking(
 	ACTIVITY_EVENTS.forEach((type) => document.addEventListener(type, markActivity, { capture: true, passive: true }))
 	provider.addSpanProcessor(new SessionSpanProcessor({ allSpansAreActivity, useLocalStorage }))
 
-	updateSessionStatus({ useLocalStorage, forceStore: true })
-
 	return {
 		deinit: () => {
 			ACTIVITY_EVENTS.forEach((type) => document.removeEventListener(type, markActivity))
@@ -184,5 +202,5 @@ export function getRumSessionId({ useLocalStorage }: { useLocalStorage: boolean
 		return window['SplunkRumNative'].getNativeSessionId()
 	}
 
-	return getCurrentSessionState({ useLocalStorage, forceStoreRead: true })?.id
+	return getCurrentSessionId({ useLocalStorage, forceStore: true })
 }

packages/web/tests/SessionBasedSampler.test.ts

@@ -18,7 +18,7 @@
 
 import { InternalEventTarget } from '../src/EventTarget'
 import { SessionBasedSampler } from '../src/SessionBasedSampler'
-import { initSessionTracking, updateSessionStatus } from '../src/session'
+import { initSessionTracking, getOrCreateSessionIdAndUpdateExpirationIfNecessary } from '../src/session'
 import { context, SamplingDecision } from '@opentelemetry/api'
 import { SESSION_INACTIVITY_TIMEOUT_MS, SESSION_STORAGE_KEY } from '../src/session/constants'
 import { describe, it, expect } from 'vitest'
@@ -37,7 +37,8 @@ describe('Session based sampler', () => {
 		)
 		document.cookie = SESSION_STORAGE_KEY + '=' + lowCookieValue + '; path=/; max-age=' + 10
 		const provider = createWebTracerProvider()
-		initSessionTracking(provider, lowSessionId, new InternalEventTarget())
+		initSessionTracking(provider, new InternalEventTarget())
+		getOrCreateSessionIdAndUpdateExpirationIfNecessary({ useLocalStorage: false, forceStore: true })
 
 		const sampler = new SessionBasedSampler({ ratio: 0.5 })
 		expect(
@@ -55,7 +56,7 @@ describe('Session based sampler', () => {
 			}),
 		)
 		document.cookie = SESSION_STORAGE_KEY + '=' + highCookieValue + '; path=/; max-age=' + 10
-		updateSessionStatus({
+		getOrCreateSessionIdAndUpdateExpirationIfNecessary({
 			forceStore: true,
 			useLocalStorage: false,
 		})

packages/web/tests/SplunkOtelWeb.test.ts

@@ -18,7 +18,7 @@
 
 import { SpanAttributes } from '@opentelemetry/api'
 import SplunkRum from '../src'
-import { updateSessionStatus } from '../src/session'
+import { getOrCreateSessionIdAndUpdateExpirationIfNecessary } from '../src/session'
 import { describe, expect, it, afterEach } from 'vitest'
 
 describe('SplunkOtelWeb', () => {
@@ -124,7 +124,7 @@ describe('SplunkOtelWeb', () => {
 			})
 
 			document.body.click()
-			updateSessionStatus({ forceStore: false, useLocalStorage: false })
+			getOrCreateSessionIdAndUpdateExpirationIfNecessary({ forceStore: false, useLocalStorage: false })
 
 			// Wait for promise chain to resolve
 			await Promise.resolve()
@@ -135,17 +135,17 @@ describe('SplunkOtelWeb', () => {
 
 	describe('.inited', () => {
 		it('should follow lifecycle', () => {
-			expect(SplunkRum.inited).toBe(false, 'Should be false in the beginning.')
+			expect(SplunkRum.inited, 'Should be false in the beginning.').toBe(false)
 
 			SplunkRum.init({
 				applicationName: 'app-name',
 				beaconEndpoint: 'https://beacon',
 				rumAccessToken: '<token>',
 			})
-			expect(SplunkRum.inited).toBe(true, 'Should be true after creating.')
+			expect(SplunkRum.inited, 'Should be true after creating.').toBe(true)
 
 			SplunkRum.deinit()
-			expect(SplunkRum.inited).toBe(false, 'Should be false after destroying.')
+			expect(SplunkRum.inited, 'Should be false after destroying.').toBe(false)
 		})
 	})
 })

packages/web/tests/session.test.ts

@@ -17,11 +17,15 @@
  */
 
 import { InternalEventTarget } from '../src/EventTarget'
-import { initSessionTracking, getRumSessionId, updateSessionStatus } from '../src/session'
+import {
+	initSessionTracking,
+	getRumSessionId,
+	getOrCreateSessionIdAndUpdateExpirationIfNecessary,
+} from '../src/session'
 import { SESSION_STORAGE_KEY, SESSION_INACTIVITY_TIMEOUT_MS } from '../src/session/constants'
-import { clearSessionCookie, cookieStore } from '../src/session/cookie-session'
+import { clearSessionCookie } from '../src/session/cookie-session'
 import { clearSessionStateFromLocalStorage } from '../src/session/local-storage-session'
-import { expect, it, describe, beforeEach, afterEach, vi, MockInstance } from 'vitest'
+import { expect, it, describe, beforeEach, afterEach } from 'vitest'
 import { createWebTracerProvider } from './utils'
 
 describe('Session tracking', () => {
@@ -34,14 +38,16 @@ describe('Session tracking', () => {
 	})
 
 	it('should correctly handle expiry, garbage values, (in)activity, etc.', async () => {
-		// the init tests have possibly already started the setInterval for updateSessionStatus.  Try to accomodate this.
 		const provider = createWebTracerProvider()
-		const trackingHandle = initSessionTracking(provider, '1234', new InternalEventTarget())
-		const firstSessionId = getRumSessionId({ useLocalStorage: false })
+		const trackingHandle = initSessionTracking(provider, new InternalEventTarget())
+		const firstSessionId = getOrCreateSessionIdAndUpdateExpirationIfNecessary({
+			useLocalStorage: false,
+			forceStore: true,
+		})
 
 		expect(firstSessionId).toHaveLength(32)
 		// no marked activity, should keep same state
-		updateSessionStatus({ forceStore: false, useLocalStorage: false })
+		getOrCreateSessionIdAndUpdateExpirationIfNecessary({ forceStore: false, useLocalStorage: false })
 		expect(firstSessionId).toBe(getRumSessionId({ useLocalStorage: false }))
 
 		// set cookie to expire in 2 seconds, mark activity, and then updateSessionStatus.
@@ -55,7 +61,7 @@ describe('Session tracking', () => {
 		)
 		document.cookie = SESSION_STORAGE_KEY + '=' + cookieValue + '; path=/; max-age=' + 2
 		document.body.dispatchEvent(new Event('click'))
-		updateSessionStatus({ forceStore: false, useLocalStorage: false })
+		getOrCreateSessionIdAndUpdateExpirationIfNecessary({ forceStore: false, useLocalStorage: false })
 
 		await new Promise((resolve) => setTimeout(resolve, 4000))
 
@@ -75,7 +81,7 @@ describe('Session tracking', () => {
 		)
 		document.cookie = SESSION_STORAGE_KEY + '=' + tooOldCookieValue + '; path=/; max-age=' + 4
 
-		updateSessionStatus({ forceStore: true, useLocalStorage: false })
+		getOrCreateSessionIdAndUpdateExpirationIfNecessary({ forceStore: true, useLocalStorage: false })
 		expect(document.cookie.includes(SESSION_STORAGE_KEY)).toBeTruthy()
 		const newSessionId = getRumSessionId({ useLocalStorage: false })
 		expect(newSessionId?.length).toBe(32)
@@ -85,38 +91,6 @@ describe('Session tracking', () => {
 	})
 })
 
-describe('Activity tracking', () => {
-	let cookieSetStoreSpy: MockInstance<(value: string) => void>
-	beforeEach(() => {
-		cookieSetStoreSpy = vi.spyOn(cookieStore, 'set')
-	})
-
-	afterEach(() => {
-		cookieSetStoreSpy.mockRestore()
-	})
-
-	function subject(allSpansAreActivity = false) {
-		const provider = createWebTracerProvider()
-
-		initSessionTracking(provider, '1234', new InternalEventTarget(), undefined, allSpansAreActivity)
-
-		provider.getTracer('tracer').startSpan('any-span').end()
-		updateSessionStatus({ forceStore: false, useLocalStorage: false })
-	}
-
-	it('non-activity spans do not trigger a new session', () => {
-		subject()
-
-		expect(cookieSetStoreSpy).toHaveBeenCalledTimes(1)
-	})
-
-	it('activity spans do trigger a new session when opt-in', () => {
-		subject(true)
-
-		expect(cookieSetStoreSpy).toHaveBeenCalledTimes(2)
-	})
-})
-
 describe('Session tracking - localStorage', () => {
 	beforeEach(() => {
 		clearSessionStateFromLocalStorage()
@@ -129,17 +103,9 @@ describe('Session tracking - localStorage', () => {
 	it('should save session state to local storage', () => {
 		const useLocalStorage = true
 		const provider = createWebTracerProvider()
-		const trackingHandle = initSessionTracking(
-			provider,
-			'1234',
-			new InternalEventTarget(),
-			undefined,
-			undefined,
-			useLocalStorage,
-		)
+		const trackingHandle = initSessionTracking(provider, new InternalEventTarget(), undefined, undefined)
 
-		const firstSessionId = getRumSessionId({ useLocalStorage })
-		updateSessionStatus({ forceStore: true, useLocalStorage })
+		const firstSessionId = getOrCreateSessionIdAndUpdateExpirationIfNecessary({ useLocalStorage, forceStore: true })
 		expect(firstSessionId).toBe(getRumSessionId({ useLocalStorage }))
 
 		trackingHandle.deinit()