Merge pull request #1328 from nickcurie/nick/diff-asset-cost-change

Diff asset tracks cost changes

pkg/kubecost/asset.go

@@ -3,6 +3,7 @@ package kubecost
 import (
 	"encoding"
 	"fmt"
+	"math"
 	"strings"
 	"sync"
 	"time"
@@ -2893,36 +2894,46 @@ type DiffKind string
 const (
 	DiffAdded   DiffKind = "added"
 	DiffRemoved          = "removed"
+	DiffChanged          = "changed"
 )
 
 // Diff stores an object and a string that denotes whether that object was
 // added or removed from a set of those objects
 type Diff[T any] struct {
-	Entity T
+	Before T
+	After  T
 	Kind   DiffKind
 }
 
-// DiffAsset takes two AssetSets and returns a slice of Diffs by checking
-// the keys of each AssetSet. If a key is not found, a Diff is generated
-// and added to the slice.
-func DiffAsset(before, after *AssetSet) []Diff[Asset] {
-	changedItems := []Diff[Asset]{}
+// DiffAsset takes two AssetSets and returns a map of keys to Diffs by checking
+// the keys of each AssetSet. If a key is not found or is found with a different total cost,
+// a Diff is generated and added to the map. A found asset will only be added to the map if the new
+// total cost is greater than ratioCostChange * the old total cost
+func DiffAsset(before, after *AssetSet, ratioCostChange float64) (map[string]Diff[Asset], error) {
+	if ratioCostChange < 0.0 {
+		return nil, fmt.Errorf("Percent cost change cannot be less than 0")
+	}
+
+	changedItems := map[string]Diff[Asset]{}
 
 	for assetKey1, asset1 := range before.assets {
-		if _, ok := after.assets[assetKey1]; !ok {
-			d := Diff[Asset]{asset1, DiffRemoved}
-			changedItems = append(changedItems, d)
+		if asset2, ok := after.assets[assetKey1]; !ok {
+			d := Diff[Asset]{asset1, nil, DiffRemoved}
+			changedItems[assetKey1] = d
+		} else if math.Abs(asset1.TotalCost()-asset2.TotalCost()) > ratioCostChange*asset1.TotalCost() { //check if either value exceeds the other by more than pctCostChange
+			d := Diff[Asset]{asset1, asset2, DiffChanged}
+			changedItems[assetKey1] = d
 		}
 	}
 
 	for assetKey2, asset2 := range after.assets {
 		if _, ok := before.assets[assetKey2]; !ok {
-			d := Diff[Asset]{asset2, DiffAdded}
-			changedItems = append(changedItems, d)
+			d := Diff[Asset]{nil, asset2, DiffAdded}
+			changedItems[assetKey2] = d
 		}
 	}
 
-	return changedItems
+	return changedItems, nil
 }
 
 // AssetSetRange is a thread-safe slice of AssetSets. It is meant to

pkg/kubecost/diff_test.go

@@ -4,105 +4,125 @@ import (
 	"reflect"
 	"testing"
 	"time"
-
-	"golang.org/x/exp/slices"
 )
 
 func TestDiff(t *testing.T) {
 
 	start := time.Now().AddDate(0, 0, -1)
-	end :=  time.Now()
+	end := time.Now()
 	window1 := NewWindow(&start, &end)
 
 	node1 := NewNode("node1", "cluster1", "123abc", start, end, window1)
+	node1.CPUCost = 10
+	node1b := node1.Clone().(*Node)
+	node1b.CPUCost = 20
+	node1Key, _ := key(node1, nil)
 	node2 := NewNode("node2", "cluster1", "123abc", start, end, window1)
+	node2.CPUCost = 100
+	node2b := node2.Clone().(*Node)
+	node2b.CPUCost = 105
+	node2Key, _ := key(node2, nil)
 	node3 := NewNode("node3", "cluster1", "123abc", start, end, window1)
+	node3Key, _ := key(node3, nil)
 	node4 := NewNode("node4", "cluster1", "123abc", start, end, window1)
+	node4Key, _ := key(node4, nil)
 	disk1 := NewDisk("disk1", "cluster1", "123abc", start, end, window1)
+	disk1Key, _ := key(disk1, nil)
 	disk2 := NewDisk("disk2", "cluster1", "123abc", start, end, window1)
+	disk2Key, _ := key(disk2, nil)
 
 	cases := map[string]struct {
 		inputAssetsBefore []Asset
 		inputAssetsAfter  []Asset
-		expected          []Diff[Asset]
+		costChangeRatio   float64
+		expected          map[string]Diff[Asset]
 	}{
-		"added node":            {
-			inputAssetsBefore: []Asset{node1, node2}, 
-			inputAssetsAfter:  []Asset{node1, node2, node3}, 
-			expected:          []Diff[Asset]{{node3, DiffAdded}},
+		"added node": {
+			inputAssetsBefore: []Asset{node1, node2},
+			inputAssetsAfter:  []Asset{node1, node2, node3},
+			expected:          map[string]Diff[Asset]{node3Key: {nil, node3, DiffAdded}},
 		},
-		"multiple adds":         {
-			inputAssetsBefore: []Asset{node1, node2}, 
-			inputAssetsAfter:  []Asset{node1, node2, node3, node4}, 
-			expected:          []Diff[Asset]{{node3, DiffAdded}, {node4, DiffAdded}},
+		"multiple adds": {
+			inputAssetsBefore: []Asset{node1, node2},
+			inputAssetsAfter:  []Asset{node1, node2, node3, node4},
+			expected:          map[string]Diff[Asset]{node3Key: {nil, node3, DiffAdded}, node4Key: {nil, node4, DiffAdded}},
 		},
-		"removed node":          {
-			inputAssetsBefore: []Asset{node1, node2}, 
-			inputAssetsAfter:  []Asset{node2}, 
-			expected:          []Diff[Asset]{{node1, DiffRemoved}},
+		"removed node": {
+			inputAssetsBefore: []Asset{node1, node2},
+			inputAssetsAfter:  []Asset{node2},
+			expected:          map[string]Diff[Asset]{node1Key: {node1, nil, DiffRemoved}},
 		},
-		"multiple removes":      {
-			inputAssetsBefore: []Asset{node1, node2, node3}, 
-			inputAssetsAfter:  []Asset{node2}, 
-			expected:          []Diff[Asset]{{node1, DiffRemoved}, {node3, DiffRemoved}},
+		"multiple removes": {
+			inputAssetsBefore: []Asset{node1, node2, node3},
+			inputAssetsAfter:  []Asset{node2},
+			expected:          map[string]Diff[Asset]{node1Key: {node1, nil, DiffRemoved}, node3Key: {node3, nil, DiffRemoved}},
 		},
-		"remove all":            {
-			inputAssetsBefore: []Asset{node1, node2}, 
-			inputAssetsAfter:  []Asset{}, 
-			expected:          []Diff[Asset]{{node1, DiffRemoved}, {node2, DiffRemoved}},
+		"remove all": {
+			inputAssetsBefore: []Asset{node1, node2},
+			inputAssetsAfter:  []Asset{},
+			expected:          map[string]Diff[Asset]{node1Key: {node1, nil, DiffRemoved}, node2Key: {node2, nil, DiffRemoved}},
 		},
-		"add and remove":        {
-			inputAssetsBefore: []Asset{node1, node2}, 
-			inputAssetsAfter:  []Asset{node2, node3}, 
-			expected:          []Diff[Asset]{{node1, DiffRemoved}, {node3, DiffAdded}},
+		"add and remove": {
+			inputAssetsBefore: []Asset{node1, node2},
+			inputAssetsAfter:  []Asset{node2, node3},
+			expected:          map[string]Diff[Asset]{node1Key: {node1, nil, DiffRemoved}, node3Key: {nil, node3, DiffAdded}},
 		},
-		"no change":             {
-			inputAssetsBefore: []Asset{node1, node2}, 
-			inputAssetsAfter:  []Asset{node1, node2}, 
-			expected:          []Diff[Asset]{},
+		"no change": {
+			inputAssetsBefore: []Asset{node1, node2},
+			inputAssetsAfter:  []Asset{node1, node2},
+			expected:          map[string]Diff[Asset]{},
 		},
-		"order switch":          {
-			inputAssetsBefore: []Asset{node2, node1}, 
-			inputAssetsAfter:  []Asset{node1, node2}, 
-			expected:          []Diff[Asset]{},
+		"order switch": {
+			inputAssetsBefore: []Asset{node2, node1},
+			inputAssetsAfter:  []Asset{node1, node2},
+			expected:          map[string]Diff[Asset]{},
 		},
-		"disk add":              {
-			inputAssetsBefore: []Asset{disk1, node1}, 
-			inputAssetsAfter:  []Asset{disk1, node1, disk2}, 
-			expected:          []Diff[Asset]{{disk2, DiffAdded}},
+		"disk add": {
+			inputAssetsBefore: []Asset{disk1, node1},
+			inputAssetsAfter:  []Asset{disk1, node1, disk2},
+			expected:          map[string]Diff[Asset]{disk2Key: {nil, disk2, DiffAdded}},
 		},
-		"disk and node add":     {
-			inputAssetsBefore: []Asset{disk1, node1}, 
-			inputAssetsAfter:  []Asset{disk1, node1, disk2, node2}, 
-			expected:          []Diff[Asset]{{disk2, DiffAdded}, {node2, DiffAdded}},
+		"disk and node add": {
+			inputAssetsBefore: []Asset{disk1, node1},
+			inputAssetsAfter:  []Asset{disk1, node1, disk2, node2},
+			expected:          map[string]Diff[Asset]{disk2Key: {nil, disk2, DiffAdded}, node2Key: {nil, node2, DiffAdded}},
 		},
 		"disk and node removed": {
-			inputAssetsBefore: []Asset{disk1, node1, disk2, node2}, 
-			inputAssetsAfter:  []Asset{disk2, node2}, 
-			expected:          []Diff[Asset]{{disk1, DiffRemoved}, {node1, DiffRemoved}},
+			inputAssetsBefore: []Asset{disk1, node1, disk2, node2},
+			inputAssetsAfter:  []Asset{disk2, node2},
+			expected:          map[string]Diff[Asset]{disk1Key: {disk1, nil, DiffRemoved}, node1Key: {node1, nil, DiffRemoved}},
+		},
+		"cost change more than 10%": {
+			inputAssetsBefore: []Asset{node1},
+			inputAssetsAfter:  []Asset{node1b},
+			costChangeRatio:   0.1,
+			expected:          map[string]Diff[Asset]{node1Key: {node1, node1b, DiffChanged}},
+		},
+		"cost change less than 10%": {
+			inputAssetsBefore: []Asset{node2},
+			inputAssetsAfter:  []Asset{node2b},
+			costChangeRatio:   0.1,
+			expected:          map[string]Diff[Asset]{},
 		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			as1 := NewAssetSet(start, end, tc.inputAssetsBefore...)
+
 			as2 := NewAssetSet(start, end, tc.inputAssetsAfter...)
 
-			result := DiffAsset(as1.Clone(), as2.Clone())
+			result, err := DiffAsset(as1.Clone(), as2.Clone(), tc.costChangeRatio)
 
-			slices.SortFunc(result, func(a, b Diff[Asset]) bool {
-				return a.Entity.Properties().Name < b.Entity.Properties().Name
-			})
+			if err != nil {
+				t.Fatalf("error; got %s", err)
+			}
 
-			slices.SortFunc(tc.expected, func(a, b Diff[Asset]) bool {
-				return a.Entity.Properties().Name < b.Entity.Properties().Name
-			})
-	
 			if !reflect.DeepEqual(result, tc.expected) {
 				t.Fatalf("expected %+v; got %+v", tc.expected, result)
 			}
-			
+
 		})
 	}
 
-}
+}