Skip to content

Conversation

@thomas-lebeau
Copy link
Collaborator

Motivation

When deploying to a new DC, the API key for uploading sourcemaps is not set up yet, in that case we should skip uploading source maps.

Changes

Test instructions

Checklist

  • Tested locally
  • Tested on staging
  • Added unit tests for this change.
  • Added e2e/integration tests for this change.

@thomas-lebeau thomas-lebeau marked this pull request as ready for review November 19, 2025 10:36
@thomas-lebeau thomas-lebeau requested a review from a team as a code owner November 19, 2025 10:36
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

headers: {
Accept: 'application/json',
'DD-API-KEY': getTelemetryOrgApiKey(site),
'DD-APPLICATION-KEY': getTelemetryOrgApplicationKey(site),

P1 Badge Handle optional telemetry keys in monitor check headers

After getTelemetryOrgApiKey and getTelemetryOrgApplicationKey were changed to return string | undefined, this call site still forwards their result directly into the headers literal. Under tsconfig.scripts.json (strict mode), { 'DD-API-KEY': string | undefined } is no longer assignable to HeadersInit, so the script fails to type‑check and cannot run. Even at runtime, an undefined secret would now be sent as the literal string 'undefined' instead of failing fast. The headers should guard against missing secrets or cast to a definite string before use.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@thomas-lebeau
Copy link
Collaborator Author

thomas-lebeau commented Nov 19, 2025

codex is kind of right, however, these monitor check only happens when monitorIdsByDatacenter[datacenter] is defined. It is expected that the key get set up by the time the monitors are created and setup.

I plan to refactor the way we monitor when release to a DC so we can get rid of the hardcoded list of datacenter in the repo. This will be done in a followup PR, for now this is fine.

@thomas-lebeau thomas-lebeau merged commit 5de28a8 into main Nov 19, 2025
22 checks passed
@thomas-lebeau thomas-lebeau deleted the thomas.lebeau/allow-undefined-api-key branch November 19, 2025 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants