Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-- No rollback for the payload rewrite: the original monthly_cost=0 values
-- were themselves incorrect (stale pre-PR-#254 data). Reverting to 0
-- would re-introduce the very bug this migration fixes. The correct
-- monthly_cost values will be written on the next scheduled collection.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
-- Rewrite stale pre-PR-#254 recommendation payloads that store
-- monthly_cost as a literal JSON 0 (from the old float64 struct field).
-- After PR #254, monthly_cost is *float64 so a real zero means "no
-- recurring charge" (e.g. all-upfront), while null means "provider API
-- did not return a monthly breakdown".
--
-- Pre-deploy rows have monthly_cost=0 from the old float64 zero — they
-- are stale and would render as "$0" in the UI even after the code fix.
-- Setting them to null here causes the frontend to render "—" instead,
-- which is at least accurate ("data not yet collected") until the next
-- scheduler tick re-collects with correct values.
--
-- NOTE: We do NOT truncate the table or reset last_collected_at because
-- Azure recommendation collection alone exceeds the API Lambda's 60s
-- timeout — a forced cold-start collect would cause a 502 on the next
-- request. The daily scheduled collector will write correct values on
-- its next run. See GitHub issue #256 companion issue for the Lambda
-- timeout root cause.
UPDATE recommendations
SET payload = jsonb_set(payload, '{monthly_cost}', 'null'::jsonb)
WHERE (payload->>'monthly_cost')::text = '0';
20 changes: 20 additions & 0 deletions providers/azure/internal/recommendations/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ type ExtractedFields struct {
EstimatedSavings float64
Term string
Scope string
// RecurringMonthlyCost is the monthly recurring charge for this
// commitment. Azure Reservation recommendations are all all-upfront
// (a single payment, no recurring monthly charge), so this is always
// a pointer to 0.0 — meaning "no recurring charge" rather than
// "data not available" (which would be nil).
RecurringMonthlyCost *float64
}

// float64Ptr returns a pointer to the given float64 value. Used to
// distinguish "explicitly zero" from "not provided" (nil) on pointer fields.
func float64Ptr(v float64) *float64 {
return &v
}

// Extract reads the Azure reservation recommendation payload into
Expand Down Expand Up @@ -101,6 +113,10 @@ func extractLegacy(rec *armconsumption.LegacyReservationRecommendation) *Extract
// treat EstimatedSavings as lookback-period ≈ monthly.
out.EstimatedSavings = *props.NetSavings
}
// Azure Reservation recommendations are always all-upfront (single payment,
// no monthly recurring charge). Set to 0 (not nil) so the frontend renders
// "$0" rather than "—" (which would imply "data not available").
out.RecurringMonthlyCost = float64Ptr(0)
return out
}

Expand Down Expand Up @@ -133,6 +149,10 @@ func extractModern(rec *armconsumption.ModernReservationRecommendation) *Extract
out.OnDemandCost = amountValue(props.CostWithNoReservedInstances)
out.CommitmentCost = amountValue(props.TotalCostWithReservedInstances)
out.EstimatedSavings = amountValue(props.NetSavings)
// Azure Reservation recommendations are always all-upfront (single payment,
// no monthly recurring charge). Set to 0 (not nil) so the frontend renders
// "$0" rather than "—" (which would imply "data not available").
out.RecurringMonthlyCost = float64Ptr(0)

return out
}
Expand Down
30 changes: 30 additions & 0 deletions providers/azure/internal/recommendations/converter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,21 @@ func TestExtract_MissingCostsReadAsZero(t *testing.T) {
assert.InDelta(t, 0.0, f.EstimatedSavings, 1e-9)
}

