feat(aws): Use DescribeSpotPriceHistory as fallback for spot pricing

When the AWS spot data feed is delayed (up to 15+ hours in practice),
OpenCost was falling back to on-demand pricing for spot nodes, overstating
costs by ~2.6x. This adds a DescribeSpotPriceHistory API fallback that
provides accurate spot pricing when the spot data feed hasn't yet
delivered data for a running instance.

The fallback is always initialized regardless of whether the spot data
feed is configured, so it also helps clusters that use spot instances
but haven't set up the spot data feed at all.

Fixes https://github.com/opencost/opencost/issues/3725

Signed-off-by: Warwick Peatey <warwick@automatic.systems>
Assisted-by: Claude Code

pkg/cloud/aws/provider.go

@@ -53,6 +53,7 @@ const (
 
 	APIPricingSource              = "Public API"
 	SpotPricingSource             = "Spot Data Feed"
+	SpotPriceHistorySource        = "Spot Price History"
 	ReservedInstancePricingSource = "Savings Plan, Reserved Instance, and Out-Of-Cluster"
 	FargatePricingSource          = "Fargate"
 
@@ -96,11 +97,7 @@ func (aws *AWS) PricingSourceStatus() map[string]*models.PricingSource {
 		Enabled: true,
 	}
 
-	if !aws.SpotRefreshEnabled() {
-		sps.Available = false
-		sps.Error = "Spot instances not set up"
-		sps.Enabled = false
-	} else {
+	if aws.SpotFeedRefreshEnabled() {
 		sps.Error = ""
 		if aws.SpotPricingError != nil {
 			sps.Error = aws.SpotPricingError.Error()
@@ -112,9 +109,28 @@ func (aws *AWS) PricingSourceStatus() map[string]*models.PricingSource {
 		} else {
 			sps.Error = "No spot instances detected"
 		}
+	} else {
+		sps.Available = false
+		sps.Error = "Spot instances not set up"
+		sps.Enabled = false
 	}
 	sources[SpotPricingSource] = sps
 
+	sphs := &models.PricingSource{
+		Name:    SpotPriceHistorySource,
+		Enabled: true,
+	}
+	if aws.SpotPriceHistoryError != nil {
+		sphs.Error = aws.SpotPriceHistoryError.Error()
+		sphs.Available = false
+	} else if aws.SpotPriceHistoryCache == nil {
+		sphs.Error = "Not yet initialized"
+		sphs.Available = false
+	} else {
+		sphs.Available = true
+	}
+	sources[SpotPriceHistorySource] = sphs
+
 	rps := &models.PricingSource{
 		Name:    ReservedInstancePricingSource,
 		Enabled: true,
@@ -185,6 +201,8 @@ type AWS struct {
 	SpotRefreshRunning          bool
 	SpotPricingLock             sync.RWMutex
 	SpotPricingError            error
+	SpotPriceHistoryCache       *SpotPriceHistoryCache
+	SpotPriceHistoryError       error
 	RIPricingByInstanceID       map[string]*RIData
 	RIPricingError              error
 	RIDataRunning               bool
@@ -848,8 +866,8 @@ func (aws *AWS) getRegionPricing(nodeList []*clustercache.Node) (*http.Response,
 	return resp, pricingURL, err
 }
 
-// SpotRefreshEnabled determines whether the required configs to run the spot feed query have been set up
-func (aws *AWS) SpotRefreshEnabled() bool {
+// SpotFeedRefreshEnabled determines whether the required configs to run the spot feed query have been set up
+func (aws *AWS) SpotFeedRefreshEnabled() bool {
 	// Guard against nil receiver
 	if aws == nil {
 		return false
@@ -1019,28 +1037,36 @@ func (aws *AWS) DownloadPricingData() error {
 	}
 	log.Infof("Finished downloading \"%s\"", pricingURL)
 
-	if !aws.SpotRefreshEnabled() {
-		return nil
+	// Initialize a spot price history cache if not already initialized.
+	// Reset error to allow retrying on subsequent DownloadPricingData calls.
+	if aws.SpotPriceHistoryCache == nil {
+		aws.SpotPriceHistoryError = nil
+		aws.SpotPriceHistoryCache, aws.SpotPriceHistoryError = aws.initializeSpotPriceHistoryCache()
+		if aws.SpotPriceHistoryError != nil {
+			log.Errorf("Failed to initialize spot price history manager: %s", aws.SpotPriceHistoryError)
+		}
 	}
 
-	// Always run spot pricing refresh when performing download
-	aws.refreshSpotPricing(true)
+	if aws.SpotFeedRefreshEnabled() {
+		// Always run spot pricing refresh when performing download
+		aws.refreshSpotPricing(true)
 
-	// Only start a single refresh goroutine
-	if !aws.SpotRefreshRunning {
-		aws.SpotRefreshRunning = true
+		// Only start a single refresh goroutine
+		if !aws.SpotRefreshRunning {
+			aws.SpotRefreshRunning = true
 
-		go func() {
-			defer errs.HandlePanic()
+			go func() {
+				defer errs.HandlePanic()
 
-			for {
-				log.Infof("Spot Pricing Refresh scheduled in %.2f minutes.", SpotRefreshDuration.Minutes())
-				time.Sleep(SpotRefreshDuration)
+				for {
+					log.Infof("Spot Pricing Refresh scheduled in %.2f minutes.", SpotRefreshDuration.Minutes())
+					time.Sleep(SpotRefreshDuration)
 
-				// Reoccurring refresh checks update times
-				aws.refreshSpotPricing(false)
-			}
-		}()
+					// Reoccurring refresh checks update times
+					aws.refreshSpotPricing(false)
+				}
+			}()
+		}
 	}
 
 	return nil
@@ -1278,6 +1304,60 @@ func (aws *AWS) refreshSpotPricing(force bool) {
 	aws.SpotPricingByInstanceID = sp
 }
 
+func (aws *AWS) initializeSpotPriceHistoryCache() (*SpotPriceHistoryCache, error) {
+	log.Info("Initializing AWS Spot Price History Manager")
+
+	// Get AWS access key for creating config
+	accessKey, err := aws.GetAWSAccessKey()
+	if err != nil {
+		return nil, fmt.Errorf("getting AWS access key for spot price history: %v", err)
+	}
+
+	// Use the cluster region to create the initial AWS config and credentials.
+	// The SpotPriceHistoryFetcher itself can query multiple regions by creating
+	// region-specific EC2 clients as needed.
+	if aws.ClusterRegion == "" {
+		return nil, fmt.Errorf("no cluster region configured")
+	}
+
+	// Create config for the cluster region
+	awsConfig, err := accessKey.CreateConfig(aws.ClusterRegion)
+	if err != nil {
+		return nil, fmt.Errorf("creating AWS config for spot price history: %v", err)
+	}
+
+	return NewSpotPriceHistoryCache(NewAWSSpotPriceHistoryFetcher(awsConfig)), nil
+}
+
+func (aws *AWS) spotPricingFromHistory(k models.Key) (*SpotPriceHistoryEntry, bool) {
+	if aws.SpotPriceHistoryCache == nil {
+		return nil, false
+	}
+
+	// Extract region, instance type, and availability zone from the key
+	awsKey, ok := k.(*awsKey)
+	if !ok {
+		log.DedupedWarningf(10, "Failed to cast key to awsKey for spot price history lookup: %s", k.ID())
+		return nil, false
+	}
+
+	region, regionOk := util.GetRegion(awsKey.Labels)
+	instanceType, instanceTypeOk := util.GetInstanceType(awsKey.Labels)
+	availabilityZone, availabilityZoneOk := util.GetZone(awsKey.Labels)
+	// Skip lookup if any required information is missing
+	if !regionOk || !instanceTypeOk || !availabilityZoneOk {
+		log.DedupedWarningf(10, "Missing required info for spot price history lookup (region: %s, instanceType: %s, zone: %s): %s", region, instanceType, availabilityZone, k.ID())
+		return nil, false
+	}
+
+	price, err := aws.SpotPriceHistoryCache.GetSpotPrice(region, instanceType, availabilityZone)
+	if err != nil {
+		log.DedupedWarningf(10, "Failed to get spot price history for instance %s: %s", k.ID(), err.Error())
+		return nil, false
+	}
+	return price, true
+}
+
 // Stubbed NetworkPricing for AWS. Pull directly from aws.json for now
 func (aws *AWS) NetworkPricing() (*models.Network, error) {
 	cpricing, err := aws.Config.GetCustomPricingData()
@@ -1403,11 +1483,29 @@ func (aws *AWS) createNode(terms *AWSProductTerms, usageType string, k models.Ke
 			UsageType:    PreemptibleType,
 		}, meta, nil
 	} else if aws.isPreemptible(key) { // Preemptible but we don't have any data in the pricing report.
-		if aws.SpotRefreshEnabled() {
-			log.DedupedWarningf(5, "Node %s marked preemptible but we have no data in spot feed", k.ID())
+		log.DedupedWarningf(5, "Node %s marked preemptible but we have no data in spot feed", k.ID())
+
+		// Try to get spot pricing from DescribeSpotPriceHistory API
+		if historyEntry, ok := aws.spotPricingFromHistory(k); ok {
+			log.DedupedInfof(5, "Using spot price history data for node %s: $%f", k.ID(), historyEntry.SpotPrice)
+			spotHistoryCost := fmt.Sprintf("%f", historyEntry.SpotPrice)
+			meta.Source = SpotPriceHistorySource
+			return &models.Node{
+				Cost:         spotHistoryCost,
+				VCPU:         terms.VCpu,
+				RAM:          terms.Memory,
+				GPU:          terms.GPU,
+				Storage:      terms.Storage,
+				BaseCPUPrice: aws.BaseCPUPrice,
+				BaseRAMPrice: aws.BaseRAMPrice,
+				BaseGPUPrice: aws.BaseGPUPrice,
+				UsageType:    PreemptibleType,
+			}, meta, nil
 		}
+
 		if publicPricingFound {
 			// return public price if found
+			log.DedupedWarningf(5, "No spot price history available for %s, falling back to on-demand pricing", k.ID())
 			return &models.Node{
 				Cost:         cost,
 				VCPU:         terms.VCpu,
@@ -1421,9 +1519,7 @@ func (aws *AWS) createNode(terms *AWSProductTerms, usageType string, k models.Ke
 			}, meta, nil
 		} else {
 			// return defaults if public pricing not found
-			if aws.SpotRefreshEnabled() {
-				log.DedupedWarningf(5, "Could not find Node %s's public pricing info, using default configured spot prices instead", k.ID())
-			}
+			log.DedupedWarningf(5, "Could not find Node %s's public pricing info, using default configured spot prices instead", k.ID())
 			return &models.Node{
 				VCPU:         terms.VCpu,
 				VCPUCost:     aws.BaseSpotCPUPrice,

pkg/cloud/aws/provider_test.go

@@ -867,7 +867,7 @@ func (f *fakeProviderConfig) ConfigFileManager() *config.ConfigFileManager {
 	return nil
 }
 
-func TestAWS_SpotRefreshEnabled(t *testing.T) {
+func TestAWS_SpotFeedRefreshEnabled(t *testing.T) {
 	tests := []struct {
 		name                string
 		spotDataBucket      string
@@ -955,9 +955,9 @@ func TestAWS_SpotRefreshEnabled(t *testing.T) {
 				},
 			}
 
-			got := aws.SpotRefreshEnabled()
+			got := aws.SpotFeedRefreshEnabled()
 			if got != tt.want {
-				t.Errorf("AWS.SpotRefreshEnabled() = %v, want %v", got, tt.want)
+				t.Errorf("AWS.SpotFeedRefreshEnabled() = %v, want %v", got, tt.want)
 			}
 		})
 	}
@@ -971,10 +971,10 @@ func TestAWS_SpotRefreshEnabled(t *testing.T) {
 			Config:         nil, // nil Config should not cause panic
 		}
 
-		got := aws.SpotRefreshEnabled()
+		got := aws.SpotFeedRefreshEnabled()
 		want := true // Should fall back to field-based check
 		if got != want {
-			t.Errorf("AWS.SpotRefreshEnabled() with nil Config = %v, want %v", got, want)
+			t.Errorf("AWS.SpotFeedRefreshEnabled() with nil Config = %v, want %v", got, want)
 		}
 	})
 
@@ -986,10 +986,10 @@ func TestAWS_SpotRefreshEnabled(t *testing.T) {
 			Config:         nil, // nil Config should not cause panic
 		}
 
-		got := aws.SpotRefreshEnabled()
+		got := aws.SpotFeedRefreshEnabled()
 		want := false // No fields set, should return false
 		if got != want {
-			t.Errorf("AWS.SpotRefreshEnabled() with nil Config and no fields = %v, want %v", got, want)
+			t.Errorf("AWS.SpotFeedRefreshEnabled() with nil Config and no fields = %v, want %v", got, want)
 		}
 	})
 }

pkg/cloud/aws/spotpricehistory.go

@@ -0,0 +1,186 @@
+package aws
+
+import (
+	"context"
+	"fmt"
+	"strconv"
+	"sync"
+	"time"
+
+	awsSDK "github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/ec2"
+	ec2Types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
+
+	"github.com/opencost/opencost/core/pkg/log"
+)
+
+// SpotPriceHistoryKey uniquely identifies a spot price lookup by region,
+// instance type, and availability zone.
+type SpotPriceHistoryKey struct {
+	Region           string
+	InstanceType     string
+	AvailabilityZone string
+}
+
+func (key SpotPriceHistoryKey) String() string {
+	return fmt.Sprintf("%s/%s/%s", key.Region, key.InstanceType, key.AvailabilityZone)
+}
+
+const (
+	SpotPriceHistoryCacheAge = 1 * time.Hour
+)
+
+// SpotPriceHistoryEntry holds a cached spot price from the DescribeSpotPriceHistory API.
+type SpotPriceHistoryEntry struct {
+	SpotPrice float64
+	Timestamp time.Time
+
+	RetrievedAt time.Time
+	Error       error // Negative cache
+}
+
+func (spe SpotPriceHistoryEntry) shouldRefresh() bool {
+	return time.Since(spe.RetrievedAt) > SpotPriceHistoryCacheAge
+}
+
+// SpotPriceHistoryCache provides a thread-safe, on-demand cache for spot prices
+// retrieved via the DescribeSpotPriceHistory API. Entries are cached for
+// SpotPriceHistoryCacheAge and include negative caching for errors.
+type SpotPriceHistoryCache struct {
+	cache          map[SpotPriceHistoryKey]*SpotPriceHistoryEntry
+	mutex          sync.Mutex
+	refreshRunning map[SpotPriceHistoryKey]bool
+	refreshCond    *sync.Cond
+
+	fetcher SpotPriceHistoryFetcher
+}
+
+func NewSpotPriceHistoryCache(fetcher SpotPriceHistoryFetcher) *SpotPriceHistoryCache {
+	cache := &SpotPriceHistoryCache{
+		cache:          make(map[SpotPriceHistoryKey]*SpotPriceHistoryEntry),
+		refreshRunning: make(map[SpotPriceHistoryKey]bool),
+
+		fetcher: fetcher,
+	}
+	cache.refreshCond = sync.NewCond(&cache.mutex)
+	return cache
+}
+
+// GetSpotPrice returns the cached spot price for the given region, instance type,
+// and availability zone. If the cache entry is missing or stale, it fetches a
+// fresh value from the underlying SpotPriceHistoryFetcher.
+func (sph *SpotPriceHistoryCache) GetSpotPrice(region, instanceType, availabilityZone string) (*SpotPriceHistoryEntry, error) {
+	key := SpotPriceHistoryKey{
+		Region:           region,
+		InstanceType:     instanceType,
+		AvailabilityZone: availabilityZone,
+	}
+	sph.mutex.Lock()
+	for sph.refreshRunning[key] {
+		sph.refreshCond.Wait()
+	}
+	// Check if we have cached price. If so, return it.
+	entry, exists := sph.cache[key]
+	if exists && !entry.shouldRefresh() {
+		sph.mutex.Unlock()
+		return entry, entry.Error
+	}
+	// Either a cache entry does not exist or it is stale. Refresh it.
+	sph.refreshRunning[key] = true
+	sph.mutex.Unlock()
+
+	// Fetch the entry
+	entry, err := sph.fetcher.FetchSpotPrice(key)
+	if err != nil {
+		// If we fail to fetch, create a negative cache entry.
+		entry = &SpotPriceHistoryEntry{
+			RetrievedAt: time.Now(),
+			Error:       err,
+		}
+	}
+
+	// Store it into the cache
+	sph.mutex.Lock()
+	sph.cache[key] = entry
+	delete(sph.refreshRunning, key)
+	sph.refreshCond.Broadcast()
+	sph.mutex.Unlock()
+	return entry, entry.Error
+}
+
+// SpotPriceHistoryFetcher is the interface for fetching spot prices from the
+// DescribeSpotPriceHistory API (or a mock for testing).
+type SpotPriceHistoryFetcher interface {
+	FetchSpotPrice(key SpotPriceHistoryKey) (*SpotPriceHistoryEntry, error)
+}
+
+// AWSSpotPriceHistoryFetcher implements SpotPriceHistoryFetcher using the real
+// AWS EC2 DescribeSpotPriceHistory API. It maintains a pool of per-region
+// EC2 clients.
+type AWSSpotPriceHistoryFetcher struct {
+	awsConfig       awsSDK.Config
+	ec2ClientsMutex sync.Mutex
+	ec2Clients      map[string]*ec2.Client
+}
+
+func NewAWSSpotPriceHistoryFetcher(awsConfig awsSDK.Config) *AWSSpotPriceHistoryFetcher {
+	return &AWSSpotPriceHistoryFetcher{
+		awsConfig:  awsConfig,
+		ec2Clients: make(map[string]*ec2.Client),
+	}
+}
+
+func (a *AWSSpotPriceHistoryFetcher) getEC2Client(region string) *ec2.Client {
+	a.ec2ClientsMutex.Lock()
+	defer a.ec2ClientsMutex.Unlock()
+	if client, ok := a.ec2Clients[region]; ok {
+		return client
+	}
+	config := a.awsConfig
+	config.Region = region
+	client := ec2.NewFromConfig(config)
+	a.ec2Clients[region] = client
+	return client
+}
+
+func (a *AWSSpotPriceHistoryFetcher) FetchSpotPrice(key SpotPriceHistoryKey) (*SpotPriceHistoryEntry, error) {
+	log.Infof("Retrieving spot price history for %s", key)
+	client := a.getEC2Client(key.Region)
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	input := &ec2.DescribeSpotPriceHistoryInput{
+		InstanceTypes:    []ec2Types.InstanceType{ec2Types.InstanceType(key.InstanceType)},
+		AvailabilityZone: awsSDK.String(key.AvailabilityZone),
+		// Only retrieve Linux/UNIX (Amazon VPC) prices. The non-VPC
+		// "Linux/UNIX" variant was for EC2-Classic, which was fully retired in
+		// August 2023.
+		ProductDescriptions: []string{
+			"Linux/UNIX (Amazon VPC)",
+		},
+		// Only retrieve the latest price.
+		MaxResults: awsSDK.Int32(1),
+	}
+
+	resp, err := client.DescribeSpotPriceHistory(ctx, input)
+	if err != nil {
+		return nil, fmt.Errorf("describing spot price history for %s: %w", key, err)
+	}
+	if len(resp.SpotPriceHistory) == 0 {
+		return nil, fmt.Errorf("no spot price history found for %s", key)
+	}
+	spotPrice := resp.SpotPriceHistory[0]
+
+	if spotPrice.SpotPrice == nil || spotPrice.Timestamp == nil {
+		return nil, fmt.Errorf("missing required spot price history data")
+	}
+	price, err := strconv.ParseFloat(*spotPrice.SpotPrice, 64)
+	if err != nil {
+		return nil, fmt.Errorf("parsing spot price: %w", err)
+	}
+	return &SpotPriceHistoryEntry{
+		SpotPrice:   price,
+		Timestamp:   *spotPrice.Timestamp,
+		RetrievedAt: time.Now(),
+	}, nil
+}

pkg/cloud/aws/spotpricehistory_test.go

@@ -0,0 +1,199 @@
+package aws
+
+import (
+	"errors"
+	"testing"
+	"time"
+)
+
+type mockSpotPriceHistoryFetcher struct {
+	fetchFunc func(key SpotPriceHistoryKey) (*SpotPriceHistoryEntry, error)
+}
+
+func (m *mockSpotPriceHistoryFetcher) FetchSpotPrice(key SpotPriceHistoryKey) (*SpotPriceHistoryEntry, error) {
+	if m.fetchFunc != nil {
+		return m.fetchFunc(key)
+	}
+	return &SpotPriceHistoryEntry{
+		SpotPrice:   0.05,
+		Timestamp:   time.Now(),
+		RetrievedAt: time.Now(),
+	}, nil
+}
+
+func TestSpotPriceHistoryCache_GetSpotPrice_CacheHit(t *testing.T) {
+	mockFetcher := &mockSpotPriceHistoryFetcher{}
+	cache := NewSpotPriceHistoryCache(mockFetcher)
+
+	region := "us-west-2"
+	instanceType := "m5.large"
+	availabilityZone := "us-west-2a"
+
+	key := SpotPriceHistoryKey{
+		Region:           region,
+		InstanceType:     instanceType,
+		AvailabilityZone: availabilityZone,
+	}
+
+	cachedEntry := &SpotPriceHistoryEntry{
+		SpotPrice:   0.08,
+		Timestamp:   time.Now(),
+		RetrievedAt: time.Now(),
+	}
+	cache.cache[key] = cachedEntry
+
+	entry, err := cache.GetSpotPrice(region, instanceType, availabilityZone)
+	if err != nil {
+		t.Errorf("Expected no error, got %v", err)
+	}
+	if entry.SpotPrice != 0.08 {
+		t.Errorf("Expected spot price 0.08, got %f", entry.SpotPrice)
+	}
+}
+
+func TestSpotPriceHistoryCache_GetSpotPrice_CacheMiss(t *testing.T) {
+	fetchCalled := false
+	mockFetcher := &mockSpotPriceHistoryFetcher{
+		fetchFunc: func(key SpotPriceHistoryKey) (*SpotPriceHistoryEntry, error) {
+			fetchCalled = true
+			return &SpotPriceHistoryEntry{
+				SpotPrice:   0.12,
+				Timestamp:   time.Now(),
+				RetrievedAt: time.Now(),
+			}, nil
+		},
+	}
+	cache := NewSpotPriceHistoryCache(mockFetcher)
+
+	entry, err := cache.GetSpotPrice("us-west-2", "m5.large", "us-west-2a")
+	if err != nil {
+		t.Errorf("Expected no error, got %v", err)
+	}
+	if !fetchCalled {
+		t.Error("Expected fetcher to be called for cache miss")
+	}
+	if entry.SpotPrice != 0.12 {
+		t.Errorf("Expected spot price 0.12, got %f", entry.SpotPrice)
+	}
+}
+
+func TestSpotPriceHistoryCache_GetSpotPrice_StaleEntry(t *testing.T) {
+	fetchCalled := false
+	mockFetcher := &mockSpotPriceHistoryFetcher{
+		fetchFunc: func(key SpotPriceHistoryKey) (*SpotPriceHistoryEntry, error) {
+			fetchCalled = true
+			return &SpotPriceHistoryEntry{
+				SpotPrice:   0.15,
+				Timestamp:   time.Now(),
+				RetrievedAt: time.Now(),
+			}, nil
+		},
+	}
+	cache := NewSpotPriceHistoryCache(mockFetcher)
+
+	key := SpotPriceHistoryKey{
+		Region:           "us-west-2",
+		InstanceType:     "m5.large",
+		AvailabilityZone: "us-west-2a",
+	}
+
+	staleEntry := &SpotPriceHistoryEntry{
+		SpotPrice:   0.08,
+		Timestamp:   time.Now(),
+		RetrievedAt: time.Now().Add(-2 * time.Hour),
+	}
+	cache.cache[key] = staleEntry
+
+	entry, err := cache.GetSpotPrice("us-west-2", "m5.large", "us-west-2a")
+	if err != nil {
+		t.Errorf("Expected no error, got %v", err)
+	}
+	if !fetchCalled {
+		t.Error("Expected fetcher to be called for stale entry")
+	}
+	if entry.SpotPrice != 0.15 {
+		t.Errorf("Expected refreshed spot price 0.15, got %f", entry.SpotPrice)
+	}
+}
+
+func TestSpotPriceHistoryCache_GetSpotPrice_FetchError(t *testing.T) {
+	expectedError := errors.New("fetch failed")
+	mockFetcher := &mockSpotPriceHistoryFetcher{
+		fetchFunc: func(key SpotPriceHistoryKey) (*SpotPriceHistoryEntry, error) {
+			return nil, expectedError
+		},
+	}
+	cache := NewSpotPriceHistoryCache(mockFetcher)
+
+	_, err := cache.GetSpotPrice("us-west-2", "m5.large", "us-west-2a")
+	if err == nil {
+		t.Error("Expected error from failed fetch")
+	}
+	if !errors.Is(err, expectedError) {
+		t.Errorf("Expected error %v, got %v", expectedError, err)
+	}
+
+	key := SpotPriceHistoryKey{
+		Region:           "us-west-2",
+		InstanceType:     "m5.large",
+		AvailabilityZone: "us-west-2a",
+	}
+	cachedEntry := cache.cache[key]
+	if !errors.Is(cachedEntry.Error, expectedError) {
+		t.Errorf("Expected cached entry error %v, got %v", expectedError, cachedEntry.Error)
+	}
+}
+
+func TestSpotPriceHistoryEntry_shouldRefresh(t *testing.T) {
+	now := time.Now()
+
+	tests := []struct {
+		name        string
+		retrievedAt time.Time
+		expected    bool
+	}{
+		{
+			name:        "fresh entry",
+			retrievedAt: now,
+			expected:    false,
+		},
+		{
+			name:        "stale entry",
+			retrievedAt: now.Add(-2 * time.Hour),
+			expected:    true,
+		},
+		{
+			name:        "borderline entry",
+			retrievedAt: now.Add(-SpotPriceHistoryCacheAge + 1*time.Minute),
+			expected:    false,
+		},
+		{
+			name:        "expired entry",
+			retrievedAt: now.Add(-SpotPriceHistoryCacheAge - 1*time.Minute),
+			expected:    true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			entry := SpotPriceHistoryEntry{
+				RetrievedAt: tt.retrievedAt,
+			}
+			if got := entry.shouldRefresh(); got != tt.expected {
+				t.Errorf("shouldRefresh() = %v, want %v", got, tt.expected)
+			}
+		})
+	}
+}
+
+func TestSpotPriceHistoryKey_String(t *testing.T) {
+	key := SpotPriceHistoryKey{
+		Region:           "us-west-2",
+		InstanceType:     "m5.large",
+		AvailabilityZone: "us-west-2a",
+	}
+	expected := "us-west-2/m5.large/us-west-2a"
+	if got := key.String(); got != expected {
+		t.Errorf("String() = %v, want %v", got, expected)
+	}
+}