Improve AWS runner attribution and watchdog coverage - #3851
Open
shi-eric wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAWS CI workflows now propagate normalized trigger categories and run metadata to EC2 runner tags. A CloudFormation watchdog scans overdue runners, publishes metrics, and sends alerts. Documentation and tests cover deployment procedures and workflow contracts. ChangesAWS CI observability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EventBridge
participant WatchdogLambda
participant EC2
participant CloudWatch
participant SNS
EventBridge->>WatchdogLambda: invoke every four hours
WatchdogLambda->>EC2: scan running attributed runners
EC2-->>WatchdogLambda: return instance tags and metadata
WatchdogLambda->>CloudWatch: publish overdue-runner metrics
CloudWatch->>SNS: send alarm or recovery notification
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/mujoco_warp_tests.yml:
- Around line 94-98: JSON-encode the free-form trigger-category input before
constructing the aws-resource-tags JSON, then use the encoded value for the
Newton-Trigger tag in both .github/workflows/mujoco_warp_tests.yml lines 94-98
and .github/workflows/warp_nightly_tests.yml lines 94-98. Apply the same
workflow-level encoding approach at both sites so characters such as quotes
cannot break the tag array.
In `@scripts/ci/aws/overdue-newton-github-runner-watchdog.yaml`:
- Around line 212-229: The CloudFormation deployment procedure must seed the
watchdog metric before treating the alarm as operational. In
scripts/ci/aws/overdue-newton-github-runner-watchdog.yaml lines 212-229, update
the alarm configuration or stack outputs as needed to support waiting for the
alarm to leave INSUFFICIENT_DATA, then publish a successful OverdueRunnerCount=0
datapoint using the Newton/GitHubRunnerWatchdog dimension before accepting the
result. Document this exact post-stack procedure in scripts/ci/aws/README.md
lines 99-104.
In `@scripts/ci/tests/test_watchdog_logic.py`:
- Around line 64-65: Add a narrow, documented Ruff suppression for S102 directly
on the intentional exec call in the test setup that compiles TEMPLATE; keep the
suppression scoped to this test-only execution and do not alter global lint
configuration.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 363800ce-926f-43c3-b171-7e688efba1cb
📒 Files selected for processing (14)
.github/workflows/aws_gpu_benchmarks.yml.github/workflows/aws_gpu_tests.yml.github/workflows/merge_queue_aws_gpu.yml.github/workflows/minimum_deps_tests.yml.github/workflows/mujoco_warp_tests.yml.github/workflows/pr_target_aws_gpu_benchmarks.yml.github/workflows/pr_target_aws_gpu_tests.yml.github/workflows/push_aws_gpu.yml.github/workflows/scheduled_nightly.yml.github/workflows/warp_nightly_tests.ymlscripts/ci/aws/README.mdscripts/ci/aws/overdue-newton-github-runner-watchdog.yamlscripts/ci/tests/test_runner_workflow_contract.pyscripts/ci/tests/test_watchdog_logic.py
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
EC2 runners carried only ownership and repository tags. This made AWS spend difficult to map to the workflow run and workload that created it. Add stable trigger and workload categories plus run identifiers at instance and volume creation time. Version the overdue-runner watchdog beside the CI code and broaden its ownership-tag scan to include permitted development runs from forks. Keep alert routing deployment-specific and require change-set review for live updates.
shi-eric
force-pushed
the
ershi/update-aws-tags
branch
from
August 8, 2026 21:42
719df10 to
01d8b74
Compare
shi-eric
marked this pull request as ready for review
August 8, 2026 21:46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
EC2 instances and attached EBS volumes created by Newton's AWS runner workflows now receive launch-time tags for the repository, normalized trigger category, workload, GitHub run ID, and run attempt. The repository tag uses
${{ github.repository }}so authorized fork runs remain attributable.Caller workflows explicitly classify pull requests, merge queue runs, pushes, scheduled nightly runs, and manual dispatches. This makes AWS Console spend easier to map back to the CI activity that created it.
This PR also checks the existing overdue-runner watchdog CloudFormation template into
scripts/ci/awswith deployment instructions. The watchdog scans all running instances with Newton's ownership tag instead of filtering on a single repository, closing the monitoring gap for temporary development runs from authorized forks. Instances created before the new attribution tags are still reported with unknown metadata rather than ignored.The watchdog continues to calculate runner age from the EC2 launch time, so this change does not add an expiration tag. It implements the attribution portion of the Newton AWS EC2 runner mitigation plan without changing the runner provisioning or cleanup mechanism.
This is a CI infrastructure change and does not affect Newton's public API.
Checklist
Test plan
uv run --no-project -m unittest discover -s scripts/ci/tests -vuvx pre-commit run -ashi-eric/newtonand confirmed that the EC2 instance and attached EBS volume received the expected dynamic repository, trigger, workload, run ID, and run-attempt tags before cleanup: https://github.com/shi-eric/newton/actions/runs/31274733298UPDATE_COMPLETE, drift status wasIN_SYNC, and the alarm wasOKThe local tests cover workflow configuration contracts and watchdog logic. They do not call AWS; the fork workflow run and watchdog deployment provide the live AWS verification.
Summary by CodeRabbit