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

Merge branch 'develop' into disale-healthz-endpoint-logging

Matt Ray 2 лет назад
Родитель
Сommit
b356f6c6a3
44 измененных файлов с 1124 добавлено и 393 удалено
  1. 5 3
      pkg/cloud/alibaba/provider.go
  2. 3 0
      pkg/cloud/aws/athenaquerier.go
  3. 55 22
      pkg/cloud/aws/provider.go
  4. 11 9
      pkg/cloud/azure/provider.go
  5. 100 54
      pkg/cloud/gcp/provider.go
  6. 140 158
      pkg/cloud/gcp/provider_test.go
  7. 319 0
      pkg/cloud/gcp/test/skus.json
  8. 1 1
      pkg/cloud/models/models.go
  9. 7 0
      pkg/cloud/models/pricing.go
  10. 4 3
      pkg/cloud/provider/csvprovider.go
  11. 4 2
      pkg/cloud/provider/customprovider.go
  12. 5 3
      pkg/cloud/scaleway/provider.go
  13. 1 1
      pkg/costmodel/allocation.go
  14. 15 4
      pkg/costmodel/allocation_helpers.go
  15. 1 0
      pkg/costmodel/allocation_types.go
  16. 1 1
      pkg/costmodel/assets.go
  17. 7 0
      pkg/costmodel/cluster.go
  18. 1 1
      pkg/costmodel/costmodel.go
  19. 18 8
      pkg/costmodel/intervals.go
  20. 53 25
      pkg/costmodel/intervals_test.go
  21. 34 16
      pkg/costmodel/metrics.go
  22. 2 1
      pkg/filter21/allocation/fields.go
  23. 3 0
      pkg/kubecost/allocation.go
  24. 68 1
      pkg/kubecost/allocation_json_test.go
  25. 39 2
      pkg/kubecost/allocationfilter_test.go
  26. 26 2
      pkg/kubecost/allocationmatcher.go
  27. 18 3
      pkg/kubecost/asset.go
  28. 9 1
      pkg/kubecost/asset_json.go
  29. 8 2
      pkg/kubecost/asset_json_test.go
  30. 2 2
      pkg/kubecost/bingen.go
  31. 52 7
      pkg/kubecost/kubecost_codecs.go
  32. 2 2
      pkg/kubecost/mock.go
  33. 7 6
      pkg/kubecost/query.go
  34. 1 1
      pkg/metrics/deploymentmetrics.go
  35. 1 1
      pkg/metrics/namespacemetrics.go
  36. 1 1
      pkg/metrics/nodemetrics.go
  37. 1 31
      pkg/metrics/podlabelmetrics.go
  38. 1 1
      pkg/metrics/podmetrics.go
  39. 1 1
      pkg/metrics/servicemetrics.go
  40. 1 1
      pkg/metrics/statefulsetmetrics.go
  41. 2 1
      pkg/metrics/telemetry.go
  42. 14 0
      pkg/prom/metrics.go
  43. 65 0
      pkg/prom/metrics_test.go
  44. 15 15
      test/cloud_test.go

+ 5 - 3
pkg/cloud/alibaba/provider.go

@@ -514,23 +514,25 @@ func (alibaba *Alibaba) AllNodePricing() (interface{}, error) {
 }
 
 // NodePricing gives pricing information of a specific node given by the key
-func (alibaba *Alibaba) NodePricing(key models.Key) (*models.Node, error) {
+func (alibaba *Alibaba) NodePricing(key models.Key) (*models.Node, models.PricingMetadata, error) {
 	alibaba.DownloadPricingDataLock.RLock()
 	defer alibaba.DownloadPricingDataLock.RUnlock()
 
 	// Get node features for the key
 	keyFeature := key.Features()
 
+	meta := models.PricingMetadata{}
+
 	pricing, ok := alibaba.Pricing[keyFeature]
 	if !ok {
 		log.Errorf("Node pricing information not found for node with feature: %s", keyFeature)
-		return nil, fmt.Errorf("Node pricing information not found for node with feature: %s letting it use default values", keyFeature)
+		return nil, meta, fmt.Errorf("Node pricing information not found for node with feature: %s letting it use default values", keyFeature)
 	}
 
 	log.Debugf("returning the node price for the node with feature: %s", keyFeature)
 	returnNode := pricing.Node
 
-	return returnNode, nil
+	return returnNode, meta, nil
 }
 
 // PVPricing gives a pricing information of a specific PV given by PVkey

+ 3 - 0
pkg/cloud/aws/athenaquerier.go

@@ -110,6 +110,9 @@ func (aq *AthenaQuerier) queryAthenaPaginated(ctx context.Context, query string,
 
 	// Create Athena Client
 	cli, err := aq.GetAthenaClient()
+	if err != nil {
+		return fmt.Errorf("QueryAthenaPaginated: GetAthenaClient error: %s", err.Error())
+	}
 
 	// Query Athena
 	startQueryExecutionOutput, err := cli.StartQueryExecution(ctx, startQueryExecutionInput)

+ 55 - 22
pkg/cloud/aws/provider.go

@@ -5,6 +5,7 @@ import (
 	"compress/gzip"
 	"context"
 	"encoding/csv"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -15,6 +16,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/aws/smithy-go"
 	"github.com/opencost/opencost/pkg/cloud/models"
 	"github.com/opencost/opencost/pkg/cloud/utils"
 	"github.com/opencost/opencost/pkg/kubecost"
@@ -1240,9 +1242,11 @@ func (aws *AWS) savingsPlanPricing(instanceID string) (*SavingsPlanData, bool) {
 	return data, ok
 }
 
-func (aws *AWS) createNode(terms *AWSProductTerms, usageType string, k models.Key) (*models.Node, error) {
+func (aws *AWS) createNode(terms *AWSProductTerms, usageType string, k models.Key) (*models.Node, models.PricingMetadata, error) {
 	key := k.Features()
 
+	meta := models.PricingMetadata{}
+
 	if spotInfo, ok := aws.spotPricing(k.ID()); ok {
 		var spotcost string
 		log.DedupedInfof(5, "Looking up spot data from feed for node %s", k.ID())
@@ -1262,7 +1266,7 @@ func (aws *AWS) createNode(terms *AWSProductTerms, usageType string, k models.Ke
 			BaseRAMPrice: aws.BaseRAMPrice,
 			BaseGPUPrice: aws.BaseGPUPrice,
 			UsageType:    PreemptibleType,
-		}, nil
+		}, meta, nil
 	} else if aws.isPreemptible(key) { // Preemptible but we don't have any data in the pricing report.
 		log.DedupedWarningf(5, "Node %s marked preemptible but we have no data in spot feed", k.ID())
 		return &models.Node{
@@ -1275,7 +1279,7 @@ func (aws *AWS) createNode(terms *AWSProductTerms, usageType string, k models.Ke
 			BaseRAMPrice: aws.BaseRAMPrice,
 			BaseGPUPrice: aws.BaseGPUPrice,
 			UsageType:    PreemptibleType,
-		}, nil
+		}, meta, nil
 	} else if sp, ok := aws.savingsPlanPricing(k.ID()); ok {
 		strCost := fmt.Sprintf("%f", sp.EffectiveCost)
 		return &models.Node{
@@ -1288,7 +1292,7 @@ func (aws *AWS) createNode(terms *AWSProductTerms, usageType string, k models.Ke
 			BaseRAMPrice: aws.BaseRAMPrice,
 			BaseGPUPrice: aws.BaseGPUPrice,
 			UsageType:    usageType,
-		}, nil
+		}, meta, nil
 
 	} else if ri, ok := aws.reservedInstancePricing(k.ID()); ok {
 		strCost := fmt.Sprintf("%f", ri.EffectiveCost)
@@ -1302,7 +1306,7 @@ func (aws *AWS) createNode(terms *AWSProductTerms, usageType string, k models.Ke
 			BaseRAMPrice: aws.BaseRAMPrice,
 			BaseGPUPrice: aws.BaseGPUPrice,
 			UsageType:    usageType,
-		}, nil
+		}, meta, nil
 
 	}
 	var cost string
@@ -1315,7 +1319,7 @@ func (aws *AWS) createNode(terms *AWSProductTerms, usageType string, k models.Ke
 		if ok {
 			cost = c.PricePerUnit.CNY
 		} else {
-			return nil, fmt.Errorf("Could not fetch data for \"%s\"", k.ID())
+			return nil, meta, fmt.Errorf("Could not fetch data for \"%s\"", k.ID())
 		}
 	}
 
@@ -1329,11 +1333,11 @@ func (aws *AWS) createNode(terms *AWSProductTerms, usageType string, k models.Ke
 		BaseRAMPrice: aws.BaseRAMPrice,
 		BaseGPUPrice: aws.BaseGPUPrice,
 		UsageType:    usageType,
-	}, nil
+	}, meta, nil
 }
 
 // NodePricing takes in a key from GetKey and returns a Node object for use in building the cost model.
-func (aws *AWS) NodePricing(k models.Key) (*models.Node, error) {
+func (aws *AWS) NodePricing(k models.Key) (*models.Node, models.PricingMetadata, error) {
 	aws.DownloadPricingDataLock.RLock()
 	defer aws.DownloadPricingDataLock.RUnlock()
 
@@ -1343,6 +1347,8 @@ func (aws *AWS) NodePricing(k models.Key) (*models.Node, error) {
 		usageType = PreemptibleType
 	}
 
+	meta := models.PricingMetadata{}
+
 	terms, ok := aws.Pricing[key]
 	if ok {
 		return aws.createNode(terms, usageType, k)
@@ -1358,7 +1364,7 @@ func (aws *AWS) NodePricing(k models.Key) (*models.Node, error) {
 				BaseGPUPrice:     aws.BaseGPUPrice,
 				UsageType:        usageType,
 				UsesBaseCPUPrice: true,
-			}, err
+			}, meta, err
 		}
 		terms, termsOk := aws.Pricing[key]
 		if !termsOk {
@@ -1369,11 +1375,11 @@ func (aws *AWS) NodePricing(k models.Key) (*models.Node, error) {
 				BaseGPUPrice:     aws.BaseGPUPrice,
 				UsageType:        usageType,
 				UsesBaseCPUPrice: true,
-			}, fmt.Errorf("Unable to find any Pricing data for \"%s\"", key)
+			}, meta, fmt.Errorf("Unable to find any Pricing data for \"%s\"", key)
 		}
 		return aws.createNode(terms, usageType, k)
 	} else { // Fall back to base pricing if we can't find the key. Base pricing is handled at the costmodel level.
-		return nil, fmt.Errorf("Invalid Pricing Key \"%s\"", key)
+		return nil, meta, fmt.Errorf("Invalid Pricing Key \"%s\"", key)
 
 	}
 }
@@ -1555,8 +1561,20 @@ func (aws *AWS) getAllAddresses() ([]*ec2Types.Address, error) {
 			// Query for first page of volume results
 			resp, err := aws.getAddressesForRegion(context.TODO(), region)
 			if err != nil {
-				errorCh <- err
-				return
+				var awsErr smithy.APIError
+				if errors.As(err, &awsErr) {
+					switch awsErr.ErrorCode() {
+					case "AuthFailure", "InvalidClientTokenId", "UnauthorizedOperation":
+						log.DedupedInfof(5, "Unable to get addresses for region %s due to AWS permissions, error message: %s", r, awsErr.ErrorMessage())
+						return
+					default:
+						errorCh <- err
+						return
+					}
+				} else {
+					errorCh <- err
+					return
+				}
 			}
 			addressCh <- resp
 		}(r)
@@ -1657,8 +1675,20 @@ func (aws *AWS) getAllDisks() ([]*ec2Types.Volume, error) {
 			// Query for first page of volume results
 			resp, err := aws.getDisksForRegion(context.TODO(), region, 1000, nil)
 			if err != nil {
-				errorCh <- err
-				return
+				var awsErr smithy.APIError
+				if errors.As(err, &awsErr) {
+					switch awsErr.ErrorCode() {
+					case "AuthFailure", "InvalidClientTokenId", "UnauthorizedOperation":
+						log.DedupedInfof(5, "Unable to get disks for region %s due to AWS permissions, error message: %s", r, awsErr.ErrorMessage())
+						return
+					default:
+						errorCh <- err
+						return
+					}
+				} else {
+					errorCh <- err
+					return
+				}
 			}
 			volumeCh <- resp
 
@@ -1740,14 +1770,17 @@ func (aws *AWS) isDiskOrphaned(vol *ec2Types.Volume) bool {
 }
 
 func (aws *AWS) GetOrphanedResources() ([]models.OrphanedResource, error) {
-	volumes, err := aws.getAllDisks()
-	if err != nil {
-		return nil, err
-	}
+	volumes, volumesErr := aws.getAllDisks()
+	addresses, addressesErr := aws.getAllAddresses()
 
-	addresses, err := aws.getAllAddresses()
-	if err != nil {
-		return nil, err
+	// If we have any orphaned resources - prioritize returning them over returning errors
+	if len(addresses) == 0 && len(volumes) == 0 {
+		if volumesErr != nil {
+			return nil, volumesErr
+		}
+		if addressesErr != nil {
+			return nil, addressesErr
+		}
 	}
 
 	var orphanedResources []models.OrphanedResource

+ 11 - 9
pkg/cloud/azure/provider.go

@@ -1079,7 +1079,7 @@ func (az *Azure) AllNodePricing() (interface{}, error) {
 }
 
 // NodePricing returns Azure pricing data for a single node
-func (az *Azure) NodePricing(key models.Key) (*models.Node, error) {
+func (az *Azure) NodePricing(key models.Key) (*models.Node, models.PricingMetadata, error) {
 	az.DownloadPricingDataLock.RLock()
 	defer az.DownloadPricingDataLock.RUnlock()
 	pricingDataExists := true
@@ -1088,9 +1088,11 @@ func (az *Azure) NodePricing(key models.Key) (*models.Node, error) {
 		log.DedupedWarningf(1, "Unable to download Azure pricing data")
 	}
 
+	meta := models.PricingMetadata{}
+
 	azKey, ok := key.(*azureKey)
 	if !ok {
-		return nil, fmt.Errorf("azure: NodePricing: key is of type %T", key)
+		return nil, meta, fmt.Errorf("azure: NodePricing: key is of type %T", key)
 	}
 	config, _ := az.GetConfig()
 
@@ -1105,7 +1107,7 @@ func (az *Azure) NodePricing(key models.Key) (*models.Node, error) {
 			if azKey.isValidGPUNode() {
 				n.Node.GPU = "1" // TODO: support multiple GPUs
 			}
-			return n.Node, nil
+			return n.Node, meta, nil
 		}
 		log.Infof("[Info] found spot instance, trying to get retail price for %s: %s, ", spotFeatures, azKey)
 		spotCost, err := getRetailPrice(region, instance, config.CurrencyCode, true)
@@ -1124,7 +1126,7 @@ func (az *Azure) NodePricing(key models.Key) (*models.Node, error) {
 			az.addPricing(spotFeatures, &AzurePricing{
 				Node: spotNode,
 			})
-			return spotNode, nil
+			return spotNode, meta, nil
 		}
 	}
 
@@ -1136,13 +1138,13 @@ func (az *Azure) NodePricing(key models.Key) (*models.Node, error) {
 			if azKey.isValidGPUNode() {
 				n.Node.GPU = azKey.GetGPUCount()
 			}
-			return n.Node, nil
+			return n.Node, meta, nil
 		}
 		log.DedupedWarningf(5, "No pricing data found for node %s from key %s", azKey, azKey.Features())
 	}
 	c, err := az.GetConfig()
 	if err != nil {
-		return nil, fmt.Errorf("No default pricing data available")
+		return nil, meta, fmt.Errorf("No default pricing data available")
 	}
 
 	// GPU Node
@@ -1153,7 +1155,7 @@ func (az *Azure) NodePricing(key models.Key) (*models.Node, error) {
 			UsesBaseCPUPrice: true,
 			GPUCost:          c.GPU,
 			GPU:              azKey.GetGPUCount(),
-		}, nil
+		}, meta, nil
 	}
 
 	// Serverless Node. This is an Azure Container Instance, and no pods can be
@@ -1163,7 +1165,7 @@ func (az *Azure) NodePricing(key models.Key) (*models.Node, error) {
 		return &models.Node{
 			VCPUCost: "0",
 			RAMCost:  "0",
-		}, nil
+		}, meta, nil
 	}
 
 	// Regular Node
