diff --git a/.gitignore b/.gitignore index 95712e548..ac1dbb21c 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,5 @@ tests_output/ vitest-report __screenshots__ + +html diff --git a/eslint.config.mjs b/eslint.config.mjs index 9929fa096..7b1b99ba8 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -253,6 +253,7 @@ export default [ '.vscode/', '**/.next/', 'vitest-report', + 'html', ], }, ] diff --git a/packages/web/src/SessionBasedSampler.ts b/packages/web/src/SessionBasedSampler.ts index 0e0db5023..f3472f899 100644 --- a/packages/web/src/SessionBasedSampler.ts +++ b/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 diff --git a/packages/web/src/SplunkSpanAttributesProcessor.ts b/packages/web/src/SplunkSpanAttributesProcessor.ts index 09b389dea..c3fe5e9bf 100644 --- a/packages/web/src/SplunkSpanAttributesProcessor.ts +++ b/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 { diff --git a/packages/web/src/index.ts b/packages/web/src/index.ts index 59a8da8cd..56a8f33d6 100644 --- a/packages/web/src/index.ts +++ b/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, diff --git a/packages/web/src/session/session.ts b/packages/web/src/session/session.ts index 20726066b..f1c5a858c 100644 --- a/packages/web/src/session/session.ts +++ b/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 { @@ -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 }) } diff --git a/packages/web/tests/SessionBasedSampler.test.ts b/packages/web/tests/SessionBasedSampler.test.ts index 2ffdc518f..7575d7dc3 100644 --- a/packages/web/tests/SessionBasedSampler.test.ts +++ b/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, }) diff --git a/packages/web/tests/SplunkOtelWeb.test.ts b/packages/web/tests/SplunkOtelWeb.test.ts index 7fc50c970..17e287cfe 100644 --- a/packages/web/tests/SplunkOtelWeb.test.ts +++ b/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: '', }) - 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) }) }) }) diff --git a/packages/web/tests/session.test.ts b/packages/web/tests/session.test.ts index 579d99a19..c7e7b7726 100644 --- a/packages/web/tests/session.test.ts +++ b/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()