func TestExtract_Legacy_RecurringMonthlyCostIsZeroPointer(t *testing.T) {
// Azure reservations are always all-upfront (single payment, no monthly
// recurring charge). RecurringMonthlyCost must be a non-nil pointer to 0
// so the frontend renders "$0" instead of "—" (which would mean unknown).
rec := mocks.BuildLegacyReservationRecommendation(
mocks.WithRegion("eastus"),
mocks.WithNormalizedSize("Standard_D2s_v3"),
mocks.WithCosts(100, 70, 30),
)
f := Extract(rec)
require.NotNil(t, f)
require.NotNil(t, f.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil for Azure reservations")
assert.InDelta(t, 0.0, *f.RecurringMonthlyCost, 1e-9)
}

// --- Modern (MCA billing account) ----------------------------------------

func TestExtract_Modern_NilProperties(t *testing.T) {
Expand Down Expand Up @@ -209,3 +224,18 @@ func TestExtract_Modern_MissingCostAmountsReadAsZero(t *testing.T) {
assert.InDelta(t, 0.0, f.CommitmentCost, 1e-9)
assert.InDelta(t, 0.0, f.EstimatedSavings, 1e-9)
}

func TestExtract_Modern_RecurringMonthlyCostIsZeroPointer(t *testing.T) {
// Azure Reservation recommendations are always all-upfront regardless
// of billing account type. RecurringMonthlyCost must be a non-nil pointer
// to 0 for both Legacy and Modern response shapes.
rec := mocks.BuildModernReservationRecommendation(
mocks.WithModernRegion("westeurope"),
mocks.WithModernSKUName("Standard_D4s_v5"),
mocks.WithModernCosts(400, 260, 140),
)
f := Extract(rec)
require.NotNil(t, f)
require.NotNil(t, f.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil for Azure reservations")
assert.InDelta(t, 0.0, *f.RecurringMonthlyCost, 1e-9)
}
29 changes: 15 additions & 14 deletions providers/azure/services/cache/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -597,20 +597,21 @@ func (c *CacheClient) convertAzureRedisRecommendation(ctx context.Context, azure
details.Shards = entry.shardCount
}
return &common.Recommendation{
Provider: common.ProviderAzure,
Service: common.ServiceCache,
Account: c.subscriptionID,
Region: f.Region,
ResourceType: f.ResourceType,
Count: f.Count,
OnDemandCost: f.OnDemandCost,
CommitmentCost: f.CommitmentCost,
EstimatedSavings: f.EstimatedSavings,
CommitmentType: common.CommitmentReservedInstance,
Term: f.Term,
PaymentOption: "upfront",
Timestamp: time.Now(),
Details: details,
Provider: common.ProviderAzure,
Service: common.ServiceCache,
Account: c.subscriptionID,
Region: f.Region,
ResourceType: f.ResourceType,
Count: f.Count,
OnDemandCost: f.OnDemandCost,
CommitmentCost: f.CommitmentCost,
EstimatedSavings: f.EstimatedSavings,
RecurringMonthlyCost: f.RecurringMonthlyCost,
CommitmentType: common.CommitmentReservedInstance,
Term: f.Term,
PaymentOption: "upfront",
Timestamp: time.Now(),
Details: details,
}
}

Expand Down
29 changes: 15 additions & 14 deletions providers/azure/services/compute/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -729,20 +729,21 @@ func (c *ComputeClient) convertAzureVMRecommendation(ctx context.Context, azureR
}
}
return &common.Recommendation{
Provider: common.ProviderAzure,
Service: common.ServiceCompute,
Account: c.subscriptionID,
Region: f.Region,
ResourceType: f.ResourceType,
Count: f.Count,
OnDemandCost: f.OnDemandCost,
CommitmentCost: f.CommitmentCost,
EstimatedSavings: f.EstimatedSavings,
CommitmentType: common.CommitmentReservedInstance,
Term: f.Term,
PaymentOption: "upfront",
Timestamp: time.Now(),
Details: details,
Provider: common.ProviderAzure,
Service: common.ServiceCompute,
Account: c.subscriptionID,
Region: f.Region,
ResourceType: f.ResourceType,
Count: f.Count,
OnDemandCost: f.OnDemandCost,
CommitmentCost: f.CommitmentCost,
EstimatedSavings: f.EstimatedSavings,
RecurringMonthlyCost: f.RecurringMonthlyCost,
CommitmentType: common.CommitmentReservedInstance,
Term: f.Term,
PaymentOption: "upfront",
Timestamp: time.Now(),
Details: details,
}
}

