fix(cost-optimization): improve monthly cost estimation and prevent d… - #164
Open
ankit3890 wants to merge 1 commit into
Open
fix(cost-optimization): improve monthly cost estimation and prevent d…#164ankit3890 wants to merge 1 commit into
ankit3890 wants to merge 1 commit into
Conversation
…ivision by zero ## Summary Fixes an incorrect monthly cost projection in `CostOptimizationAnalyzer` and adds a defensive guard against a potential divide-by-zero. ## Problem `estimateMonthlyCost()` previously calculated: ```ts (totalCost / executionCount) * 30 ``` This assumes executions occur at a steady rate (~1/day), which isn't true for most workflows—especially idle ones. Since `detectIdleResources()` uses this value to populate `IdleResource.estimatedMonthlyCost`, it could significantly overestimate the savings reported by `terminate_idle` recommendations. ### Example - **2 executions** at **$1 each** over the course of a year - **Previous estimate:** `$30/month` - **Actual recent spend:** `$2/month` (or `$0/month` if no executions occurred in the last 30 days) As a result, long-idle or low-volume workflows could report inflated monthly savings. ## Fix - Updated `estimateMonthlyCost()` to sum the actual cost of executions within the trailing **30-day window** instead of extrapolating from lifetime averages. - Added an explicit guard when calculating `avgCostPerExecution` to safely handle potential divide-by-zero scenarios during future refactors. ## Impact - More accurate `IdleResource.estimatedMonthlyCost`. - More realistic `terminate_idle` recommendation savings. - `estimatedTotalMonthlySavings` now better reflects actual recent usage. - No API, type, or method signature changes. ## Testing - [ ] Verify sparse historical executions no longer produce inflated monthly costs. - [ ] Verify workflows with recent executions report actual 30-day spend. - [ ] Confirm empty execution datasets still return an efficiency score of **100** (regression check). ## Notes for Reviewers `estimateMonthlyCost()` filters using `created_at`, while idle detection uses `updated_at`. This is intentional, as cost should reflect **when an execution occurred** rather than its latest update time. Happy to align these semantics if a different interpretation is preferred.
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.
Summary
Fixes an incorrect monthly cost projection in
CostOptimizationAnalyzerand adds a defensive guard against a potential divide-by-zero.Problem
estimateMonthlyCost()previously calculated:This assumes executions occur at a steady rate (~1/day), which isn't true for most workflows—especially idle ones. Since
detectIdleResources()uses this value to populateIdleResource.estimatedMonthlyCost, it could significantly overestimate the savings reported byterminate_idlerecommendations.Example
$30/month$2/month(or$0/monthif no executions occurred in the last 30 days)As a result, long-idle or low-volume workflows could report inflated monthly savings.
Fix
estimateMonthlyCost()to calculate monthly cost by summing the actual cost of executions within the trailing 30-day window.avgCostPerExecutionto safely handle potential divide-by-zero scenarios during future refactors.Impact
IdleResource.estimatedMonthlyCost.terminate_idlerecommendation savings.estimatedTotalMonthlySavingsnow better reflects actual recent usage.Notes for Reviewers
estimateMonthlyCost()filters executions usingcreated_at, while idle detection usesupdated_at. This is intentional, as monthly cost should reflect when an execution occurred, whereas idleness is determined by the resource's latest activity.Performance & Observability Impact
Performance
Observability
User Impact