Repository navigation
fix(cli): Batch objectstore requests in snapshots upload - #2062
Conversation
`sentry snapshots upload` sent one HEAD per image (8 at a time) and then
one PUT per missing image. These go through Sentry's objectstore proxy
(`/api/0/organizations/{org}/objectstore/`), which rate-limits each HTTP
request. A suite of about 60 images got HTTP 429 on HEAD within about a
second.
The upload now uses objectstore's multipart batch endpoint
(`/v1/objects:batch/{usecase}/{scope}/`), like the Rust client's
`Session::many()` that sentry-cli uses. All HEADs go in one request.
Files up to 1 MB are inserted in batches of up to 1000 operations and
100 MB. Larger files still get individual PUTs, 5 at a time.
Co-Authored-By: Jamie Quadri <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
BYK
left a comment
There was a problem hiding this comment.
- Keep concurrent uploads still (we can do concurrent batches?)
- Move all this logic to
@sentry/apipackage and use it from there
Runs up to 4 batch requests at once for both existence checks and uploads, and uploads oversized files in parallel with the batches. Lowers the per-batch body limit to 25 MB so in-flight upload bodies stay around 100 MB, the same bound as before.
|
Thanks @BYK.
|
|
Follow-up on item 2: I opened getsentry/sentry-api-schema#133. It adds the objectstore client, exported from |
New lint rules go in .oxlintrc.json or the CLI Oxlint JS plugin, not ast-grep. Co-Authored-By: Burak Yigit Kaya <[email protected]>
Moves the Objectstore upload client from the Sentry CLI into `@sentry/api/client`, as requested in review on getsentry/toolkit#2062. That PR fixes `sentry snapshots upload` hitting HTTP 429. The new CLI sent one request per image, and the Sentry proxy in front of Objectstore rate-limits per request. The old Rust CLI batched requests (`Session::many()`), and this client does the same. The logic matches toolkit at 5459297: - `findExistingObjects` sends HEADs in multipart batches to `/v1/objects:batch/{usecase}/{scope}/`, up to 1000 operations per batch and 4 batches in flight. - `putObjects` batches inserts up to 1 MB each, within 1000 operations and 25 MB per batch, with 4 batches in flight. It PUTs larger objects one by one, up to 5 at once, in parallel with the batches. - A failed batch, a failed operation, a non-multipart response, or a response missing an operation throws. ### API (`@sentry/api/client`) ```ts findExistingObjects(config: ObjectstoreConfig, keys: string[], options?: ObjectstoreOptions): Promise<Set<string>> putObjects(config: ObjectstoreConfig, uploads: ObjectUpload[], options?: ObjectstoreOptions): Promise<void> type ObjectstoreConfig = { url; usecase; scopes: [string, string][]; authToken?: string | null; expirationPolicy } type ObjectUpload = { key: string; size: number; read: () => Promise<Uint8Array<ArrayBuffer>> } type ObjectstoreOptions = { fetch?: (input: string, init?: RequestInit) => Promise<Response> } class ObjectstoreError extends ApiError { url: string } ``` ### Choices - **Location: the shared client, not a new subpath.** `@sentry/api/client` is already the web-standard request layer that hosts inject things into. Putting the client there means `tsconfig.browser.json` and `portability.test.ts` check it automatically. - **Browser-safe.** There are no `node:` imports and no `Buffer`. Multipart encoding and parsing work on bytes (`TextEncoder`, `Uint8Array`), and boundaries come from `crypto.randomUUID()`. The host supplies file contents through `read()`, which runs only when that upload's request starts. The CLI will pass `() => readFile(path)` and the size from `stat`. - **Injectable `fetch`.** It defaults to `globalThis.fetch`. The CLI will pass its custom-CA fetch. - **Errors.** `ObjectstoreError` extends this package's `ApiError`, so `instanceof SentryApiError` still matches. It adds `url`, which is the failing object's URL or the request URL, and the CLI's own error type needs it. - **No new dependencies.** A small local limiter replaces `p-limit`. Once a task fails, queued tasks don't start. - `putObject` and `buildObjectUrl` stay internal because the CLI only uses the two functions above. ### Verification - `test/api-client/objectstore.test.ts` (13 tests, ported from toolkit) covers batch shape, key encoding, concurrency (3 batches in flight for 2,500 keys and for 60 MB), per-operation, request, and protocol errors, and individual PUTs. All 13 pass, and so does the whole `test/api-client` suite (370 tests). - I also ran an E2E test of the built `dist` against a local `ghcr.io/getsentry/objectstore:nightly`. 1,200 objects took 2 batched HEAD requests and 3 upload requests (2 batches plus 1 PUT for a 2 MB object). Non-ASCII keys round-tripped, and the bytes read back matched. - `pnpm build` and `tsc -p tsconfig.browser.json` pass. - `pnpm typecheck` and `pnpm test` fail on `main` without this change, and they fail the same way with it. `test/typecheck.ts` and two `smoke.test.ts` cases expect a `feature_id` field that was added to the schema in the last generated spec. Main's Build has been red since a47f626. Toolkit will switch `packages/cli` to this client after the next `@sentry/api` release. Toolkit currently pins 1.26.0, so it needs a catalog bump at the same time. <!-- junior-request-attribution:start --> via @BYK. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3A790a7f162e08c84f43da8a5304ce4980 --> -- [View Session](https://junior-prod.sentry.dev/conversations/agent%3A790a7f162e08c84f43da8a5304ce4980) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3A790a7f162e08c84f43da8a5304ce4980/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Bumps @sentry/api to 1.44.0, which ships the batched Objectstore client from getsentry/sentry-api-schema#133, and replaces the CLI's own protocol code with it. src/lib/objectstore.ts is now a thin adapter: it reads files from disk when their request starts, sends requests through customFetch so custom CAs still apply, and rethrows ObjectstoreError as the CLI's ApiError. Batching and limits are unchanged: 4 concurrent batches, 25 MB per batch, and 5 concurrent PUTs for files over 1 MB. Protocol tests now live in @sentry/api; the CLI tests cover the adapter. The bump also regenerates the alert and event skill references, which pick up field descriptions from the newer schema. Co-Authored-By: Jamie Quadri <[email protected]> Co-Authored-By: Burak Yigit Kaya <[email protected]>
…2073) This is step 1 of moving the CLI onto `@sentry/api` generated operations. The goal is an Oxlint rule that bans the `apiRequest*` helpers with no allowlist. @BYK chose that scope ("Also ban apiRequest* helpers") when the lint follow-up to #2062 was scoped. Since the rule can't land until every call site has moved, it isn't in this PR. #2062 bumps `@sentry/api` to 1.44.0 too and moves `lib/objectstore.ts`. This PR leaves `objectstore.ts` alone, so whichever PR merges second only needs a lockfile rebase. ## What changed - `@sentry/api` 1.26.0 → 1.44.0 in the catalog and in `minimumReleaseAgeExclude`. The regenerated `alert.md` and `event.md` skill references are included. - 18 `apiRequest*` call sites now use generated operations: - **alerts**: `deleteIssueAlertRule` → `deleteOrganizationWorkflow` - **chunk-upload**: - `getChunkUploadOptions` → `getOrganizationChunkUploadOptions`. `ChunkServerOptionsSchema` still validates the response because the SDK type doesn't include `maxWait`. - New `assembleDebugFiles` → `assembleProjectDebugFiles` - New `assembleArtifactBundle` → `assembleOrganizationArtifactBundle` - **sourcemaps, dart-symbols, proguard, debug-files**: their assemble and poll calls now go through the chunk-upload wrappers above (7 sites). `pollAssembly`/`pollDifAssembly` take an `assemble` callback. - **conversations**: `getConversationSpans` → `retrieveOrganizationAiConversation` - **issues**: `getIssueByShortId` → `resolveOrganizationShortId` - **preprod-artifacts**: - `getBuildInstallDetails` → `getOrganizationPreprodArtifactInstallDetails` - Latest base snapshot → `getOrganizationPreprodArtifactSnapshotLatestBase` (a 404 still returns `null`) - **releases**: `setCommitsWithRefs` → `updateOrganizationRelease`. The `.`/`..` path-segment checks stay because the SDK's path serializer lets them through. - **replays**: `getReplay` → `getOrganizationReplay` - **traces**: - `getTraceMeta` → `getOrganizationTraceMeta` - `fetchTransactionsPage`/`fetchSpansPage` → `listOrganizationEvents` - `infrastructure.ts` gets `validateResponse` and `unwrapValidatedResult`, so call sites that already validated responses with Zod still do that on SDK results. No new `as` casts on query or body types. ### Behavior notes - HTTP error messages for the moved calls now read `<context>: <status>` (from `unwrapResult`/`throwApiError`) instead of `API request failed: …`. The 401/403 enrichment is unchanged. - `throwApiError` doesn't attach the `api_response_headers` Sentry context that the helper path set. - Pagination, region routing (`getOrgSdkConfig`), caching and idempotency are unchanged. ## Tests - `chunk-upload.test.ts` and `proguard.test.ts` now test at the fetch level (`mockFetch` + `setOrgRegion`). New cases cover `maxWait`, schema mismatch, 404, and both assemble endpoints. - `debug-files.test.ts` mocks the new `assembleDebugFiles`. - `preprod-artifacts.test.ts` mocks `getOrgSdkConfig`, with new cases for a null body, a 500, and a schema mismatch. - The existing fetch-level tests for alerts, short IDs, conversations, releases and traces pass unchanged. ## Gate - `pnpm run tsc` and `pnpm run lint`: pass. - `pnpm run test`: fails only on sandbox environment problems that also fail on `main`. CI is the real check. - The root run aborts in `codemod-cli3to4` because the sandbox lacks `GLIBC_2.35`. - `pnpm -r --no-bail run test`: every package passes except the CLI, which has 5 failures (12881 passed). All 5 are file-permission tests that fail on `main` in the same sandbox: `completions` (2), `upgrade` acquireLock, `db/resilient` readonly, and `dsn/code-scanner` unreadable files. ## Remaining call sites (39 + 2 raw fetches) ### (a) Missing from the public schema: needs getsentry/sentry API docs - `auth.ts` GET `/api/0/` (root). Used for the token check; dynamic. - `dashboards.ts` GET `/organizations/{org}/dashboards/{id}/revisions/` - `dashboards.ts` POST `/organizations/{org}/dashboards/{id}/revisions/{rev}/restore/` - `issues.ts` GET `/issues/{id}/` (unscoped issue lookup) - `issues.ts` PUT `/issues/{id}/` (unscoped update fallback) - `issues.ts` raw `customFetch` GET `/shared/issues/{shareId}/` (`getSharedIssue`) - `logs.ts` GET `/organizations/{org}/trace-logs/` - `org-auth-tokens.ts` GET, POST `/organizations/{org}/org-auth-tokens/`, DELETE `…/{tokenId}/` - `organizations.ts` GET `/users/me/regions/` - `preprod-artifacts.ts`: - POST `/projects/{org}/{project}/files/preprodartifacts/assemble/` (build assemble) - GET `…/preprodartifacts/snapshots/upload-options/` - Snapshot archive status and trigger at `…/snapshots/{id}/archive/` - Raw `customFetch` `openSnapshotArchive` - `projects.ts` GET `/organizations/{org}/projects/?query=dsn:{key}` (×2). The list op has no `query` param. - `releases.ts` GET `/organizations/{org}/releases/{version}/previous-with-commits/` - `traces.ts` GET `/projects/{org}/{project}/trace-items/{itemId}/` - `trials.ts` GET `/customers/{org}/` and PUT `/customers/{org}/product-trial/` - `users.ts` GET `/auth/` These look intentionally private, so publishing them needs a call from the API owners: `/customers/*` (billing, getsentry), `/users/me/regions/`, the `/api/0/` root, `/auth/`, `previous-with-commits`, `trace-logs`, `trace-items`, and the preprod/snapshot internals. ### (b) In the schema, but the generated types don't match what the CLI sends or reads - `alerts.ts` ×2, GET `/organizations/{org}/workflows/`: no `projectSlug` filter, and the CLI sends string ids. - `code-mappings.ts` POST bulk code mappings: the CLI sends a camelCase body; the schema is snake_case. - `conversations.ts` list: no `sort` query param. - `dashboards.ts` `events-timeseries`: `yAxis` is an array and `dataset` is optional in the CLI. - `dashboards.ts` `events`: `dataset` can be null or a value outside the enum. - `explore.ts` `events-timeseries`: array `yAxis`, string `interval`. - `explore.ts` `events`: free-form dataset. - `issues.ts` GET org issue: the `collapse` enum is only `release|stats|tags`. - `issues.ts` PUT org issue: every update body field is required. - `issues.ts` PUT `/organizations/{org}/issues/` (merge): `updateOrganizationIssues` also requires every body field. - `replays.ts` list: the `field` enum lacks `info_ids`/`warning_ids`. - `replays.ts` `replay-count`: no `data_source=events`. - `seer.ts` ×3, `/organizations/{org}/issues/{id}/autofix/`: no `mode=explorer`. - `traces.ts` GET `/organizations/{org}/trace/{id}/`: no `timestamp`/`limit`/`project` params. - `preprod-artifacts.ts` POST `…/preprodartifacts/snapshots/`: the body type is `never`, and the CLI sends a zstd body. ### (c) Dynamic or upload transport, not Sentry API operations - Chunk uploads POST to the server-provided chunk URL. - Preprod build download fetches the external `installUrl`. - `rawApiRequest`/`prepareRawApiRequest` back the `sentry api` passthrough command. <!-- junior-request-attribution:start --> via @BYK. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3A80220b9d4c297e2a971f05dcb03af515 --> -- [View Session](https://junior-prod.sentry.dev/conversations/agent%3A80220b9d4c297e2a971f05dcb03af515) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3A80220b9d4c297e2a971f05dcb03af515/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <[email protected]>
sentry snapshots uploadnow uses objectstore's batch endpoint, so a snapshot suite takes a few HTTP requests instead of one HEAD and one PUT per image. Before this change, suites of about 60 images got HTTP 429 from the Sentry objectstore proxy.Root cause
The upload sent one HEAD per image (8 at a time), then one PUT per missing image. Example failure from getsentry/sentry-evals (job), with
[email protected]:The 429 arrives about 1.4s after the upload starts. The same error appears in two other recent failed runs of that workflow.
Those requests go through
ObjectstoreEndpointin getsentry/sentry. It setsauthentication_classes = ()andRateLimitConfig(group="CLI"). Since Django sees no authenticated user, the request should fall into theIPcategory of theCLIgroup, which isSENTRY_RATELIMITER_DEFAULT. That is 40 requests per second in getsentry. This category mapping comes from reading the code. I did not confirm it from the 429 response body, because the CLI only prints the status text.The Rust
sentry-clidoes not hit this because it usesobjectstore-client'sSession::many(). That sends HEADs and small PUTs as multipart requests to/v1/objects:batch/{usecase}/{scope}/. The proxy accepts that path (is_valid_object_pathallowsobjects:*).Change
lib/objectstore.ts: adds a small client for the batch protocol. It encodes per-part headers likex-sn-batch-operation-kindandx-sn-batch-operation-key, escaping keys the wayencode_header_valuedoes. It parses the multipart response byx-sn-batch-operation-indexandx-sn-batch-operation-status.findExistingObjects: sends batched HEADs, at most 1000 per request.putObjects: inserts files up to 1 MB in batches of up to 1000 operations and 100 MB. Batches run one at a time to keep memory bounded. Larger files still get individual PUTs, 5 at a time. These limits match the Rust client and the server'sMAX_OPERATIONSandMAX_FIELD_SIZE.ApiError.snapshots/upload.ts: uses the two functions above.runPooledandUPLOAD_CONCURRENCYare removed, and so isobjectExists, which nothing else used.Testing
snapshots uploadcommand tests are updated to the new functions.ghcr.io/getsentry/objectstore:nightly, filesystem backend) with 62 images of about 20 KB plus one 1.5 MB image:x-sn-expirationwas stored on both batched and individual uploads.tsc --noEmitandpnpm run lintpass. I ran the CLI vitest suites for objectstore and snapshots, not the full rootpnpm run test.Not covered
us.sentry.ioproxy. Proxy support for the batch path comes from reading the code and from the Rust CLI using it.via jamieq.
--
View Session [Sentry]