Expand Down
5 changes: 5 additions & 0 deletions providers/azure/services/compute/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,11 @@ func TestComputeClient_ConvertAzureVMRecommendation_PopulatesAllFields(t *testin
assert.Equal(t, "3yr", out.Term)
assert.Equal(t, "upfront", out.PaymentOption)

// Azure reservations are all-upfront: RecurringMonthlyCost must be a
// non-nil pointer to 0 so the frontend shows "$0" not "—" (unknown).
require.NotNil(t, out.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil for Azure compute recs")
assert.InDelta(t, 0.0, *out.RecurringMonthlyCost, 1e-9)

// Details is populated from the payload's ResourceType (InstanceType
// only — Platform/Tenancy/Scope are deferred to batched enrichment).
require.NotNil(t, out.Details)
Expand Down
29 changes: 15 additions & 14 deletions providers/azure/services/cosmosdb/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,20 +601,21 @@ func (c *CosmosDBClient) convertAzureCosmosRecommendation(ctx context.Context, a
details.APIType = api
}
return &common.Recommendation{
Provider: common.ProviderAzure,
Service: common.ServiceNoSQL,
Account: c.subscriptionID,
Region: f.Region,
ResourceType: f.ResourceType,
Count: f.Count,
OnDemandCost: f.OnDemandCost,
CommitmentCost: f.CommitmentCost,
EstimatedSavings: f.EstimatedSavings,
CommitmentType: common.CommitmentReservedInstance,
Term: f.Term,
PaymentOption: "upfront",
Timestamp: time.Now(),
Details: details,
Provider: common.ProviderAzure,
Service: common.ServiceNoSQL,
Account: c.subscriptionID,
Region: f.Region,
ResourceType: f.ResourceType,
Count: f.Count,
OnDemandCost: f.OnDemandCost,
CommitmentCost: f.CommitmentCost,
EstimatedSavings: f.EstimatedSavings,
RecurringMonthlyCost: f.RecurringMonthlyCost,
CommitmentType: common.CommitmentReservedInstance,
Term: f.Term,
PaymentOption: "upfront",
Timestamp: time.Now(),
Details: details,
}
}

Expand Down
29 changes: 15 additions & 14 deletions providers/azure/services/database/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,20 +599,21 @@ func (c *DatabaseClient) convertAzureSQLRecommendation(ctx context.Context, azur
details.EngineVersion = entry.engineVersion
}
return &common.Recommendation{
Provider: common.ProviderAzure,
Service: common.ServiceRelationalDB,
Account: c.subscriptionID,
Region: f.Region,
ResourceType: f.ResourceType,
Count: f.Count,
OnDemandCost: f.OnDemandCost,
CommitmentCost: f.CommitmentCost,
EstimatedSavings: f.EstimatedSavings,
CommitmentType: common.CommitmentReservedInstance,
Term: f.Term,
PaymentOption: "upfront",
Timestamp: time.Now(),
Details: details,
Provider: common.ProviderAzure,
Service: common.ServiceRelationalDB,
Account: c.subscriptionID,
Region: f.Region,
ResourceType: f.ResourceType,
Count: f.Count,
OnDemandCost: f.OnDemandCost,
CommitmentCost: f.CommitmentCost,
EstimatedSavings: f.EstimatedSavings,
RecurringMonthlyCost: f.RecurringMonthlyCost,
CommitmentType: common.CommitmentReservedInstance,
Term: f.Term,
PaymentOption: "upfront",
Timestamp: time.Now(),
Details: details,
}
}

Expand Down