feat(admin): add streaming upload heapsnapshot endpoint #2957
Conversation
|
WalkthroughThis pull request migrates heap snapshot functionality from local filesystem storage to S3-based cloud storage. The main snapshot route handler is refactored to establish an S3 client, stream heap snapshots directly to S3 with configurable parallelism settings, and include progress logging with performance metrics. Docker infrastructure is updated with a MinIO service to provide local S3-compatible storage for development, including automatic bucket initialization. A new AWS SDK dependency is added to support S3 upload operations with progress tracking capabilities. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/webapp/app/routes/admin.api.v1.snapshot.ts`:
- Around line 31-55: Replace direct process.env reads in getS3Config with the
shared env export: import the env object and read env.SNAPSHOT_S3_BUCKET,
env.SNAPSHOT_S3_REGION, env.SNAPSHOT_S3_ENDPOINT, env.SNAPSHOT_S3_ACCESS_KEY_ID,
and env.SNAPSHOT_S3_SECRET_ACCESS_KEY inside getS3Config instead of process.env;
update the env schema (the env export) to declare these keys (and ensure the
existing ECS metadata key is present) so validation passes, and preserve the
existing logic for default region and conditional credentials/endpoint handling.
In `@apps/webapp/package.json`:
- Around line 32-35: The dependency `@aws-sdk/client-sqs` is pinned to 3.445.0 and
is out of sync with the rest of the AWS SDK packages (e.g., `@aws-sdk/client-s3`
and `@aws-sdk/lib-storage` at 3.936.0); update the `@aws-sdk/client-sqs` entry in
package.json to a matching 3.936.0 (or the same 3.x minor version used across
the repo), reinstall dependencies (npm/yarn install) to update the lockfile, and
run the test/build to ensure no breakages from the SDK version alignment.
In `@docker/docker-compose.yml`:
- Around line 219-254: The docker-compose uses floating :latest images and an
unreliable healthcheck; update the minio service to pin the image tag to a
specific stable MinIO release (replace minio/minio:latest with a concrete
RELEASE.* tag) and update the healthcheck test to use the HTTP readiness
endpoint (GET /minio/health/ready) instead of ["CMD","mc","ready","local"] for
reliability; for minio-setup either pin minio/mc to the matching RELEASE.* tag
or remove the separate minio-setup container and perform setup using the server
image (server images include mc), ensure mc alias/mb entrypoint logic still runs
against the pinned host/credentials, and run Prettier on docker-compose.yml
before committing to fix formatting.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
apps/webapp/app/routes/admin.api.v1.snapshot.tsapps/webapp/package.jsondocker/docker-compose.yml
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
docker/docker-compose.ymlapps/webapp/package.jsonapps/webapp/app/routes/admin.api.v1.snapshot.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/routes/admin.api.v1.snapshot.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/admin.api.v1.snapshot.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/routes/admin.api.v1.snapshot.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/routes/admin.api.v1.snapshot.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/routes/admin.api.v1.snapshot.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/routes/admin.api.v1.snapshot.ts
🧠 Learnings (1)
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to {packages,integrations}/**/* : Add a changeset when modifying any public package in `packages/*` or `integrations/*` using `pnpm run changeset:add`
Applied to files:
apps/webapp/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
🔇 Additional comments (3)
docker/docker-compose.yml (1)
3-11: MinIO volume addition looks good.
Keeps MinIO data persistent across restarts.apps/webapp/app/routes/admin.api.v1.snapshot.ts (2)
1-12: Clear upload tuning defaults.
The constants are well documented and easy to adjust.
74-161: No changes needed. Theprogress.loadedvalue in@aws-sdk/lib-storage'sUploadclass is cumulative across the entire multipart upload (tracked internally asbytesUploadedSoFar), not per-part. ThetotalBytesaccumulation andsizeBytesreporting are correct as-is.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| function getS3Config() { | ||
| const bucket = process.env.SNAPSHOT_S3_BUCKET; | ||
| const region = process.env.SNAPSHOT_S3_REGION ?? "us-east-1"; | ||
|
|
||
| if (!bucket) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // Optional - only needed for non-AWS S3 (MinIO, R2, etc.) or local dev | ||
| const endpoint = process.env.SNAPSHOT_S3_ENDPOINT; | ||
| const accessKeyId = process.env.SNAPSHOT_S3_ACCESS_KEY_ID; | ||
| const secretAccessKey = process.env.SNAPSHOT_S3_SECRET_ACCESS_KEY; | ||
|
|
||
| // If explicit credentials provided, use them (local dev / non-AWS) | ||
| // Otherwise, SDK uses default credential chain (IAM role, env vars, etc.) | ||
| const credentials = | ||
| accessKeyId && secretAccessKey ? { accessKeyId, secretAccessKey } : undefined; | ||
|
|
||
| return { | ||
| bucket, | ||
| region, | ||
| endpoint, | ||
| credentials, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Route env access through env.server.
Direct process.env access in webapp routes bypasses shared env validation and violates the webapp env access rule. Please use the env export and ensure the schema includes these keys (and the existing ECS metadata key in this file).
🛠️ Proposed fix
import { S3Client } from "@aws-sdk/client-s3";
import { Upload } from "@aws-sdk/lib-storage";
import { type DataFunctionArgs } from "@remix-run/node";
import v8 from "v8";
import { prisma } from "~/db.server";
import { authenticateApiRequestWithPersonalAccessToken } from "~/services/personalAccessToken.server";
import { logger } from "~/services/logger.server";
+import { env } from "~/env.server";
function getS3Config() {
- const bucket = process.env.SNAPSHOT_S3_BUCKET;
- const region = process.env.SNAPSHOT_S3_REGION ?? "us-east-1";
+ const bucket = env.SNAPSHOT_S3_BUCKET;
+ const region = env.SNAPSHOT_S3_REGION ?? "us-east-1";
// Optional - only needed for non-AWS S3 (MinIO, R2, etc.) or local dev
- const endpoint = process.env.SNAPSHOT_S3_ENDPOINT;
- const accessKeyId = process.env.SNAPSHOT_S3_ACCESS_KEY_ID;
- const secretAccessKey = process.env.SNAPSHOT_S3_SECRET_ACCESS_KEY;
+ const endpoint = env.SNAPSHOT_S3_ENDPOINT;
+ const accessKeyId = env.SNAPSHOT_S3_ACCESS_KEY_ID;
+ const secretAccessKey = env.SNAPSHOT_S3_SECRET_ACCESS_KEY;As per coding guidelines, access environment variables via the env export from env.server.ts.
🤖 Prompt for AI Agents
In `@apps/webapp/app/routes/admin.api.v1.snapshot.ts` around lines 31 - 55,
Replace direct process.env reads in getS3Config with the shared env export:
import the env object and read env.SNAPSHOT_S3_BUCKET, env.SNAPSHOT_S3_REGION,
env.SNAPSHOT_S3_ENDPOINT, env.SNAPSHOT_S3_ACCESS_KEY_ID, and
env.SNAPSHOT_S3_SECRET_ACCESS_KEY inside getS3Config instead of process.env;
update the env schema (the env export) to declare these keys (and ensure the
existing ECS metadata key is present) so validation passes, and preserve the
existing logic for default region and conditional credentials/endpoint handling.
| "@aws-sdk/client-ecr": "^3.931.0", | ||
| "@aws-sdk/client-s3": "^3.936.0", | ||
| "@aws-sdk/lib-storage": "^3.936.0", | ||
| "@aws-sdk/client-sqs": "^3.445.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@aws-sdk/lib-storage 3.936.0 security advisories npm registry
💡 Result:
No security advisories/vulnerabilities are currently listed for @aws-sdk/lib-storage@3.936.0 in the sources that track npm-registry package advisories/vulns:
- Release date:
3.936.0was published Nov 19, 2025. [1] - Snyk: reports no direct vulnerabilities for
@aws-sdk/lib-storage(note: this excludes issues that may exist only in transitive dependencies). [2] - ReversingLabs (Spectra Assure Community): reports no known vulnerabilities detected for
@aws-sdk/lib-storage. [3]
Sources: [1] [2] [3]
🌐 Web query:
@aws-sdk/lib-storage latest version compatibility AWS SDK v3
💡 Result:
- Latest
@aws-sdk/lib-storage(AWS SDK for JavaScript v3): v3.975.0 (tag:latest). [1] - Compatibility with AWS SDK v3:
@aws-sdk/lib-storageis a v3 package (3.x) intended to be used with v3 clients like@aws-sdk/client-s3(it provides theUploadhelper used for multipart uploads). [3] - Node.js compatibility (important for “latest”): AWS SDK v3 v3.968.0+ requires Node.js >= 20; since
@aws-sdk/lib-storage@3.975.0is >= 3.968.0, it likewise requires Node >= 20. If you must stay on Node 18, pin to@aws-sdk/lib-storage <= 3.967.x. [2]
Rule of thumb: keep your AWS SDK v3 packages on the same 3.x minor version line (e.g., @aws-sdk/lib-storage and @aws-sdk/client-s3 both 3.975.x) to avoid dependency mismatches. [2]
Sources: [1] [2] [3]
@aws-sdk/lib-storage@3.936.0 is safe, but consider aligning @aws-sdk/client-sqs version.
No security advisories exist for @aws-sdk/lib-storage@3.936.0. This version correctly aligns with @aws-sdk/client-s3 (both 3.936.0), which is the proper pairing for the library. However, @aws-sdk/client-sqs at 3.445.0 is significantly out of sync (~500 versions behind). AWS SDK v3 best practice recommends keeping all packages on the same 3.x minor version line to avoid dependency mismatches. Consider upgrading @aws-sdk/client-sqs to a version closer to 3.936.0 or later.
🤖 Prompt for AI Agents
In `@apps/webapp/package.json` around lines 32 - 35, The dependency
`@aws-sdk/client-sqs` is pinned to 3.445.0 and is out of sync with the rest of the
AWS SDK packages (e.g., `@aws-sdk/client-s3` and `@aws-sdk/lib-storage` at 3.936.0);
update the `@aws-sdk/client-sqs` entry in package.json to a matching 3.936.0 (or
the same 3.x minor version used across the repo), reinstall dependencies
(npm/yarn install) to update the lockfile, and run the test/build to ensure no
breakages from the SDK version alignment.
| minio: | ||
| container_name: minio | ||
| image: minio/minio:latest | ||
| restart: always | ||
| volumes: | ||
| - minio-data:/data | ||
| ports: | ||
| - "9002:9000" | ||
| - "9003:9001" | ||
| environment: | ||
| MINIO_ROOT_USER: admin | ||
| MINIO_ROOT_PASSWORD: adminpassword | ||
| networks: | ||
| - app_network | ||
| command: server /data --console-address ":9001" | ||
| healthcheck: | ||
| test: ["CMD", "mc", "ready", "local"] | ||
| interval: 5s | ||
| timeout: 5s | ||
| retries: 5 | ||
|
|
||
| minio-setup: | ||
| container_name: minio-setup | ||
| image: minio/mc:latest | ||
| depends_on: | ||
| minio: | ||
| condition: service_healthy | ||
| networks: | ||
| - app_network | ||
| entrypoint: > | ||
| /bin/sh -c " | ||
| mc alias set local http://minio:9000 admin adminpassword; | ||
| mc mb --ignore-existing local/snapshots; | ||
| mc mb --ignore-existing local/packets; | ||
| exit 0; | ||
| " |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does minio/minio container image include the mc client and what are the current stable tags for minio/minio
💡 Result:
-
Does
minio/minioinclude themcclient? Yes. MinIO’s container images (includingminio/minioandquay.io/minio/minio) include the MinIO Client (mc) built-in (since RELEASE.2022-12-02T19-19-22Z) primarily for container-level debugging. (min.io) -
Current stable tags for
minio/minio(server):- MinIO uses date-based release tags:
RELEASE.<YYYY-MM-DD>T<hh-mm-ss>Z(pin these for production). (github.com) - Latest upstream stable release tag (GitHub):
RELEASE.2025-10-15T17-29-55Z. (github.com) - Latest tag shown on Docker Hub page for
minio/minio(as published there):RELEASE.2025-09-07T16-13-09Z-cpuv1(Docker Hub page indicates it was updated ~5 months ago). (hub.docker.com) - MinIO’s container docs also reference pulling stable images from
quay.io/minio/minio(example shown with a pinnedRELEASE.*tag). (min.io)
- MinIO uses date-based release tags:
Sources (numbered):
- MinIO container install docs (mc included in images): (min.io)
- MinIO server releases (latest release tag): (github.com)
- Docker Hub
minio/miniopage (recent tag shown): (hub.docker.com) - MinIO upgrade doc example referencing quay.io pinned
RELEASE.*: (min.io)
Citations:
- 1: https://min.io/docs/minio/container/operations/installation.html?utm_source=openai
- 2: https://github.com/minio/minio/releases?utm_source=openai
- 3: https://github.com/minio/minio/releases?utm_source=openai
- 4: https://hub.docker.com/r/minio/minio?utm_source=openai
- 5: https://min.io/docs/minio/container/operations/install-deploy-manage/upgrade-minio-deployment.html?utm_source=openai
- 6: https://min.io/docs/minio/container/operations/installation.html?utm_source=openai
- 7: https://github.com/minio/minio/releases?utm_source=openai
- 8: https://hub.docker.com/r/minio/minio?utm_source=openai
- 9: https://min.io/docs/minio/container/operations/install-deploy-manage/upgrade-minio-deployment.html?utm_source=openai
🌐 Web query:
minio mc healthcheck alternative HTTP readiness endpoint
💡 Result:
If you want an HTTP(S) readiness endpoint instead of using mc (e.g., for Kubernetes/Docker/LB health checks), MinIO exposes unauthenticated Healthcheck APIs:
- Readiness (node ready to serve requests):
GET /minio/health/ready(HTTP200= ready) (min.io) - Liveness (process is up):
GET /minio/health/live(HTTP200= alive) (min.io)
If you specifically want “cluster is ready/quorum OK” style checks, MinIO also provides:
- Cluster write readiness:
GET /minio/health/cluster(docs.min.io) - Cluster read readiness:
GET /minio/health/cluster/read(docs.min.io)
If you still want a CLI-based alternative, mc ready <alias> checks readiness by hitting these health endpoints (and can check read quorum with --cluster-read). (blog.min.io)
Citations:
- 1: https://min.io/docs/minio/linux/operations/monitoring/healthcheck-probe.html?utm_source=openai
- 2: https://min.io/docs/minio/linux/operations/monitoring/healthcheck-probe.html?utm_source=openai
- 3: https://docs.min.io/enterprise/aistor-object-store/operations/monitoring/healthcheck-probe/?utm_source=openai
- 4: https://docs.min.io/enterprise/aistor-object-store/operations/monitoring/healthcheck-probe/?utm_source=openai
- 5: https://blog.min.io/supportability-minio-subnet/?utm_source=openai
🏁 Script executed:
# Check if the docker-compose.yml file exists and verify its current state
cd docker && cat -n docker-compose.yml | sed -n '219,254p'Repository: triggerdotdev/trigger.dev
Length of output: 1170
🏁 Script executed:
# Check if Prettier is configured in the project and what formatting issues might exist
git ls-files | grep -E "(\.prettierrc|prettier\.config|package\.json)"Repository: triggerdotdev/trigger.dev
Length of output: 2359
Pin MinIO image tags to specific release versions.
Both minio/minio:latest and minio/mc:latest need pinning to stable release tags (e.g., RELEASE.2025-10-15T17-29-55Z). The mc client is included in MinIO server images and the healthcheck will work as-is, but consider using the HTTP readiness endpoint GET /minio/health/ready instead of mc ready for better reliability. Also format this YAML file with Prettier before committing.
🤖 Prompt for AI Agents
In `@docker/docker-compose.yml` around lines 219 - 254, The docker-compose uses
floating :latest images and an unreliable healthcheck; update the minio service
to pin the image tag to a specific stable MinIO release (replace
minio/minio:latest with a concrete RELEASE.* tag) and update the healthcheck
test to use the HTTP readiness endpoint (GET /minio/health/ready) instead of
["CMD","mc","ready","local"] for reliability; for minio-setup either pin
minio/mc to the matching RELEASE.* tag or remove the separate minio-setup
container and perform setup using the server image (server images include mc),
ensure mc alias/mb entrypoint logic still runs against the pinned
host/credentials, and run Prettier on docker-compose.yml before committing to
fix formatting.
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
No description provided.