Parcourir la source

Custom Cost Inconsistencies - ENG-2212 (#2778)

* Updated Custom Cost total request parser to account for the accumulate parameter.

Signed-off-by: Nik Willwerth <nwillwerth@kubecost.com>

* Added unit tests.

Signed-off-by: Nik Willwerth <nwillwerth@kubecost.com>

---------

Signed-off-by: Nik Willwerth <nwillwerth@kubecost.com>
nik-kc il y a 1 an
Parent
commit
13f299635a

+ 7 - 4
pkg/customcost/queryservice_helper.go

@@ -5,10 +5,10 @@ import (
 
 	"github.com/opencost/opencost/core/pkg/filter"
 	"github.com/opencost/opencost/core/pkg/opencost"
-	"github.com/opencost/opencost/core/pkg/util/httputil"
+	"github.com/opencost/opencost/core/pkg/util/mapper"
 )
 
-func ParseCustomCostTotalRequest(qp httputil.QueryParams) (*CostTotalRequest, error) {
+func ParseCustomCostTotalRequest(qp mapper.PrimitiveMap) (*CostTotalRequest, error) {
 	windowStr := qp.Get("window", "")
 	if windowStr == "" {
 		return nil, fmt.Errorf("missing require window param")
@@ -28,6 +28,8 @@ func ParseCustomCostTotalRequest(qp httputil.QueryParams) (*CostTotalRequest, er
 		return nil, err
 	}
 
+	accumulate := opencost.ParseAccumulate(qp.Get("accumulate", "day"))
+
 	var filter filter.Filter
 	filterString := qp.Get("filter", "")
 	if filterString != "" {
@@ -42,13 +44,14 @@ func ParseCustomCostTotalRequest(qp httputil.QueryParams) (*CostTotalRequest, er
 		Start:       *window.Start(),
 		End:         *window.End(),
 		AggregateBy: aggregateBy,
+		Accumulate:  accumulate,
 		Filter:      filter,
 	}
 
 	return opts, nil
 }
 
-func ParseCustomCostTimeseriesRequest(qp httputil.QueryParams) (*CostTimeseriesRequest, error) {
+func ParseCustomCostTimeseriesRequest(qp mapper.PrimitiveMap) (*CostTimeseriesRequest, error) {
 	windowStr := qp.Get("window", "")
 	if windowStr == "" {
 		return nil, fmt.Errorf("missing require window param")
@@ -68,7 +71,7 @@ func ParseCustomCostTimeseriesRequest(qp httputil.QueryParams) (*CostTimeseriesR
 		return nil, err
 	}
 
-	accumulate := opencost.ParseAccumulate(qp.Get("accumulate", ""))
+	accumulate := opencost.ParseAccumulate(qp.Get("accumulate", "day"))
 
 	var filter filter.Filter
 	filterString := qp.Get("filter", "")

+ 53 - 0
pkg/customcost/queryservice_helper_test.go

@@ -0,0 +1,53 @@
+package customcost
+
+import (
+	"testing"
+
+	"github.com/opencost/opencost/core/pkg/opencost"
+	"github.com/opencost/opencost/core/pkg/util/httputil"
+)
+
+// Test_ParseCustomCostRequest_Accumulate focuses on testing that both Custom Cost request parsing functions properly
+// set the `accumulate` field, inspired by a desire to prevent a regression of https://kubecost.atlassian.net/browse/ENG-2212
+func Test_ParseCustomCostRequest_Accumulate(t *testing.T) {
+	testCases := map[string]struct {
+		accumulateString   string
+		expectedAccumulate opencost.AccumulateOption
+	}{
+		"no accumulate": {
+			accumulateString:   "",
+			expectedAccumulate: opencost.AccumulateOptionDay,
+		},
+		"hour accumulate": {
+			accumulateString:   "hour",
+			expectedAccumulate: opencost.AccumulateOptionHour,
+		},
+		"day accumulate": {
+			accumulateString:   "day",
+			expectedAccumulate: opencost.AccumulateOptionDay,
+		},
+	}
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			qp := httputil.NewQueryParams(map[string][]string{})
+			qp.Set("window", "7d")
+			if len(tc.accumulateString) > 0 {
+				qp.Set("accumulate", tc.accumulateString)
+			}
+
+			totalRequest, err := ParseCustomCostTotalRequest(qp)
+			if err != nil {
+				t.Fatalf("expected no error, got: %v", err)
+			} else if totalRequest.Accumulate != tc.expectedAccumulate {
+				t.Fatalf("expected %v, got %v", tc.expectedAccumulate, totalRequest.Accumulate)
+			}
+
+			timeseriesRequest, err := ParseCustomCostTimeseriesRequest(qp)
+			if err != nil {
+				t.Fatalf("expected no error, got: %v", err)
+			} else if timeseriesRequest.Accumulate != tc.expectedAccumulate {
+				t.Fatalf("expected %v, got %v", tc.expectedAccumulate, timeseriesRequest.Accumulate)
+			}
+		})
+	}
+}