@@ -1171,7 +1173,7 @@ func (az *Azure) NodePricing(key models.Key) (*models.Node, error) {
 		VCPUCost:         c.CPU,
 		RAMCost:          c.RAM,
 		UsesBaseCPUPrice: true,
-	}, nil
+	}, meta, nil
 }
 
 // Stubbed NetworkPricing for Azure. Pull directly from azure.json for now

+ 100 - 54
pkg/cloud/gcp/provider.go

@@ -37,6 +37,7 @@ import (
 
 const GKE_GPU_TAG = "cloud.google.com/gke-accelerator"
 const BigqueryUpdateType = "bigqueryupdate"
+const BillingAPIURLFmt = "https://cloudbilling.googleapis.com/v1/services/6F81-5844-456A/skus?key=%s&currencyCode=%s"
 
 const (
 	GCPHourlyPublicIPCost = 0.01
@@ -152,7 +153,7 @@ func (gcp *GCP) GetLocalStorageQuery(window, offset time.Duration, rate bool, us
 	}
 	fmtWindow := timeutil.DurationString(window)
 
-	return fmt.Sprintf(fmtQuery, env.GetPromClusterFilter(), baseMetric, fmtWindow, fmtOffset, env.GetPromClusterLabel(), localStorageCost)
+	return fmt.Sprintf(fmtQuery, baseMetric, env.GetPromClusterFilter(), fmtWindow, fmtOffset, env.GetPromClusterLabel(), localStorageCost)
 }
 
 func (gcp *GCP) GetConfig() (*models.CustomPricing, error) {
@@ -627,7 +628,7 @@ func (gcp *GCP) parsePage(r io.Reader, inputKeys map[string]models.Key, pvKeys m
 		if err == io.EOF {
 			break
 		} else if err != nil {
-			return nil, "", fmt.Errorf("Error parsing GCP pricing page: %s", err)
+			return nil, "", fmt.Errorf("error parsing GCP pricing page: %s", err)
 		}
 		if t == "skus" {
 			_, err := dec.Token() // consumes [
@@ -760,19 +761,19 @@ func (gcp *GCP) parsePage(r io.Reader, inputKeys map[string]models.Key, pvKeys m
 				partialCPUMap["e2micro"] = 0.25
 				partialCPUMap["e2small"] = 0.5
 				partialCPUMap["e2medium"] = 1
-				/*
-					var partialCPU float64
-					if strings.ToLower(instanceType) == "f1micro" {
-						partialCPU = 0.2
-					} else if strings.ToLower(instanceType) == "g1small" {
-						partialCPU = 0.5
-					}
-				*/
+
+				if (instanceType == "ram" || instanceType == "cpu") && strings.Contains(strings.ToUpper(product.Description), "T2D AMD") {
+					instanceType = "t2dstandard"
+				}
+				if (instanceType == "ram" || instanceType == "cpu") && strings.Contains(strings.ToUpper(product.Description), "T2A ARM") {
+					instanceType = "t2astandard"
+				}
+
 				var gpuType string
 				for matchnum, group := range nvidiaGPURegex.FindStringSubmatch(product.Description) {
 					if matchnum == 1 {
 						gpuType = strings.ToLower(strings.Join(strings.Split(group, " "), "-"))
-						log.Debug("GPU type found: " + gpuType)
+						log.Debugf("GCP Billing API: GPU type found: '%s'", gpuType)
 					}
 				}
 
@@ -826,16 +827,15 @@ func (gcp *GCP) parsePage(r io.Reader, inputKeys map[string]models.Key, pvKeys m
 						// (E.g., SKU "2013-37B4-22EA")
 						// and are excluded from cost computations
 						if hourlyPrice == 0 {
-							log.Infof("Excluding reserved GPU SKU #%s", product.SKUID)
+							log.Debugf("GCP Billing API: excluding reserved GPU SKU #%s", product.SKUID)
 							continue
 						}
 
 						for k, key := range inputKeys {
 							if key.GPUType() == gpuType+","+usageType {
 								if region == strings.Split(k, ",")[0] {
-									log.Infof("Matched GPU to node in region \"%s\"", region)
-									log.Debugf("PRODUCT DESCRIPTION: %s", product.Description)
 									matchedKey := key.Features()
+									log.Debugf("GCP Billing API: matched GPU to node: %s: %s", matchedKey, product.Description)
 									if pl, ok := gcpPricingList[matchedKey]; ok {
 										pl.Node.GPUName = gpuType
 										pl.Node.GPUCost = strconv.FormatFloat(hourlyPrice, 'f', -1, 64)
@@ -848,7 +848,6 @@ func (gcp *GCP) parsePage(r io.Reader, inputKeys map[string]models.Key, pvKeys m
 										}
 										gcpPricingList[matchedKey] = product
 									}
-									log.Infof("Added data for " + matchedKey)
 								}
 							}
 						}
@@ -856,14 +855,18 @@ func (gcp *GCP) parsePage(r io.Reader, inputKeys map[string]models.Key, pvKeys m
 						_, ok := inputKeys[candidateKey]
 						_, ok2 := inputKeys[candidateKeyGPU]
 						if ok || ok2 {
-							lastRateIndex := len(product.PricingInfo[0].PricingExpression.TieredRates) - 1
 							var nanos float64
 							var unitsBaseCurrency int
-							if lastRateIndex > -1 && len(product.PricingInfo) > 0 {
-								nanos = product.PricingInfo[0].PricingExpression.TieredRates[lastRateIndex].UnitPrice.Nanos
-								unitsBaseCurrency, err = strconv.Atoi(product.PricingInfo[0].PricingExpression.TieredRates[lastRateIndex].UnitPrice.Units)
-								if err != nil {
-									return nil, "", fmt.Errorf("error parsing base unit price for instance: %w", err)
+							if len(product.PricingInfo) > 0 {
+								lastRateIndex := len(product.PricingInfo[0].PricingExpression.TieredRates) - 1
+								if lastRateIndex >= 0 {
+									nanos = product.PricingInfo[0].PricingExpression.TieredRates[lastRateIndex].UnitPrice.Nanos
+									unitsBaseCurrency, err = strconv.Atoi(product.PricingInfo[0].PricingExpression.TieredRates[lastRateIndex].UnitPrice.Units)
+									if err != nil {
+										return nil, "", fmt.Errorf("error parsing base unit price for instance: %w", err)
+									}
+								} else {
+									continue
 								}
 							} else {
 								continue
@@ -877,69 +880,73 @@ func (gcp *GCP) parsePage(r io.Reader, inputKeys map[string]models.Key, pvKeys m
 								continue
 							} else if strings.Contains(strings.ToUpper(product.Description), "RAM") {
 								if instanceType == "custom" {
-									log.Debug("RAM custom sku is: " + product.Name)
+									log.Debugf("GCP Billing API: RAM custom sku '%s'", product.Name)
 								}
 								if _, ok := gcpPricingList[candidateKey]; ok {
+									log.Debugf("GCP Billing API: key '%s': RAM price: %f", candidateKey, hourlyPrice)
 									gcpPricingList[candidateKey].Node.RAMCost = strconv.FormatFloat(hourlyPrice, 'f', -1, 64)
 								} else {
-									product = &GCPPricing{}
-									product.Node = &models.Node{
+									log.Debugf("GCP Billing API: key '%s': RAM price: %f", candidateKey, hourlyPrice)
+									pricing := &GCPPricing{}
+									pricing.Node = &models.Node{
 										RAMCost: strconv.FormatFloat(hourlyPrice, 'f', -1, 64),
 									}
 									partialCPU, pcok := partialCPUMap[instanceType]
 									if pcok {
-										product.Node.VCPU = fmt.Sprintf("%f", partialCPU)
+										pricing.Node.VCPU = fmt.Sprintf("%f", partialCPU)
 									}
-									product.Node.UsageType = usageType
-									gcpPricingList[candidateKey] = product
+									pricing.Node.UsageType = usageType
+									gcpPricingList[candidateKey] = pricing
 								}
 								if _, ok := gcpPricingList[candidateKeyGPU]; ok {
-									log.Infof("Adding RAM %f for %s", hourlyPrice, candidateKeyGPU)
+									log.Debugf("GCP Billing API: key '%s': RAM price: %f", candidateKeyGPU, hourlyPrice)
 									gcpPricingList[candidateKeyGPU].Node.RAMCost = strconv.FormatFloat(hourlyPrice, 'f', -1, 64)
 								} else {
-									log.Infof("Adding RAM %f for %s", hourlyPrice, candidateKeyGPU)
-									product = &GCPPricing{}
-									product.Node = &models.Node{
+									log.Debugf("GCP Billing API: key '%s': RAM price: %f", candidateKeyGPU, hourlyPrice)
+									pricing := &GCPPricing{}
+									pricing.Node = &models.Node{
 										RAMCost: strconv.FormatFloat(hourlyPrice, 'f', -1, 64),
 									}
 									partialCPU, pcok := partialCPUMap[instanceType]
 									if pcok {
-										product.Node.VCPU = fmt.Sprintf("%f", partialCPU)
+										pricing.Node.VCPU = fmt.Sprintf("%f", partialCPU)
 									}
-									product.Node.UsageType = usageType
-									gcpPricingList[candidateKeyGPU] = product
+									pricing.Node.UsageType = usageType
+									gcpPricingList[candidateKeyGPU] = pricing
 								}
-								break
 							} else {
 								if _, ok := gcpPricingList[candidateKey]; ok {
+									log.Debugf("GCP Billing API: key '%s': CPU price: %f", candidateKey, hourlyPrice)
 									gcpPricingList[candidateKey].Node.VCPUCost = strconv.FormatFloat(hourlyPrice, 'f', -1, 64)
 								} else {
-									product = &GCPPricing{}
-									product.Node = &models.Node{
+									log.Debugf("GCP Billing API: key '%s': CPU price: %f", candidateKey, hourlyPrice)
+									pricing := &GCPPricing{}
+									pricing.Node = &models.Node{
 										VCPUCost: strconv.FormatFloat(hourlyPrice, 'f', -1, 64),
 									}
 									partialCPU, pcok := partialCPUMap[instanceType]
 									if pcok {
-										product.Node.VCPU = fmt.Sprintf("%f", partialCPU)
+										pricing.Node.VCPU = fmt.Sprintf("%f", partialCPU)
 									}
-									product.Node.UsageType = usageType
-									gcpPricingList[candidateKey] = product
+									pricing.Node.UsageType = usageType
+									gcpPricingList[candidateKey] = pricing
 								}
 								if _, ok := gcpPricingList[candidateKeyGPU]; ok {
+									log.Debugf("GCP Billing API: key '%s': CPU price: %f", candidateKeyGPU, hourlyPrice)
 									gcpPricingList[candidateKeyGPU].Node.VCPUCost = strconv.FormatFloat(hourlyPrice, 'f', -1, 64)
 								} else {
-									product = &GCPPricing{}
-									product.Node = &models.Node{
+									log.Debugf("GCP Billing API: key '%s': CPU price: %f", candidateKeyGPU, hourlyPrice)
+									pricing := &GCPPricing{}
+									pricing.Node = &models.Node{
 										VCPUCost: strconv.FormatFloat(hourlyPrice, 'f', -1, 64),
 									}
 									partialCPU, pcok := partialCPUMap[instanceType]
 									if pcok {
-										product.Node.VCPU = fmt.Sprintf("%f", partialCPU)
+										pricing.Node.VCPU = fmt.Sprintf("%f", partialCPU)
 									}
-									product.Node.UsageType = usageType
-									gcpPricingList[candidateKeyGPU] = product
+									pricing.Node.UsageType = usageType
+									gcpPricingList[candidateKeyGPU] = pricing
 								}
-								break
 							}
 						}
 					}
@@ -962,13 +969,19 @@ func (gcp *GCP) parsePage(r io.Reader, inputKeys map[string]models.Key, pvKeys m
 	return gcpPricingList, nextPageToken, nil
 }
 
+func (gcp *GCP) getBillingAPIURL(apiKey, currencyCode string) string {
+	return fmt.Sprintf(BillingAPIURLFmt, apiKey, currencyCode)
+}
+
 func (gcp *GCP) parsePages(inputKeys map[string]models.Key, pvKeys map[string]models.PVKey) (map[string]*GCPPricing, error) {
 	var pages []map[string]*GCPPricing
 	c, err := gcp.GetConfig()
 	if err != nil {
 		return nil, err
 	}
-	url := "https://cloudbilling.googleapis.com/v1/services/6F81-5844-456A/skus?key=" + gcp.APIKey + "&currencyCode=" + c.CurrencyCode
+
+	url := gcp.getBillingAPIURL(gcp.APIKey, c.CurrencyCode)
+
 	log.Infof("Fetch GCP Billing Data from URL: %s", url)
 	var parsePagesHelper func(string) error
 	parsePagesHelper = func(pageToken string) error {
@@ -1019,6 +1032,7 @@ func (gcp *GCP) parsePages(inputKeys map[string]models.Key, pvKeys map[string]mo
 			}
 		}
 	}
+
 	log.Debugf("ALL PAGES: %+v", returnPages)
 	for k, v := range returnPages {
 		if v.Node != nil {
@@ -1539,25 +1553,57 @@ func (gcp *GCP) isValidPricingKey(key models.Key) bool {
 }
 
 // NodePricing returns GCP pricing data for a single node
-func (gcp *GCP) NodePricing(key models.Key) (*models.Node, error) {
+func (gcp *GCP) NodePricing(key models.Key) (*models.Node, models.PricingMetadata, error) {
+	meta := models.PricingMetadata{}
+
+	c, err := gcp.Config.GetCustomPricingData()
+	if err != nil {
+		meta.Warnings = append(meta.Warnings, fmt.Sprintf("failed to detect currency: %s", err))
+	} else {
+		meta.Currency = c.CurrencyCode
+	}
+
 	if n, ok := gcp.getPricing(key); ok {
 		log.Debugf("Returning pricing for node %s: %+v from SKU %s", key, n.Node, n.Name)
+
+		// Add pricing URL, but redact the key (hence, "***"")
+		meta.Source = fmt.Sprintf("Downloaded pricing from %s", gcp.getBillingAPIURL("***", c.CurrencyCode))
+
 		n.Node.BaseCPUPrice = gcp.BaseCPUPrice
-		return n.Node, nil
+
+		return n.Node, meta, nil
 	} else if ok := gcp.isValidPricingKey(key); ok {
+		meta.Warnings = append(meta.Warnings, fmt.Sprintf("No pricing found, but key is valid: %s", key.Features()))
+
 		err := gcp.DownloadPricingData()
 		if err != nil {
-			return nil, fmt.Errorf("Download pricing data failed: %s", err.Error())
+			log.Warnf("no pricing data found for %s", key.Features())
+
+			meta.Warnings = append(meta.Warnings, "Failed to download pricing data")
+
+			return nil, meta, fmt.Errorf("failed to download pricing data: %w", err)
 		}
 		if n, ok := gcp.getPricing(key); ok {
 			log.Debugf("Returning pricing for node %s: %+v from SKU %s", key, n.Node, n.Name)
+
+			// Add pricing URL, but redact the key (hence, "***"")
+			meta.Source = fmt.Sprintf("Downloaded pricing from %s", gcp.getBillingAPIURL("***", c.CurrencyCode))
+
 			n.Node.BaseCPUPrice = gcp.BaseCPUPrice
-			return n.Node, nil
+
+			return n.Node, meta, nil
 		}
-		log.Warnf("no pricing data found for %s: %s", key.Features(), key)
-		return nil, fmt.Errorf("Warning: no pricing data found for %s", key)
+
+		log.Warnf("no pricing data found for %s", key.Features())
+
+		meta.Warnings = append(meta.Warnings, "Failed to find pricing after downloading data, but key is valid")
+
+		return nil, meta, fmt.Errorf("failed to find pricing data: %s", key.Features())
 	}
-	return nil, fmt.Errorf("Warning: no pricing data found for %s", key)
+
+	meta.Warnings = append(meta.Warnings, fmt.Sprintf("No pricing found, and key is not valid: %s", key.Features()))
+
+	return nil, meta, fmt.Errorf("no pricing data found for %s", key.Features())
 }
 
 func (gcp *GCP) ServiceAccountStatus() *models.ServiceAccountStatus {

+ 140 - 158
pkg/cloud/gcp/provider_test.go

@@ -2,7 +2,8 @@ package gcp
 
 import (
 	"bytes"
-	"io/ioutil"
+	"encoding/json"
+	"os"
 	"reflect"
 	"testing"
 
@@ -118,169 +119,80 @@ func TestGetUsageType(t *testing.T) {
 	}
 }
 
-// tests basic parsing of GCP pricing API responses
-// Load a reader object on a portion of a GCP api response
-// Confirm that the resting *GCP object contains the correctly parsed pricing info
-func TestParsePage(t *testing.T) {
+func TestKeyFeatures(t *testing.T) {
+	type testCase struct {
+		key *gcpKey
+		exp string
+	}
 
-	gcpSkuString := `
-	{
-		"skus": [
-			{
-				"name": "services/6F81-5844-456A/skus/039F-D0DA-4055",
-				"skuId": "039F-D0DA-4055",
-				"description": "Nvidia Tesla A100 GPU running in Americas",
-				"category": {
-				  "serviceDisplayName": "Compute Engine",
-				  "resourceFamily": "Compute",
-				  "resourceGroup": "GPU",
-				  "usageType": "OnDemand"
+	testCases := []testCase{
+		{
+			key: &gcpKey{
+				Labels: map[string]string{
+					"node.kubernetes.io/instance-type": "n2-standard-4",
+					"topology.kubernetes.io/region":    "us-east1",
+				},
+			},
+			exp: "us-east1,n2standard,ondemand",
+		},
+		{
+			key: &gcpKey{
+				Labels: map[string]string{
+					"node.kubernetes.io/instance-type": "e2-standard-8",
+					"topology.kubernetes.io/region":    "us-west1",
+					"cloud.google.com/gke-preemptible": "true",
 				},
-				"serviceRegions": [
-				  "us-central1",
-				  "us-east1",
-				  "us-west1"
-				],
-				"pricingInfo": [
-				  {
-					"summary": "",
-					"pricingExpression": {
-					  "usageUnit": "h",
-					  "displayQuantity": 1,
-					  "tieredRates": [
-						{
-						  "startUsageAmount": 0,
-						  "unitPrice": {
-							"currencyCode": "USD",
-							"units": "2",
-							"nanos": 933908000
-						  }
-						}
-					  ],
-					  "usageUnitDescription": "hour",
-					  "baseUnit": "s",
-					  "baseUnitDescription": "second",
-					  "baseUnitConversionFactor": 3600
-					},
-					"currencyConversionRate": 1,
-					"effectiveTime": "2023-03-24T10:52:50.681Z"
-				  }
-				],
-				"serviceProviderName": "Google",
-				"geoTaxonomy": {
-				  "type": "MULTI_REGIONAL",
-				  "regions": [
-					"us-central1",
-					"us-east1",
-					"us-west1"
-				  ]
-				}
 			},
-			{
-				"name": "services/6F81-5844-456A/skus/2390-DCAF-DA38",
-				"skuId": "2390-DCAF-DA38",
-				"description": "A2 Instance Ram running in Americas",
-				"category": {
-				  "serviceDisplayName": "Compute Engine",
-				  "resourceFamily": "Compute",
-				  "resourceGroup": "RAM",
-				  "usageType": "OnDemand"
+			exp: "us-west1,e2standard,preemptible",
+		},
+		{
+			key: &gcpKey{
+				Labels: map[string]string{
+					"node.kubernetes.io/instance-type": "a2-highgpu-1g",
+					"cloud.google.com/gke-gpu":         "true",
+					"cloud.google.com/gke-accelerator": "nvidia-tesla-a100",
+					"topology.kubernetes.io/region":    "us-central1",
 				},
-				"serviceRegions": [
-				  "us-central1",
-				  "us-east1",
-				  "us-west1"
-				],
-				"pricingInfo": [
-				  {
-					"summary": "",
-					"pricingExpression": {
-					  "usageUnit": "GiBy.h",
-					  "displayQuantity": 1,
-					  "tieredRates": [
-						{
-						  "startUsageAmount": 0,
-						  "unitPrice": {
-							"currencyCode": "USD",
-							"units": "0",
-							"nanos": 4237000
-						  }
-						}
-					  ],
-					  "usageUnitDescription": "gibibyte hour",
-					  "baseUnit": "By.s",
-					  "baseUnitDescription": "byte second",
-					  "baseUnitConversionFactor": 3865470566400
-					},
-					"currencyConversionRate": 1,
-					"effectiveTime": "2023-03-24T10:52:50.681Z"
-				  }
-				],
-				"serviceProviderName": "Google",
-				"geoTaxonomy": {
-				  "type": "MULTI_REGIONAL",
-				  "regions": [
-					"us-central1",
-					"us-east1",
-					"us-west1"
-				  ]
-				}
 			},
-			{
-				"name": "services/6F81-5844-456A/skus/2922-40C5-B19F",
-				"skuId": "2922-40C5-B19F",
-				"description": "A2 Instance Core running in Americas",
-				"category": {
-				  "serviceDisplayName": "Compute Engine",
-				  "resourceFamily": "Compute",
-				  "resourceGroup": "CPU",
-				  "usageType": "OnDemand"
+			exp: "us-central1,a2highgpu,ondemand,gpu",
+		},
+		{
+			key: &gcpKey{
+				Labels: map[string]string{
+					"node.kubernetes.io/instance-type": "t2d-standard-1",
+					"topology.kubernetes.io/region":    "asia-southeast1",
 				},
-				"serviceRegions": [
-				  "us-central1",
-				  "us-east1",
-				  "us-west1"
-				],
-				"pricingInfo": [
-				  {
-					"summary": "",
-					"pricingExpression": {
-					  "usageUnit": "h",
-					  "displayQuantity": 1,
-					  "tieredRates": [
-						{
-						  "startUsageAmount": 0,
-						  "unitPrice": {
-							"currencyCode": "USD",
-							"units": "0",
-							"nanos": 31611000
-						  }
-						}
-					  ],
-					  "usageUnitDescription": "hour",
-					  "baseUnit": "s",
-					  "baseUnitDescription": "second",
-					  "baseUnitConversionFactor": 3600
-					},
-					"currencyConversionRate": 1,
-					"effectiveTime": "2023-03-24T10:52:50.681Z"
-				  }
-				],
-				"serviceProviderName": "Google",
-				"geoTaxonomy": {
-				  "type": "MULTI_REGIONAL",
-				  "regions": [
-					"us-central1",
-					"us-east1",
-					"us-west1"
-				  ]
-				}
+			},
+			exp: "asia-southeast1,t2dstandard,ondemand",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.exp, func(t *testing.T) {
+			act := tc.key.Features()
+			if act != tc.exp {
+				t.Errorf("expected '%s'; got '%s'", tc.exp, act)
 			}
-		],
-			"nextPageToken": "APKCS1HVa0YpwgyTFbqbJ1eGwzKZmsPwLqzMZPTSNia5ck1Hc54Tx_Kz3oBxwSnRIdGVxXoSPdf-XlDpyNBf4QuxKcIEgtrQ1LDLWAgZowI0ns7HjrGta2s="
-		}
-	`
-	reader := ioutil.NopCloser(bytes.NewBufferString(gcpSkuString))
+		})
+	}
+}
+
+// tests basic parsing of GCP pricing API responses
+// Load a reader object on a portion of a GCP api response
+// Confirm that the resting *GCP object contains the correctly parsed pricing info
+func TestParsePage(t *testing.T) {
+	// NOTE: SKUs here are copied directly from GCP Billing API. Some of them
+	// are in currency IDR, which relates directly to ticket GTM-52, for which
+	// some of this work was done. So if the prices look huge... don't panic.
+	// The only thing we're testing here is that, given these instance types
+	// and regions and prices, those same prices get set appropriately into
+	// the returned pricing map.
+	skuFilePath := "./test/skus.json"
+	fileBytes, err := os.ReadFile(skuFilePath)
+	if err != nil {
+		t.Fatalf("failed to open file '%s': %s", skuFilePath, err)
+	}
+	reader := bytes.NewReader(fileBytes)
 
 	testGcp := &GCP{}
 
@@ -293,6 +205,24 @@ func TestParsePage(t *testing.T) {
 				"topology.kubernetes.io/region":    "us-central1",
 			},
 		},
+		"us-central1,e2medium,ondemand": &gcpKey{
+			Labels: map[string]string{
+				"node.kubernetes.io/instance-type": "e2-medium",
+				"topology.kubernetes.io/region":    "us-central1",
+			},
+		},
+		"us-central1,e2standard,ondemand": &gcpKey{
+			Labels: map[string]string{
+				"node.kubernetes.io/instance-type": "e2-standard",
+				"topology.kubernetes.io/region":    "us-central1",
+			},
+		},
+		"asia-southeast1,t2dstandard,ondemand": &gcpKey{
+			Labels: map[string]string{
+				"node.kubernetes.io/instance-type": "t2d-standard-1",
+				"topology.kubernetes.io/region":    "asia-southeast1",
+			},
+		},
 	}
 
 	pvKeys := map[string]models.PVKey{}
@@ -361,9 +291,61 @@ func TestParsePage(t *testing.T) {
 				UsageType:        "ondemand",
 			},
 		},
+		"us-central1,e2medium,ondemand": {
+			Node: &models.Node{
+				VCPU:             "1.000000",
+				VCPUCost:         "327.173848364",
+				RAMCost:          "43.85294978",
+				UsesBaseCPUPrice: false,
+				UsageType:        "ondemand",
+			},
+		},
+		"us-central1,e2medium,ondemand,gpu": {
+			Node: &models.Node{
+				VCPU:             "1.000000",
+				VCPUCost:         "327.173848364",
+				RAMCost:          "43.85294978",
+				UsesBaseCPUPrice: false,
+				UsageType:        "ondemand",
+			},
+		},
+		"us-central1,e2standard,ondemand": {
+			Node: &models.Node{
+				VCPUCost:         "327.173848364",
+				RAMCost:          "43.85294978",
+				UsesBaseCPUPrice: false,
+				UsageType:        "ondemand",
+			},
+		},
+		"us-central1,e2standard,ondemand,gpu": {
+			Node: &models.Node{
+				VCPUCost:         "327.173848364",
+				RAMCost:          "43.85294978",
+				UsesBaseCPUPrice: false,
+				UsageType:        "ondemand",
+			},
+		},
+		"asia-southeast1,t2dstandard,ondemand": {
+			Node: &models.Node{
+				VCPUCost:         "508.934997455",
+				RAMCost:          "68.204999658",
+				UsesBaseCPUPrice: false,
+				UsageType:        "ondemand",
+			},
+		},
+		"asia-southeast1,t2dstandard,ondemand,gpu": {
+			Node: &models.Node{
+				VCPUCost:         "508.934997455",
+				RAMCost:          "68.204999658",
+				UsesBaseCPUPrice: false,
+				UsageType:        "ondemand",
+			},
+		},
 	}
 
 	if !reflect.DeepEqual(actualPrices, expectedActualPrices) {
-		t.Fatalf("error parsing GCP prices. parsed %v but expected %v", actualPrices, expectedActualPrices)
+		act, _ := json.Marshal(actualPrices)
+		exp, _ := json.Marshal(expectedActualPrices)
+		t.Errorf("error parsing GCP prices: parsed \n%s\n expected \n%s\n", string(act), string(exp))
 	}
 }

+ 319 - 0
pkg/cloud/gcp/test/skus.json

@@ -0,0 +1,319 @@
+{
+    "skus": [
+        {
+            "name": "services/6F81-5844-456A/skus/039F-D0DA-4055",
+            "skuId": "039F-D0DA-4055",
+            "description": "Nvidia Tesla A100 GPU running in Americas",
+            "category": {
+              "serviceDisplayName": "Compute Engine",
+              "resourceFamily": "Compute",
+              "resourceGroup": "GPU",
+              "usageType": "OnDemand"
+            },
+            "serviceRegions": [
+              "us-central1",
+              "us-east1",
+              "us-west1"
+            ],
+            "pricingInfo": [
+              {
+                "summary": "",
+                "pricingExpression": {
+                  "usageUnit": "h",
+                  "displayQuantity": 1,
+                  "tieredRates": [
+                    {
+                      "startUsageAmount": 0,
+                      "unitPrice": {
+                        "currencyCode": "USD",
+                        "units": "2",
+                        "nanos": 933908000
+                      }
+                    }
+                  ],
+                  "usageUnitDescription": "hour",
+                  "baseUnit": "s",
+                  "baseUnitDescription": "second",
+                  "baseUnitConversionFactor": 3600
+                },
+                "currencyConversionRate": 1,
+                "effectiveTime": "2023-03-24T10:52:50.681Z"
+              }
+            ],
+            "serviceProviderName": "Google",
+            "geoTaxonomy": {
+              "type": "MULTI_REGIONAL",
+              "regions": [
+                "us-central1",
+                "us-east1",
+                "us-west1"
+              ]
+            }
+        },
+        {
+            "name": "services/6F81-5844-456A/skus/2390-DCAF-DA38",
+            "skuId": "2390-DCAF-DA38",
+            "description": "A2 Instance Ram running in Americas",
+            "category": {
+              "serviceDisplayName": "Compute Engine",
+              "resourceFamily": "Compute",
+              "resourceGroup": "RAM",
+              "usageType": "OnDemand"
+            },
+            "serviceRegions": [
+              "us-central1",
+              "us-east1",
+              "us-west1"
+            ],
+            "pricingInfo": [
+              {
+                "summary": "",
+                "pricingExpression": {
+                  "usageUnit": "GiBy.h",
+                  "displayQuantity": 1,
+                  "tieredRates": [
+                    {
+                      "startUsageAmount": 0,
+                      "unitPrice": {
+                        "currencyCode": "USD",
+                        "units": "0",
+                        "nanos": 4237000
+                      }
+                    }
+                  ],
+                  "usageUnitDescription": "gibibyte hour",
+                  "baseUnit": "By.s",
+                  "baseUnitDescription": "byte second",
+                  "baseUnitConversionFactor": 3865470566400
+                },
+                "currencyConversionRate": 1,
+                "effectiveTime": "2023-03-24T10:52:50.681Z"
+              }
+            ],
+            "serviceProviderName": "Google",
+            "geoTaxonomy": {
+              "type": "MULTI_REGIONAL",
+              "regions": [
+                "us-central1",
+                "us-east1",
+                "us-west1"
+              ]
+            }
+        },
+        {
+            "name": "services/6F81-5844-456A/skus/2922-40C5-B19F",
+            "skuId": "2922-40C5-B19F",
+            "description": "A2 Instance Core running in Americas",
+            "category": {
+              "serviceDisplayName": "Compute Engine",
+              "resourceFamily": "Compute",
+              "resourceGroup": "CPU",
+              "usageType": "OnDemand"
+            },
+            "serviceRegions": [
+              "us-central1",
+              "us-east1",
+              "us-west1"
+            ],
+            "pricingInfo": [
+              {
+                "summary": "",
+                "pricingExpression": {
+                  "usageUnit": "h",
+                  "displayQuantity": 1,
+                  "tieredRates": [
+                    {
+                      "startUsageAmount": 0,
+                      "unitPrice": {
+                        "currencyCode": "USD",
+                        "units": "0",
+                        "nanos": 31611000
+                      }
+                    }
+                  ],
+                  "usageUnitDescription": "hour",
+                  "baseUnit": "s",
+                  "baseUnitDescription": "second",
+                  "baseUnitConversionFactor": 3600
+                },
+                "currencyConversionRate": 1,
+                "effectiveTime": "2023-03-24T10:52:50.681Z"
+              }
+            ],
+            "serviceProviderName": "Google",
+            "geoTaxonomy": {
+              "type": "MULTI_REGIONAL",
+              "regions": [
+                "us-central1",
+                "us-east1",
+                "us-west1"
+              ]
+            }
+        },
+        {
+            "name": "services/6F81-5844-456A/skus/4756-01E4-0F32",
+            "skuId": "4756-01E4-0F32",
+            "description": "T2D AMD Instance Ram running in Singapore",
+            "category": {
+                "serviceDisplayName": "Compute Engine",
+                "resourceFamily": "Compute",
+                "resourceGroup": "RAM",
+                "usageType": "OnDemand"
+            },
+            "serviceRegions": [
+                "asia-southeast1"
+            ],
+            "pricingInfo": [
+                {
+                    "summary": "",
+                    "pricingExpression": {
+                        "usageUnit": "GiBy.h",
+                        "usageUnitDescription": "gibibyte hour",
+                        "baseUnit": "By.s",
+                        "displayQuantity": 1,
+                        "tieredRates": [
+                            {
+                                "startUsageAmount": 0,
+                                "unitPrice": {
+                                    "currencyCode": "IDR",
+                                    "units": "68",
+                                    "nanos": 204999658
+                                }
+                            }
+                        ]
+                    },
+                    "currencyConversionRate": 14999.999925,
+                    "EffectiveTime": "2023-08-10T22:49:22.905126Z"
+                }
+            ],
+            "serviceProviderName": "Google",
+            "node": null,
+            "pv": null
+        },
+        {
+            "name": "services/6F81-5844-456A/skus/9E37-EAF4-1576",
+            "skuId": "9E37-EAF4-1576",
+            "description": "T2D AMD Instance Core running in Singapore",
+            "category": {
+                "serviceDisplayName": "Compute Engine",
+                "resourceFamily": "Compute",
+                "resourceGroup": "CPU",
+                "usageType": "OnDemand"
+            },
+            "serviceRegions": [
+                "asia-southeast1"
+            ],
+            "pricingInfo": [
+                {
+                    "summary": "",
+                    "pricingExpression": {
+                        "usageUnit": "h",
+                        "usageUnitDescription": "hour",
+                        "baseUnit": "s",
+                        "displayQuantity": 1,
+                        "tieredRates": [
+                            {
+                                "startUsageAmount": 0,
+                                "unitPrice": {
+                                    "currencyCode": "IDR",
+                                    "units": "508",
+                                    "nanos": 934997455
+                                }
+                            }
+                        ]
+                    },
+                    "currencyConversionRate": 14999.999925,
+                    "EffectiveTime": "2023-08-10T22:49:22.905126Z"
+                }
+            ],
+            "serviceProviderName": "Google",
+            "node": null,
+            "pv": null
+        },
+        {
+            "name": "services/6F81-5844-456A/skus/CF4E-A0C7-E3BF",
+            "skuId": "CF4E-A0C7-E3BF",
+            "description": "E2 Instance Core running in Americas",
+            "category": {
+                "serviceDisplayName": "Compute Engine",
+                "resourceFamily": "Compute",
+                "resourceGroup": "CPU",
+                "usageType": "OnDemand"
+            },
+            "serviceRegions": [
+                "us-central1",
+                "us-east1",
+                "us-west1"
+            ],
+            "pricingInfo": [
+                {
+                    "summary": "",
+                    "pricingExpression": {
+                        "usageUnit": "h",
+                        "usageUnitDescription": "hour",
+                        "baseUnit": "s",
+                        "displayQuantity": 1,
+                        "tieredRates": [
+                            {
+                                "startUsageAmount": 0,
+                                "unitPrice": {
+                                    "currencyCode": "IDR",
+                                    "units": "327",
+                                    "nanos": 173848364
+                                }
+                            }
+                        ]
+                    },
+                    "currencyConversionRate": 14999.999925,
+                    "EffectiveTime": "2023-08-09T07:28:37.555408Z"
+                }
+            ],
+            "serviceProviderName": "Google",
+            "node": null,
+            "pv": null
+        },
+        {
+            "name": "services/6F81-5844-456A/skus/F449-33EC-A5EF",
+            "skuId": "F449-33EC-A5EF",
+            "description": "E2 Instance Ram running in Americas",
+            "category": {
+                "serviceDisplayName": "Compute Engine",
+                "resourceFamily": "Compute",
+                "resourceGroup": "RAM",
+                "usageType": "OnDemand"
+            },
+            "serviceRegions": [
+                "us-central1",
+                "us-east1",
+                "us-west1"
+            ],
+            "pricingInfo": [
+                {
+                    "summary": "",
+                    "pricingExpression": {
+                        "usageUnit": "GiBy.h",
+                        "usageUnitDescription": "gibibyte hour",
+                        "baseUnit": "By.s",
+                        "displayQuantity": 1,
+                        "tieredRates": [
+                            {
+                                "startUsageAmount": 0,
+                                "unitPrice": {
+                                    "currencyCode": "IDR",
+                                    "units": "43",
+                                    "nanos": 852949780
+                                }
+                            }
+                        ]
+                    },
+                    "currencyConversionRate": 14999.999925,
+                    "EffectiveTime": "2023-08-09T07:28:37.555408Z"
+                }
+            ],
+            "serviceProviderName": "Google",
+            "node": null,
+            "pv": null
+        }
+    ],
+    "nextPageToken": "APKCS1HVa0YpwgyTFbqbJ1eGwzKZmsPwLqzMZPTSNia5ck1Hc54Tx_Kz3oBxwSnRIdGVxXoSPdf-XlDpyNBf4QuxKcIEgtrQ1LDLWAgZowI0ns7HjrGta2s="
+}

+ 1 - 1
pkg/cloud/models/models.go

@@ -273,7 +273,7 @@ type Provider interface {
 	GetAddresses() ([]byte, error)
 	GetDisks() ([]byte, error)
 	GetOrphanedResources() ([]OrphanedResource, error)
-	NodePricing(Key) (*Node, error)
+	NodePricing(Key) (*Node, PricingMetadata, error)
 	PVPricing(PVKey) (*PV, error)
 	NetworkPricing() (*Network, error)           // TODO: add key interface arg for dynamic price fetching
 	LoadBalancerPricing() (*LoadBalancer, error) // TODO: add key interface arg for dynamic price fetching

+ 7 - 0
pkg/cloud/models/pricing.go

@@ -0,0 +1,7 @@
+package models
+
+type PricingMetadata struct {
+	Currency string   `json:"currency"`
+	Source   string   `json:"source"`
+	Warnings []string `json:"warnings,omitempty"`
+}

+ 4 - 3
pkg/cloud/provider/csvprovider.go

@@ -228,9 +228,10 @@ func (k *csvKey) ID() string {
 	return k.ProviderID
 }
 
-func (c *CSVProvider) NodePricing(key models.Key) (*models.Node, error) {
+func (c *CSVProvider) NodePricing(key models.Key) (*models.Node, models.PricingMetadata, error) {
 	c.DownloadPricingDataLock.RLock()
 	defer c.DownloadPricingDataLock.RUnlock()
+	meta := models.PricingMetadata{}
 	var node *models.Node
 	if p, ok := c.Pricing[key.ID()]; ok {
 		node = &models.Node{
@@ -277,9 +278,9 @@ func (c *CSVProvider) NodePricing(key models.Key) (*models.Node, error) {
 			}
 			node.Cost = fmt.Sprintf("%f", nc+totalCost)
 		}
-		return node, nil
+		return node, meta, nil
 	} else {
-		return nil, fmt.Errorf("Unable to find Node matching `%s`:`%s`", key.ID(), key.Features())
+		return nil, meta, fmt.Errorf("Unable to find Node matching `%s`:`%s`", key.ID(), key.Features())
 	}
 }
 

+ 4 - 2
pkg/cloud/provider/customprovider.go

@@ -172,10 +172,12 @@ func (cp *CustomProvider) AllNodePricing() (interface{}, error) {
 	return cp.Pricing, nil
 }
 
-func (cp *CustomProvider) NodePricing(key models.Key) (*models.Node, error) {
+func (cp *CustomProvider) NodePricing(key models.Key) (*models.Node, models.PricingMetadata, error) {
 	cp.DownloadPricingDataLock.RLock()
 	defer cp.DownloadPricingDataLock.RUnlock()
 
+	meta := models.PricingMetadata{}
+
 	k := key.Features()
 	var gpuCount string
 	if _, ok := cp.Pricing[k]; !ok {
@@ -205,7 +207,7 @@ func (cp *CustomProvider) NodePricing(key models.Key) (*models.Node, error) {
 		RAMCost:  ramCost,
 		GPUCost:  gpuCost,
 		GPU:      gpuCount,
-	}, nil
+	}, meta, nil
 }
 
 func (cp *CustomProvider) DownloadPricingData() error {

+ 5 - 3
pkg/cloud/scaleway/provider.go

@@ -132,10 +132,12 @@ func (k *scalewayKey) ID() string {
 	return ""
 }
 
-func (c *Scaleway) NodePricing(key models.Key) (*models.Node, error) {
+func (c *Scaleway) NodePricing(key models.Key) (*models.Node, models.PricingMetadata, error) {
 	c.DownloadPricingDataLock.RLock()
 	defer c.DownloadPricingDataLock.RUnlock()
 
+	meta := models.PricingMetadata{}
+
 	// There is only the zone and the instance ID in the providerID, hence we must use the features
 	split := strings.Split(key.Features(), ",")
 	if pricing, ok := c.Pricing[split[0]]; ok {
@@ -151,12 +153,12 @@ func (c *Scaleway) NodePricing(key models.Key) (*models.Node, error) {
 				InstanceType: split[1],
 				Region:       split[0],
 				GPUName:      key.GPUType(),
-			}, nil
+			}, meta, nil
 
 		}
 
 	}
-	return nil, fmt.Errorf("Unable to find node pricing matching thes features `%s`", key.Features())
+	return nil, meta, fmt.Errorf("Unable to find node pricing matching thes features `%s`", key.Features())
 }
 
 func (c *Scaleway) LoadBalancerPricing() (*models.LoadBalancer, error) {

+ 1 - 1
pkg/costmodel/allocation.go

@@ -665,7 +665,7 @@ func (cm *CostModel) computeAllocation(start, end time.Time, resolution time.Dur
 	// split appropriately among each pod's container allocation.
 	podPVCMap := map[podKey][]*pvc{}
 	buildPodPVCMap(podPVCMap, pvMap, pvcMap, podMap, resPodPVCAllocation, podUIDKeyMap, ingestPodUID)
-	applyPVCsToPods(window, podMap, podPVCMap, pvcMap, resolution)
+	applyPVCsToPods(window, podMap, podPVCMap, pvcMap)
 
 	// Identify PVCs without pods and add pv costs to the unmounted Allocation for the pvc's cluster
 	applyUnmountedPVCs(window, podMap, pvcMap)

+ 15 - 4
pkg/costmodel/allocation_helpers.go

@@ -1392,6 +1392,7 @@ func getLoadBalancerCosts(lbMap map[serviceKey]*lbCost, resLBCost, resLBActiveMi
 			lb.End = lb.End.Add(resolution)
 
 			lb.TotalCost += lbPricePerHr * resultHours * scaleFactor
+			lb.Ip = ip
 			lb.Private = privateIPCheck(ip)
 		} else {
 			log.DedupedWarningf(20, "CostModel: found minutes for key that does not exist: %s", serviceKey)
@@ -1449,6 +1450,7 @@ func applyLoadBalancersToPods(window kubecost.Window, podMap map[podKey]*pod, lb
 					Service: sKey.Namespace + "/" + sKey.Service,
 					Cost:    alloc.LoadBalancerCost,
 					Private: lb.Private,
+					Ip:      lb.Ip,
 				}
 			}
 		}
@@ -1959,7 +1961,7 @@ func buildPodPVCMap(podPVCMap map[podKey][]*pvc, pvMap map[pvKey]*pv, pvcMap map
 	}
 }
 
-func applyPVCsToPods(window kubecost.Window, podMap map[podKey]*pod, podPVCMap map[podKey][]*pvc, pvcMap map[pvcKey]*pvc, resolution time.Duration) {
+func applyPVCsToPods(window kubecost.Window, podMap map[podKey]*pod, podPVCMap map[podKey][]*pvc, pvcMap map[pvcKey]*pvc) {
 	// Because PVCs can be shared among pods, the respective pv cost
 	// needs to be evenly distributed to those pods based on time
 	// running, as well as the amount of time the pvc was shared.
@@ -2004,12 +2006,20 @@ func applyPVCsToPods(window kubecost.Window, podMap map[podKey]*pod, podPVCMap m
 
 		pvc, ok := pvcMap[thisPVCKey]
 		if !ok {
-			log.DedupedWarningf(5, "Missing pvc with key %s", thisPVCKey)
+			log.Warnf("Allocation: Compute: applyPVCsToPods: missing pvc with key %s", thisPVCKey)
+			continue
+		}
+		if pvc == nil {
+			log.Warnf("Allocation: Compute: applyPVCsToPods: nil pvc with key %s", thisPVCKey)
 			continue
 		}
 
 		// Determine coefficients for each pvc-pod relation.
-		sharedPVCCostCoefficients := getPVCCostCoefficients(intervals, pvc, resolution)
+		sharedPVCCostCoefficients, err := getPVCCostCoefficients(intervals, pvc)
+		if err != nil {
+			log.Warnf("Allocation: Compute: applyPVCsToPods: getPVCCostCoefficients: %s", err)
+			continue
+		}
 
 		// Distribute pvc costs to Allocations
 		for thisPodKey, coeffComponents := range sharedPVCCostCoefficients {
@@ -2041,8 +2051,9 @@ func applyPVCsToPods(window kubecost.Window, podMap map[podKey]*pod, podPVCMap m
 					Cluster: pvc.Volume.Cluster,
 					Name:    pvc.Volume.Name,
 				}
+
 				// Both Cost and byteHours should be multiplied by the coef and divided by count
-				// so that you if all allocations with a given pv key are summed the result of those
+				// so that if all allocations with a given pv key are summed the result of those
 				// would be equal to the values of the original pv
 				count := float64(len(pod.Allocations))
 				alloc.PVs[pvKey] = &kubecost.PVAllocation{

+ 1 - 0
pkg/costmodel/allocation_types.go

@@ -212,4 +212,5 @@ type lbCost struct {
 	Start     time.Time
 	End       time.Time
 	Private   bool
+	Ip        string
 }

+ 1 - 1
pkg/costmodel/assets.go

@@ -84,7 +84,7 @@ func (cm *CostModel) ComputeAssets(start, end time.Time) (*kubecost.AssetSet, er
 			e = end
 		}
 
-		loadBalancer := kubecost.NewLoadBalancer(lb.Name, lb.Cluster, lb.ProviderID, s, e, kubecost.NewWindow(&start, &end), lb.Private)
+		loadBalancer := kubecost.NewLoadBalancer(lb.Name, lb.Cluster, lb.ProviderID, s, e, kubecost.NewWindow(&start, &end), lb.Private, lb.Ip)
 		cm.PropertiesFromCluster(loadBalancer.Properties)
 		loadBalancer.Cost = lb.Cost
 		assetSet.Insert(loadBalancer, nil)

+ 7 - 0
pkg/costmodel/cluster.go

@@ -716,6 +716,7 @@ type LoadBalancer struct {
 	End        time.Time
 	Minutes    float64
 	Private    bool
+	Ip         string
 }
 
 func ClusterLoadBalancers(client prometheus.Client, start, end time.Time) (map[LoadBalancerIdentifier]*LoadBalancer, error) {
@@ -851,6 +852,12 @@ func ClusterLoadBalancers(client prometheus.Client, start, end time.Time) (map[L
 
 			hrs := (lb.Minutes * scaleFactor) / 60.0
 			lb.Cost += lbPricePerHr * hrs
+
+			if lb.Ip != "" && lb.Ip != providerID {
+				log.DedupedWarningf(5, "ClusterLoadBalancers: multiple IPs per load balancer not supported, using most recent IP")
+			}
+			lb.Ip = providerID
+
 			lb.Private = privateIPCheck(providerID)
 		} else {
 			log.DedupedWarningf(20, "ClusterLoadBalancers: found minutes for key that does not exist: %v", key)

+ 1 - 1
pkg/costmodel/costmodel.go

@@ -1035,7 +1035,7 @@ func (cm *CostModel) GetNodeCost(cp costAnalyzerCloud.Provider) (map[string]*cos
 
 		pmd.TotalNodes++
 
-		cnode, err := cp.NodePricing(cp.GetKey(nodeLabels, n))
+		cnode, _, err := cp.NodePricing(cp.GetKey(nodeLabels, n))
 		if err != nil {
 			log.Infof("Error getting node pricing. Error: %s", err.Error())
 			if cnode != nil {

+ 18 - 8
pkg/costmodel/intervals.go

@@ -1,6 +1,7 @@
 package costmodel
 
 import (
+	"fmt"
 	"sort"
 	"time"
 
@@ -79,40 +80,49 @@ func getIntervalPointsFromWindows(windows map[podKey]kubecost.Window) IntervalPo
 // getPVCCostCoefficients gets a coefficient which represents the scale
 // factor that each PVC in a pvcIntervalMap and corresponding slice of
 // IntervalPoints intervals uses to calculate a cost for that PVC's PV.
-func getPVCCostCoefficients(intervals IntervalPoints, thisPVC *pvc, resolution time.Duration) map[podKey][]CoefficientComponent {
+func getPVCCostCoefficients(intervals IntervalPoints, thisPVC *pvc) (map[podKey][]CoefficientComponent, error) {
 	// pvcCostCoefficientMap has a format such that the individual coefficient
 	// components are preserved for testing purposes.
 	pvcCostCoefficientMap := make(map[podKey][]CoefficientComponent)
 
-	// Reset the start time by the offset as well. so that offset is not used in coefficient calculation!
-	startTime := thisPVC.Start.Add(resolution)
-	pvcWindow := kubecost.NewWindow(&startTime, &thisPVC.End)
+	pvcWindow := kubecost.NewWindow(&thisPVC.Start, &thisPVC.End)
+	pvcWindowDurationMinutes := pvcWindow.Duration().Minutes()
+	if pvcWindowDurationMinutes <= 0.0 {
+		// Protect against Inf and NaN issues that would be caused by dividing
+		// by zero later on.
+		return nil, fmt.Errorf("detected PVC with window of zero duration: %s/%s/%s", thisPVC.Cluster, thisPVC.Namespace, thisPVC.Name)
+	}
+
 	unmountedKey := getUnmountedPodKey(thisPVC.Cluster)
 
 	var void struct{}
 	activeKeys := map[podKey]struct{}{}
 
-	currentTime := startTime
+	currentTime := thisPVC.Start
+
 	// For each interval i.e. for any time a pod-PVC relation ends or starts...
 	for _, point := range intervals {
 		// If the current point happens at a later time than the previous point
 		if !point.Time.Equal(currentTime) {
+			// If there are active keys, attribute one unit of proportion to
+			// each active key.
 			for key := range activeKeys {
 				pvcCostCoefficientMap[key] = append(
 					pvcCostCoefficientMap[key],
 					CoefficientComponent{
-						Time:       point.Time.Sub(currentTime).Minutes() / pvcWindow.Duration().Minutes(),
+						Time:       point.Time.Sub(currentTime).Minutes() / pvcWindowDurationMinutes,
 						Proportion: 1.0 / float64(len(activeKeys)),
 					},
 				)
 
 			}
+
 			// If there are no active keys attribute all cost to the unmounted pv
 			if len(activeKeys) == 0 {
 				pvcCostCoefficientMap[unmountedKey] = append(
 					pvcCostCoefficientMap[unmountedKey],
 					CoefficientComponent{
-						Time:       point.Time.Sub(currentTime).Minutes() / pvcWindow.Duration().Minutes(),
+						Time:       point.Time.Sub(currentTime).Minutes() / pvcWindowDurationMinutes,
 						Proportion: 1.0,
 					},
 				)
@@ -142,7 +152,7 @@ func getPVCCostCoefficients(intervals IntervalPoints, thisPVC *pvc, resolution t
 			},
 		)
 	}
-	return pvcCostCoefficientMap
+	return pvcCostCoefficientMap, nil
 }
 
 // getCoefficientFromComponents takes the components of a PVC-pod PV cost coefficient

+ 53 - 25
pkg/costmodel/intervals_test.go

@@ -1,6 +1,7 @@
 package costmodel
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 	"time"
@@ -150,32 +151,39 @@ func TestGetIntervalPointsFromWindows(t *testing.T) {
 }
 
 func TestGetPVCCostCoefficients(t *testing.T) {
+	pod1Key := newPodKey("cluster1", "namespace1", "pod1")
+	pod2Key := newPodKey("cluster1", "namespace1", "pod2")
+	pod3Key := newPodKey("cluster1", "namespace1", "pod3")
+	pod4Key := newPodKey("cluster1", "namespace1", "pod4")
+	ummountedPodKey := newPodKey("cluster1", kubecost.UnmountedSuffix, kubecost.UnmountedSuffix)
+
 	pvc1 := &pvc{
-		Bytes:     0,
+		Bytes:     100 * 1024 * 1024 * 1024,
 		Name:      "pvc1",
 		Cluster:   "cluster1",
 		Namespace: "namespace1",
 		Start:     time.Date(2021, 2, 19, 8, 0, 0, 0, time.UTC),
 		End:       time.Date(2021, 2, 19, 9, 0, 0, 0, time.UTC),
 	}
-	pod1Key := newPodKey("cluster1", "namespace1", "pod1")
-	pod2Key := newPodKey("cluster1", "namespace1", "pod2")
-	pod3Key := newPodKey("cluster1", "namespace1", "pod3")
-	pod4Key := newPodKey("cluster1", "namespace1", "pod4")
-	ummountedPodKey := newPodKey("cluster1", kubecost.UnmountedSuffix, kubecost.UnmountedSuffix)
 
-	zeroDuration, _ := time.ParseDuration("0m0s")
-	fiveMinOffset, _ := time.ParseDuration("5m")
-	// Reflects the pvc that is offset by duration, the actual case that happens in allocation workflow.
-	// before core-370, the offset was causing a unmounted shared pvc coefficient map for the duration of the offset.
-	pvcWithDurationOffset := &pvc{
-		Bytes:     0,
-		Name:      "pvc1",
+	pvc2 := &pvc{
+		Bytes:     100 * 1024 * 1024 * 1024,
+		Name:      "pvc2",
 		Cluster:   "cluster1",
 		Namespace: "namespace1",
-		Start:     time.Date(2021, 2, 19, 8, 0, 0, 0, time.UTC).Add(-fiveMinOffset),
+		Start:     time.Date(2021, 2, 19, 8, 0, 0, 0, time.UTC),
 		End:       time.Date(2021, 2, 19, 9, 0, 0, 0, time.UTC),
 	}
+
+	pvc3 := &pvc{
+		Bytes:     100 * 1024 * 1024 * 1024,
+		Name:      "pvc3",
+		Cluster:   "cluster1",
+		Namespace: "namespace1",
+		Start:     time.Date(2021, 2, 19, 8, 0, 0, 0, time.UTC),
+		End:       time.Date(2021, 2, 19, 8, 0, 0, 0, time.UTC),
+	}
+
 	cases := []struct {
 		name           string
 		pvc            *pvc
@@ -183,6 +191,7 @@ func TestGetPVCCostCoefficients(t *testing.T) {
 		intervals      []IntervalPoint
 		resolution     time.Duration
 		expected       map[podKey][]CoefficientComponent
+		expError       error
 	}{
 		{
 			name: "four pods w/ various overlaps",
@@ -197,7 +206,7 @@ func TestGetPVCCostCoefficients(t *testing.T) {
 				NewIntervalPoint(time.Date(2021, 2, 19, 9, 0, 0, 0, time.UTC), "end", pod3Key),
 				NewIntervalPoint(time.Date(2021, 2, 19, 9, 0, 0, 0, time.UTC), "end", pod1Key),
 			},
-			resolution: zeroDuration,
+			expError: nil,
 			expected: map[podKey][]CoefficientComponent{
 				pod1Key: {
 					{0.5, 0.25},
@@ -226,7 +235,7 @@ func TestGetPVCCostCoefficients(t *testing.T) {
 				NewIntervalPoint(time.Date(2021, 2, 19, 8, 30, 0, 0, time.UTC), "end", pod1Key),
 				NewIntervalPoint(time.Date(2021, 2, 19, 9, 0, 0, 0, time.UTC), "end", pod2Key),
 			},
-			resolution: zeroDuration,
+			expError: nil,
 			expected: map[podKey][]CoefficientComponent{
 				pod1Key: {
 					{1.0, 0.5},
@@ -245,7 +254,7 @@ func TestGetPVCCostCoefficients(t *testing.T) {
 				NewIntervalPoint(time.Date(2021, 2, 19, 9, 0, 0, 0, time.UTC), "end", pod1Key),
 				NewIntervalPoint(time.Date(2021, 2, 19, 9, 0, 0, 0, time.UTC), "end", pod2Key),
 			},
-			resolution: zeroDuration,
+			expError: nil,
 			expected: map[podKey][]CoefficientComponent{
 				pod1Key: {
 					{0.5, 0.5},
@@ -265,7 +274,7 @@ func TestGetPVCCostCoefficients(t *testing.T) {
 				NewIntervalPoint(time.Date(2021, 2, 19, 8, 0, 0, 0, time.UTC), "start", pod1Key),
 				NewIntervalPoint(time.Date(2021, 2, 19, 9, 0, 0, 0, time.UTC), "end", pod1Key),
 			},
-			resolution: zeroDuration,
+			expError: nil,
 			expected: map[podKey][]CoefficientComponent{
 				pod1Key: {
 					{1.0, 1.0},
@@ -281,7 +290,7 @@ func TestGetPVCCostCoefficients(t *testing.T) {
 				NewIntervalPoint(time.Date(2021, 2, 19, 8, 45, 0, 0, time.UTC), "start", pod2Key),
 				NewIntervalPoint(time.Date(2021, 2, 19, 9, 0, 0, 0, time.UTC), "end", pod2Key),
 			},
-			resolution: zeroDuration,
+			expError: nil,
 			expected: map[podKey][]CoefficientComponent{
 				pod1Key: {
 					{1.0, 0.25},
@@ -301,7 +310,7 @@ func TestGetPVCCostCoefficients(t *testing.T) {
 				NewIntervalPoint(time.Date(2021, 2, 19, 8, 15, 0, 0, time.UTC), "start", pod1Key),
 				NewIntervalPoint(time.Date(2021, 2, 19, 8, 45, 0, 0, time.UTC), "end", pod1Key),
 			},
-			resolution: zeroDuration,
+			expError: nil,
 			expected: map[podKey][]CoefficientComponent{
 				pod1Key: {
 					{1.0, 0.5},
@@ -312,17 +321,16 @@ func TestGetPVCCostCoefficients(t *testing.T) {
 				},
 			},
 		},
-		// Test case to ensure the offset doesnt cause any unmounted
 		{
-			name: "pvcMap with duration offset not causing any unmounted entry in sharedPV Coefficient",
-			pvc:  pvcWithDurationOffset,
+			name: "back to back pods, full coverage",
+			pvc:  pvc2,
 			intervals: []IntervalPoint{
 				NewIntervalPoint(time.Date(2021, 2, 19, 8, 0, 0, 0, time.UTC), "start", pod1Key),
 				NewIntervalPoint(time.Date(2021, 2, 19, 8, 30, 0, 0, time.UTC), "start", pod2Key),
 				NewIntervalPoint(time.Date(2021, 2, 19, 8, 30, 0, 0, time.UTC), "end", pod1Key),
 				NewIntervalPoint(time.Date(2021, 2, 19, 9, 0, 0, 0, time.UTC), "end", pod2Key),
 			},
-			resolution: fiveMinOffset,
+			expError: nil,
 			expected: map[podKey][]CoefficientComponent{
 				pod1Key: {
 					{1.0, 0.5},
@@ -332,11 +340,31 @@ func TestGetPVCCostCoefficients(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "zero duration",
+			pvc:  pvc3,
+			intervals: []IntervalPoint{
+				NewIntervalPoint(time.Date(2021, 2, 19, 8, 0, 0, 0, time.UTC), "start", pod1Key),
+				NewIntervalPoint(time.Date(2021, 2, 19, 8, 0, 0, 0, time.UTC), "end", pod1Key),
+			},
+			expError: fmt.Errorf("detected PVC with window of zero duration: %s/%s/%s", "cluster1", "namespace1", "pvc3"),
+			expected: nil,
+		},
 	}
 
 	for _, testCase := range cases {
 		t.Run(testCase.name, func(t *testing.T) {
-			result := getPVCCostCoefficients(testCase.intervals, testCase.pvc, testCase.resolution)
+			result, err := getPVCCostCoefficients(testCase.intervals, testCase.pvc)
+			if err != nil {
+				if testCase.expError == nil {
+					t.Errorf("getPVCCostCoefficients failed: got unexpected error: %v", err)
+				}
+				return
+			}
+
+			if err == nil && testCase.expError != nil {
+				t.Errorf("getPVCCostCoefficients failed: did not get expected error: %v", testCase.expError)
+			}
 
 			if !reflect.DeepEqual(result, testCase.expected) {
 				t.Errorf("getPVCCostCoefficients test failed: %s: Got %+v but expected %+v", testCase.name, result, testCase.expected)

+ 34 - 16
pkg/costmodel/metrics.go

@@ -187,7 +187,7 @@ func initCostModelMetrics(clusterCache clustercache.ClusterCache, provider model
 		spotGv = prometheus.NewGaugeVec(prometheus.GaugeOpts{
 			Name: "kubecost_node_is_spot",
 			Help: "kubecost_node_is_spot Cloud provider info about node preemptibility",
-		}, []string{"instance", "node", "instance_type", "region", "provider_id"})
+		}, []string{"instance", "node", "instance_type", "region", "provider_id", "arch"})
 		if _, disabled := disabledMetrics["kubecost_node_is_spot"]; !disabled {
 			toRegisterGV = append(toRegisterGV, spotGv)
 		}
@@ -513,7 +513,7 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 
 				totalCost := cpu*cpuCost + ramCost*(ram/1024/1024/1024) + gpu*gpuCost
 
-				labelKey := getKeyFromLabelStrings(nodeName, nodeName, nodeType, nodeRegion, node.ProviderID)
+				labelKey := getKeyFromLabelStrings(nodeName, nodeName, nodeType, nodeRegion, node.ProviderID, node.ArchType)
 
 				avgCosts, ok := nodeCostAverages[labelKey]
 
@@ -558,9 +558,9 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 				nodeCostAverages[labelKey] = avgCosts
 
 				if node.IsSpot() {
-					cmme.NodeSpotRecorder.WithLabelValues(nodeName, nodeName, nodeType, nodeRegion, node.ProviderID).Set(1.0)
+					cmme.NodeSpotRecorder.WithLabelValues(nodeName, nodeName, nodeType, nodeRegion, node.ProviderID, node.ArchType).Set(1.0)
 				} else {
-					cmme.NodeSpotRecorder.WithLabelValues(nodeName, nodeName, nodeType, nodeRegion, node.ProviderID).Set(0.0)
+					cmme.NodeSpotRecorder.WithLabelValues(nodeName, nodeName, nodeType, nodeRegion, node.ProviderID, node.ArchType).Set(0.0)
 				}
 				nodeSeen[labelKey] = true
 			}
@@ -666,45 +666,48 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 				pvSeen[labelKey] = true
 			}
 
+			// Remove metrics on Nodes/LoadBalancers/Containers/PVs that no
+			// longer exist
 			for labelString, seen := range nodeSeen {
 				if !seen {
 					log.Debugf("Removing %s from nodes", labelString)
 					labels := getLabelStringsFromKey(labelString)
+
 					ok := cmme.NodeTotalPriceRecorder.DeleteLabelValues(labels...)
 					if ok {
 						log.Debugf("removed %s from totalprice", labelString)
 					} else {
-						log.Infof("FAILURE TO REMOVE %s from totalprice", labelString)
+						log.Errorf("FAILURE TO REMOVE %s from totalprice", labelString)
 					}
 					ok = cmme.NodeSpotRecorder.DeleteLabelValues(labels...)
 					if ok {
 						log.Debugf("removed %s from spot records", labelString)
 					} else {
-						log.Infof("FAILURE TO REMOVE %s from spot records", labelString)
+						log.Errorf("FAILURE TO REMOVE %s from spot records", labelString)
 					}
 					ok = cmme.CPUPriceRecorder.DeleteLabelValues(labels...)
 					if ok {
 						log.Debugf("removed %s from cpuprice", labelString)
 					} else {
-						log.Infof("FAILURE TO REMOVE %s from cpuprice", labelString)
+						log.Errorf("FAILURE TO REMOVE %s from cpuprice", labelString)
 					}
 					ok = cmme.GPUPriceRecorder.DeleteLabelValues(labels...)
 					if ok {
 						log.Debugf("removed %s from gpuprice", labelString)
 					} else {
-						log.Infof("FAILURE TO REMOVE %s from gpuprice", labelString)
+						log.Errorf("FAILURE TO REMOVE %s from gpuprice", labelString)
 					}
 					ok = cmme.GPUCountRecorder.DeleteLabelValues(labels...)
 					if ok {
 						log.Debugf("removed %s from gpucount", labelString)
 					} else {
-						log.Infof("FAILURE TO REMOVE %s from gpucount", labelString)
+						log.Errorf("FAILURE TO REMOVE %s from gpucount", labelString)
 					}
 					ok = cmme.RAMPriceRecorder.DeleteLabelValues(labels...)
 					if ok {
 						log.Debugf("removed %s from ramprice", labelString)
 					} else {
-						log.Infof("FAILURE TO REMOVE %s from ramprice", labelString)
+						log.Errorf("FAILURE TO REMOVE %s from ramprice", labelString)
 					}
 					delete(nodeSeen, labelString)
 					delete(nodeCostAverages, labelString)
@@ -717,7 +720,7 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 					labels := getLabelStringsFromKey(labelString)
 					ok := cmme.LBCostRecorder.DeleteLabelValues(labels...)
 					if !ok {
-						log.Warnf("Metric emission: failed to delete LoadBalancer with labels: %v", labels)
+						log.Errorf("Metric emission: failed to delete LoadBalancer with labels: %v", labels)
 					}
 					delete(loadBalancerSeen, labelString)
 				} else {
@@ -727,9 +730,18 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 			for labelString, seen := range containerSeen {
 				if !seen {
 					labels := getLabelStringsFromKey(labelString)
-					cmme.RAMAllocationRecorder.DeleteLabelValues(labels...)
-					cmme.CPUAllocationRecorder.DeleteLabelValues(labels...)
-					cmme.GPUAllocationRecorder.DeleteLabelValues(labels...)
+					ok := cmme.RAMAllocationRecorder.DeleteLabelValues(labels...)
+					if !ok {
+						log.Errorf("Metric emission: failed to delete RAMAllocation with labels: %v", labels)
+					}
+					ok = cmme.CPUAllocationRecorder.DeleteLabelValues(labels...)
+					if !ok {
+						log.Errorf("Metric emission: failed to delete CPUAllocation with labels: %v", labels)
+					}
+					ok = cmme.GPUAllocationRecorder.DeleteLabelValues(labels...)
+					if !ok {
+						log.Errorf("Metric emission: failed to delete GPUAllocation with labels: %v", labels)
+					}
 					delete(containerSeen, labelString)
 				} else {
 					containerSeen[labelString] = false
@@ -738,7 +750,10 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 			for labelString, seen := range pvSeen {
 				if !seen {
 					labels := getLabelStringsFromKey(labelString)
-					cmme.PersistentVolumePriceRecorder.DeleteLabelValues(labels...)
+					ok := cmme.PersistentVolumePriceRecorder.DeleteLabelValues(labels...)
+					if !ok {
+						log.Errorf("Metric emission: failed to delete PVPrice with labels: %v", labels)
+					}
 					delete(pvSeen, labelString)
 				} else {
 					pvSeen[labelString] = false
@@ -747,7 +762,10 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 			for labelString, seen := range pvcSeen {
 				if !seen {
 					labels := getLabelStringsFromKey(labelString)
-					cmme.PVAllocationRecorder.DeleteLabelValues(labels...)
+					ok := cmme.PVAllocationRecorder.DeleteLabelValues(labels...)
+					if !ok {
+						log.Errorf("Metric emission: failed to delete PVAllocation with labels: %v", labels)
+					}
 					delete(pvcSeen, labelString)
 				} else {
 					pvcSeen[labelString] = false

+ 2 - 1
pkg/filter21/allocation/fields.go

@@ -25,7 +25,8 @@ const (
 // Filtering based on label aliases (team, department, etc.) should be a
 // responsibility of the query handler. By the time it reaches this
 // structured representation, we shouldn't have to be aware of what is
-// aliased to what.
+// aliased to what. The aliases correspond to either a label or annotation,
+// defined by the user.
 type AllocationAlias string
 
 const (

+ 3 - 0
pkg/kubecost/allocation.go

@@ -110,6 +110,8 @@ func (orig LbAllocations) Clone() LbAllocations {
 		newAllocs[key] = &LbAllocation{
 			Service: lbAlloc.Service,
 			Cost:    lbAlloc.Cost,
+			Private: lbAlloc.Private,
+			Ip:      lbAlloc.Ip,
 		}
 	}
 	return newAllocs
@@ -119,6 +121,7 @@ type LbAllocation struct {
 	Service string  `json:"service"`
 	Cost    float64 `json:"cost"`
 	Private bool    `json:"private"`
+	Ip      string  `json:"ip"` //@bingen:field[version=19]
 }
 
 func (lba *LbAllocation) SanitizeNaN() {

+ 68 - 1
pkg/kubecost/allocation_json_test.go

@@ -2,10 +2,11 @@ package kubecost
 
 import (
 	"encoding/json"
-	"github.com/opencost/opencost/pkg/util/mathutil"
 	"math"
 	"testing"
 	"time"
+
+	"github.com/opencost/opencost/pkg/util/mathutil"
 )
 
 func TestAllocation_MarshalJSON(t *testing.T) {
@@ -155,6 +156,72 @@ func TestPVAllocations_MarshalJSON(t *testing.T) {
 
 }
 
+func TestLbAllocation_MarshalJSON(t *testing.T) {
+	testCases := map[string]LbAllocations{
+		"empty": {},
+		"single": {
+			"cluster1/namespace1/ingress": {
+				Service: "namespace1/ingress",
+				Cost:    1,
+				Private: false,
+				Ip:      "127.0.0.1",
+			},
+		},
+		"multi": {
+			"cluster1/namespace1/ingress": {
+				Service: "namespace1/ingress",
+				Cost:    1,
+				Private: false,
+				Ip:      "127.0.0.1",
+			},
+			"cluster1/namespace1/frontend": {
+				Service: "namespace1/frontend",
+				Cost:    1,
+				Private: false,
+				Ip:      "127.0.0.2",
+			},
+		},
+		"emptyLB": {
+			"cluster1/namespace1/pod": {},
+		},
+	}
+
+	for name, before := range testCases {
+		t.Run(name, func(t *testing.T) {
+			data, err := json.Marshal(before)
+			if err != nil {
+				t.Fatalf("LbAllocations.MarshalJSON: unexpected error: %s", err)
+			}
+
+			after := LbAllocations{}
+			err = json.Unmarshal(data, &after)
+			if err != nil {
+				t.Fatalf("LbAllocations.UnmarshalJSON: unexpected error: %s", err)
+			}
+
+			if len(before) != len(after) {
+				t.Fatalf("LbAllocations.MarshalJSON: before and after are not equal")
+			}
+
+			for serviceKey, beforeLB := range before {
+				afterLB, ok := after[serviceKey]
+				if !ok {
+					t.Fatalf("LbAllocations.MarshalJSON: after missing serviceKey %s", serviceKey)
+				}
+				if beforeLB.Cost != afterLB.Cost {
+					t.Fatalf("LbAllocations.MarshalJSON: LbAllocation Cost not equal for serviceKey %s", serviceKey)
+				}
+
+				if beforeLB.Ip != afterLB.Ip {
+					t.Fatalf("LbAllocations.MarshalJSON: LbAllocation Ip not equal for serviceKey %s", serviceKey)
+				}
+			}
+
+		})
+	}
+
+}
+
 func TestFormatFloat64ForResponse(t *testing.T) {
 	type formatTestCase struct {
 		name          string

+ 39 - 2
pkg/kubecost/allocationfilter_test.go

@@ -440,11 +440,11 @@ func Test_AllocationFilterCondition_Matches(t *testing.T) {
 			expected: true,
 		},
 		{
-			name: `product != unallocated -> true`,
+			name: `department != unallocated -> true`,
 			a: &Allocation{
 				Properties: &AllocationProperties{
 					Annotations: AllocationAnnotations{
-						"keyproduct": "foo",
+						"keydepartment": "foo",
 					},
 				},
 			},
@@ -460,6 +460,43 @@ func Test_AllocationFilterCondition_Matches(t *testing.T) {
 			},
 			expected: true,
 		},
+		{
+			name: `product == unallocated -> true`,
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Annotations: AllocationAnnotations{
+						"keydepartment": "foo",
+					},
+				},
+			},
+			filter: &ast.EqualOp{
+				Left: ast.Identifier{
+					Field: ast.NewAliasField(afilter.AliasProduct),
+				},
+				Right: UnallocatedSuffix,
+			},
+			expected: true,
+		},
+		{
+			name: `product == "" -> true`,
+			a: &Allocation{
+				Properties: &AllocationProperties{
+					Labels: AllocationLabels{
+						"keydepartment": "foo",
+					},
+					Annotations: AllocationAnnotations{
+						"keyowner": "bar",
+					},
+				},
+			},
+			filter: &ast.EqualOp{
+				Left: ast.Identifier{
+					Field: ast.NewAliasField(afilter.AliasProduct),
+				},
+				Right: "",
+			},
+			expected: true,
+		},
 	}
 
 	for _, c := range cases {

+ 26 - 2
pkg/kubecost/allocationmatcher.go

@@ -244,7 +244,9 @@ func convertAliasFilterToLabelAnnotationFilter(aliasKey string, filterValue stri
 		return nil, fmt.Errorf("unsupported op type '%s' for alias conversion", op)
 	}
 
-	return ops.Or(
+	// This handles the case where a label EXISTS/IS PRESENT for (is extant)
+	// for an aliased field. That's the primary case.
+	extantCaseNode := ops.Or(
 		ops.And(
 			ops.Contains(afilter.FieldLabel, aliasKey),
 			labelOp,
@@ -256,5 +258,27 @@ func convertAliasFilterToLabelAnnotationFilter(aliasKey string, filterValue stri
 				annotationOp,
 			),
 		),
-	), nil
+	)
+	var node ast.FilterNode
+	// This handles the special case of unallocated aliased value. There's
+	// two forms of this; first is where the label/annotation exists, but
+	// has an empty string value. That's actually handled by the extant case,
+	// because the API passes through that empty string. The other is when
+	// the aliased label/annotation doesn't exist for an allocation. That's
+	// what this modification to the tree handles. This matters when you're
+	// trying to drill into/identify workloads "not allocated" within that
+	// specific aliased field.
+	if filterValue == "" || filterValue == UnallocatedSuffix {
+		node = ops.Or(
+			extantCaseNode,
+			ops.And(
+				ops.Not(ops.Contains(afilter.FieldLabel, aliasKey)),
+				ops.Not(ops.Contains(afilter.FieldAnnotation, aliasKey)),
+			),
+		)
+	} else {
+		node = extantCaseNode
+	}
+
+	return node, nil
 }

+ 18 - 3
pkg/kubecost/asset.go

@@ -2360,11 +2360,12 @@ type LoadBalancer struct {
 	Window     Window
 	Adjustment float64
 	Cost       float64
-	Private    bool // @bingen:field[version=20]
+	Private    bool   // @bingen:field[version=20]
+	Ip         string // @bingen:field[version=21]
 }
 
 // NewLoadBalancer instantiates and returns a new LoadBalancer
-func NewLoadBalancer(name, cluster, providerID string, start, end time.Time, window Window, private bool) *LoadBalancer {
+func NewLoadBalancer(name, cluster, providerID string, start, end time.Time, window Window, private bool, ip string) *LoadBalancer {
 	properties := &AssetProperties{
 		Category:   NetworkCategory,
 		Name:       name,
@@ -2380,6 +2381,7 @@ func NewLoadBalancer(name, cluster, providerID string, start, end time.Time, win
 		End:        end,
 		Window:     window,
 		Private:    private,
+		Ip:         ip,
 	}
 }
 
@@ -2526,6 +2528,11 @@ func (lb *LoadBalancer) add(that *LoadBalancer) {
 
 	lb.Cost += that.Cost
 	lb.Adjustment += that.Adjustment
+
+	if lb.Ip != that.Ip {
+		//TODO: should we add to an array here or just ignore?
+		log.DedupedWarningf(5, "LoadBalancer add: load balancer ip fields (%s and %s) do not match. ignoring...", lb.Ip, that.Ip)
+	}
 }
 
 // Clone returns a cloned instance of the given Asset
@@ -2538,10 +2545,12 @@ func (lb *LoadBalancer) Clone() Asset {
 		Window:     lb.Window.Clone(),
 		Adjustment: lb.Adjustment,
 		Cost:       lb.Cost,
+		Private:    lb.Private,
+		Ip:         lb.Ip,
 	}
 }
 
-// Equal returns true if the tow Assets match precisely
+// Equal returns true if the two Assets match precisely
 func (lb *LoadBalancer) Equal(a Asset) bool {
 	that, ok := a.(*LoadBalancer)
 	if !ok {
@@ -2569,6 +2578,12 @@ func (lb *LoadBalancer) Equal(a Asset) bool {
 	if lb.Cost != that.Cost {
 		return false
 	}
+	if lb.Private != that.Private {
+		return false
+	}
+	if lb.Ip != that.Ip {
+		return false
+	}
 
 	return true
 }

+ 9 - 1
pkg/kubecost/asset_json.go

@@ -613,7 +613,9 @@ func (lb *LoadBalancer) MarshalJSON() ([]byte, error) {
 	jsonEncodeString(buffer, "end", lb.End.Format(time.RFC3339), ",")
 	jsonEncodeFloat64(buffer, "minutes", lb.Minutes(), ",")
 	jsonEncodeFloat64(buffer, "adjustment", lb.Adjustment, ",")
-	jsonEncodeFloat64(buffer, "totalCost", lb.TotalCost(), "")
+	jsonEncodeFloat64(buffer, "totalCost", lb.TotalCost(), ",")
+	jsonEncode(buffer, "private", lb.Private, ",")
+	jsonEncodeString(buffer, "ip", lb.Ip, "")
 	buffer.WriteString("}")
 	return buffer.Bytes(), nil
 }
@@ -675,6 +677,12 @@ func (lb *LoadBalancer) InterfaceToLoadBalancer(itf interface{}) error {
 	if Cost, err := getTypedVal(fmap["totalCost"]); err == nil {
 		lb.Cost = Cost.(float64) - lb.Adjustment
 	}
+	if private, err := getTypedVal(fmap["private"]); err == nil {
+		lb.Private = private.(bool)
+	}
+	if ip, err := getTypedVal(fmap["ip"]); err == nil {
+		lb.Ip = ip.(string)
+	}
 
 	return nil
 

+ 8 - 2
pkg/kubecost/asset_json_test.go

@@ -419,7 +419,7 @@ func TestNode_Unmarshal(t *testing.T) {
 
 func TestLoadBalancer_Unmarshal(t *testing.T) {
 
-	lb1 := NewLoadBalancer("loadbalancer1", "cluster1", "provider1", *unmarshalWindow.start, *unmarshalWindow.end, unmarshalWindow, false)
+	lb1 := NewLoadBalancer("loadbalancer1", "cluster1", "provider1", *unmarshalWindow.start, *unmarshalWindow.end, unmarshalWindow, false, "127.0.0.1")
 	lb1.Cost = 12.0
 	lb1.SetAdjustment(4.0)
 
@@ -457,6 +457,12 @@ func TestLoadBalancer_Unmarshal(t *testing.T) {
 	if lb1.Cost != lb2.Cost {
 		t.Fatalf("LoadBalancer Unmarshal: cost mutated in unmarshal")
 	}
+	if lb1.Private != lb2.Private {
+		t.Fatalf("LoadBalancer Unmarshal: private mutated in unmarshal")
+	}
+	if lb1.Ip != lb2.Ip {
+		t.Fatalf("LoadBalancer Unmarshal: ip mutated in unmarshal")
+	}
 
 	// As a final check, make sure the above checks out
 	if !lb1.Equal(lb2) {
@@ -515,7 +521,7 @@ func TestAssetset_Unmarshal(t *testing.T) {
 	disk := NewDisk("disk1", "cluster1", "disk1", *unmarshalWindow.start, *unmarshalWindow.end, unmarshalWindow)
 	network := NewNetwork("network1", "cluster1", "provider1", *unmarshalWindow.start, *unmarshalWindow.end, unmarshalWindow)
 	node := NewNode("node1", "cluster1", "provider1", *unmarshalWindow.start, *unmarshalWindow.end, unmarshalWindow)
-	lb := NewLoadBalancer("loadbalancer1", "cluster1", "provider1", *unmarshalWindow.start, *unmarshalWindow.end, unmarshalWindow, false)
+	lb := NewLoadBalancer("loadbalancer1", "cluster1", "provider1", *unmarshalWindow.start, *unmarshalWindow.end, unmarshalWindow, false, "127.0.0.1")
 	sa := NewSharedAsset("sharedasset1", unmarshalWindow)
 
 	assetList := []Asset{any, cloud, cm, disk, network, node, lb, sa}

+ 2 - 2
pkg/kubecost/bingen.go

@@ -26,7 +26,7 @@ package kubecost
 // @bingen:generate:CoverageSet
 
 // Asset Version Set: Includes Asset pipeline specific resources
-// @bingen:set[name=Assets,version=20]
+// @bingen:set[name=Assets,version=21]
 // @bingen:generate:Any
 // @bingen:generate:Asset
 // @bingen:generate:AssetLabels
@@ -46,7 +46,7 @@ package kubecost
 // @bingen:end
 
 // Allocation Version Set: Includes Allocation pipeline specific resources
-// @bingen:set[name=Allocation,version=18]
+// @bingen:set[name=Allocation,version=19]
 // @bingen:generate:Allocation
 // @bingen:generate[stringtable]:AllocationSet
 // @bingen:generate:AllocationSetRange

+ 52 - 7
pkg/kubecost/kubecost_codecs.go

@@ -13,11 +13,12 @@ package kubecost
 
 import (
 	"fmt"
-	util "github.com/opencost/opencost/pkg/util"
 	"reflect"
 	"strings"
 	"sync"
 	"time"
+
+	util "github.com/opencost/opencost/pkg/util"
 )
 
 const (
@@ -33,17 +34,17 @@ const (
 )
 
 const (
+	// AssetsCodecVersion is used for any resources listed in the Assets version set
+	AssetsCodecVersion uint8 = 21
+
+	// AllocationCodecVersion is used for any resources listed in the Allocation version set
+	AllocationCodecVersion uint8 = 19
+
 	// CloudCostCodecVersion is used for any resources listed in the CloudCost version set
 	CloudCostCodecVersion uint8 = 2
 
 	// DefaultCodecVersion is used for any resources listed in the Default version set
 	DefaultCodecVersion uint8 = 17
-
-	// AssetsCodecVersion is used for any resources listed in the Assets version set
-	AssetsCodecVersion uint8 = 20
-
-	// AllocationCodecVersion is used for any resources listed in the Allocation version set
-	AllocationCodecVersion uint8 = 18
 )
 
 //--------------------------------------------------------------------------
@@ -5426,6 +5427,12 @@ func (target *LbAllocation) MarshalBinaryWithContext(ctx *EncodingContext) (err
 	}
 	buff.WriteFloat64(target.Cost) // write float64
 	buff.WriteBool(target.Private) // write bool
+	if ctx.IsStringTable() {
+		b := ctx.Table.AddOrGet(target.Ip)
+		buff.WriteInt(b) // write table index
+	} else {
+		buff.WriteString(target.Ip) // write string
+	}
 	return nil
 }
 
@@ -5499,6 +5506,22 @@ func (target *LbAllocation) UnmarshalBinaryWithContext(ctx *DecodingContext) (er
 	e := buff.ReadBool() // read bool
 	target.Private = e
 
+	// field version check
+	if uint8(19) <= version {
+		var g string
+		if ctx.IsStringTable() {
+			h := buff.ReadInt() // read string index
+			g = ctx.Table[h]
+		} else {
+			g = buff.ReadString() // read string
+		}
+		f := g
+		target.Ip = f
+
+	} else {
+		target.Ip = "" // default
+	}
+
 	return nil
 }
 
@@ -5612,6 +5635,12 @@ func (target *LoadBalancer) MarshalBinaryWithContext(ctx *EncodingContext) (err
 	buff.WriteFloat64(target.Adjustment) // write float64
 	buff.WriteFloat64(target.Cost)       // write float64
 	buff.WriteBool(target.Private)       // write bool
+	if ctx.IsStringTable() {
+		e := ctx.Table.AddOrGet(target.Ip)
+		buff.WriteInt(e) // write table index
+	} else {
+		buff.WriteString(target.Ip) // write string
+	}
 	return nil
 }
 
@@ -5770,6 +5799,22 @@ func (target *LoadBalancer) UnmarshalBinaryWithContext(ctx *DecodingContext) (er
 		target.Private = false // default
 	}
 
+	// field version check
+	if uint8(21) <= version {
+		var y string
+		if ctx.IsStringTable() {
+			aa := buff.ReadInt() // read string index
+			y = ctx.Table[aa]
+		} else {
+			y = buff.ReadString() // read string
+		}
+		x := y
+		target.Ip = x
+
+	} else {
+		target.Ip = "" // default
+	}
+
 	return nil
 }
 

+ 2 - 2
pkg/kubecost/mock.go

@@ -548,10 +548,10 @@ func GenerateMockAssetSets(start, end time.Time) []*AssetSet {
 	node3Network.Cost = 2.0
 
 	// Add LoadBalancers
-	cluster2LoadBalancer1 := NewLoadBalancer("namespace2/loadBalancer1", "cluster2", "lb1", start, end, NewWindow(&start, &end), false)
+	cluster2LoadBalancer1 := NewLoadBalancer("namespace2/loadBalancer1", "cluster2", "lb1", start, end, NewWindow(&start, &end), false, "127.0.0.1")
 	cluster2LoadBalancer1.Cost = 10.0
 
-	cluster2LoadBalancer2 := NewLoadBalancer("namespace2/loadBalancer2", "cluster2", "lb2", start, end, NewWindow(&start, &end), false)
+	cluster2LoadBalancer2 := NewLoadBalancer("namespace2/loadBalancer2", "cluster2", "lb2", start, end, NewWindow(&start, &end), false, "127.0.0.1")
 	cluster2LoadBalancer2.Cost = 15.0
 
 	assetSet1 := NewAssetSet(start, end, cluster1Nodes, cluster2Node1, cluster2Node2, cluster2Node3, cluster2Disk1,

+ 7 - 6
pkg/kubecost/query.go

@@ -60,12 +60,13 @@ type AllocationQueryOptions struct {
 type AccumulateOption string
 
 const (
-	AccumulateOptionNone  AccumulateOption = ""
-	AccumulateOptionAll   AccumulateOption = "all"
-	AccumulateOptionHour  AccumulateOption = "hour"
-	AccumulateOptionDay   AccumulateOption = "day"
-	AccumulateOptionWeek  AccumulateOption = "week"
-	AccumulateOptionMonth AccumulateOption = "month"
+	AccumulateOptionNone    AccumulateOption = ""
+	AccumulateOptionAll     AccumulateOption = "all"
+	AccumulateOptionHour    AccumulateOption = "hour"
+	AccumulateOptionDay     AccumulateOption = "day"
+	AccumulateOptionWeek    AccumulateOption = "week"
+	AccumulateOptionMonth   AccumulateOption = "month"
+	AccumulateOptionQuarter AccumulateOption = "quarter"
 )
 
 // AssetQueryOptions defines optional parameters for querying an Asset Store

+ 1 - 1
pkg/metrics/deploymentmetrics.go

@@ -42,7 +42,7 @@ func (kdc KubecostDeploymentCollector) Collect(ch chan<- prometheus.Metric) {
 		deploymentName := deployment.GetName()
 		deploymentNS := deployment.GetNamespace()
 
-		labels, values := prom.KubeLabelsToLabels(deployment.Spec.Selector.MatchLabels)
+		labels, values := prom.KubeLabelsToLabels(prom.SanitizeLabels(deployment.Spec.Selector.MatchLabels))
 		if len(labels) > 0 {
 			m := newDeploymentMatchLabelsMetric(deploymentName, deploymentNS, "deployment_match_labels", labels, values)
 			ch <- m

+ 1 - 1
pkg/metrics/namespacemetrics.go

@@ -139,7 +139,7 @@ func (nsac KubeNamespaceCollector) Collect(ch chan<- prometheus.Metric) {
 	for _, namespace := range namespaces {
 		nsName := namespace.GetName()
 
-		labels, values := prom.KubeLabelsToLabels(namespace.Labels)
+		labels, values := prom.KubeLabelsToLabels(prom.SanitizeLabels(namespace.Labels))
 		if len(labels) > 0 {
 			m := newNamespaceAnnotationsMetric("kube_namespace_labels", nsName, labels, values)
 			ch <- m

+ 1 - 1
pkg/metrics/nodemetrics.go

@@ -120,7 +120,7 @@ func (nsac KubeNodeCollector) Collect(ch chan<- prometheus.Metric) {
 
 		// node labels
 		if _, disabled := disabledMetrics["kube_node_labels"]; !disabled {
-			labelNames, labelValues := prom.KubePrependQualifierToLabels(node.GetLabels(), "label_")
+			labelNames, labelValues := prom.KubePrependQualifierToLabels(prom.SanitizeLabels(node.GetLabels()), "label_")
 			ch <- newKubeNodeLabelsMetric(nodeName, "kube_node_labels", labelNames, labelValues)
 		}
 

+ 1 - 31
pkg/metrics/podlabelmetrics.go

@@ -6,36 +6,6 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-//--------------------------------------------------------------------------
-//  KubecostPodCollector
-//--------------------------------------------------------------------------
-
-// KubecostPodCollector is a prometheus collector that emits pod metrics
-type KubecostPodLabelsCollector struct {
-	KubeClusterCache clustercache.ClusterCache
-}
-
-// Describe sends the super-set of all possible descriptors of metrics
-// collected by this Collector.
-func (kpmc KubecostPodLabelsCollector) Describe(ch chan<- *prometheus.Desc) {
-	ch <- prometheus.NewDesc("kube_pod_annotations", "All annotations for each pod prefix with annotation_", []string{}, nil)
-}
-
-// Collect is called by the Prometheus registry when collecting metrics.
-func (kpmc KubecostPodLabelsCollector) Collect(ch chan<- prometheus.Metric) {
-	pods := kpmc.KubeClusterCache.GetAllPods()
-	for _, pod := range pods {
-		podName := pod.GetName()
-		podNS := pod.GetNamespace()
-
-		// Pod Annotations
-		labels, values := prom.KubeAnnotationsToLabels(pod.Annotations)
-		if len(labels) > 0 {
-			ch <- newPodAnnotationMetric("kube_pod_annotations", podNS, podName, labels, values)
-		}
-	}
-}
-
 //--------------------------------------------------------------------------
 //  KubePodLabelsCollector
 //--------------------------------------------------------------------------
@@ -71,7 +41,7 @@ func (kpmc KubePodLabelsCollector) Collect(ch chan<- prometheus.Metric) {
 
 		// Pod Labels
 		if _, disabled := disabledMetrics["kube_pod_labels"]; !disabled {
-			labelNames, labelValues := prom.KubePrependQualifierToLabels(pod.GetLabels(), "label_")
+			labelNames, labelValues := prom.KubePrependQualifierToLabels(prom.SanitizeLabels(pod.GetLabels()), "label_")
 			ch <- newKubePodLabelsMetric("kube_pod_labels", podNS, podName, podUID, labelNames, labelValues)
 		}
 

+ 1 - 1
pkg/metrics/podmetrics.go

@@ -135,7 +135,7 @@ func (kpmc KubePodCollector) Collect(ch chan<- prometheus.Metric) {
 
 		// Pod Labels
 		if _, disabled := disabledMetrics["kube_pod_labels"]; !disabled {
-			labelNames, labelValues := prom.KubePrependQualifierToLabels(pod.GetLabels(), "label_")
+			labelNames, labelValues := prom.KubePrependQualifierToLabels(prom.SanitizeLabels(pod.GetLabels()), "label_")
 			ch <- newKubePodLabelsMetric("kube_pod_labels", podNS, podName, podUID, labelNames, labelValues)
 		}
 

+ 1 - 1
pkg/metrics/servicemetrics.go

@@ -42,7 +42,7 @@ func (sc KubecostServiceCollector) Collect(ch chan<- prometheus.Metric) {
 		serviceName := svc.GetName()
 		serviceNS := svc.GetNamespace()
 
-		labels, values := prom.KubeLabelsToLabels(svc.Spec.Selector)
+		labels, values := prom.KubeLabelsToLabels(prom.SanitizeLabels(svc.Spec.Selector))
 		if len(labels) > 0 {
 			m := newServiceSelectorLabelsMetric(serviceName, serviceNS, "service_selector_labels", labels, values)
 			ch <- m

+ 1 - 1
pkg/metrics/statefulsetmetrics.go

@@ -41,7 +41,7 @@ func (sc KubecostStatefulsetCollector) Collect(ch chan<- prometheus.Metric) {
 		statefulsetName := statefulset.GetName()
 		statefulsetNS := statefulset.GetNamespace()
 
-		labels, values := prom.KubeLabelsToLabels(statefulset.Spec.Selector.MatchLabels)
+		labels, values := prom.KubeLabelsToLabels(prom.SanitizeLabels(statefulset.Spec.Selector.MatchLabels))
 		if len(labels) > 0 {
 			m := newStatefulsetMatchLabelsMetric(statefulsetName, statefulsetNS, "statefulSet_match_labels", labels, values)
 			ch <- m

+ 2 - 1
pkg/metrics/telemetry.go

@@ -2,9 +2,10 @@ package metrics
 
 import (
 	"fmt"
-	"github.com/opencost/opencost/pkg/version"
 	"sync"
 
+	"github.com/opencost/opencost/pkg/version"
+
 	"github.com/kubecost/events"
 	"github.com/prometheus/client_golang/prometheus"
 )

+ 14 - 0
pkg/prom/metrics.go

@@ -104,3 +104,17 @@ func KubeAnnotationsToLabels(labels map[string]string) ([]string, []string) {
 func SanitizeLabelName(s string) string {
 	return invalidLabelCharRE.ReplaceAllString(s, "_")
 }
+
+// SanitizeLabels sanitizes all label names in the given map. This may cause
+// collisions, which is intentional as collisions that are not caught prior to
+// attempted emission will cause fatal errors. In the case of a collision, the
+// last value seen will be set, and all previous values will be overwritten.
+func SanitizeLabels(labels map[string]string) map[string]string {
+	response := make(map[string]string, len(labels))
+
+	for k, v := range labels {
+		response[SanitizeLabelName(k)] = v
+	}
+
+	return response
+}

+ 65 - 0
pkg/prom/metrics_test.go

@@ -2,6 +2,7 @@ package prom
 
 import (
 	"fmt"
+	"reflect"
 	"testing"
 )
 
@@ -93,3 +94,67 @@ func TestKubeLabelsToPromLabels(t *testing.T) {
 		t.Errorf("%s", err)
 	}
 }
+
+func TestSanitizeLabels(t *testing.T) {
+	type testCase struct {
+		in  map[string]string
+		exp map[string]string
+	}
+
+	tcs := map[string]testCase{
+		"empty labels": {
+			in:  map[string]string{},
+			exp: map[string]string{},
+		},
+		"no op": {
+			in: map[string]string{
+				"foo": "bar",
+				"baz": "loo",
+			},
+			exp: map[string]string{
+				"foo": "bar",
+				"baz": "loo",
+			},
+		},
+		"modification, no collisions": {
+			in: map[string]string{
+				"foo-foo":   "bar",
+				"baz---baz": "loo",
+			},
+			exp: map[string]string{
+				"foo_foo":   "bar",
+				"baz___baz": "loo",
+			},
+		},
+		"modification, one collision": {
+			in: map[string]string{
+				"foo-foo":   "bar",
+				"foo+foo":   "bar",
+				"baz---baz": "loo",
+			},
+			exp: map[string]string{
+				"foo_foo":   "bar",
+				"baz___baz": "loo",
+			},
+		},
+		"modification, all collisions": {
+			in: map[string]string{
+				"foo-foo": "bar",
+				"foo+foo": "bar",
+				"foo_foo": "bar",
+			},
+			exp: map[string]string{
+				"foo_foo": "bar",
+			},
+		},
+	}
+
+	for name, tc := range tcs {
+		t.Run(name, func(t *testing.T) {
+			act := SanitizeLabels(tc.in)
+			if !reflect.DeepEqual(tc.exp, act) {
+				t.Errorf("sanitizing labels failed for case %s: %+v != %+v", name, tc.exp, act)
+			}
+		})
+	}
+}

+ 15 - 15
test/cloud_test.go

@@ -194,7 +194,7 @@ func TestNodePriceFromCSVWithGPU(t *testing.T) {
 
 	c.DownloadPricingData()
 	k := c.GetKey(n.Labels, n)
-	resN, err := c.NodePricing(k)
+	resN, _, err := c.NodePricing(k)
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -210,7 +210,7 @@ func TestNodePriceFromCSVWithGPU(t *testing.T) {
 	}
 
 	k2 := c.GetKey(n2.Labels, n2)
-	resN2, err := c.NodePricing(k2)
+	resN2, _, err := c.NodePricing(k2)
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -249,7 +249,7 @@ func TestNodePriceFromCSVSpecialChar(t *testing.T) {
 	}
 	c.DownloadPricingData()
 	k := c.GetKey(n.Labels, n)
-	resN, err := c.NodePricing(k)
+	resN, _, err := c.NodePricing(k)
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -285,7 +285,7 @@ func TestNodePriceFromCSV(t *testing.T) {
 	}
 	c.DownloadPricingData()
 	k := c.GetKey(n.Labels, n)
-	resN, err := c.NodePricing(k)
+	resN, _, err := c.NodePricing(k)
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -302,7 +302,7 @@ func TestNodePriceFromCSV(t *testing.T) {
 	unknownN.Labels["foo"] = labelFooWant
 	unknownN.Labels["topology.kubernetes.io/region"] = "fakeregion"
 	k2 := c.GetKey(unknownN.Labels, unknownN)
-	resN2, _ := c.NodePricing(k2)
+	resN2, _, _ := c.NodePricing(k2)
 	if resN2 != nil {
 		t.Errorf("CSV provider should return nil on missing node")
 	}
@@ -314,7 +314,7 @@ func TestNodePriceFromCSV(t *testing.T) {
 		},
 	}
 	k3 := c.GetKey(n.Labels, n)
-	resN3, _ := c2.NodePricing(k3)
+	resN3, _, _ := c2.NodePricing(k3)
 	if resN3 != nil {
 		t.Errorf("CSV provider should return nil on missing csv")
 	}
@@ -361,7 +361,7 @@ func TestNodePriceFromCSVWithRegion(t *testing.T) {
 	}
 	c.DownloadPricingData()
 	k := c.GetKey(n.Labels, n)
-	resN, err := c.NodePricing(k)
+	resN, _, err := c.NodePricing(k)
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -371,7 +371,7 @@ func TestNodePriceFromCSVWithRegion(t *testing.T) {
 		}
 	}
 	k2 := c.GetKey(n2.Labels, n2)
-	resN2, err := c.NodePricing(k2)
+	resN2, _, err := c.NodePricing(k2)
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -381,7 +381,7 @@ func TestNodePriceFromCSVWithRegion(t *testing.T) {
 		}
 	}
 	k3 := c.GetKey(n3.Labels, n3)
-	resN3, err := c.NodePricing(k3)
+	resN3, _, err := c.NodePricing(k3)
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -398,7 +398,7 @@ func TestNodePriceFromCSVWithRegion(t *testing.T) {
 	unknownN.Labels["topology.kubernetes.io/region"] = "fakeregion"
 	unknownN.Labels["foo"] = labelFooWant
 	k4 := c.GetKey(unknownN.Labels, unknownN)
-	resN4, _ := c.NodePricing(k4)
+	resN4, _, _ := c.NodePricing(k4)
 	if resN4 != nil {
 		t.Errorf("CSV provider should return nil on missing node, instead returned %+v", resN4)
 	}
@@ -410,7 +410,7 @@ func TestNodePriceFromCSVWithRegion(t *testing.T) {
 		},
 	}
 	k5 := c.GetKey(n.Labels, n)
-	resN5, _ := c2.NodePricing(k5)
+	resN5, _, _ := c2.NodePricing(k5)
 	if resN5 != nil {
 		t.Errorf("CSV provider should return nil on missing csv")
 	}
@@ -501,7 +501,7 @@ func TestSourceMatchesFromCSV(t *testing.T) {
 	n2.Labels["foo"] = "labelFooWant"
 
 	k := c.GetKey(n2.Labels, n2)
-	resN, err := c.NodePricing(k)
+	resN, _, err := c.NodePricing(k)
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -567,7 +567,7 @@ func TestNodePriceFromCSVWithCase(t *testing.T) {
 
 	c.DownloadPricingData()
 	k := c.GetKey(n.Labels, n)
-	resN, err := c.NodePricing(k)
+	resN, _, err := c.NodePricing(k)
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -602,7 +602,7 @@ func TestNodePriceFromCSVByClass(t *testing.T) {
 	c.DownloadPricingData()
 
 	k := c.GetKey(n.Labels, n)
-	resN, err := c.NodePricing(k)
+	resN, _, err := c.NodePricing(k)
 	if err != nil {
 		t.Errorf("Error in NodePricing: %s", err.Error())
 	} else {
@@ -620,7 +620,7 @@ func TestNodePriceFromCSVByClass(t *testing.T) {
 	k2 := c.GetKey(n2.Labels, n)
 
 	c.DownloadPricingData()
-	resN2, err := c.NodePricing(k2)
+	resN2, _, err := c.NodePricing(k2)
 
 	if resN2 != nil {
 		t.Errorf("CSV provider should return nil on missing node, instead returned %+v", resN2)