Просмотр исходного кода

fix: address Copilot review feedback on PR #3735

Addresses 5 review comments from copilot-pull-request-reviewer:

1. pkg/costmodel/currency_helper.go:40 -- normalise target currency
   BEFORE checking for the USD no-op. Previously "usd" or " USD "
   would fall through to the full conversion path.

2. pkg/costmodel/currency_helper.go:29 -- prevent log storms. Each
   handler-facing entry (ConvertAllocation, ConvertAllocationSet,
   ConvertAllocationSetRange, convertCloudCostSetRange,
   convertCustomCostResponse, convertCustomCostTimeseriesResponse)
   now calls Converter.GetRate("USD", target) upfront. If the probe
   fails (invalid currency, API down) the helper returns an error
   without mutating anything and the caller logs a single
   per-request warning. Without this, a failing converter produced
   one warning per cost field per allocation -- potentially
   thousands of log lines per request.

3. pkg/costmodel/aggregation.go:183 -- always rebuild the summary
   allocation set range after attempting conversion. The helper is
   best-effort and may have partially mutated the underlying data
   even on a non-nil error, so the pre-conversion summary was
   potentially inconsistent with the (partly converted) data.
   Extracted the per-field work into an unexported
   convertAllocationFields() so inner loops don't re-probe the rate.

4. pkg/customcost/queryservice.go:66 -- the handler's err-branch was
   previously dead code (helper always returned nil). The helper now
   returns a real error on upfront rate-lookup failure, keeping the
   handler's "Continue with USD values" comment and log-once branch
   meaningful.

5. pkg/cloudcost/queryservice.go:75 -- same treatment as #4.

Added tests for the new behaviour:

  * TestConvertAllocation_USDNormalized -- "usd", " USD ", "Usd", ""
    all no-op without converter activity.
  * TestConvertAllocation_GetRateFailsReturnsErrorNoMutation
  * TestConvertAllocationSetRange_GetRateFailsReturnsError
  * TestConvertCloudCostSetRange_USDNormalized
  * TestConvertCloudCostSetRange_GetRateFailsReturnsError
  * TestConvertCustomCostResponse_USDNormalized
  * TestConvertCustomCostResponse_GetRateFailsReturnsError

mockConverter in each package gained getRateFail / getRateCalls
fields to exercise the probe.

Signed-off-by: Warwick Peatey <warwick@automatic.systems>
Assisted-by: Claude Code
Warwick Peatey 1 месяц назад
Родитель
Сommit
b1edcab182

+ 12 - 5
pkg/cloudcost/queryservice.go

