Skip to content

Commit 7776ffa

Browse files
authored
👷[RUM-11488] enforce specifying an expiration date for temporary telemetry (#3825)
* 👷add eslint rule to enforce `monitor-until` comments * 👷define expiration date or expire exiting telemetry * 👷add eslint rule to detect expired monitor-until comments * 👷add a scheduled job to check expired telemetry * 👌remove unnecessary ssh eval + report * 👌adjust replay telemetry expiration
1 parent 740f39f commit 7776ffa

File tree

25 files changed

+117
-83
lines changed

25 files changed

+117
-83
lines changed

‎.gitlab-ci.yml‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,3 +586,30 @@ bump-chrome-version-scheduled-failure:
586586
- postmessage "#browser-sdk-deploy" "$MESSAGE_TEXT"
587587
dependencies:
588588
- bump-chrome-version-scheduled
589+
590+
########################################################################################################################
591+
# Check expired telemetry
592+
########################################################################################################################
593+
594+
check-expired-telemetry-scheduled:
595+
stage: task
596+
extends: .base-configuration
597+
only:
598+
variables:
599+
- $TARGET_TASK_NAME == "check-expired-telemetry"
600+
script:
601+
- yarn
602+
- yarn build
603+
- MONITOR_UNTIL_COMMENT_EXPIRED_LEVEL=error yarn lint
604+
605+
check-expired-telemetry-scheduled-failure:
606+
extends: .prepare_notification
607+
only:
608+
variables:
609+
- $TARGET_TASK_NAME == "check-expired-telemetry"
610+
when: on_failure
611+
script:
612+
- 'MESSAGE_TEXT=":fire: [*$CI_PROJECT_NAME*] <$BUILD_URL|Expired telemetry detected> :fire:"'
613+
- postmessage "#rum-browser-sdk-ops" "$MESSAGE_TEXT"
614+
dependencies:
615+
- check-expired-telemetry-scheduled

‎eslint-local-rules/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,5 @@ module.exports = {
1818
'disallow-non-scripts': require('./disallowNonScripts'),
1919
'enforce-prod-deps-imports': require('./enforceProdDepsImports.js'),
2020
'secure-command-execution': require('./secureCommandExecution'),
21+
...require('./monitorUntilCommentRules'),
2122
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
const METHODS_TO_CHECK = ['addTelemetryDebug', 'addTelemetryMetrics']
2+
const MONITOR_COMMENT_FORMAT = /^\s*monitor-until: (\d{4}-\d{2}-\d{2}|forever)/
3+
4+
module.exports = {
5+
'enforce-monitor-until-comment': {
6+
meta: {
7+
docs: {
8+
description:
9+
'Force to specify an expiration date to telemetry debug and metrics events in order to clean them regularly',
10+
recommended: false,
11+
},
12+
schema: [],
13+
},
14+
create(context) {
15+
const sourceCode = context.getSourceCode()
16+
17+
return {
18+
CallExpression(node) {
19+
const methodName = node.callee.name
20+
if (!METHODS_TO_CHECK.includes(methodName)) {
21+
return
22+
}
23+
24+
const monitorComment = sourceCode
25+
.getCommentsBefore(node)
26+
.find((comment) => MONITOR_COMMENT_FORMAT.test(comment.value))
27+
28+
if (!monitorComment) {
29+
context.report(node, 'Missing `// monitor-until: YYYY-MM-DD` or `// monitor-until: forever` comment')
30+
}
31+
},
32+
}
33+
},
34+
},
35+
'monitor-until-comment-expired': {
36+
meta: {
37+
docs: {
38+
description: 'Report expired monitor-until comments',
39+
recommended: false,
40+
},
41+
schema: [],
42+
},
43+
create(context) {
44+
return {
45+
Program() {
46+
const now = new Date()
47+
const comments = context.getSourceCode().getAllComments()
48+
comments.forEach((comment) => {
49+
const monitorCommentMatch = comment.value.match(MONITOR_COMMENT_FORMAT)
50+
if (!monitorCommentMatch || monitorCommentMatch[1] === 'forever') {
51+
return
52+
}
53+
54+
if (new Date(monitorCommentMatch[1]) < now) {
55+
context.report(comment, `Expired: ${comment.value}`)
56+
}
57+
})
58+
},
59+
}
60+
},
61+
},
62+
}

‎eslint.config.mjs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import globals from 'globals'
1111
import eslintLocalRules from './eslint-local-rules/index.js'
1212

1313
const SPEC_FILES = '**/*.{spec,specHelper}.{ts,tsx,js}'
14+
const MONITOR_UNTIL_COMMENT_EXPIRED_LEVEL = process.env.MONITOR_UNTIL_COMMENT_EXPIRED_LEVEL || 'warn'
1415

1516
// eslint-disable-next-line import/no-default-export
1617
export default tseslint.config(
@@ -357,6 +358,8 @@ export default tseslint.config(
357358
files: ['packages/*/src/**/*.ts'],
358359
ignores: [SPEC_FILES],
359360
rules: {
361+
'local-rules/enforce-monitor-until-comment': 'error',
362+
'local-rules/monitor-until-comment-expired': MONITOR_UNTIL_COMMENT_EXPIRED_LEVEL,
360363
'local-rules/disallow-side-effects': 'error',
361364
'local-rules/disallow-zone-js-patched-values': 'error',
362365
'local-rules/disallow-url-constructor-patched-values': 'error',

‎packages/core/src/domain/allowedTrackingOrigins.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export function isAllowedTrackingOrigins(
1919
display.warn(WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN)
2020

2121
const extensionUrl = extractExtensionUrlFromStack(errorStack)
22+
// monitor-until: 2026-01-01
2223
addTelemetryDebug(WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN, {
2324
extensionUrl: extensionUrl || 'unknown',
2425
})

‎packages/core/src/domain/session/sessionManager.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ function trackResume(configuration: Configuration, cb: () => void) {
174174

175175
async function reportUnexpectedSessionState() {
176176
const rawSession = retrieveSessionCookie()
177+
// monitor-until: forever, could be handy to troubleshoot issues until session manager rework
177178
addTelemetryDebug('Unexpected session state', {
178179
session: rawSession,
179180
isSyntheticsTest: isSyntheticsTest(),
@@ -212,6 +213,7 @@ function detectSessionIdChange(configuration: Configuration, initialSessionState
212213
const time = dateNow() - sdkInitTime
213214
getSessionCookies()
214215
.then((cookie) => {
216+
// monitor-until: 2025-10-01, after investigation done
215217
addTelemetryDebug('Session cookie changed', {
216218
time,
217219
session_age: sessionAge,

‎packages/core/src/domain/session/sessionStoreOperations.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { setTimeout } from '../../tools/timer'
22
import { generateUUID } from '../../tools/utils/stringUtils'
33
import type { TimeStamp } from '../../tools/utils/timeUtils'
44
import { elapsed, ONE_SECOND, timeStampNow } from '../../tools/utils/timeUtils'
5-
import { addTelemetryDebug } from '../telemetry'
65
import type { SessionStoreStrategy } from './storeStrategies/sessionStoreStrategy'
76
import type { SessionState } from './sessionState'
87
import { expandSessionState, isSessionInExpiredState } from './sessionState'
@@ -46,9 +45,6 @@ export function processSessionStoreOperations(
4645
return
4746
}
4847
if (isLockEnabled && numberOfRetries >= LOCK_MAX_TRIES) {
49-
addTelemetryDebug('Aborted session operation after max lock retries', {
50-
currentStore: retrieveStore(),
51-
})
5248
next(sessionStoreStrategy)
5349
return
5450
}

‎packages/logs/src/boot/startLogs.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export function startLogs(
8080
const accountContext = startAccountContext(hooks, configuration, LOGS_STORAGE_KEY)
8181
const userContext = startUserContext(hooks, configuration, session, LOGS_STORAGE_KEY)
8282
const globalContext = startGlobalContext(hooks, configuration, LOGS_STORAGE_KEY, false)
83-
const { stop } = startRUMInternalContext(hooks)
83+
startRUMInternalContext(hooks)
8484

8585
startNetworkErrorCollection(configuration, lifeCycle)
8686
startRuntimeErrorCollection(configuration, lifeCycle, bufferedDataObservable)
@@ -114,7 +114,6 @@ export function startLogs(
114114
userContext,
115115
stop: () => {
116116
cleanupTasks.forEach((task) => task())
117-
stop()
118117
},
119118
}
120119
}

‎packages/logs/src/domain/contexts/rumInternalContext.spec.ts‎

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,21 @@
11
import type { RelativeTime } from '@datadog/browser-core'
22
import { HookNames } from '@datadog/browser-core'
3-
import { mockSyntheticsWorkerValues, startMockTelemetry } from '@datadog/browser-core/test'
3+
import { mockSyntheticsWorkerValues } from '@datadog/browser-core/test'
44
import type { Hooks } from '../hooks'
55
import { createHooks } from '../hooks'
66
import { startRUMInternalContext } from './rumInternalContext'
77

88
describe('startRUMInternalContext', () => {
99
let hooks: Hooks
10-
let stopRUMInternalContext: () => void
1110

1211
beforeEach(() => {
1312
hooks = createHooks()
14-
;({ stop: stopRUMInternalContext } = startRUMInternalContext(hooks))
13+
startRUMInternalContext(hooks)
1514
})
1615

1716
afterEach(() => {
1817
delete window.DD_RUM
1918
delete window.DD_RUM_SYNTHETICS
20-
stopRUMInternalContext()
2119
})
2220

2321
describe('assemble hook', () => {
@@ -61,33 +59,6 @@ describe('startRUMInternalContext', () => {
6159
})
6260
expect(defaultLogsEventAttributes).toEqual({ foo: 'bar' })
6361
})
64-
65-
it('adds a telemetry debug event when RUM has not been injected yet', async () => {
66-
const telemetry = startMockTelemetry()
67-
hooks.triggerHook(HookNames.Assemble, {
68-
startTime: 0 as RelativeTime,
69-
})
70-
expect(await telemetry.getEvents()).toEqual([
71-
jasmine.objectContaining({
72-
message: 'Logs sent before RUM is injected by the synthetics worker',
73-
status: 'debug',
74-
type: 'log',
75-
testId: 'test-id',
76-
resultId: 'result-id',
77-
}),
78-
])
79-
})
80-
81-
it('adds the telemetry debug event only once', async () => {
82-
const telemetry = startMockTelemetry()
83-
hooks.triggerHook(HookNames.Assemble, {
84-
startTime: 0 as RelativeTime,
85-
})
86-
hooks.triggerHook(HookNames.Assemble, {
87-
startTime: 0 as RelativeTime,
88-
})
89-
expect((await telemetry.getEvents()).length).toEqual(1)
90-
})
9162
})
9263
})
9364

‎packages/logs/src/domain/contexts/rumInternalContext.ts‎

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,5 @@
11
import type { RelativeTime, RumInternalContext } from '@datadog/browser-core'
2-
import {
3-
globalObject,
4-
willSyntheticsInjectRum,
5-
addTelemetryDebug,
6-
getSyntheticsTestId,
7-
getSyntheticsResultId,
8-
HookNames,
9-
SKIPPED,
10-
} from '@datadog/browser-core'
2+
import { globalObject, willSyntheticsInjectRum, HookNames, SKIPPED } from '@datadog/browser-core'
113
import type { Hooks } from '../hooks'
124

135
interface Rum {
@@ -21,7 +13,6 @@ interface BrowserWindow {
2113

2214
export function startRUMInternalContext(hooks: Hooks) {
2315
const browserWindow = globalObject as BrowserWindow
24-
let logsSentBeforeRumInjectionTelemetryAdded = false
2516

2617
hooks.register(HookNames.Assemble, ({ startTime }) => {
2718
const internalContext = getRUMInternalContext(startTime)
@@ -54,25 +45,11 @@ export function startRUMInternalContext(hooks: Hooks) {
5445
if (rumContext) {
5546
return rumContext
5647
}
57-
58-
if (willSyntheticsInjectRumResult && !logsSentBeforeRumInjectionTelemetryAdded) {
59-
logsSentBeforeRumInjectionTelemetryAdded = true
60-
addTelemetryDebug('Logs sent before RUM is injected by the synthetics worker', {
61-
testId: getSyntheticsTestId(),
62-
resultId: getSyntheticsResultId(),
63-
})
64-
}
6548
}
6649

6750
function getInternalContextFromRumGlobal(startTime?: RelativeTime, rumGlobal?: Rum): RumInternalContext | undefined {
6851
if (rumGlobal && rumGlobal.getInternalContext) {
6952
return rumGlobal.getInternalContext(startTime)
7053
}
7154
}
72-
73-
return {
74-
stop: () => {
75-
logsSentBeforeRumInjectionTelemetryAdded = false
76-
},
77-
}
7855
}

0 commit comments

Comments
 (0)