infra: Add a new specific task definition for the admin API to enable sentry tracing for admin requests#6496
Conversation
matthewelwell
requested review from
gagantrivedi
and removed request for
a team
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Docker builds report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6496 +/- ##
=======================================
Coverage 98.23% 98.23%
=======================================
Files 1311 1311
Lines 48474 48474
=======================================
Hits 47617 47617
Misses 857 857 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
matthewelwell
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| with: | ||
| cluster: ${{ inputs.aws_ecs_cluster_name }} | ||
| service: ${{ inputs.aws_ecs_sdk_service_name }} | ||
| task-definition: ${{ steps.task-def-admin-api.outputs.task-definition }} |
There was a problem hiding this comment.
Admin and SDK task definitions deployed to wrong services
High Severity
The task definitions are swapped between services. The step "Deploy Amazon ECS SDK API task definition" deploys task-def-sdk-api to aws_ecs_service_name (the admin/main service), while "Deploy Amazon ECS SDK service" deploys task-def-admin-api to aws_ecs_sdk_service_name (the SDK service). The verification step at line 167 confirms aws_ecs_service_name is the admin service by naming its output RUNNING_ADMIN_API_TASK_DEF. This means the admin service gets the SDK config and the SDK service gets the admin config.
Additional Locations (1)
| task-definition: ${{ steps.task-def-sdk-api.outputs.task-definition }} | ||
|
|
||
| - name: Deploy Amazon ECS SDK service with same task definition | ||
| run: | |
There was a problem hiding this comment.
Removed step ID breaks deployment verification step
High Severity
The old step id: deploy-api-task-def was removed when renaming the deploy step, but line 166 still references steps.deploy-api-task-def.outputs.task-definition-arn. This will resolve to an empty string, causing EXPECTED_TASK_DEF to be empty and the "Verify correct version is running" check to always fail, blocking every deployment.
| { | ||
| "name": "DASHBOARD_ENDPOINTS_SENTRY_TRACE_SAMPLE_RATE", | ||
| "value": "0.002" | ||
| "value": "0" |
There was a problem hiding this comment.
Dashboard sentry trace rate set to zero despite discussion
Medium Severity
DASHBOARD_ENDPOINTS_SENTRY_TRACE_SAMPLE_RATE was changed from "0.002" to "0", completely disabling dashboard endpoint sentry tracing on the SDK API. The PR discussion explicitly concluded this value should be left as it was, but it remains set to "0" in the current diff.
| "containerPort": 8000, | ||
| "hostPort": 8000, | ||
| "protocol": "tcp" | ||
| } |
There was a problem hiding this comment.
New task definitions missing Prometheus port 9100
Medium Severity
Both new task definition files only expose port 8000 but set PROMETHEUS_ENABLED to "True". Every other task definition in the codebase (production SDK API, staging admin API, both task processors) exposes port 9100 alongside 8000 for Prometheus metrics scraping. Without port 9100, Prometheus won't be able to collect metrics from the production admin API or staging SDK API despite the feature being enabled.
Additional Locations (1)
3 participants
Changes
The purpose of this PR is to enable sentry tracing on all requests to the newly separated Admin API service.
In order to do so, I have created a new task definition instead of reusing the one for the SDK API.
How did you test this code?
TODO