Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions scripts/deploy/deploy-prod-dc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>> = mock.fn()
const hasTelemetryCredentialsMock: Mock<(datacenters: string[]) => Promise<boolean>> = 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

Expand All @@ -30,17 +32,23 @@ describe('deploy-prod-dc', () => {
})
await mockModule(path.resolve(import.meta.dirname, './lib/checkTelemetryErrors.ts'), {
checkTelemetryErrors: checkTelemetryErrorsMock,
hasTelemetryCredentials: hasTelemetryCredentialsMock,
})
})

beforeEach(() => {
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)
Expand Down Expand Up @@ -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<void> {
Expand Down
10 changes: 8 additions & 2 deletions scripts/deploy/deploy-prod-dc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -49,7 +49,13 @@ export async function main(...args: string[]): Promise<void> {
}

// 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
Comment on lines +55 to +57

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the gate on credential lookup failures

issue: When AWS SSM is temporarily unavailable or the job loses permission, both telemetry credential getters catch every command failure and return undefined, so this condition disables the pre-deploy check and the entire post-deploy gate while the production workflow proceeds with deployment. Before this change, the gate retried credential reads on every iteration and could recover from a transient failure; distinguish a genuinely missing parameter from lookup errors, or keep reevaluating credentials during the gate.

Useful? React with 馃憤聽/ 馃憥.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BenoitZugmeyer should we only skip when crendentials are confirmed to be missing ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a commit for that. The command will now crash if something goes wrong instead of proceed forward.

}

if (shouldCheckTelemetryErrors) {
// Make sure system is in a good state before deploying
Expand Down
6 changes: 6 additions & 0 deletions scripts/deploy/lib/checkTelemetryErrors.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
30 changes: 28 additions & 2 deletions scripts/deploy/lib/checkTelemetryErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ export async function checkTelemetryErrors(datacenters: string[], version: strin
}
}

async function checkDatacenterTelemetryErrors(datacenter: string, queries: Query[], agent: Agent): Promise<void> {
async function getDatacenterTelemetryCredentials(
datacenter: string
): Promise<{ site: string; apiKey: string; applicationKey: string } | undefined> {
const datacenterMetadata = await getDatacenterMetadata(datacenter)

if (!datacenterMetadata?.site) {
Expand All @@ -91,10 +93,34 @@ 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<boolean> {
for (const datacenter of datacenters) {
if ((await getDatacenterTelemetryCredentials(datacenter)) !== undefined) {
return true
}
}
return false
}

async function checkDatacenterTelemetryErrors(datacenter: string, queries: Query[], agent: Agent): Promise<void> {
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)
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) {
Expand Down
58 changes: 58 additions & 0 deletions scripts/lib/secrets.spec.ts
Original file line number Diff line number Diff line change
@@ -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/)
})
})
29 changes: 19 additions & 10 deletions scripts/lib/secrets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
}
Loading