Skip to content

👷 Make json-schemas sync reliable: authenticate API requests and format output deterministically - #4832

Merged
bdibon merged 2 commits into
mainfrom
boris.dibon/json-schemas-auth-rate-limit
Jun 29, 2026
Merged

👷 Make json-schemas sync reliable: authenticate API requests and format output deterministically#4832
bdibon merged 2 commits into
mainfrom
boris.dibon/json-schemas-auth-rate-limit

Conversation

@bdibon

@bdibon bdibon commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Motivation

Two issues made yarn json-schemas:sync unreliable:

  1. Rate limiting — the branch-resolution step resolves the latest rum-events-format commit through an unauthenticated GitHub REST API call, capped at 60/hour per IP. Behind a shared corporate NAT that budget is consumed by everyone on the same address, so the sync fails with HTTP 403:

    API rate limit exceeded for <ip>. ... Authenticated requests get a higher rate limit.
    x-ratelimit-limit: 60   x-ratelimit-remaining: 0
    
  2. Non-deterministic formattingjson-schema-to-typescript formats the generated *.types.ts files with whatever prettier it resolves at runtime, which differs between environments (e.g. a nested prettier left by a prior fork build locally vs. a freshly built fork in CI). In CI this collapsed union types onto a single line, so the check-schemas job reported a diff against the committed (repo-prettier-formatted) files and failed.

Changes

Both changes are in scripts/json-schemas.ts:

  • Authenticate GitHub requests: resolve the branch using the developer's existing gh CLI token (authenticated limit: 5000/hour). Try gh auth token; on success attach an Authorization: token … header (style matches the existing call in scripts/lib/gitUtils.ts). If gh is missing or the user isn't logged in, print a warning and fall back to the previous unauthenticated behavior rather than failing. No env-var setup is required.
  • Deterministic formatting: skip json-schema-to-typescript's internal prettier pass (format: false) and format the compiled output with the repo's pinned prettier (parser: 'typescript') before writing. Generation is now identical across environments and matches prettier --check ., so check-schemas stays green.

(The PR also includes a routine rum-events-format schema bump produced by running the now-reliable sync end-to-end.)

Test instructions

  1. Auth — happy path (logged into gh):
    node scripts/json-schemas.ts --update master
    Resolves the latest commit with no 403.
  2. Auth — fallback path (simulate gh unavailable):
    d=$(mktemp -d); printf '#!/bin/sh\nexit 1\n' > "$d/gh"; chmod +x "$d/gh"
    PATH="$d:$PATH" node scripts/json-schemas.ts --update master
    Prints the warning and proceeds with an unauthenticated request instead of crashing.
  3. Formatting determinism: from a clean tree, regenerate and confirm no diff:
    yarn json-schemas:generate && git diff --stat   # expect: no changes
    Then force a fresh generator build (the original CI failure mode) and repeat:
    rm -rf node_modules/json-schema-to-typescript/dist node_modules/json-schema-to-typescript/node_modules/prettier
    yarn json-schemas:generate && git diff --stat   # still expect: no changes
  4. Run the CI check end-to-end (from a clean tree): node scripts/check-schemas.ts exits 0.

Checklist

  • Tested locally
  • Tested on staging
  • Added unit tests for this change.
  • Added e2e/integration tests for this change.
  • Updated documentation and/or relevant AGENTS.md file

🤖 Generated with Claude Code

`yarn json-schemas:sync` resolves the latest rum-events-format commit via an
unauthenticated GitHub API call, capped at 60 requests/hour per IP. Behind a
shared NAT that budget is exhausted by everyone on the same address, producing
a 403 rate-limit error.

Use the developer's `gh auth token` to authenticate the request (5000/hour). If
`gh` is unavailable or not logged in, warn and fall back to the previous
unauthenticated behavior instead of failing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Jun 29, 2026

Copy link
Copy Markdown

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 77.20% (+0.00%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 35906e8 | Docs | Datadog PR Page | Give us feedback!

@cit-pr-commenter-54b7da

Copy link
Copy Markdown

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 172.75 KiB 172.75 KiB 0 B 0.00%
Rum Profiler 8.22 KiB 8.22 KiB 0 B 0.00%
Rum Recorder 21.14 KiB 21.14 KiB 0 B 0.00%
Logs 54.44 KiB 54.44 KiB 0 B 0.00%
Rum Slim 130.27 KiB 130.27 KiB 0 B 0.00%
Worker 22.96 KiB 22.96 KiB 0 B 0.00%

@bdibon bdibon changed the title 👷 Authenticate GitHub API requests in json-schemas sync 👷 Make json-schemas sync reliable: authenticate API requests and format output deterministically Jun 29, 2026
@bdibon
bdibon marked this pull request as ready for review June 29, 2026 09:38
@bdibon
bdibon requested a review from a team as a code owner June 29, 2026 09:38

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a947cff29d

ℹ️ 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".

Comment thread scripts/json-schemas.ts
// behind a shared NAT. Authenticate with the user's `gh` CLI token to get the 5000/hour limit.
let token = ''
try {
token = command`gh auth token`.run().trim()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Pin gh token lookup to github.com

issue: If a developer has GH_HOST set to a GitHub Enterprise host, gh auth token can return that host's token—the GitHub CLI manual says gh auth token chooses a default host without --hostname (https://cli.github.com/manual/gh_auth_token) and GH_HOST supplies that host (https://cli.github.com/manual/gh_help_environment). This code then sends the token to the hard-coded api.github.com URL, so yarn json-schemas:sync can fail instead of falling back and may expose the wrong token; request the token with gh auth token --hostname github.com for this GitHub.com API call.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let's keep it simple and not rely on env variables

Comment thread package.json Outdated
},
"devDependencies": {
"@datadog/rum-events-format": "DataDog/rum-events-format#commit=02c94b31a2676458a156ed627f3edcd87254beb7",
"@datadog/rum-events-format": "DataDog/rum-events-format#commit=eb646464de64b1296c61738d98f87bf0a84ec207",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Normalize unknown quota reasons before using the new schema

issue: This bump brings in a schema that enumerates _dd.profiling.quota_reason, but the SDK still forwards arbitrary backend reason strings: parseQuotaResult casts attrs.reason directly in packages/browser-rum/src/domain/profiling/quotaCheck.ts:23-24, and the existing spec covers unknown_reason in datadogProfiler.spec.ts:1184-1194. If the quota API returns a new reason, startProfilingContext attaches that value to VIEW/LONG_TASK/ACTION/VITAL events, so those events no longer validate against the schema used by our format checks; map unknown reasons to an allowed value (or update the schema) with this bump.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let's drop the middle commit, it should work anyway

json-schema-to-typescript formats its output with whatever prettier it
resolves at runtime, which differs between environments and collapsed
union types onto a single line in CI, breaking the check-schemas job.
Format with the repo's prettier instead so generation is deterministic
and matches `prettier --check .`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bdibon
bdibon force-pushed the boris.dibon/json-schemas-auth-rate-limit branch from a947cff to 35906e8 Compare June 29, 2026 11:08
@bdibon
bdibon merged commit ce5e758 into main Jun 29, 2026
30 of 31 checks passed
@bdibon
bdibon deleted the boris.dibon/json-schemas-auth-rate-limit branch June 29, 2026 11:22
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 29, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants