From 2d6e34eb60b60f3d9ebb23983315c7f17262733f Mon Sep 17 00:00:00 2001 From: Benoit Zugmeyer Date: Fri, 28 Aug 2026 11:32:14 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20skip=20telemetry=20error=20c?= =?UTF-8?q?hecks=20when=20no=20datacenter=20has=20credentials?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deploy-prod-dc telemetry gate runs for 30 minutes even when every datacenter lacks a configured API/application key, with each iteration just logging 'skipping...' and the loop spinning uselessly. Detect this upfront via hasTelemetryCredentials() and skip the pre-deploy check and the gate entirely when no datacenter can be queried. --- scripts/deploy/deploy-prod-dc.spec.ts | 28 ++++++++++++++++++++++ scripts/deploy/deploy-prod-dc.ts | 10 ++++++-- scripts/deploy/lib/checkTelemetryErrors.ts | 28 +++++++++++++++++++++- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/scripts/deploy/deploy-prod-dc.spec.ts b/scripts/deploy/deploy-prod-dc.spec.ts index 24cc99801f..6efce82a49 100644 --- a/scripts/deploy/deploy-prod-dc.spec.ts +++ b/scripts/deploy/deploy-prod-dc.spec.ts @@ -11,10 +11,12 @@ const currentBrowserSdkVersionMajor = browserSdkVersion.split('.')[0] describe('deploy-prod-dc', () => { const commandMock = mock.fn() const checkTelemetryErrorsMock: Mock<(datacenters: string[], version: string) => Promise> = mock.fn() + const hasTelemetryCredentialsMock: Mock<(datacenters: string[]) => Promise> = mock.fn() const fetchHandlingErrorMock = mock.fn() let commands: CommandDetail[] let checkTelemetryErrorsCalls: Array<{ version: string; datacenters: string[] }> + let hasTelemetryCredentialsCalls: string[][] let mockTime: number const originalDateNow = Date.now @@ -30,6 +32,7 @@ describe('deploy-prod-dc', () => { }) await mockModule(path.resolve(import.meta.dirname, './lib/checkTelemetryErrors.ts'), { checkTelemetryErrors: checkTelemetryErrorsMock, + hasTelemetryCredentials: hasTelemetryCredentialsMock, }) }) @@ -37,10 +40,15 @@ describe('deploy-prod-dc', () => { mockDatacenters(MOCK_DATACENTERS) commands = mockCommandImplementation(commandMock) checkTelemetryErrorsCalls = [] + hasTelemetryCredentialsCalls = [] checkTelemetryErrorsMock.mock.mockImplementation((datacenters: string[], version: string) => { checkTelemetryErrorsCalls.push({ version, datacenters }) return Promise.resolve() }) + hasTelemetryCredentialsMock.mock.mockImplementation((datacenters: string[]) => { + hasTelemetryCredentialsCalls.push(datacenters) + return Promise.resolve(true) + }) // Mock time control mockTime = Date.UTC(2026, 0, 16, 12, 0, 0) @@ -118,12 +126,32 @@ describe('deploy-prod-dc', () => { // gov datacenters should not be checked for telemetry errors assert.strictEqual(checkTelemetryErrorsCalls.length, 0) + // credentials should not be checked either, since gov skips telemetry entirely + assert.strictEqual(hasTelemetryCredentialsCalls.length, 0) assert.deepEqual(commands, [ { command: 'node ./scripts/deploy/deploy.ts prod v6 root' }, { command: 'node ./scripts/deploy/upload-source-maps.ts v6 root' }, ]) }) + + it('should skip telemetry error checks when no datacenter has telemetry credentials', async () => { + hasTelemetryCredentialsMock.mock.mockImplementation((datacenters: string[]) => { + hasTelemetryCredentialsCalls.push(datacenters) + return Promise.resolve(false) + }) + + await runScript('./deploy-prod-dc.ts', 'v6', 'us1', '--check-telemetry-errors') + + // credentials are checked once, but no telemetry error checks are performed + assert.strictEqual(hasTelemetryCredentialsCalls.length, 1) + assert.strictEqual(checkTelemetryErrorsCalls.length, 0) + + assert.deepEqual(commands, [ + { command: 'node ./scripts/deploy/deploy.ts prod v6 us1' }, + { command: 'node ./scripts/deploy/upload-source-maps.ts v6 us1' }, + ]) + }) }) async function runScript(scriptPath: string, ...args: string[]): Promise { diff --git a/scripts/deploy/deploy-prod-dc.ts b/scripts/deploy/deploy-prod-dc.ts index cba846658a..25b119f953 100644 --- a/scripts/deploy/deploy-prod-dc.ts +++ b/scripts/deploy/deploy-prod-dc.ts @@ -3,7 +3,7 @@ import { printLog, runMain, timeout } from '../lib/executionUtils.ts' import { command } from '../lib/command.ts' import { DatacenterType, getAllDatacentersMetadata } from '../lib/datacenter.ts' import { browserSdkVersion } from '../lib/browserSdkVersion.ts' -import { checkTelemetryErrors } from './lib/checkTelemetryErrors.ts' +import { checkTelemetryErrors, hasTelemetryCredentials } from './lib/checkTelemetryErrors.ts' /** * Orchestrate the deployments of the artifacts for specific DCs @@ -49,7 +49,13 @@ export async function main(...args: string[]): Promise { } // Skip all telemetry error checks for gov datacenter deployments - const shouldCheckTelemetryErrors = checkTelemetryErrorsFlag && !datacenters.every((dc) => dc === 'gov') + const isGovOnly = datacenters.every((dc) => dc === 'gov') + let shouldCheckTelemetryErrors = checkTelemetryErrorsFlag && !isGovOnly + + if (shouldCheckTelemetryErrors && !(await hasTelemetryCredentials(datacenters))) { + printLog('No telemetry credentials found for any datacenter, skipping telemetry error checks.') + shouldCheckTelemetryErrors = false + } if (shouldCheckTelemetryErrors) { // Make sure system is in a good state before deploying diff --git a/scripts/deploy/lib/checkTelemetryErrors.ts b/scripts/deploy/lib/checkTelemetryErrors.ts index 1f93331c16..b689b71da2 100644 --- a/scripts/deploy/lib/checkTelemetryErrors.ts +++ b/scripts/deploy/lib/checkTelemetryErrors.ts @@ -73,7 +73,9 @@ export async function checkTelemetryErrors(datacenters: string[], version: strin } } -async function checkDatacenterTelemetryErrors(datacenter: string, queries: Query[], agent: Agent): Promise { +async function getDatacenterTelemetryCredentials( + datacenter: string +): Promise<{ site: string; apiKey: string; applicationKey: string } | undefined> { const datacenterMetadata = await getDatacenterMetadata(datacenter) if (!datacenterMetadata?.site) { @@ -91,6 +93,30 @@ async function checkDatacenterTelemetryErrors(datacenter: string, queries: Query return } + return { site, apiKey, applicationKey } +} + +/** + * Returns true if at least one of the given datacenters has telemetry credentials configured. + * Use this to skip telemetry checks entirely when no datacenter can be queried. + */ +export async function hasTelemetryCredentials(datacenters: string[]): Promise { + for (const datacenter of datacenters) { + if ((await getDatacenterTelemetryCredentials(datacenter)) !== undefined) { + return true + } + } + return false +} + +async function checkDatacenterTelemetryErrors(datacenter: string, queries: Query[], agent: Agent): Promise { + const credentials = await getDatacenterTelemetryCredentials(datacenter) + if (!credentials) { + return + } + + const { site, apiKey, applicationKey } = credentials + for (let i = 0; i < queries.length; i++) { const query = queries[i] const buckets = await queryLogsApi(site, apiKey, applicationKey, query, agent) From 9c94412a3b9d91b78053a43fbc55954ed134d48c Mon Sep 17 00:00:00 2001 From: Benoit Zugmeyer Date: Fri, 28 Aug 2026 11:37:16 +0200 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=90=9B=20fix=20'undefined'=20event=20?= =?UTF-8?q?count=20in=20telemetry=20error=20logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the logs API returns no buckets, the computed count is undefined, producing logs like 'found undefined event(s)'. Default to 0 instead. --- .../deploy/lib/checkTelemetryErrors.spec.ts | 6 ++ scripts/deploy/lib/checkTelemetryErrors.ts | 2 +- scripts/lib/secrets.spec.ts | 58 +++++++++++++++++++ scripts/lib/secrets.ts | 29 ++++++---- 4 files changed, 84 insertions(+), 11 deletions(-) create mode 100644 scripts/lib/secrets.spec.ts diff --git a/scripts/deploy/lib/checkTelemetryErrors.spec.ts b/scripts/deploy/lib/checkTelemetryErrors.spec.ts index 9243c27e9f..8dcaf72e9b 100644 --- a/scripts/deploy/lib/checkTelemetryErrors.spec.ts +++ b/scripts/deploy/lib/checkTelemetryErrors.spec.ts @@ -131,6 +131,12 @@ describe('check-telemetry-errors', () => { ) }) + it('should not throw and report 0 events when no buckets are returned', async () => { + mockFetch([[], [], []]) + + await assert.doesNotReject(() => checkTelemetryErrors(['us1'], '6.2.1')) + }) + it('should throw an error if the API returns an unexpected response format', async () => { // Mock first API call with invalid response (missing data.buckets) fetchMock.mock.mockImplementationOnce( diff --git a/scripts/deploy/lib/checkTelemetryErrors.ts b/scripts/deploy/lib/checkTelemetryErrors.ts index b689b71da2..17a5d97ac5 100644 --- a/scripts/deploy/lib/checkTelemetryErrors.ts +++ b/scripts/deploy/lib/checkTelemetryErrors.ts @@ -120,7 +120,7 @@ async function checkDatacenterTelemetryErrors(datacenter: string, queries: Query for (let i = 0; i < queries.length; i++) { const query = queries[i] const buckets = await queryLogsApi(site, apiKey, applicationKey, query, agent) - const count = buckets[0]?.computes?.c0 + const count = buckets[0]?.computes?.c0 ?? 0 // buckets are sorted by count, so we only need to check the first one if (count > query.threshold) { diff --git a/scripts/lib/secrets.spec.ts b/scripts/lib/secrets.spec.ts new file mode 100644 index 0000000000..1bb1167fb0 --- /dev/null +++ b/scripts/lib/secrets.spec.ts @@ -0,0 +1,58 @@ +import assert from 'node:assert/strict' +import path from 'node:path' +import { afterEach, before, describe, it, mock } from 'node:test' +import { mockModule } from '../deploy/lib/testHelpers.ts' + +describe('secrets', () => { + let getTelemetryOrgApiKey: (site: string) => string | undefined + const commandMock = mock.fn<(...args: any[]) => any>() + + before(async () => { + await mockModule(path.resolve(import.meta.dirname, './command.ts'), { command: commandMock }) + ;({ getTelemetryOrgApiKey } = await import('./secrets.ts')) + }) + + afterEach(() => { + commandMock.mock.resetCalls() + }) + + function mockCommandRun(run: () => string): void { + commandMock.mock.mockImplementation(() => { + const chain = { + withInput: () => chain, + withEnvironment: () => chain, + withCurrentWorkingDirectory: () => chain, + withLogs: () => chain, + run, + } + return chain + }) + } + + it('returns the secret value when the parameter exists', () => { + mockCommandRun(() => 'a-secret-value\n') + + assert.strictEqual(getTelemetryOrgApiKey('datadoghq.com'), 'a-secret-value') + }) + + it('returns undefined when the parameter is not found', () => { + mockCommandRun(() => { + throw new Error( + 'Command failed with exit status 255: aws ssm get-parameter\n---- stderr: ----\n' + + 'aws: [ERROR]: An error occurred (ParameterNotFound) when calling the GetParameter operation:\n----' + ) + }) + + assert.strictEqual(getTelemetryOrgApiKey('datadoghq.com'), undefined) + }) + + it('rethrows errors that are not ParameterNotFound', () => { + mockCommandRun(() => { + throw new Error( + 'Command failed with exit status 255: aws ssm get-parameter\n---- stderr: ----\nNetwork error\n----' + ) + }) + + assert.throws(() => getTelemetryOrgApiKey('datadoghq.com'), /Network error/) + }) +}) diff --git a/scripts/lib/secrets.ts b/scripts/lib/secrets.ts index 8d429a6bd9..3a6744abcd 100644 --- a/scripts/lib/secrets.ts +++ b/scripts/lib/secrets.ts @@ -51,20 +51,12 @@ export function getOrg2AppKey(): string { export function getTelemetryOrgApiKey(site: string): string | undefined { const normalizedSite = site.replaceAll('.', '-') - try { - return getSecretKey(`ci.browser-sdk.source-maps.${normalizedSite}.ci_api_key`) - } catch { - return - } + return getOptionalSecretKey(`ci.browser-sdk.source-maps.${normalizedSite}.ci_api_key`) } export function getTelemetryOrgApplicationKey(site: string): string | undefined { const normalizedSite = site.replaceAll('.', '-') - try { - return getSecretKey(`ci.browser-sdk.telemetry.${normalizedSite}.ci_app_key`) - } catch { - return - } + return getOptionalSecretKey(`ci.browser-sdk.telemetry.${normalizedSite}.ci_app_key`) } export function getNpmToken(): string { @@ -122,3 +114,20 @@ function getSecretKey(name: string): string { .run() .trim() } + +/** + * Like getSecretKey, but returns undefined when the parameter does not exist instead of throwing. + * + * Other errors (e.g. AWS permission issues or temporary network failures) are rethrown so they are + * not silently mistaken for a missing parameter, which could otherwise disable safety checks. + */ +function getOptionalSecretKey(name: string): string | undefined { + try { + return getSecretKey(name) + } catch (error) { + if (error instanceof Error && /ParameterNotFound/.test(error.message)) { + return + } + throw error + } +}