diff --git a/CHANGELOG.md b/CHANGELOG.md index 4999e4a1d3..53f046eba6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,80 @@ --- +## v0.2.3 + +- āœØ A session sample rate published from the console that rises above 0 now ends the running + session of a visitor whose session was drawn at 0, so collection starts at their next interaction + instead of waiting for that session to end on its own ā€” up to four hours. This is the case where + waiting shows an operator who has just switched collection on nothing at all, which is + indistinguishable from a broken integration. It joins the two changes that already did not wait: + a stricter Session Replay privacy level, and a rate of 0. Nothing here happens without + `remoteConfigurationEnabled: true`. +- šŸ“ Only a session drawn AT 0 is re-drawn, not every session that is not being collected. Those + are different populations: a visitor who lost a draw at 30 had a coin flipped for them, and + re-rolling the losers while the winners keep their sessions would put the real rate above the + published one. While a rate of 0 is in force nothing is collected and no coin is flipped, so + re-drawing everyone lands exactly on the new rate. A rate rising from one real value to another + therefore still waits for the next session, as before. +- šŸ“ The rate a sampled-out session was drawn at is now recorded alongside the one a collected + session was drawn at, in the same single `localStorage` entry this SDK already keeps for the + draw. No new entry, no extra request. Without it a page that did not perform the draw ā€” the + second page of a visit, or another tab ā€” could not tell the two populations above apart. The + decision reads that record straight off storage rather than off what the page last saw of the + draw: two sampled-out sessions look alike to the session store, so a tab can miss another tab + ending one and drawing the next, and storage is the one place the current draw is always found. +- šŸ“ What you will see on the day you lift a rate off 0: visitors who were invisible start + appearing within seconds of loading a page rather than at their next session, so collected volume + climbs the same day rather than the next. That is the change taking effect, not a defect. + +--- + +## v0.2.2 + +- šŸ› The settings cache no longer grows by one entry per release of your site. Entries are keyed by + application version, because two releases served at the same time are entitled to different rates + ā€” but the entry a previous release used was never read or removed again, so on a site that + deploys often they accumulated in the storage quota the page shares. New entries now record when + they were refreshed, and one left untouched for two days is removed when the SDK starts. Entries + written by older SDK builds are kept because they carry no refresh time, leaving a finite legacy + residue while preventing the cache from growing without bound. + +## v0.2.1 + +- āœØ Two changes published from the console now end the running session, so they reach the visitor + at their next interaction instead of waiting for that session to end on its own: a stricter + Session Replay privacy level, and a session sample rate of 0 ā€” the emergency stop, which took up + to four hours to stop anything before this. Both apply only while the visitor is being collected; + one who is not records nothing and sends nothing, so neither change has anything to act on there. + The session that ends is collected to its end as it began, so no recording is left masked in one + half and plain in the other. Every other change still waits for the next session, including a + loosening privacy level and a rate rising to 100 ā€” for "collect this visitor now" there is + `setForcedSession()`. Custom values wait on their own too, but not once `beforeSampling` turns + them into a rate of 0. Nothing here happens without `remoteConfigurationEnabled: true`. +- šŸ“ How soon "does not wait" is depends on when this client next hears of the change, and it hears + only at page load and at each new session ā€” there is no timer. A visitor who keeps loading pages + hears within seconds of the publish and their session ends there. A single tab that is never + reloaded hears nothing until its session reaches the four-hour cap, so an always-on screen is the + case this does least for; any other tab the same visitor loads ends the session they share. +- šŸ“ What you will see on the day you publish one of the two: session counts rise and average + session length drops, because each affected visitor's running session is split at that moment, + and a replay in progress ends at the split ā€” the session that follows draws again, so it carries + a new recording only if that draw keeps one. That is the change taking effect, not a defect. +- šŸ“ The two act on what actually changed, not on the activation mode recorded with the publish: a + change the console files as "next session" still ends the running session if it is one of them. +- šŸ“ `beforeSampling` is now also consulted when settings arrive, away from any draw, to work out + which rate would apply. It must stay free of side effects and answer the same way for the same + input: a callback that draws its own lottery ā€” answering 0 at random ā€” can end a session that a + steady answer would have left running. +- šŸ“ A session forced with `setForcedSession()` is not ended by a rate: forcing decides whether this + visitor is collected, and every draw the page makes is collected whatever the console says, so + ending it would only produce the same session again. A stricter privacy level still ends it, + because forcing says nothing about how much of the page may be uploaded in the clear. The page + forces the next session on its own, so the visit continues as two sessions. +- šŸ“ Turning remote configuration off is itself a change: the rates go back to the ones passed to + `init`. On a site whose init rate is 0, switching it off stops collection at once rather than at + the next session. + ## v0.2.0 - šŸ’„ **Breaking**: `remoteConfigurationId` is gone from `RumInitConfiguration`. It fetched a diff --git a/developer-extension/package.json b/developer-extension/package.json index 64e65b75a2..21084ec20a 100644 --- a/developer-extension/package.json +++ b/developer-extension/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-sdk-developer-extension", - "version": "0.2.0", + "version": "0.2.3", "private": true, "scripts": { "build": "rm -rf dist && webpack --mode production", diff --git a/lerna.json b/lerna.json index 3b191b9b28..17fc086141 100644 --- a/lerna.json +++ b/lerna.json @@ -1,4 +1,4 @@ { "npmClient": "yarn", - "version": "0.2.0" + "version": "0.2.3" } diff --git a/packages/core/package.json b/packages/core/package.json index 5f28ebec36..681fcd12f8 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-core", - "version": "0.2.0", + "version": "0.2.3", "license": "Apache-2.0", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/packages/flagging/package.json b/packages/flagging/package.json index 11ca96cea3..28650c2fd4 100644 --- a/packages/flagging/package.json +++ b/packages/flagging/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-flagging", - "version": "0.2.0", + "version": "0.2.3", "license": "Apache-2.0", "private": true, "main": "cjs/entries/main.js", @@ -18,7 +18,7 @@ "@flashcatcloud/browser-core": "workspace:*" }, "peerDependencies": { - "@flashcatcloud/browser-rum": "0.2.0" + "@flashcatcloud/browser-rum": "0.2.3" }, "peerDependenciesMeta": { "@flashcatcloud/browser-rum": { diff --git a/packages/logs/package.json b/packages/logs/package.json index 389efefc03..b12b307d92 100644 --- a/packages/logs/package.json +++ b/packages/logs/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-logs", - "version": "0.2.0", + "version": "0.2.3", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", @@ -17,7 +17,7 @@ "@flashcatcloud/browser-core": "workspace:*" }, "peerDependencies": { - "@flashcatcloud/browser-rum": "0.2.0" + "@flashcatcloud/browser-rum": "0.2.3" }, "peerDependenciesMeta": { "@flashcatcloud/browser-rum": { diff --git a/packages/rum-core/package.json b/packages/rum-core/package.json index a289b5f804..6f633a85a2 100644 --- a/packages/rum-core/package.json +++ b/packages/rum-core/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum-core", - "version": "0.2.0", + "version": "0.2.3", "license": "Apache-2.0", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index ee15cedb80..f449070d4f 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -62,6 +62,12 @@ export interface RumInitConfiguration extends InitConfiguration { * a single session. Keep it a pure decision: side effects will be repeated, and only the last * call's return value is used. * + * The SDK also calls it away from a draw: when new settings arrive it asks which rate would + * apply now, to decide whether the running session has to end for them to take effect. So it + * must answer the same way for the same input ā€” one that answers differently each time can end a + * session that a steady one would have left running ā€” and anything it does besides returning a + * rate (a metric, a log, a counter) happens more often than there are sessions. + * * Its failure modes never reach session creation: a thrown error or an out-of-range value leaves * the incoming rate in place, and a value that is not a function at all is reported once and * then ignored rather than refusing `init`. @@ -86,9 +92,31 @@ export interface RumInitConfiguration extends InitConfiguration { * Take the sampling rates from the application's settings in the console instead of only from the * values passed here, so they can be changed without releasing a new version of this site. * - * A change applies to sessions started after it arrives; a session already under way keeps the - * decision it was created with. The values below stay in use until the first settings arrive, and - * whenever the settings cannot be reached. + * A change applies to sessions started after it arrives, and a session already under way is never + * re-decided in place. Three changes do not wait for that session to end on its own, because + * their effect on it can be told without drawing again: a stricter `defaultPrivacyLevel`, and a + * session sample rate of 0, both while the visitor is being collected ā€” one who is not records + * nothing and sends nothing, so neither has anything to act on there ā€” and a rate above 0 for a + * visitor whose session was drawn AT 0, who was never in a draw at all and now could be. Any of + * the three ends the current session, and the visitor's next action starts a new one under the + * new settings; the old session is collected to its end as it was begun, so no recording is left + * masked in one half and plain in the other. + * + * Every other change waits for the next session, a loosening privacy level included, and so does + * a rate rising from one real value to another: only a second draw could say whether a session + * drawn at 30 should have been kept at 80, and drawing twice turns a rate p into pĀ². Re-drawing + * only the visitors who are not collected would spare the winners and re-roll the losers, which + * lifts the real rate above the published one. A rate of 0 is the one value with no winners to + * spare, which is why leaving it is decidable and leaving 30 is not. For "collect this one + * visitor now" at any rate, there is `setForcedSession()`. + * + * How soon "does not wait" is depends on when this client next hears of the change, and it hears + * only at page load and at each new session. A visitor who keeps loading pages hears within + * seconds; a single tab that is never reloaded hears nothing until its session reaches the + * four-hour cap. + * + * The values below stay in use until the first settings arrive, and whenever the settings cannot + * be reached. * * Requires `localStorage`. Sessions themselves are kept in a cookie unless `sessionPersistence` * says otherwise, but this SDK already reads one `localStorage` entry on every site ā€” the record diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts index 214a097342..c3e409ab8c 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts @@ -1,4 +1,4 @@ -import { INTAKE_SITE_US1, ONE_SECOND, display, isIntakeUrl } from '@flashcatcloud/browser-core' +import { INTAKE_SITE_US1, ONE_DAY, ONE_SECOND, dateNow, display, isIntakeUrl } from '@flashcatcloud/browser-core' import type { Clock, MockXhr } from '@flashcatcloud/browser-core/test' import { interceptRequests, mockClock, registerCleanupTask } from '@flashcatcloud/browser-core/test' import { mockRumConfiguration } from '../../../test' @@ -196,6 +196,83 @@ describe('remoteConfiguration', () => { }) }) + describe('announcing that new settings are in storage', () => { + function watchStoredNotifications() { + const notified = jasmine.createSpy('remoteConfigurationStored') + lifeCycle.subscribe(LifeCycleEventType.REMOTE_CONFIGURATION_STORED, notified) + return notified + } + + it('announces settings that reached storage, so a subscriber can act on them', (done) => { + const notified = watchStoredNotifications() + + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ rum: { sessionSampleRate: 0 } })) + + expect(notified).toHaveBeenCalledTimes(1) + done() + }) + start(configurationWith()) + }) + + it('stays silent about settings it refused as older than the ones it holds', (done) => { + localStorage.setItem(setup!.storeKey, JSON.stringify({ sessionSampleRate: 42, version: 8 })) + const notified = watchStoredNotifications() + + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ version: 7, rum: { sessionSampleRate: 0 } })) + + // Nothing changed in storage, so nothing downstream may behave as though it had. + expect(notified).not.toHaveBeenCalled() + done() + }) + start(configurationWith()) + }) + + it('stays silent about an answer that repeats the settings it already holds', (done) => { + localStorage.setItem(setup!.storeKey, JSON.stringify({ sessionSampleRate: 42, version: 7 })) + const notified = watchStoredNotifications() + + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ version: 7, rum: { sessionSampleRate: 42 } })) + + // The ordinary answer: every new session asks again and most find nothing changed. A + // subscriber woken by those would act on no news, once per session, for as long as the + // visitor stays. + expect(notified).not.toHaveBeenCalled() + done() + }) + start(configurationWith()) + }) + + it('stays silent when the answer never reached storage', (done) => { + const notified = watchStoredNotifications() + spyOn(Storage.prototype, 'setItem').and.throwError('storage is full') + + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ rum: { sessionSampleRate: 0 } })) + + // The next draw will not find these settings, so ending a session for their sake would end + // it for nothing. + expect(notified).not.toHaveBeenCalled() + done() + }) + start(configurationWith()) + }) + + it('stays silent about an answer that never made it', (done) => { + const notified = watchStoredNotifications() + + interceptor.withMockXhr((xhr) => { + xhr.complete(500) + + expect(notified).not.toHaveBeenCalled() + done() + }) + start(configurationWith()) + }) + }) + describe('refusing a payload it cannot read', () => { const STORED = { sessionSampleRate: 42, version: 2 } @@ -672,6 +749,102 @@ describe('remoteConfiguration', () => { }) }) + describe('sweeping the entries of releases nobody runs', () => { + // The key carries the application version, so every release leaves one behind. Without a sweep + // they accumulate for good in a quota the host application shares. + const otherReleaseKey = buildRemoteConfigSetup({ ...INIT_CONFIGURATION, version: '0.9.0' })!.storeKey + const drawKey = buildDrawStoreKey(INIT_CONFIGURATION) + const foreignKey = 'a-key-the-host-application-owns' + + beforeEach(() => { + registerCleanupTask(() => { + localStorage.removeItem(otherReleaseKey) + localStorage.removeItem(drawKey) + localStorage.removeItem(foreignKey) + }) + }) + + function writeEntryAged(key: string, age: number, values: Record = { version: 4 }) { + localStorage.setItem(key, JSON.stringify({ ...values, t: dateNow() - age })) + } + + function writeTimeOf(key: string) { + return (JSON.parse(localStorage.getItem(key)!) as { t?: number }).t + } + + it('removes an entry nothing has refreshed for longer than the threshold', () => { + writeEntryAged(otherReleaseKey, 3 * ONE_DAY) + + start(configurationWith()) + + expect(localStorage.getItem(otherReleaseKey)).toBeNull() + }) + + it('keeps an entry a page refreshed recently, which is how a live one looks', () => { + writeEntryAged(otherReleaseKey, ONE_DAY) + + start(configurationWith()) + + expect(localStorage.getItem(otherReleaseKey)).not.toBeNull() + }) + + it('keeps an entry left by a build that did not record when it was written', () => { + // An old build still using this origin cannot add a write time when it refreshes the entry, + // so absence alone cannot distinguish a live release from an abandoned one. + localStorage.setItem(otherReleaseKey, JSON.stringify({ version: 4, sessionSampleRate: 42 })) + + start(configurationWith()) + + expect(localStorage.getItem(otherReleaseKey)).not.toBeNull() + }) + + it("never removes this page's own entry, however old it looks", () => { + // It holds the version floor that lets a late answer be refused, and the request this very + // initialisation is starting is about to read it. + localStorage.setItem(setup!.storeKey, JSON.stringify({ version: 8, sessionSampleRate: 42 })) + + start(configurationWith()) + + expect(readRemoteConfig(setup).version).toBe(8) + }) + + it('leaves alone every key it did not write', () => { + writeEntryAged(drawKey, 3 * ONE_DAY) + localStorage.setItem(foreignKey, 'not ours to parse') + + start(configurationWith()) + + expect(localStorage.getItem(drawKey)).not.toBeNull() + expect(localStorage.getItem(foreignKey)).toBe('not ours to parse') + }) + + it('records when an entry was written, so a later sweep can tell its age', (done) => { + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ rum: { sessionSampleRate: 42 } })) + + expect(dateNow() - writeTimeOf(setup!.storeKey)!).toBeLessThan(ONE_SECOND) + done() + }) + start(configurationWith()) + }) + + it('refreshes the write time of an entry whose values it refuses', (done) => { + // The entry a client is stuck on when a server breaks the only-goes-up contract is the one + // entry no successful write refreshes. Without this its settings would be swept out from + // under it while it was still asking for them. + writeEntryAged(setup!.storeKey, 3 * ONE_DAY, { version: 8, sessionSampleRate: 42 }) + + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ rum: { sessionSampleRate: 1 }, version: 7 })) + + expect(readRemoteConfig(setup)).toEqual({ version: 8, sessionSampleRate: 42 }) + expect(dateNow() - writeTimeOf(setup!.storeKey)!).toBeLessThan(ONE_SECOND) + done() + }) + start(configurationWith()) + }) + }) + describe('the storage key', () => { it('separates applications, environments and versions', () => { const keyOf = (partial: Partial) => diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts index 2180dfbc8f..2c0ee781f9 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts @@ -2,9 +2,11 @@ import { addEventListener, clearTimeout, createEndpointUrlBuilder, + dateNow, display, noop, setTimeout, + ONE_DAY, ONE_SECOND, } from '@flashcatcloud/browser-core' import type { DefaultPrivacyLevel, TimeoutId } from '@flashcatcloud/browser-core' @@ -20,17 +22,23 @@ declare const __BUILD_ENV__SDK_VERSION__: string * masks a page by default. * * A change only affects sessions created after it arrives, so a visitor is never dropped halfway - * through and never starts being recorded halfway through. Fetching follows the same rhythm: once - * at start-up and once whenever a new session begins ā€” a change can only matter at the next draw, - * so asking more often than sessions are drawn would be requests for nothing. There is no timer - * between sessions. + * through and never starts being recorded halfway through. What "immediately" means for the + * handful of changes that cannot wait is therefore not a flip of the running session but its end: + * see `endSessionIfSettingsAreDecisive` in the session manager, which subscribes to the event this + * module emits once new settings are in storage. + * + * Fetching follows the session's rhythm: once at start-up and once whenever a new session begins ā€” + * a change can only matter at a draw, and every draw is a new session ā€” so asking more often than + * sessions are drawn would be requests for nothing. There is no timer between sessions. The cost + * of that rhythm is that a visitor who never goes idle stays on one session, and so on one set of + * settings, for as long as they keep using the site. * * Three fields the server sends are accepted and ignored, deliberately: `ttl` and * `refresh_on_foreground`, which describe when to ask again and are moot without a timer, and - * `activation`, which offers to end a running session so a change applies at once. Everything here - * is next-session, so a console that ever offers "apply immediately" would not be obeyed by this - * build ā€” named here so the mismatch is found by reading rather than by an operator wondering why - * nothing happened. + * `activation`, which offers to end a running session so a change applies at once. This build ends + * a running session on its own reading of what changed rather than on the server's say-so, so a + * console that offers "apply immediately" as a switch would not be obeyed ā€” named here so the + * mismatch is found by reading rather than by an operator wondering why nothing happened. * * Nothing here runs unless `remoteConfigurationEnabled: true`. Left off ā€” the default ā€” the SDK makes no * extra request and behaves exactly as it did before this existed. @@ -51,6 +59,22 @@ const STORE_KEY_PREFIX = '_fc_rc_1_' const DRAW_STORE_KEY_PREFIX = '_fc_draw_1_' const DEFAULT_FETCH_TIMEOUT = 3 * ONE_SECOND +/** + * How long an entry may go unrefreshed before `sweepAbandonedEntries` treats it as belonging to a + * release nobody is running any more. + * + * An entry written by this SDK that is still being read is also being rewritten: the page reading + * it refetches at every session renewal and stores the answer. So the threshold only has to clear + * the longest a live entry can legitimately stay silent, which is the longest session (four hours, + * after which a renewal refetches) plus the longest endpoint outage we are willing to survive + * without dropping anyone ā€” a failed fetch stores nothing. Two days leaves better than a day and a + * half of outage, and still bounds the leak at the entries of two days of releases. + * + * Erring long is deliberate. Deleting an entry too early costs the page reading it one session on + * its init values; keeping a dead one costs a few hundred bytes. + */ +const STORE_ENTRY_MAX_AGE = 2 * ONE_DAY + /** * A failed fetch is retried quickly, then patiently, then not at all until the next natural * trigger (a new session, or the next page load). The budget is deliberately tiny ā€” two extra @@ -112,6 +136,9 @@ export interface BeforeSamplingContext { * The application's last word on the sampling of the session about to be drawn ā€” see the * `beforeSampling` init option. Returning nothing, or an out-of-range rate, leaves the incoming * value in place. + * + * Must be free of side effects and answer the same way for the same input: it is also called away + * from a draw, to work out which rate newly delivered settings would actually apply. */ export type BeforeSamplingCallback = ( context: BeforeSamplingContext @@ -277,7 +304,13 @@ function keepConfigFresh(configuration: RumConfiguration, setup: RemoteConfigSet } if (response) { failedAttempts = 0 - store(setup, response) + if (store(setup, response)) { + // Announced only once new settings are in storage, because that is where the next draw + // reads them: a subscriber that ends the running session so the new values can take + // effect immediately has to be sure the draw that follows will find them, and must not + // be woken by an answer that changed nothing. + lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) + } return } if (failedAttempts < RETRY_DELAYS.length) { @@ -296,6 +329,10 @@ function keepConfigFresh(configuration: RumConfiguration, setup: RemoteConfigSet const renewSubscription = lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, onTrigger) + // Before the first request, so the room the entries of dead releases are holding is free by the + // time there is an answer to store. See `sweepAbandonedEntries`. + sweepAbandonedEntries(setup.storeKey) + onTrigger() return () => { @@ -387,6 +424,12 @@ function fetchRemoteConfiguration( } } +/** + * Writes the response to storage, and answers whether it brought settings this client did not + * already hold. A refused or unwritable response answers `false`, and so does one that repeats the + * version already stored: settings only ever change under a higher number, so by that contract a + * repeat leaves the next draw reading what it would have read anyway. + */ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) { // Settings are published under a number that only ever goes up ā€” rolling back republishes the // old settings under a new, higher one ā€” so a response numbered below what is already stored is @@ -401,11 +444,27 @@ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) // // What it is compared against is storage, not a version held in memory here, because the two // requests that can cross are two pages, and storage is the only thing they share. - const storedVersion = readRemoteConfig(setup).version + const stored = readRemoteConfig(setup) + const storedVersion = stored.version if (storedVersion !== undefined && response.version < storedVersion) { - return + // Refused, but the entry is plainly still in use ā€” a request was just made for it and answered. + // Rewriting it unchanged is what says so: its age is the only thing the sweep reads, and this + // is the one path that reaches an entry without storing anything. A client left here by a + // server that broke the only-goes-up contract would otherwise have the settings it is still + // asking for swept out from under it. Reading a version out of the entry proves it is there, + // so nothing needs to be checked before writing it back. + writeEntry(setup, stored) + return false } + // Settings only ever change under a higher number, so a response repeating the number already + // stored carries nothing new ā€” and that is the ordinary answer, since every new session refetches + // and most of them find the settings unchanged. It is written anyway, which costs one small + // `setItem` and keeps the entry in the shape this build writes, but it is not announced: a + // subscriber that ends the running session must hear about changes only, or an unchanged answer + // arriving at every renewal would end a session per renewal, forever. + const isNew = storedVersion === undefined || response.version > storedVersion + const values: RemoteConfigValues = { version: response.version } if (response.enabled && response.rum) { // Each value is copied only when the server actually sent it. A knob nobody configured must @@ -432,15 +491,107 @@ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) values.custom = response.custom } + // Written even with nothing in it ā€” that is what "remote configuration is off, use your own + // settings" looks like ā€” so that the version is kept either way and the console can still see + // that this client is up to date with the change that turned it off. + return writeEntry(setup, values) && isNew +} + +/** + * What actually sits in storage: the values, plus when they were last written. + * + * `t` is not one of the values and is never handed on ā€” `readStoredValues` drops it with everything + * else it does not recognise. It exists for `sweepAbandonedEntries` alone, which is why it is not + * spelled out on `RemoteConfigValues` where a reader would take it for something the server sends. + */ +interface StoredEntry extends RemoteConfigValues { + t: number +} + +/** + * The one place an entry is written, so that every entry carries the write time the sweep reads. + * + * Answers whether the values are now where the next draw will look for them. A failure ā€” storage + * unavailable, or the origin out of room ā€” leaves the previous entry exactly as it was, which is + * the same "keep what is already working" answer a failed request gets: the client goes on applying + * the settings it last stored, and goes on reporting their version. It is still reported as a + * failure, because nothing downstream may act on settings the next draw will not find. + */ +function writeEntry(setup: RemoteConfigSetup, values: RemoteConfigValues) { + try { + const entry: StoredEntry = { ...values, t: dateNow() } + localStorage.setItem(setup.storeKey, JSON.stringify(entry)) + return true + } catch { + return false + } +} + +/** + * Delete the entries of releases nobody is running any more. + * + * The store key carries the application version, because two releases live at the same time are + * entitled to different rates and one entry between them would have each overwrite the other's at + * every fetch. The cost of that is an entry per release, and nothing ever read or removed them + * again ā€” on a site that deploys daily they accumulate for good, in a quota the host application + * shares. + * + * Run once per initialisation rather than at every write. Sweeping on write was the shape tried + * first and it is the wrong one: `localStorage` is synchronous, a session renewal is a hot path, + * and the walk would repeat for no new information. Once per page also puts it *before* the first + * write, which is what lets it free room on an origin that is already out of it ā€” the very state + * the leak produces. + * + * This page's own entry is never a candidate: it holds the version floor that lets a late answer + * be refused, and it is about to be read by the request this initialisation is starting. + * + * An entry with no write time at all was left by a build older than this one. It is kept because an + * old build still running on the origin cannot add the write time when it refreshes the entry, so + * absence alone cannot distinguish a live release from an abandoned one. That leaves a finite set + * of entries from before the write time existed; every entry written from this build onward is + * timestamped, so the cache no longer grows without bound. + */ +function sweepAbandonedEntries(keepKey: string) { + try { + const now = dateNow() + const abandoned: string[] = [] + + // Collected in full before anything is removed: removing during the walk shifts the indices + // `key()` reads, and whatever slid into the freed slot would be stepped over. + for (let i = 0; i < localStorage.length; i += 1) { + const key = localStorage.key(i) + if (key === null || key === keepKey || key.indexOf(STORE_KEY_PREFIX) !== 0) { + continue + } + const writeTime = readWriteTime(key) + if (writeTime !== undefined && now - writeTime > STORE_ENTRY_MAX_AGE) { + abandoned.push(key) + } + } + + abandoned.forEach((key) => localStorage.removeItem(key)) + } catch { + // Storage unavailable, or an entry that is not ours to parse. Housekeeping is never worth + // failing an initialisation over, and the next page load tries again. + } +} + +/** + * When the entry under `key` was last written, or undefined when it does not say. Anything in a + * browser profile can be edited by hand, so a time that is not a finite number is read as no time + * at all rather than trusted into the arithmetic above. + */ +function readWriteTime(key: string) { try { - // Written even with nothing in it ā€” that is what "remote configuration is off, use your own - // settings" looks like ā€” so that the version is kept either way and the console can still see - // that this client is up to date with the change that turned it off. - localStorage.setItem(setup.storeKey, JSON.stringify(values)) + const stored = localStorage.getItem(key) + const parsed: unknown = stored ? JSON.parse(stored) : undefined + if (!parsed || typeof parsed !== 'object') { + return undefined + } + const { t } = parsed as Partial + return typeof t === 'number' && isFinite(t) ? t : undefined } catch { - // Storage unavailable, or the origin is out of room. The previous entry stays as it is, which - // is the same "keep what is already working" answer a failed request gets ā€” the client goes on - // applying the settings it last stored, and goes on reporting their version. + return undefined } } @@ -497,19 +648,11 @@ function validFetchTimeout(timeout: number | undefined) { * as long as both are being served ā€” so the version has to stay, and cannot be dropped to make the * limitation below go away. * - * KNOWN LIMITATION - that costs an entry per deploy. The first session after a release reads the - * local settings, and the entry the release before it used is never read again and never removed, - * so they accumulate in a quota the host application shares. - * - * Sweeping them on write is not the answer, and was tried: nothing here can tell an abandoned entry - * from the entry of a tab still open on yesterday's release, and deleting the latter drops that tab - * to its local settings for a whole session ā€” after which the two tabs delete each other's entry at - * every renewal, which is a worse failure than the leak. A correct fix needs a way to know that no - * page is still reading an entry: an age written beside the values would do it, and is the shape to - * reach for if the accumulation ever bites. Two things to get right if it is ever built ā€” the - * threshold has to clear the longest session AND the longest plausible endpoint outage, since only - * a stored response refreshes the age, and the stale-version early return above skips that write, - * so it must refresh the age even when it declines the values. + * That costs an entry per release ā€” the first session after one reads the local settings, and the + * entry the release before it used is never read again ā€” so the entries are swept by age rather + * than left to accumulate in a quota the host application shares. See `sweepAbandonedEntries` for + * why age is the only thing that can tell an abandoned entry from the entry of a tab still open on + * yesterday's release. */ function buildStoreKey(initConfiguration: RumInitConfiguration) { return buildKey(STORE_KEY_PREFIX, identityParts(initConfiguration).concat(initConfiguration.version ?? '')) diff --git a/packages/rum-core/src/domain/lifeCycle.ts b/packages/rum-core/src/domain/lifeCycle.ts index b1abd3fb46..78b6d9fccb 100644 --- a/packages/rum-core/src/domain/lifeCycle.ts +++ b/packages/rum-core/src/domain/lifeCycle.ts @@ -37,6 +37,18 @@ export const enum LifeCycleEventType { RAW_RUM_EVENT_COLLECTED, RUM_EVENT_COLLECTED, RAW_ERROR_COLLECTED, + + // FLASHCAT FORK - a remote configuration response has just changed what is in storage. Emitted + // only when the write actually happened and actually changed something, so a response refused as + // stale, one that merely repeats the settings already held, and a storage failure all stay + // silent: a subscriber acting on settings the next draw would have read anyway would be acting + // on no news at all. + // + // Added last on purpose. The values of a const enum are inlined at build time and shift when an + // entry is inserted, and everything above this line is upstream's ā€” keeping the fork's own entry + // at the end leaves upstream's numbering alone and keeps this file out of the way of the next + // upstream merge. + REMOTE_CONFIGURATION_STORED, } // This is a workaround for an issue occurring when the Browser SDK is included in a TypeScript @@ -69,6 +81,7 @@ declare const LifeCycleEventTypeAsConst: { RAW_RUM_EVENT_COLLECTED: LifeCycleEventType.RAW_RUM_EVENT_COLLECTED RUM_EVENT_COLLECTED: LifeCycleEventType.RUM_EVENT_COLLECTED RAW_ERROR_COLLECTED: LifeCycleEventType.RAW_ERROR_COLLECTED + REMOTE_CONFIGURATION_STORED: LifeCycleEventType.REMOTE_CONFIGURATION_STORED } // Note: this interface needs to be exported even if it is not used outside of this module, else TS @@ -93,6 +106,7 @@ export interface LifeCycleEventMap { error: RawError customerContext?: Context } + [LifeCycleEventTypeAsConst.REMOTE_CONFIGURATION_STORED]: void } export interface RawRumEventCollectedData { diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index f122708907..8cf789409e 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -820,6 +820,549 @@ describe('rum session manager', () => { }) }) + describe('restarting the session when the settings are decisive', () => { + const STORE_KEY = 'test-decisive-settings' + const DRAW_KEY = 'test-decisive-settings-draw' + const REMOTE_SETUP = { + buildUrl: () => 'https://example.com/config', + storeKey: STORE_KEY, + fetchTimeout: 3000, + } + + afterEach(() => localStorage.removeItem(DRAW_KEY)) + + function storeRemote(stored: object) { + localStorage.setItem(STORE_KEY, JSON.stringify(stored)) + registerCleanupTask(() => localStorage.removeItem(STORE_KEY)) + } + + function startWith(configuration: Partial = {}) { + return startRumSessionManagerWithDefaults({ + configuration: { remoteConfig: REMOTE_SETUP, drawStoreKey: DRAW_KEY, ...configuration }, + }) + } + + // Settings reach storage first and are announced afterwards, the order the fetcher uses: the + // draw that may follow reads storage, so it has to find them already there. + function deliver(stored: object) { + storeRemote(stored) + lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) + } + + function isSessionEnded() { + return getSessionState(SESSION_STORE_KEY).isExpired === '1' + } + + describe('the three changes it can decide on its own', () => { + it('ends a session drawn at zero when the rate rises above it', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 0 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + + // Nothing was collected and no coin was flipped, so re-drawing this visitor lands exactly + // on the new rate ā€” and until it happens an operator who has just switched collection on + // sees nothing at all, which is indistinguishable from broken. + deliver({ version: 2, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends a session drawn at zero even when the new rate is a partial one', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 0 }) + + deliver({ version: 2, sessionSampleRate: 30 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends a session drawn on an init rate of zero when the first settings deliver a rate', () => { + // Nothing in storage yet, so this session was drawn on the init values ā€” and a draw landing + // exactly on them records nothing, which is why zero can only be read back off init here. + // This is the application that never collects until the console says so. + startWith({ sessionSampleRate: 0 }) + + deliver({ version: 1, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('collects the session that follows a rate lifted off zero', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 0 }) + + deliver({ version: 2, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + clock.tick(STORAGE_POLL_DELAY) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITH_SESSION_REPLAY) + }) + + it('ends a session being collected when the rate goes to zero', () => { + storeRemote({ version: 1, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + startWith({ sessionSampleRate: 100 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITH_SESSION_REPLAY) + + deliver({ version: 2, sessionSampleRate: 0, sessionReplaySampleRate: 0 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends the session when the privacy level tightens', () => { + storeRemote({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + startWith({ sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + + deliver({ version: 2, sessionSampleRate: 100, defaultPrivacyLevel: 'mask-user-input' }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends the session on the tightening step that masks everything', () => { + storeRemote({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'mask-user-input' }) + startWith({ sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + + deliver({ version: 2, sessionSampleRate: 100, defaultPrivacyLevel: 'mask' }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends a session drawn before any settings arrived when the first ones tighten the level', () => { + // Nothing in storage yet, so this session was drawn on the init values ā€” and a draw that + // lands exactly on them records nothing, which is why the level it runs under can only be + // read back off init. The recorder falls back the same way, so this is the level the page + // is really being masked with. + startWith({ sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + + deliver({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'mask' }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends a collected session when the callback turns the delivered values into a zero', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startWith({ + sessionSampleRate: 100, + beforeSampling: ({ custom }) => (custom?.optOut === true ? { sessionSampleRate: 0 } : undefined), + }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).not.toBe(RumTrackingType.NOT_TRACKED) + + // The response carries no rate at all: the console ships the data and the application's own + // code turns it into the decision. Asking the callback away from a draw is the whole reason + // that decision can reach the session already running. + deliver({ version: 2, custom: { optOut: true } }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends a collected session when the settings are switched off and init never collected', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startWith({ sessionSampleRate: 0 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).not.toBe(RumTrackingType.NOT_TRACKED) + + // Turning remote configuration off in the console stores the version and nothing else, so + // the rates go back to the ones the site passed to init. That is a change like any other, + // and here it is the decisive one. + deliver({ version: 2 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('draws the session that follows on the settings that have just landed', () => { + storeRemote({ version: 1, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + startWith({ sessionSampleRate: 100 }) + + deliver({ version: 2, sessionSampleRate: 0 }) + clock.tick(STORAGE_POLL_DELAY) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + }) + }) + + describe('everything else waits for the next session', () => { + it('leaves the session alone when the rate moves to a value it cannot decide on', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startWith({ sessionSampleRate: 100 }) + + deliver({ version: 2, sessionSampleRate: 30 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('leaves a session that lost a draw at a real rate alone when the rate rises', () => { + // The regression this exists to catch: re-drawing every session that is not collected, + // while leaving the collected ones alone, spares the winners and re-rolls the losers ā€” a + // fleet drawn at 30 and moved to 80 would come out well above 80. Only a session drawn at + // zero has no winner beside it to spare. + spyOn(Math, 'random').and.returnValue(0.99) + storeRemote({ version: 1, sessionSampleRate: 30 }) + startWith({ sessionSampleRate: 0 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + + deliver({ version: 2, sessionSampleRate: 80 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('leaves a session that is not being collected alone when the privacy level tightens', () => { + storeRemote({ version: 1, sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) + startWith({ sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + + // Nothing is being recorded for this visitor, so there is no plaintext for the stricter + // level to catch and nothing to gain by ending their session. + deliver({ version: 2, sessionSampleRate: 0, defaultPrivacyLevel: 'mask' }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('does not end one sampled-out session after another as settings keep arriving', () => { + // Nothing is recorded for this visitor, so a stricter level has nothing to catch however + // many times it is announced. The rate stays at zero throughout, so the one rule that does + // act on a sampled-out session finds nothing to act on either. + storeRemote({ version: 1, sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) + startWith({ sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) + + deliver({ version: 2, sessionSampleRate: 0, defaultPrivacyLevel: 'mask' }) + clock.tick(STORAGE_POLL_DELAY) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + deliver({ version: 3, sessionSampleRate: 0, defaultPrivacyLevel: 'mask' }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('leaves the session alone when the privacy level loosens', () => { + storeRemote({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'mask' }) + startWith({ sessionSampleRate: 100 }) + + // Being slow here is the point: it leaves an operator time to undo a mistake, and what it + // costs meanwhile is more of the data already being collected. + deliver({ version: 2, sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('leaves the session alone when only the custom bag changed', () => { + storeRemote({ version: 1, sessionSampleRate: 100, custom: { cohort: 'a' } }) + startWith({ sessionSampleRate: 100 }) + + deliver({ version: 2, sessionSampleRate: 100, custom: { cohort: 'b' } }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('leaves the session alone when only the trace rate changed', () => { + storeRemote({ version: 1, sessionSampleRate: 100, traceSampleRate: 10 }) + startWith({ sessionSampleRate: 100 }) + + deliver({ version: 2, sessionSampleRate: 100, traceSampleRate: 90 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('leaves the session alone when only the replay rate changed', () => { + storeRemote({ version: 1, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + startWith({ sessionSampleRate: 100 }) + + // The replay rate is deliberately not one of the three: it decides a draw nested inside the + // session draw, and a rule for it would have to say what happens to a replay the host + // application forced on. Until that is settled, a replay rate change waits for the next + // session like every other change. + deliver({ version: 2, sessionSampleRate: 100, sessionReplaySampleRate: 0 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('does not fall over when the site never opted in and has no settings store', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 0, drawStoreKey: DRAW_KEY } }) + + // Such a site never fetches, so the announcement can only ever be reached by hand and the + // store key below is one nothing would look under. All this pins down is that the decision + // survives `remoteConfig` being undefined; that the opt-out is respected is settled where + // the fetcher is never started, not here. + deliver({ version: 2, sessionSampleRate: 100 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('has nothing to end when the session is already over', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 0 }) + expireCookie() + clock.tick(STORAGE_POLL_DELAY) + expireSessionSpy.calls.reset() + + // There is no session to read a decision off, and nothing to end: the next activity draws + // on what has just been stored, which is all this change needs. + expect(() => deliver({ version: 2, sessionSampleRate: 100 })).not.toThrow() + expect(expireSessionSpy).not.toHaveBeenCalled() + }) + }) + + describe('what it compares', () => { + it('does not answer for a sampled-out session with the record of the one it replaced', () => { + // A page that draws owns the record slot. Having drawn on the init values it has nothing to + // record, and leaving the previous session's record there would let it answer for this one: + // every sampled-out session is recorded under the same id, so unlike a collected session it + // cannot tell that the record describes somebody else. + spyOn(Math, 'random').and.returnValue(0.99) + storeRemote({ version: 1, sessionSampleRate: 0 }) + const firstPage = startWith({ sessionSampleRate: 50 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + firstPage.stop() + stopSessionManager() + + // The settings entry is gone ā€” swept as belonging to a release nobody runs any more ā€” so + // the draw that follows uses the init rate and has nothing to record. It loses too, so it + // is a sampled-out session that did not write the record it would be read under. + localStorage.removeItem(STORE_KEY) + expireCookie() + const secondPage = startWith({ sessionSampleRate: 50 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + secondPage.stop() + stopSessionManager() + + // A third page restores that session instead of drawing one, so the record is the only + // thing it can read the draw off ā€” and the only record left would be the first session's. + startWith({ sessionSampleRate: 50 }) + expireSessionSpy.calls.reset() + + // Read off the first session's record this one looks drawn at zero and is re-drawn; read + // off init, which is what it was actually drawn at, it lost a draw at fifty and stays. + deliver({ version: 2, sessionSampleRate: 80 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('reads the rate a sampled-out session was drawn at back through storage', () => { + // The case that decides whether any of this reaches a real visitor: they were drawn at zero + // on the page before, and the page acting on the change never performed that draw. A + // sampled-out session is given no id, so its draw is recorded under one no session can + // hold ā€” without that record this page falls back to the init rate and answers wrongly. + storeRemote({ version: 1, sessionSampleRate: 0 }) + const firstPage = startWith({ sessionSampleRate: 50 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + firstPage.stop() + stopSessionManager() + + // A second page load restores the same session without drawing anything of its own. Init + // says 50 here on purpose: falling back to it would read this session as one that lost a + // draw and leave it alone, which is the answer the record exists to correct. + startWith({ sessionSampleRate: 50 }) + deliver({ version: 2, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('reads the rate off storage rather than off the draw this page last saw', () => { + // Two sampled-out sessions look alike to the session store ā€” no id, the same tracking type ā€” + // so a tab that misses the expired state between them never learns the session was + // replaced: nothing expires and nothing renews here, and what this page last read of the + // draw stays as it was. Storage is the one thing the tab that drew the replacement shares + // with this one, so it is what has to be read when the decision is made. + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 50 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + + // Another tab hears a rate of 30, ends the session drawn at zero and draws the next one, + // which loses ā€” all between two of this page's storage polls. + storeRemote({ version: 2, sessionSampleRate: 30 }) + setCookie(SESSION_STORE_KEY, `rum=0&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) + localStorage.setItem( + DRAW_KEY, + JSON.stringify({ + id: 'not-tracked', + version: 2, + sessionSampleRate: 30, + sessionReplaySampleRate: 50, + traceSampleRate: 100, + defaultPrivacyLevel: 'mask', + }) + ) + clock.tick(STORAGE_POLL_DELAY) + expect(expireSessionSpy).not.toHaveBeenCalled() + + // This page's own request answers with settings newer still. Read off the draw it last saw + // the session looks drawn at zero and is ended; read off storage it lost a draw at thirty + // and is left alone. + deliver({ version: 3, sessionSampleRate: 80 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('does not consult beforeSampling when no rate could decide anything', () => { + // Resolving the rate runs the site's own code, and an announcement is not a draw. It is + // asked only where the answer is what settles whether the session ends ā€” never once per + // announcement for every visitor. + const beforeSampling = jasmine.createSpy('beforeSampling').and.returnValue(undefined) + spyOn(Math, 'random').and.returnValue(0.99) + storeRemote({ version: 1, sessionSampleRate: 30 }) + startWith({ sessionSampleRate: 0, beforeSampling }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + beforeSampling.calls.reset() + + // This visitor lost a draw at thirty, so no rate the console publishes says anything about + // the session they are on, and there is nothing to ask. + deliver({ version: 2, sessionSampleRate: 80 }) + + expect(beforeSampling).not.toHaveBeenCalled() + }) + + it('never draws again to reach its decision', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startWith({ sessionSampleRate: 100 }) + const draw = spyOn(Math, 'random').and.callThrough() + + deliver({ version: 2, sessionSampleRate: 30 }) + + // Drawing here would be a second lottery on top of the one the next session runs, quietly + // turning a rate p into pĀ². + expect(draw).not.toHaveBeenCalled() + }) + + it('compares against the level the session was drawn under, not the settings stored since', () => { + storeRemote({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'mask' }) + startWith({ sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + + // A loosening leaves the running session masking everything, as it was drawn to. + deliver({ version: 2, sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + expect(expireSessionSpy).not.toHaveBeenCalled() + + // Stricter than what was stored a moment ago, still looser than what this session actually + // masks with. Judged against the stored settings it would end a session with nothing to + // gain from restarting. + deliver({ version: 3, sessionSampleRate: 100, defaultPrivacyLevel: 'mask-user-input' }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('lets beforeSampling have the last word on the rate it judges', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startWith({ + sessionSampleRate: 100, + beforeSampling: ({ sessionSampleRate }) => ({ sessionSampleRate: sessionSampleRate === 0 ? 50 : 100 }), + }) + + // The console says zero, the application puts it back in the middle: the rate that would + // actually apply is fifty, which decides nothing. + deliver({ version: 2, sessionSampleRate: 0 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + }) + + describe('arriving more than once', () => { + it('does not end the session a second time when the same settings arrive again', () => { + storeRemote({ version: 1, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + startWith({ sessionSampleRate: 100 }) + + deliver({ version: 2, sessionSampleRate: 0 }) + expect(isSessionEnded()).toBeTrue() + + clock.tick(STORAGE_POLL_DELAY) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + expireSessionSpy.calls.reset() + + // Another tab, a retry, a reload: the same answer arrives again and finds the difference + // that justified ending a session already gone. + lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) + lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('stops re-drawing once the session that follows has lost a draw at the new rate', () => { + // The loop this could have become: the replacement is sampled out too, and if it were read + // as another session drawn at zero every further announcement would end it again. It was + // drawn at the new rate, and that is what its record says. + spyOn(Math, 'random').and.returnValue(0.99) + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 0 }) + + deliver({ version: 2, sessionSampleRate: 30 }) + expect(isSessionEnded()).toBeTrue() + + clock.tick(STORAGE_POLL_DELAY) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + expireSessionSpy.calls.reset() + + lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('stops tightening the privacy level once the session is drawn under it', () => { + storeRemote({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + startWith({ sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + + deliver({ version: 2, sessionSampleRate: 100, defaultPrivacyLevel: 'mask' }) + expect(isSessionEnded()).toBeTrue() + + clock.tick(STORAGE_POLL_DELAY) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + expireSessionSpy.calls.reset() + + lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + }) + + describe('a session the host application forced', () => { + function startForced(configuration: Partial = {}) { + const rumSessionManager = startWith({ sessionSampleRate: 0, ...configuration }) + rumSessionManager.setForcedSession() + clock.tick(STORAGE_POLL_DELAY) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITH_SESSION_REPLAY) + expireSessionSpy.calls.reset() + return rumSessionManager + } + + it('is not ended by a rate, since every draw it makes is collected anyway', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + startForced() + + // Ending it would only replace it with another forced session ā€” the same difference, for + // as long as the page lives. + deliver({ version: 2, sessionSampleRate: 0 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('is still ended when the privacy level tightens', () => { + storeRemote({ version: 1, sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) + startForced({ defaultPrivacyLevel: 'allow' }) + + // Forcing decides whether this visitor is collected. It says nothing about how much of + // their page may be uploaded in the clear. + deliver({ version: 2, sessionSampleRate: 0, defaultPrivacyLevel: 'mask' }) + + expect(isSessionEnded()).toBeTrue() + }) + }) + }) + function startRumSessionManagerWithDefaults({ configuration, trackingConsentState = createTrackingConsentState(TrackingConsent.GRANTED), diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 423c3c9c27..7617bfb0e4 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -1,6 +1,7 @@ -import type { DefaultPrivacyLevel, RelativeTime, TrackingConsentState } from '@flashcatcloud/browser-core' +import type { RelativeTime, TrackingConsentState } from '@flashcatcloud/browser-core' import { BridgeCapability, + DefaultPrivacyLevel, Observable, SESSION_TIME_OUT_DELAY, STORAGE_POLL_DELAY, @@ -98,6 +99,34 @@ export const enum SessionReplayState { FORCED, } +/** + * FLASHCAT FORK - the id the draw of a session that lost its lottery is recorded under. + * + * A session that is not collected is given no id ā€” see `sessionStore` ā€” so it has nothing to key a + * record on, and until this existed its draw was simply not recorded. That left the one question + * this SDK has to answer before it may re-draw such a visitor unanswerable: was this session drawn + * at a rate of 0, or did it lose a draw at some other rate? Getting that wrong in the second + * direction re-rolls losers while sparing winners, which quietly lifts a fleet's real sampling + * rate towards 100% ā€” see `endSessionIfSettingsAreDecisive`. + * + * Not a UUID, and not a value `generateUUID` can produce, so a record written here can never be + * mistaken for a real session's. The two are told apart by the id alone, which is what lets both + * share the single record slot: a collected session looks its own id up and a sampled-out one + * looks this up, and neither can read the other's. + * + * What it gives up, and why that is affordable: every sampled-out session matches this same id, so + * the id check that makes a stale record inert for a collected session does nothing here. What + * keeps a stale one from being read instead is that the page which draws owns the slot ā€” it writes + * its draw or clears the slot, in the same stack that created the session ā€” so the record always + * describes the most recent draw, and the most recent draw is what created the session being read. + * The one thing the record decides for a sampled-out session ā€” whether a rate leaving 0 may end + * it ā€” is read off storage at the moment of that decision rather than off the copy `trackDraw` + * took when the session was adopted. See `endSessionIfSettingsAreDecisive` for why the copy is not + * enough: the session store cannot tell one sampled-out session from the next, so a page can keep + * the copy of a session another tab has already replaced. + */ +const NOT_TRACKED_DRAW_ID = 'not-tracked' + export function startRumSessionManager( configuration: RumConfiguration, lifeCycle: LifeCycle, @@ -171,8 +200,8 @@ export function startRumSessionManager( // synchronous stack: a tab whose storage poll fell exactly between the two would find no record // and keep its own settings for that session. Writing it earlier is not possible from here ā€” the // id it belongs to is generated inside the store, as that session is persisted. The record is - // read only here, when a session is adopted, so such a tab keeps its own settings for the whole - // remaining life of that session rather than until its next poll. + // read into the history only here, when a session is adopted, so such a tab keeps its own + // settings for the whole remaining life of that session rather than until its next poll. // // Storage is also per origin while the session need not be: with `trackSessionAcrossSubdomains` // a session arrives on the next subdomain with no record waiting, and is reported and traced @@ -181,15 +210,32 @@ export function startRumSessionManager( const drawn = pendingDraw pendingDraw = undefined const sessionEntity = sessionManager.findSession() - if (!sessionEntity?.id) { + if (!sessionEntity) { return } + // A session that lost its draw has no id to be recorded under, so it is recorded under an id no + // session can hold. It has to be recorded at all for the same reason a collected one does ā€” the + // rate it was drawn at is not something a later page can work out, and here it decides whether + // a console change away from 0 may re-draw this visitor at once. Which of the two is read back + // follows from the session itself, so no record can be read for a session it does not describe. + const drawId = sessionEntity.id || NOT_TRACKED_DRAW_ID if (drawn) { - writeDrawRecord(configuration, { id: sessionEntity.id, ...drawn }) - drawnHistory.add(drawn, startTime) + // The page that draws owns the slot, and says so either way. A record is only worth keeping + // when it says something the init values do not ā€” but leaving the previous one in place + // instead would let it outlive the session it described, and a sampled-out session cannot + // spot that the way a collected one does: it matches on an id every sampled-out session + // shares. So a draw that has nothing to record clears the slot rather than passing over it. + // The cost is one `removeItem` per session drawn on a site that enabled none of this, which + // is a handful per visit. + if (isWorthRecording(configuration, drawn)) { + writeDrawRecord(configuration, { id: drawId, ...drawn }) + drawnHistory.add(drawn, startTime) + } else { + forgetDrawRecord(configuration) + } return } - const stored = readDrawRecord(configuration, sessionEntity.id) + const stored = readDrawRecord(configuration, drawId) if (stored) { drawnHistory.add(stored, startTime) } @@ -206,6 +252,122 @@ export function startRumSessionManager( lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) }) + // FLASHCAT FORK - by the time this runs the client has already downloaded the new settings and + // filed them away, and without this it would then do nothing with them until the running session + // ends on its own ā€” up to four hours. That wait is the whole problem: a visitor who keeps loading + // pages fetches the change within seconds and then carries on under the old decision for the rest + // of their visit. + // + // Three changes are not made to wait, and what makes exactly those three special is that their + // outcome for the running session can be asserted without drawing again: + // + // - a stricter default privacy level: every further second recorded is a second of plaintext + // uploaded, and masking cannot reach back for it. This is the one whose cost is not + // recoverable, and the reason the rest of this exists; + // - a session sample rate of 0 for a session being collected: nothing is meant to be collected + // any more, and this is the emergency stop the console offers ā€” one that took four hours + // would not be one; + // - a rate above 0 for a session that was drawn AT 0: this visitor was never in a draw at all, + // and now could be. Without it an application whose rate only ever comes from the console + // shows an operator who has just switched collection on precisely nothing, for as long as + // the sessions already running take to rotate ā€” and nothing at all is indistinguishable from + // broken. + // + // The first two are about a session that is being collected, and the third only ever about one + // that is not, which is why each rule checks that for itself. + // + // No rate other than 0 says anything about whether THIS session should have been kept ā€” only a + // second draw could, and drawing twice silently turns a rate p into pĀ². That is also why the + // third rule is written against the rate the session was DRAWN at rather than against whether it + // is being collected: re-drawing every session that is not collected, while leaving the collected + // ones alone, spares the winners and re-rolls the losers, so a fleet drawn at 20 and moved to 50 + // would come out at 60. A rate of 0 is the one value with no winners to spare ā€” nothing was + // collected, no coin was flipped ā€” so re-drawing everyone lands exactly on the new rate. And it + // costs nothing to end such a session: it has no id, no events and no history, so it does not + // exist in the data and ending it leaves no seam. + // + // Everything else waits for the next session, a loosening privacy level included. Loosening waits + // on purpose: the delay is what leaves an operator room to undo a mistake, and what it costs + // meanwhile is more of the data already being collected. + // + // The action is to end the session and let the next activity start a new one ā€” never to flip the + // running one, which would leave a replay masked in its first half and plain in its second, or + // invent a session that begins in the middle of a visit. + // + // It stays idempotent with no bookkeeping at all: it compares what this session was drawn under + // against what a draw would use now, and ending the session is exactly what makes that + // difference disappear. The same response arriving again ā€” another tab, a retry, a reload ā€” + // finds nothing left to act on. + // + // What it cannot reach: settings are fetched at start-up and on session renewal only, so a page + // that is never reloaded never hears of the change. A single always-visible tab is exactly that + // page ā€” the visibility timer keeps renewing it, so it fetches nothing until the four-hour cap. + // Any other tab of the same visitor that does load a page ends the session they share. + function endSessionIfSettingsAreDecisive() { + const session = sessionManager.findSession() + if (!session) { + return + } + + const remote = readRemoteConfig(configuration.remoteConfig) + + if (!isTypeTracked(session.trackingType)) { + // Read off storage rather than off `drawnHistory`, because the two can disagree here and only + // storage is right. Two sampled-out sessions look alike to the session store ā€” no id, the + // same tracking type ā€” so a page whose storage poll misses the expired state between them + // never learns that another tab ended the first and drew the second: nothing expires and + // nothing renews on this page, and the history keeps the draw of a session that is gone. The + // page that drew the replacement wrote its rate to storage in the same stack, so that is the + // one place this session's own rate can be found. A collected session cannot be confused this + // way, since its id changes with it. + // + // Nothing forced can reach this comparison as a zero: a forced draw is recorded at 100 and is + // collected besides, so the record already answers the question the tracked branch has to ask + // `forcedSession` about below. No record means the draw used the init values, see `trackDraw`. + const drawnSampleRate = + readDrawRecord(configuration, NOT_TRACKED_DRAW_ID)?.sessionSampleRate ?? configuration.sessionSampleRate + if (drawnSampleRate !== 0) { + return + } + // Asked only now, and only here, because resolving runs the site's `beforeSampling`: this + // announcement is not a draw, and the callback should be run no more often than a decision + // actually turns on its answer. + if (resolveSampleRates(configuration, remote).sessionSampleRate > 0) { + sessionManager.expire() + } + return + } + + // What this session is masking pages with right now, which is not the previously stored + // settings: settings are stored while a session runs, and the session was drawn under whatever + // was stored before that. No record means the draw used the init value, and so does the + // recorder ā€” see `startRecording`, which falls back the same way. + const drawnPrivacyLevel = drawnHistory.find()?.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel + const nextPrivacyLevel = remote.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel + if (PRIVACY_LEVEL_STRICTNESS[nextPrivacyLevel] > PRIVACY_LEVEL_STRICTNESS[drawnPrivacyLevel]) { + sessionManager.expire() + return + } + + if (forcedSession) { + // The host application has taken this page off the rates deliberately, and every draw it + // makes from now on is collected whatever the console says. Ending it on a rate would only + // replace it with an identical forced session ā€” the same difference, forever. The flag is + // this page's: another tab of the same visitor that never called `setForcedSession` reads + // the shared session as an ordinary one and may end it on a rate. + return + } + + if (resolveSampleRates(configuration, remote).sessionSampleRate === 0) { + sessionManager.expire() + } + } + + const remoteConfigSubscription = lifeCycle.subscribe( + LifeCycleEventType.REMOTE_CONFIGURATION_STORED, + endSessionIfSettingsAreDecisive + ) + sessionManager.sessionStateUpdateObservable.subscribe(({ previousState, newState }) => { if (!previousState.forcedReplay && newState.forcedReplay) { const sessionEntity = sessionManager.findSession() @@ -238,6 +400,7 @@ export function startRumSessionManager( expireObservable: sessionManager.expireObservable, stop: () => { consentSubscription.unsubscribe() + remoteConfigSubscription.unsubscribe() drawnHistory.stop() }, setForcedReplay: () => sessionManager.updateSessionState({ forcedReplay: '1' }), @@ -365,9 +528,11 @@ function computeSessionState( configuration: RumConfiguration, rawTrackingType?: string, forcedSession?: boolean, - // FLASHCAT FORK - called when a draw actually happens (never for a restored session) and lands - // on something other than the init values, with the rates the draw used and the remote version - // they came from. + // FLASHCAT FORK - called whenever a draw actually happens and never for a restored session, with + // the rates the draw used and the remote version they came from. Reporting every draw, including + // one that landed on the init values, is what lets the caller tell "this page drew" from "this + // page adopted a session somebody else drew" ā€” see `trackDraw`, where only the first may write to + // the record slot. onDraw?: (drawn: DrawnConfiguration) => void ) { let trackingType: RumTrackingType @@ -387,34 +552,7 @@ function computeSessionState( // the decision it was created with: settings arriving mid-session never start or stop // collecting for a visitor already on the site. const remote = readRemoteConfig(configuration.remoteConfig) - - let sessionSampleRate = remote.sessionSampleRate ?? configuration.sessionSampleRate - let sessionReplaySampleRate = remote.sessionReplaySampleRate ?? configuration.sessionReplaySampleRate - - // FLASHCAT FORK - the application gets the last word, right at the draw. This is what turns the - // delivered custom values into sampling decisions without a wasted first draw or a session - // restart: the console ships the data (an allow-list, a cohort rule), the application's own - // code interprets it here. Its failure modes must never reach session creation, so a thrown - // error or a value outside 0..100 leaves the incoming rate in place. - if (configuration.beforeSampling) { - try { - const override = configuration.beforeSampling({ - sessionSampleRate, - sessionReplaySampleRate, - custom: remote.custom, - }) - if (override) { - if (isRate(override.sessionSampleRate)) { - sessionSampleRate = override.sessionSampleRate - } - if (isRate(override.sessionReplaySampleRate)) { - sessionReplaySampleRate = override.sessionReplaySampleRate - } - } - } catch (e) { - display.error('beforeSampling threw an error:', e) - } - } + const { sessionSampleRate, sessionReplaySampleRate } = resolveSampleRates(configuration, remote) reportDraw(configuration, remote, sessionSampleRate, sessionReplaySampleRate, onDraw) @@ -432,17 +570,64 @@ function computeSessionState( } } +/** + * FLASHCAT FORK - the rates a draw would use right now: what the console delivered, falling back to + * what the site passed to init, with the application's `beforeSampling` given the last word. This + * is what turns the delivered custom values into sampling decisions without a wasted first draw or + * a session restart: the console ships the data (an allow-list, a cohort rule), the application's + * own code interprets it here. Its failure modes must never reach session creation, so a thrown + * error or a value outside 0..100 leaves the incoming rate in place. + * + * It resolves rates and never draws on them, which is what lets the same question be asked away + * from a draw ā€” see `endSessionIfSettingsAreDecisive`, which needs to know which rate would apply + * without spending a lottery ticket to find out. + */ +function resolveSampleRates(configuration: RumConfiguration, remote: RemoteConfigValues) { + let sessionSampleRate = remote.sessionSampleRate ?? configuration.sessionSampleRate + let sessionReplaySampleRate = remote.sessionReplaySampleRate ?? configuration.sessionReplaySampleRate + + if (configuration.beforeSampling) { + try { + const override = configuration.beforeSampling({ + sessionSampleRate, + sessionReplaySampleRate, + custom: remote.custom, + }) + if (override) { + if (isRate(override.sessionSampleRate)) { + sessionSampleRate = override.sessionSampleRate + } + if (isRate(override.sessionReplaySampleRate)) { + sessionReplaySampleRate = override.sessionReplaySampleRate + } + } + } catch (e) { + display.error('beforeSampling threw an error:', e) + } + } + + return { sessionSampleRate, sessionReplaySampleRate } +} + +/** + * FLASHCAT FORK - how much of a page each level keeps out of a recording, ordered so two levels can + * be compared. Only the direction matters: tightening is the change that cannot be undone after the + * fact, because a second already recorded in the clear has already been uploaded in the clear. + */ +const PRIVACY_LEVEL_STRICTNESS: { [level in DefaultPrivacyLevel]: number } = { + [DefaultPrivacyLevel.ALLOW]: 0, + [DefaultPrivacyLevel.MASK_USER_INPUT]: 1, + [DefaultPrivacyLevel.MASK]: 2, +} + /** * FLASHCAT FORK - hands the draw that just happened to whoever records it. Both draw branches * report the same shape and differ only in the rates: forcing pins them, an ordinary draw uses * what the console and the application settled on. * - * What decides whether a draw is worth recording is the draw itself, not which feature produced it: - * a draw that used exactly what init passed is already described by the events, so recording it - * would buy nothing and cost a storage write on every site that turned none of this on. Everything - * else is recorded ā€” including a `beforeSampling` override or a forced session on a site with - * remote configuration switched off, where the rates used and the rates init passed are precisely - * the values that differ. + * Whether the draw is worth keeping is `isWorthRecording`'s question, asked one layer up, because + * the answer there decides between writing the record and clearing it ā€” and only a caller that + * hears about every draw can clear one. */ function reportDraw( configuration: RumConfiguration, @@ -454,23 +639,32 @@ function reportDraw( if (!onDraw) { return } - const drawn: DrawnConfiguration = { + onDraw({ version: remote.version, sessionSampleRate, sessionReplaySampleRate, traceSampleRate: remote.traceSampleRate ?? initTraceRule(configuration), defaultPrivacyLevel: remote.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel, - } - if ( - drawn.version === undefined && - drawn.sessionSampleRate === configuration.sessionSampleRate && - drawn.sessionReplaySampleRate === configuration.sessionReplaySampleRate && - drawn.traceSampleRate === initTraceRule(configuration) && - drawn.defaultPrivacyLevel === configuration.defaultPrivacyLevel - ) { - return - } - onDraw(drawn) + }) +} + +/** + * FLASHCAT FORK - whether a draw says anything the init values do not. + * + * One that does not is already described by the events, so keeping it would buy nothing and cost a + * storage write on every site that turned none of this on. Asked about the draw rather than about + * which feature produced it: a `beforeSampling` override or a forced session on a site with remote + * configuration switched off is precisely the case where the rates used and the rates init passed + * are the values that differ. + */ +function isWorthRecording(configuration: RumConfiguration, drawn: DrawnConfiguration) { + return ( + drawn.version !== undefined || + drawn.sessionSampleRate !== configuration.sessionSampleRate || + drawn.sessionReplaySampleRate !== configuration.sessionReplaySampleRate || + drawn.traceSampleRate !== initTraceRule(configuration) || + drawn.defaultPrivacyLevel !== configuration.defaultPrivacyLevel + ) } /** diff --git a/packages/rum-legacy/package.json b/packages/rum-legacy/package.json index 4385086930..6de09fe87e 100644 --- a/packages/rum-legacy/package.json +++ b/packages/rum-legacy/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum-legacy", - "version": "0.2.0", + "version": "0.2.3", "license": "Apache-2.0", "private": true, "description": "RUM Browser SDK build for browsers without ES2015 support. Distributed through the CDN only.", diff --git a/packages/rum-react/package.json b/packages/rum-react/package.json index 457cc004bd..e573607070 100644 --- a/packages/rum-react/package.json +++ b/packages/rum-react/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum-react", - "version": "0.2.0", + "version": "0.2.3", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", diff --git a/packages/rum-slim/package.json b/packages/rum-slim/package.json index 55ca8aafa2..e7b30dbb9f 100644 --- a/packages/rum-slim/package.json +++ b/packages/rum-slim/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum-slim", - "version": "0.2.0", + "version": "0.2.3", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", @@ -17,7 +17,7 @@ "@flashcatcloud/browser-rum-core": "workspace:*" }, "peerDependencies": { - "@flashcatcloud/browser-logs": "0.2.0" + "@flashcatcloud/browser-logs": "0.2.3" }, "peerDependenciesMeta": { "@flashcatcloud/browser-logs": { diff --git a/packages/rum/package.json b/packages/rum/package.json index a733ea737c..4b7154fe74 100644 --- a/packages/rum/package.json +++ b/packages/rum/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum", - "version": "0.2.0", + "version": "0.2.3", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", @@ -18,7 +18,7 @@ "@flashcatcloud/browser-rum-core": "workspace:*" }, "peerDependencies": { - "@flashcatcloud/browser-logs": "0.2.0" + "@flashcatcloud/browser-logs": "0.2.3" }, "peerDependenciesMeta": { "@flashcatcloud/browser-logs": { diff --git a/packages/worker/package.json b/packages/worker/package.json index ac71b95551..540a1fc2b2 100644 --- a/packages/worker/package.json +++ b/packages/worker/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-worker", - "version": "0.2.0", + "version": "0.2.3", "license": "Apache-2.0", "main": "bundle/worker.js", "scripts": { diff --git a/performances/package.json b/performances/package.json index d7338cac34..f8034b3907 100644 --- a/performances/package.json +++ b/performances/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "performances", - "version": "0.2.0", + "version": "0.2.3", "scripts": { "start": "ts-node ./src/main.ts" }, diff --git a/test/apps/react/yarn.lock b/test/apps/react/yarn.lock index 0cf90f7d4d..54ebdf9fdf 100644 --- a/test/apps/react/yarn.lock +++ b/test/apps/react/yarn.lock @@ -6,27 +6,27 @@ __metadata: cacheKey: 10c0 "@flashcatcloud/browser-core@file:../../../packages/core/package.tgz::locator=react-app%40workspace%3A.": - version: 0.1.1 - resolution: "@flashcatcloud/browser-core@file:../../../packages/core/package.tgz#../../../packages/core/package.tgz::hash=4465e7&locator=react-app%40workspace%3A." - checksum: 10c0/46e0299a01d91d26b69488f075c3a773730c7b68b31a6acc15a627b078cd5b7d79c0f064ecc9a95f2f463545c4160a4983e4bac38b2a5228424747034fbdc4d1 + version: 0.2.3 + resolution: "@flashcatcloud/browser-core@file:../../../packages/core/package.tgz#../../../packages/core/package.tgz::hash=3757b8&locator=react-app%40workspace%3A." + checksum: 10c0/cc949e44210ec8d8546242d0b8c4cbcae46f3b53a295d20740c16fbd95ef99c85f4b11312998938156fd82cd54546f57fd12ca5c50c932be8b5c241f091237a4 languageName: node linkType: hard "@flashcatcloud/browser-rum-core@file:../../../packages/rum-core/package.tgz::locator=react-app%40workspace%3A.": - version: 0.1.1 - resolution: "@flashcatcloud/browser-rum-core@file:../../../packages/rum-core/package.tgz#../../../packages/rum-core/package.tgz::hash=9cfaab&locator=react-app%40workspace%3A." + version: 0.2.3 + resolution: "@flashcatcloud/browser-rum-core@file:../../../packages/rum-core/package.tgz#../../../packages/rum-core/package.tgz::hash=bd22d4&locator=react-app%40workspace%3A." dependencies: - "@flashcatcloud/browser-core": "npm:0.1.1" - checksum: 10c0/4ddf61c4fe3fd8d3d59d4b33e0a93490540cbb9acb28a4b6e65966b3de1018e4fb609978d702a9abf7cbe1b1848f7effefa01818567ff9d7a61b68533d62bfbb + "@flashcatcloud/browser-core": "npm:0.2.3" + checksum: 10c0/f5d6867b01ff891dbf35cd0cc2887199df5d7f941f1c1cbfba8f55387084df096d20d5b1399790897c5c93342439c0df56409df02afb5276a3d98d2ed54e902b languageName: node linkType: hard "@flashcatcloud/browser-rum-react@file:../../../packages/rum-react/package.tgz::locator=react-app%40workspace%3A.": - version: 0.1.1 - resolution: "@flashcatcloud/browser-rum-react@file:../../../packages/rum-react/package.tgz#../../../packages/rum-react/package.tgz::hash=c6584f&locator=react-app%40workspace%3A." + version: 0.2.3 + resolution: "@flashcatcloud/browser-rum-react@file:../../../packages/rum-react/package.tgz#../../../packages/rum-react/package.tgz::hash=7e444d&locator=react-app%40workspace%3A." dependencies: - "@flashcatcloud/browser-core": "npm:0.1.1" - "@flashcatcloud/browser-rum-core": "npm:0.1.1" + "@flashcatcloud/browser-core": "npm:0.2.3" + "@flashcatcloud/browser-rum-core": "npm:0.2.3" peerDependencies: react: 18 || 19 react-router-dom: 6 || 7 @@ -39,22 +39,22 @@ __metadata: optional: true react-router-dom: optional: true - checksum: 10c0/dfe0ff4d0ca4b50ce92b0c2cc190104afd0998a636382f05447bd8670c6ae9c199614898c345e3022f294859378f9357b08a8ad9319038b4523a153ad9fc1894 + checksum: 10c0/2db122bbdf63bfe0e8c900cbca0f0f5b460db811a0837b2447aaef1768c2499e11565b67a28716f8faba038dd37e584618cd068a4c59ec5b9c8780c3b5c54115 languageName: node linkType: hard "@flashcatcloud/browser-rum@file:../../../packages/rum/package.tgz::locator=react-app%40workspace%3A.": - version: 0.1.1 - resolution: "@flashcatcloud/browser-rum@file:../../../packages/rum/package.tgz#../../../packages/rum/package.tgz::hash=ab62a4&locator=react-app%40workspace%3A." + version: 0.2.3 + resolution: "@flashcatcloud/browser-rum@file:../../../packages/rum/package.tgz#../../../packages/rum/package.tgz::hash=b90a0d&locator=react-app%40workspace%3A." dependencies: - "@flashcatcloud/browser-core": "npm:0.1.1" - "@flashcatcloud/browser-rum-core": "npm:0.1.1" + "@flashcatcloud/browser-core": "npm:0.2.3" + "@flashcatcloud/browser-rum-core": "npm:0.2.3" peerDependencies: - "@flashcatcloud/browser-logs": 0.1.1 + "@flashcatcloud/browser-logs": 0.2.3 peerDependenciesMeta: "@flashcatcloud/browser-logs": optional: true - checksum: 10c0/b866d94b34d3584c52e95e6bb244dec4186fd32a2f73cf6655349c6b2f50183bc130f4a872b3346b91cc8892a7ec3f74d3bad8b92cd39608327d6eff3fc8e2ed + checksum: 10c0/34dc1334c2125ecc6dab66764d7afa57ed2e85e22b7136fa8c15464ea93af619739c705c1a41453b10e642527f548952cd273e98768c041ea6c09b90f3bdffa4 languageName: node linkType: hard diff --git a/test/apps/vanilla/yarn.lock b/test/apps/vanilla/yarn.lock index 471d88bfa9..e278764101 100644 --- a/test/apps/vanilla/yarn.lock +++ b/test/apps/vanilla/yarn.lock @@ -6,47 +6,47 @@ __metadata: cacheKey: 10c0 "@flashcatcloud/browser-core@file:../../../packages/core/package.tgz::locator=app%40workspace%3A.": - version: 0.1.1 - resolution: "@flashcatcloud/browser-core@file:../../../packages/core/package.tgz#../../../packages/core/package.tgz::hash=4465e7&locator=app%40workspace%3A." - checksum: 10c0/46e0299a01d91d26b69488f075c3a773730c7b68b31a6acc15a627b078cd5b7d79c0f064ecc9a95f2f463545c4160a4983e4bac38b2a5228424747034fbdc4d1 + version: 0.2.3 + resolution: "@flashcatcloud/browser-core@file:../../../packages/core/package.tgz#../../../packages/core/package.tgz::hash=3757b8&locator=app%40workspace%3A." + checksum: 10c0/cc949e44210ec8d8546242d0b8c4cbcae46f3b53a295d20740c16fbd95ef99c85f4b11312998938156fd82cd54546f57fd12ca5c50c932be8b5c241f091237a4 languageName: node linkType: hard "@flashcatcloud/browser-logs@file:../../../packages/logs/package.tgz::locator=app%40workspace%3A.": - version: 0.1.1 - resolution: "@flashcatcloud/browser-logs@file:../../../packages/logs/package.tgz#../../../packages/logs/package.tgz::hash=9cf51a&locator=app%40workspace%3A." + version: 0.2.3 + resolution: "@flashcatcloud/browser-logs@file:../../../packages/logs/package.tgz#../../../packages/logs/package.tgz::hash=f16bce&locator=app%40workspace%3A." dependencies: - "@flashcatcloud/browser-core": "npm:0.1.1" + "@flashcatcloud/browser-core": "npm:0.2.3" peerDependencies: - "@flashcatcloud/browser-rum": 0.1.1 + "@flashcatcloud/browser-rum": 0.2.3 peerDependenciesMeta: "@flashcatcloud/browser-rum": optional: true - checksum: 10c0/2ed31be0c03c45f1edf422e8136bfa4b804fcbdda924d8eb11996f38f2c06ebe7c5f34fad26bec8c48905463fc90b48a5ad492e147dd78d3fde1f5f0bace4424 + checksum: 10c0/997fa5864dd29469fea4a042dae3eddbc9647aee8535074288328e9eced7d8274ef78ee7b2ff311821cf15f34d2bd3347bd9d7dacb77a75c194045c3eb36a3f1 languageName: node linkType: hard "@flashcatcloud/browser-rum-core@file:../../../packages/rum-core/package.tgz::locator=app%40workspace%3A.": - version: 0.1.1 - resolution: "@flashcatcloud/browser-rum-core@file:../../../packages/rum-core/package.tgz#../../../packages/rum-core/package.tgz::hash=9cfaab&locator=app%40workspace%3A." + version: 0.2.3 + resolution: "@flashcatcloud/browser-rum-core@file:../../../packages/rum-core/package.tgz#../../../packages/rum-core/package.tgz::hash=bd22d4&locator=app%40workspace%3A." dependencies: - "@flashcatcloud/browser-core": "npm:0.1.1" - checksum: 10c0/4ddf61c4fe3fd8d3d59d4b33e0a93490540cbb9acb28a4b6e65966b3de1018e4fb609978d702a9abf7cbe1b1848f7effefa01818567ff9d7a61b68533d62bfbb + "@flashcatcloud/browser-core": "npm:0.2.3" + checksum: 10c0/f5d6867b01ff891dbf35cd0cc2887199df5d7f941f1c1cbfba8f55387084df096d20d5b1399790897c5c93342439c0df56409df02afb5276a3d98d2ed54e902b languageName: node linkType: hard "@flashcatcloud/browser-rum@file:../../../packages/rum/package.tgz::locator=app%40workspace%3A.": - version: 0.1.1 - resolution: "@flashcatcloud/browser-rum@file:../../../packages/rum/package.tgz#../../../packages/rum/package.tgz::hash=ab62a4&locator=app%40workspace%3A." + version: 0.2.3 + resolution: "@flashcatcloud/browser-rum@file:../../../packages/rum/package.tgz#../../../packages/rum/package.tgz::hash=b90a0d&locator=app%40workspace%3A." dependencies: - "@flashcatcloud/browser-core": "npm:0.1.1" - "@flashcatcloud/browser-rum-core": "npm:0.1.1" + "@flashcatcloud/browser-core": "npm:0.2.3" + "@flashcatcloud/browser-rum-core": "npm:0.2.3" peerDependencies: - "@flashcatcloud/browser-logs": 0.1.1 + "@flashcatcloud/browser-logs": 0.2.3 peerDependenciesMeta: "@flashcatcloud/browser-logs": optional: true - checksum: 10c0/b866d94b34d3584c52e95e6bb244dec4186fd32a2f73cf6655349c6b2f50183bc130f4a872b3346b91cc8892a7ec3f74d3bad8b92cd39608327d6eff3fc8e2ed + checksum: 10c0/34dc1334c2125ecc6dab66764d7afa57ed2e85e22b7136fa8c15464ea93af619739c705c1a41453b10e642527f548952cd273e98768c041ea6c09b90f3bdffa4 languageName: node linkType: hard diff --git a/yarn.lock b/yarn.lock index 432b6fd53e..d350539f33 100644 --- a/yarn.lock +++ b/yarn.lock @@ -593,7 +593,7 @@ __metadata: "@flashcatcloud/browser-core": "workspace:*" webpack: "npm:5.99.9" peerDependencies: - "@flashcatcloud/browser-rum": 0.2.0 + "@flashcatcloud/browser-rum": 0.2.3 peerDependenciesMeta: "@flashcatcloud/browser-rum": optional: true @@ -607,7 +607,7 @@ __metadata: "@flashcatcloud/browser-core": "workspace:*" webpack: "npm:5.99.9" peerDependencies: - "@flashcatcloud/browser-rum": 0.2.0 + "@flashcatcloud/browser-rum": 0.2.3 peerDependenciesMeta: "@flashcatcloud/browser-rum": optional: true @@ -667,7 +667,7 @@ __metadata: "@flashcatcloud/browser-core": "workspace:*" "@flashcatcloud/browser-rum-core": "workspace:*" peerDependencies: - "@flashcatcloud/browser-logs": 0.2.0 + "@flashcatcloud/browser-logs": 0.2.3 peerDependenciesMeta: "@flashcatcloud/browser-logs": optional: true @@ -683,7 +683,7 @@ __metadata: "@types/pako": "npm:2.0.3" pako: "npm:2.1.0" peerDependencies: - "@flashcatcloud/browser-logs": 0.2.0 + "@flashcatcloud/browser-logs": 0.2.3 peerDependenciesMeta: "@flashcatcloud/browser-logs": optional: true