feat(webapp): emit workload auth gate metrics via opentelemetry - #4822
Conversation
|
WalkthroughThe change replaces Prometheus authorization counters with tracing counters. It adds environment type and run-age bucket attributes to authorization metrics. The authorization service defines five age buckets based on hour and day thresholds. Tests cover bucket boundaries and future timestamps. Run lookups now retrieve Merge Risk: 🔵 Low · up to The PR preserves authorization decisions while moving workload gate metrics to OpenTelemetry, but the counter name may be exported with a duplicated _total suffix, which could break dashboards and queries that consume the metric. The naming issue should be corrected or explicitly accepted before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5e1dd822-a6d9-442c-b55b-7191d740493b
📒 Files selected for processing (3)
apps/webapp/app/v3/services/worker/workerGroupTokenService.server.tsapps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/test/workloadTokenAuthorization.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (32)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: runops-guard / runops-guard
- GitHub Check: typecheck / typecheck
- GitHub Check: code-quality / code-quality
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (14)
New code must target Run Engine V2 through the singleton in `app/v3/runEngine.server.ts`; do not reintroduce V1 execution paths. V1 branches may only reject or finalize gracefully with a clean 4xx.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
Never use `request.signal` to detect client disconnects. Use `getRequestAbortSignal()` from `app/services/httpAsyncStorage.server.ts`, which is wired to Express response close events.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
We use vitest exclusively. **Never mock anything** - use testcontainers instead.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/test/workloadTokenAuthorization.test.ts
Test files must not import `app/env.server.ts`; pass configuration as options instead.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/test/workloadTokenAuthorization.test.ts
For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/test/workloadTokenAuthorization.test.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/test/workloadTokenAuthorization.test.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
Use zod for validation in packages/core and apps/webapp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/test/workloadTokenAuthorization.test.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
Do not import `env.server.ts` directly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/test/workloadTokenAuthorization.test.ts
Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env`
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/test/workloadTokenAuthorization.test.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/test/workloadTokenAuthorization.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/test/workloadTokenAuthorization.test.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/test/workloadTokenAuthorization.test.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.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
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/test/workloadTokenAuthorization.test.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
🔇 Additional comments (4)
apps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.ts (1)
28-46: LGTM!apps/webapp/test/workloadTokenAuthorization.test.ts (1)
2-5: LGTM!Also applies to: 30-56
apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts (2)
16-16: LGTM!Also applies to: 25-27, 452-463, 475-480
478-478: 🚀 Performance & ScalabilityNo allowlist mapping is needed.
run.environmentTypeuses the boundedRuntimeEnvironmentTypeenum (PRODUCTION,STAGING,DEVELOPMENT,PREVIEW), and null values map to the fixed"unknown"value.
Summary
workload_auth_gate_totalrecords how each worker action authorizes: scoped by averified environment header, grandfathered by the created-at gate, or suppressed
by it. It was registered on the Prometheus registry served at
/metrics, which isper-process. With
ENABLE_CLUSTER=1every Node worker keeps its own registry, so ascrape returns whichever process happened to answer and the counter reads as a
fraction of real traffic.
This moves the counter onto the OpenTelemetry meter the webapp already uses for its
other engine metrics. Each process exports under its own
service.instance.id, sosumming across them gives the true total no matter how many workers a deployment
runs.
Attributes
The counter now carries
env_typeandrun_age_bucketalongsideoutcomeandaction.run_age_bucketis the coarse age of the run behind an untokened worker action(
lt_1h,1h_1d,1d_7d,7d_30d,gt_30d). It exists so an operator can sizeWORKLOAD_TOKEN_CUTOFFbefore committing to it: set the cutoff far in the futureand every run is grandfathered, so the age distribution of untokened traffic is
visible without anything being rejected. Both attributes come off the run row the
gate already reads, so there is no extra query.