Repository navigation
ADR-0022: idle-EC2-instance detection - the Critic's real job - #72
Merged
Merged
Conversation
Researched #71 against the real CloudWatch/EC2 API models (botocore, not assumption) before deciding anything. - CPUUtilization/NetworkIn/NetworkOut via GetMetricStatistics need no CloudWatch agent, unlike memory metrics - confirmed against the real service model, not assumed. - The threshold mirrors AWS's own published Trusted Advisor low-utilization convention rather than a number this project invents - locks the shape of the rule, flags the exact constants for re-checking against AWS's current docs before a real client ships (not before this ships fake-only). - Stop, not terminate - confirmed directly in StopInstances' own docs: restarts exactly as it was via StartInstances, billing stops immediately. Same reasoning ADR-0020 already used for Elastic IPs. - A fourth distinct idempotency shape: StopInstances doesn't fail on a repeat call the way ReleaseAddress does - check_completed has to ask DescribeInstances whether the instance is already stopped, not rely on a failure as the signal. - What the Critic is actually for here, concretely: unlike an unassociated EIP, "low utilization for N days" is a heuristic that can be wrong in nameable ways (recently launched, tagged as a standby) - the evidence handed to the Critic grows real fields (launch_time, tags, metric values) so it can add real judgement, not restate the rule's own boolean. ADR-0021's "advisory only, never a veto" isn't re-litigated - still holds exactly as locked. Closes #71
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
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.
Closes #71. Researched against the real CloudWatch/EC2 API models before deciding anything: CPUUtilization/NetworkIn/NetworkOut need no CloudWatch agent (confirmed against the model); the threshold mirrors AWS's own published Trusted Advisor convention rather than an invented number, with the exact constants flagged for re-checking before a real client ships; stop-not-terminate confirmed directly in StopInstances' own docs; check_completed here is a fourth distinct idempotency shape (state-check, since StopInstances doesn't fail on repeat); and the Critic's evidence grows real fields (launch_time, tags, metric values) so it can add genuine judgement on a case that's actually ambiguous, unlike the existing unassociated-EIP check.