@@ -218,16 +218,23 @@ func (s *QueryService) GetCloudCostViewTableHandler(tokenHook func(ViewTableRows
 }
 
 // convertCloudCostSetRange converts all cloud costs in a range from USD
-// to target currency in place. Best-effort: per-field converter failures
-// are logged and the field is left in USD rather than rolling the whole
-// response back to USD. Only CostMetric.Cost is mutated; KubernetesPercent
-// and other non-cost fields are untouched.
+// to target currency in place. Returns an error if the target rate
+// cannot be looked up upfront (no mutation occurs). Per-field converter
+// failures after the probe succeeded are handled best-effort: the field
+// is left in USD and a warning is logged. Only CostMetric.Cost is
+// mutated; KubernetesPercent and other non-cost fields are untouched.
 func convertCloudCostSetRange(ccsr *opencost.CloudCostSetRange, converter currency.Converter, targetCurrency string) error {
-	if ccsr == nil || converter == nil || targetCurrency == "USD" {
+	if ccsr == nil || converter == nil {
 		return nil
 	}
 
 	targetCurrency = strings.ToUpper(strings.TrimSpace(targetCurrency))
+	if targetCurrency == "" || targetCurrency == "USD" {
+		return nil
+	}
+	if _, err := converter.GetRate("USD", targetCurrency); err != nil {
+		return fmt.Errorf("currency rate lookup USD->%s failed: %w", targetCurrency, err)
+	}
 
 	tryConvert := func(val float64, logCtx string) float64 {
 		if val == 0 {

+ 39 - 3
pkg/cloudcost/queryservice_test.go

@@ -9,9 +9,11 @@ import (
 )
 
 type mockConverter struct {
-	rate       float64
-	failValues map[float64]bool
-	calls      int
+	rate         float64
+	failValues   map[float64]bool
+	getRateFail  bool
+	getRateCalls int
+	calls        int
 }
 
 func (m *mockConverter) Convert(amount float64, from, to string) (float64, error) {
@@ -26,6 +28,10 @@ func (m *mockConverter) Convert(amount float64, from, to string) (float64, error
 }
 
 func (m *mockConverter) GetRate(from, to string) (float64, error) {
+	m.getRateCalls++
+	if m.getRateFail {
+		return 0, fmt.Errorf("simulated rate lookup failure USD->%s", to)
+	}
 	return m.rate, nil
 }
 
@@ -107,6 +113,36 @@ func TestConvertCloudCostSetRange_MutatesAllCostMetrics(t *testing.T) {
 	}
 }
 
+func TestConvertCloudCostSetRange_GetRateFailsReturnsError(t *testing.T) {
+	ccsr := newCCSR(t)
+	m := &mockConverter{rate: 2.0, getRateFail: true}
+	err := convertCloudCostSetRange(ccsr, m, "XXX")
+	if err == nil {
+		t.Fatal("expected error when GetRate fails")
+	}
+	if m.calls != 0 {
+		t.Errorf("no Convert calls expected on precheck failure; got %d", m.calls)
+	}
+	for _, v := range ccsr.CloudCostSets[0].CloudCosts {
+		if v.ListCost.Cost != 100 {
+			t.Errorf("data must not be mutated on precheck failure; ListCost=%v", v.ListCost.Cost)
+		}
+	}
+}
+
+func TestConvertCloudCostSetRange_USDNormalized(t *testing.T) {
+	for _, target := range []string{"usd", " USD ", ""} {
+		ccsr := newCCSR(t)
+		m := &mockConverter{rate: 2.0}
+		if err := convertCloudCostSetRange(ccsr, m, target); err != nil {
+			t.Fatalf("target %q: %v", target, err)
+		}
+		if m.calls != 0 || m.getRateCalls != 0 {
+			t.Errorf("target %q: expected zero converter activity", target)
+		}
+	}
+}
+
 func TestConvertCloudCostSetRange_BestEffortOnPartialFailure(t *testing.T) {
 	ccsr := newCCSR(t)
 	// Fail only NetCost (90), leave others to convert.

+ 16 - 13
pkg/costmodel/aggregation.go

@@ -164,23 +164,26 @@ func (a *Accesses) ComputeAllocationHandlerSummary(w http.ResponseWriter, r *htt
 	}
 	sasr := opencost.NewSummaryAllocationSetRange(sasl...)
 
-	// Extract currency parameter and convert if needed
+	// Extract currency parameter and convert if needed. Currency conversion
+	// is best-effort: individual field failures are logged at source and
+	// leave USD values in place. A non-nil error from the helper means the
+	// upfront rate lookup failed and no mutation occurred.
 	currency := strings.ToUpper(strings.TrimSpace(qp.Get("currency", "USD")))
 	if currency != "USD" && a.CurrencyConverter != nil {
-		// Convert the underlying AllocationSetRange before creating summary
-		err = ConvertAllocationSetRange(asr, a.CurrencyConverter, currency)
-		if err != nil {
+		if err = ConvertAllocationSetRange(asr, a.CurrencyConverter, currency); err != nil {
 			log.Warnf("Currency conversion failed for currency %s: %v", currency, err)
-			// Continue with USD values if conversion fails
-		} else {
-			// Recreate summary allocation sets after conversion
-			sasl = []*opencost.SummaryAllocationSet{}
-			for _, as := range asr.Slice() {
-				sas := opencost.NewSummaryAllocationSet(as, nil, nil, false, false)
-				sasl = append(sasl, sas)
-			}
-			sasr = opencost.NewSummaryAllocationSetRange(sasl...)
 		}
+		// Always rebuild the summary: the helper may have partially
+		// mutated the underlying sets even when it returns an error
+		// (per-field best-effort), so the pre-conversion summary would
+		// otherwise be inconsistent with the (possibly partly converted)
+		// data.
+		sasl = sasl[:0]
+		for _, as := range asr.Slice() {
+			sas := opencost.NewSummaryAllocationSet(as, nil, nil, false, false)
+			sasl = append(sasl, sas)
+		}
+		sasr = opencost.NewSummaryAllocationSetRange(sasl...)
 	}
 
 	WriteData(w, sasr, nil)

+ 83 - 28
pkg/costmodel/currency_helper.go

@@ -1,6 +1,7 @@
 package costmodel
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/opencost/opencost/core/pkg/log"
@@ -8,15 +9,21 @@ import (
 	"github.com/opencost/opencost/pkg/currency"
 )
 
-// Currency conversion uses best-effort semantics: if a single field fails
-// to convert, it is left in USD and a warning is logged. The response may
-// therefore contain mixed currencies under partial converter failures.
-// Callers treat a non-nil error from these helpers as advisory only --
-// the mutation has already been applied where it succeeded.
+// Currency conversion uses a two-layer approach:
+//   - The handler-facing entry (Range / Set / top-level Allocation) calls
+//     Converter.GetRate once to validate the target currency. If that
+//     fails, the helper returns an error without mutating anything and
+//     the caller logs a single per-request warning -- preventing log
+//     floods when the API is unreachable or the currency code is bogus.
+//   - Per-field conversion is best-effort: if an individual Convert call
+//     fails after the rate probe succeeded (exotic case) the original
+//     USD value is retained and logged. Responses may therefore contain
+//     mixed currencies under partial converter failures.
 
 // tryConvert converts val from USD to target, returning the original
-// value (and logging) on converter error. logCtx identifies the field
-// being converted so operators can triage which fields are failing.
+// value (and logging) on converter error. Only fires after the outer
+// helper has already validated the target currency, so this path is
+// rarely hit in practice. logCtx identifies the field being converted.
 func tryConvert(converter currency.Converter, val float64, target, logCtx string) float64 {
 	if val == 0 {
 		return val
@@ -29,15 +36,49 @@ func tryConvert(converter currency.Converter, val float64, target, logCtx string
 	return converted
 }
 
+// normalizeAndProbe normalizes the target currency code and, if it is
+// not a no-op (USD or empty), probes the converter for the rate. Returns
+// the normalized currency, a no-op flag, and any rate-lookup error.
+func normalizeAndProbe(converter currency.Converter, target string) (normalized string, noop bool, err error) {
+	normalized = strings.ToUpper(strings.TrimSpace(target))
+	if normalized == "" || normalized == "USD" {
+		return normalized, true, nil
+	}
+	if _, rateErr := converter.GetRate("USD", normalized); rateErr != nil {
+		return normalized, false, fmt.Errorf("currency rate lookup USD->%s failed: %w", normalized, rateErr)
+	}
+	return normalized, false, nil
+}
+
 // ConvertAllocation converts all cost fields in an Allocation from USD
-// to target currency in place. Best-effort: per-field failures are logged
-// and skipped rather than aborting the whole allocation.
+// to target currency in place. Returns an error if the target rate
+// cannot be looked up (no mutation occurs); per-field converter failures
+// encountered after the rate probe succeeded are handled best-effort and
+// logged in place.
 func ConvertAllocation(alloc *opencost.Allocation, converter currency.Converter, targetCurrency string) error {
-	if alloc == nil || converter == nil || targetCurrency == "USD" {
+	if alloc == nil || converter == nil {
 		return nil
 	}
 
-	targetCurrency = strings.ToUpper(strings.TrimSpace(targetCurrency))
+	targetCurrency, noop, err := normalizeAndProbe(converter, targetCurrency)
+	if err != nil {
+		return err
+	}
+	if noop {
+		return nil
+	}
+
+	return convertAllocationFields(alloc, converter, targetCurrency)
+}
+
+// convertAllocationFields performs the per-field best-effort conversion.
+// The caller must have already normalised the target currency and
+// probed the rate. Returns nil today; retains the error return so the
+// signature can carry structured failure info in the future.
+func convertAllocationFields(alloc *opencost.Allocation, converter currency.Converter, targetCurrency string) error {
+	if alloc == nil {
+		return nil
+	}
 
 	// Named cost fields. Keep in sync with Allocation cost fields in
 	// core/pkg/opencost/allocation.go that appear in JSON API responses.
@@ -73,8 +114,6 @@ func ConvertAllocation(alloc *opencost.Allocation, converter currency.Converter,
 		*f.ptr = tryConvert(converter, *f.ptr, targetCurrency, "Allocation."+f.name)
 	}
 
-	// Convert PV costs (nested structure). Both Cost and Adjustment appear
-	// in JSON responses and must be converted.
 	for pvKey, pv := range alloc.PVs {
 		if pv == nil {
 			continue
@@ -85,7 +124,6 @@ func ConvertAllocation(alloc *opencost.Allocation, converter currency.Converter,
 		alloc.PVs[pvKey] = pv
 	}
 
-	// Convert LoadBalancer costs (nested structure)
 	for lbKey, lb := range alloc.LoadBalancers {
 		if lb == nil {
 			continue
@@ -94,9 +132,8 @@ func ConvertAllocation(alloc *opencost.Allocation, converter currency.Converter,
 		alloc.LoadBalancers[lbKey] = lb
 	}
 
-	// Convert SharedCostBreakdown entries. SharedCostBreakdowns is a
-	// map[string]SharedCostBreakdown (value, not pointer), so mutate a
-	// local copy and re-assign.
+	// SharedCostBreakdowns is map[string]SharedCostBreakdown (value type);
+	// mutate a local copy and re-assign.
 	for key, scb := range alloc.SharedCostBreakdown {
 		scb.TotalCost = tryConvert(converter, scb.TotalCost, targetCurrency, "Allocation.SharedCostBreakdown."+key+".TotalCost")
 		scb.CPUCost = tryConvert(converter, scb.CPUCost, targetCurrency, "Allocation.SharedCostBreakdown."+key+".CPUCost")
@@ -112,31 +149,49 @@ func ConvertAllocation(alloc *opencost.Allocation, converter currency.Converter,
 	return nil
 }
 
-// ConvertAllocationSet converts all allocations in a set (best-effort).
+// ConvertAllocationSet converts all allocations in a set. Returns an
+// error if the target rate cannot be looked up (no mutation occurs).
+// Per-allocation conversion itself is best-effort.
 func ConvertAllocationSet(set *opencost.AllocationSet, converter currency.Converter, targetCurrency string) error {
-	if set == nil || converter == nil || targetCurrency == "USD" {
+	if set == nil || converter == nil {
+		return nil
+	}
+	targetCurrency, noop, err := normalizeAndProbe(converter, targetCurrency)
+	if err != nil {
+		return err
+	}
+	if noop {
 		return nil
 	}
 
 	for _, alloc := range set.Allocations {
-		// ConvertAllocation is best-effort and never returns a non-nil
-		// error today, but retain the error-return contract in case the
-		// helper is extended later.
-		_ = ConvertAllocation(alloc, converter, targetCurrency)
+		_ = convertAllocationFields(alloc, converter, targetCurrency)
 	}
-
 	return nil
 }
 
-// ConvertAllocationSetRange converts all sets in a range (best-effort).
+// ConvertAllocationSetRange converts all sets in a range. Returns an
+// error if the target rate cannot be looked up (no mutation occurs).
+// Per-allocation conversion itself is best-effort.
 func ConvertAllocationSetRange(asr *opencost.AllocationSetRange, converter currency.Converter, targetCurrency string) error {
-	if asr == nil || converter == nil || targetCurrency == "USD" {
+	if asr == nil || converter == nil {
+		return nil
+	}
+	targetCurrency, noop, err := normalizeAndProbe(converter, targetCurrency)
+	if err != nil {
+		return err
+	}
+	if noop {
 		return nil
 	}
 
 	for _, set := range asr.Allocations {
-		_ = ConvertAllocationSet(set, converter, targetCurrency)
+		if set == nil {
+			continue
+		}
+		for _, alloc := range set.Allocations {
+			_ = convertAllocationFields(alloc, converter, targetCurrency)
+		}
 	}
-
 	return nil
 }

+ 69 - 3
pkg/costmodel/currency_helper_test.go

@@ -13,9 +13,11 @@ import (
 // which case Convert returns an error (useful for testing partial-
 // failure / best-effort behavior).
 type mockConverter struct {
-	rate       float64
-	failValues map[float64]bool
-	calls      int
+	rate         float64
+	failValues   map[float64]bool
+	getRateFail  bool
+	getRateCalls int
+	calls        int
 }
 
 func (m *mockConverter) Convert(amount float64, from, to string) (float64, error) {
@@ -30,6 +32,10 @@ func (m *mockConverter) Convert(amount float64, from, to string) (float64, error
 }
 
 func (m *mockConverter) GetRate(from, to string) (float64, error) {
+	m.getRateCalls++
+	if m.getRateFail {
+		return 0, fmt.Errorf("simulated rate lookup failure USD->%s", to)
+	}
 	return m.rate, nil
 }
 
@@ -285,3 +291,63 @@ func TestConvertAllocationSetRange_NilGuards(t *testing.T) {
 		t.Fatalf("nil converter: %v", err)
 	}
 }
+
+// TestConvertAllocation_USDNormalized exercises the normalize-before-
+// USD-check fix: lowercase and whitespace-padded "USD" must be treated
+// as a no-op without invoking the converter.
+func TestConvertAllocation_USDNormalized(t *testing.T) {
+	for _, target := range []string{"usd", " USD ", "Usd", ""} {
+		a := newFullAllocation()
+		m := &mockConverter{rate: 2.0}
+		if err := ConvertAllocation(a, m, target); err != nil {
+			t.Fatalf("target %q: unexpected error %v", target, err)
+		}
+		if m.calls != 0 || m.getRateCalls != 0 {
+			t.Errorf("target %q: expected zero converter activity, got convert=%d rate=%d",
+				target, m.calls, m.getRateCalls)
+		}
+		if a.CPUCost != 10 {
+			t.Errorf("target %q: mutated CPUCost to %v", target, a.CPUCost)
+		}
+	}
+}
+
+// TestConvertAllocation_GetRateFailsReturnsErrorNoMutation exercises the
+// upfront rate probe: when GetRate fails, the helper returns an error
+// and does NOT attempt any per-field conversion (prevents log-storms
+// when the converter is down or the currency code is invalid).
+func TestConvertAllocation_GetRateFailsReturnsErrorNoMutation(t *testing.T) {
+	a := newFullAllocation()
+	m := &mockConverter{rate: 2.0, getRateFail: true}
+
+	err := ConvertAllocation(a, m, "XXX")
+	if err == nil {
+		t.Fatal("expected error when GetRate fails")
+	}
+	if m.calls != 0 {
+		t.Errorf("expected zero Convert calls when GetRate fails; got %d", m.calls)
+	}
+	if a.CPUCost != 10 {
+		t.Errorf("allocation must not be mutated when precheck fails: CPUCost got %v want 10", a.CPUCost)
+	}
+}
+
+func TestConvertAllocationSetRange_GetRateFailsReturnsError(t *testing.T) {
+	start := time.Date(2026, 4, 16, 0, 0, 0, 0, time.UTC)
+	end := start.Add(time.Hour)
+	set := opencost.NewAllocationSet(start, end)
+	set.Insert(&opencost.Allocation{Name: "a1", Start: start, End: end, CPUCost: 5})
+	asr := opencost.NewAllocationSetRange(set)
+
+	m := &mockConverter{rate: 2.0, getRateFail: true}
+	err := ConvertAllocationSetRange(asr, m, "XXX")
+	if err == nil {
+		t.Fatal("expected error when GetRate fails")
+	}
+	if m.calls != 0 {
+		t.Errorf("no Convert calls expected when precheck fails; got %d", m.calls)
+	}
+	if set.Allocations["a1"].CPUCost != 5 {
+		t.Errorf("data must not be mutated on precheck failure")
+	}
+}

+ 35 - 9
pkg/customcost/queryservice.go

@@ -120,19 +120,34 @@ func (qs *QueryService) GetCustomCostTimeseriesHandler() func(w http.ResponseWri
 }
 
 // convertCustomCostResponse converts all cost values in a CostResponse
-// from USD to target currency in place. Best-effort: per-field converter
-// failures are logged and the field is left in USD rather than rolling
-// the whole response back to USD.
+// from USD to target currency in place. Returns an error if the target
+// rate cannot be looked up upfront (no mutation occurs). Per-field
+// converter failures after the probe succeeded are handled best-effort:
+// the field is left in USD and a warning is logged.
 //
 // CustomCost uses float32; conversion is done in float64 to avoid
 // compounding precision loss, then cast back.
 func convertCustomCostResponse(resp *CostResponse, converter currency.Converter, targetCurrency string) error {
-	if resp == nil || converter == nil || targetCurrency == "USD" {
+	if resp == nil || converter == nil {
 		return nil
 	}
 
 	targetCurrency = strings.ToUpper(strings.TrimSpace(targetCurrency))
+	if targetCurrency == "" || targetCurrency == "USD" {
+		return nil
+	}
+	if _, err := converter.GetRate("USD", targetCurrency); err != nil {
+		return fmt.Errorf("currency rate lookup USD->%s failed: %w", targetCurrency, err)
+	}
+
+	convertResponseFields(resp, converter, targetCurrency)
+	return nil
+}
 
+// convertResponseFields applies the per-field conversion without
+// re-probing the rate. The caller must have already validated the
+// target currency.
+func convertResponseFields(resp *CostResponse, converter currency.Converter, targetCurrency string) {
 	tryConvert32 := func(val float32, logCtx string) float32 {
 		if val == 0 {
 			return val
@@ -154,19 +169,30 @@ func convertCustomCostResponse(resp *CostResponse, converter currency.Converter,
 		cc.Cost = tryConvert32(cc.Cost, "CustomCost.Cost")
 		cc.ListUnitPrice = tryConvert32(cc.ListUnitPrice, "CustomCost.ListUnitPrice")
 	}
-
-	return nil
 }
 
 // convertCustomCostTimeseriesResponse converts all cost values in a
-// CostTimeseriesResponse (best-effort).
+// CostTimeseriesResponse. Returns an error if the target rate cannot be
+// looked up upfront (no mutation occurs). Per-field conversion is
+// best-effort thereafter.
 func convertCustomCostTimeseriesResponse(resp *CostTimeseriesResponse, converter currency.Converter, targetCurrency string) error {
-	if resp == nil || converter == nil || targetCurrency == "USD" {
+	if resp == nil || converter == nil {
 		return nil
 	}
 
+	targetCurrency = strings.ToUpper(strings.TrimSpace(targetCurrency))
+	if targetCurrency == "" || targetCurrency == "USD" {
+		return nil
+	}
+	if _, err := converter.GetRate("USD", targetCurrency); err != nil {
+		return fmt.Errorf("currency rate lookup USD->%s failed: %w", targetCurrency, err)
+	}
+
 	for _, costResp := range resp.Timeseries {
-		_ = convertCustomCostResponse(costResp, converter, targetCurrency)
+		if costResp == nil {
+			continue
+		}
+		convertResponseFields(costResp, converter, targetCurrency)
 	}
 
 	return nil

+ 37 - 3
pkg/customcost/queryservice_test.go

@@ -9,9 +9,11 @@ import (
 )
 
 type mockConverter struct {
-	rate       float64
-	failValues map[float64]bool
-	calls      int
+	rate         float64
+	failValues   map[float64]bool
+	getRateFail  bool
+	getRateCalls int
+	calls        int
 }
 
 func (m *mockConverter) Convert(amount float64, from, to string) (float64, error) {
@@ -26,6 +28,10 @@ func (m *mockConverter) Convert(amount float64, from, to string) (float64, error
 }
 
 func (m *mockConverter) GetRate(from, to string) (float64, error) {
+	m.getRateCalls++
+	if m.getRateFail {
+		return 0, fmt.Errorf("simulated rate lookup failure USD->%s", to)
+	}
 	return m.rate, nil
 }
 
@@ -136,6 +142,34 @@ func TestConvertCustomCostTimeseriesResponse(t *testing.T) {
 	}
 }
 
+func TestConvertCustomCostResponse_GetRateFailsReturnsError(t *testing.T) {
+	resp := newCostResponse()
+	m := &mockConverter{rate: 2.0, getRateFail: true}
+	err := convertCustomCostResponse(resp, m, "XXX")
+	if err == nil {
+		t.Fatal("expected error when GetRate fails")
+	}
+	if m.calls != 0 {
+		t.Errorf("no Convert calls expected on precheck failure; got %d", m.calls)
+	}
+	if resp.TotalCost != 150 {
+		t.Errorf("data must not be mutated on precheck failure; TotalCost=%v", resp.TotalCost)
+	}
+}
+
+func TestConvertCustomCostResponse_USDNormalized(t *testing.T) {
+	for _, target := range []string{"usd", " USD ", ""} {
+		resp := newCostResponse()
+		m := &mockConverter{rate: 2.0}
+		if err := convertCustomCostResponse(resp, m, target); err != nil {
+			t.Fatalf("target %q: %v", target, err)
+		}
+		if m.calls != 0 || m.getRateCalls != 0 {
+			t.Errorf("target %q: expected zero converter activity", target)
+		}
+	}
+}
+
 func TestConvertCustomCostTimeseriesResponse_NilGuards(t *testing.T) {
 	if err := convertCustomCostTimeseriesResponse(nil, &mockConverter{rate: 2.0}, "EUR"); err != nil {
 		t.Fatalf("nil response: %v", err)