Switch aggregate by to string argument

pkg/costmodel/aggregation.go

@@ -2117,56 +2117,21 @@ func (a *Accesses) AggregateCostModelHandler(w http.ResponseWriter, r *http.Requ
 // ParseAggregationProperties attempts to parse and return aggregation properties
 // encoded under the given key. If none exist, or if parsing fails, an error
 // is returned with empty AllocationProperties.
-func ParseAggregationProperties(qp util.QueryParams, key string) (kubecost.AllocationProperties, error) {
-	aggProps := kubecost.AllocationProperties{}
-
-	labelMap := make(map[string]string)
-	annotationMap := make(map[string]string)
-	for _, raw := range qp.GetList(key, ",") {
-		fields := strings.Split(raw, ":")
-
-		switch prop, _ := kubecost.ParseProperty(fields[0]); prop {
-		case kubecost.AllocationClusterProp:
-			aggProps.Cluster = ""
-		case kubecost.AllocationNodeProp:
-			aggProps.Node = ""
-		case kubecost.AllocationNamespaceProp:
-			aggProps.Namespace = ""
-		case kubecost.AllocationControllerKindProp:
-			aggProps.ControllerKind = ""
-		case kubecost.AllocationControllerProp:
-			aggProps.Controller = ""
-		case kubecost.AllocationPodProp:
-			aggProps.Pod = ""
-		case kubecost.AllocationContainerProp:
-			aggProps.Container = ""
-		case kubecost.AllocationServiceProp:
-			aggProps.Services = []string{}
-		case kubecost.AllocationLabelProp:
-			if len(fields) != 2 {
-				return kubecost.AllocationProperties{}, fmt.Errorf("illegal aggregate by label: %s", raw)
+func ParseAggregationProperties(qp util.QueryParams, key string) ([]string, error) {
+	AggregateBy := []string{}
+	for _, agg := range qp.GetList(key, ",") {
+		aggregate := strings.TrimSpace(agg)
+		if aggregate != "" {
+			if prop, err := kubecost.ParseProperty(aggregate); err == nil {
+				AggregateBy = append(AggregateBy, string(prop))
+			} else if strings.HasPrefix(aggregate, "label:") {
+				AggregateBy = append(AggregateBy, aggregate)
+			} else if strings.HasPrefix(aggregate, "annotation:") {
+				AggregateBy = append(AggregateBy, aggregate)
 			}
-			label := prom.SanitizeLabelName(strings.TrimSpace(fields[1]))
-			labelMap[label] = ""
-		case kubecost.AllocationAnnotationProp:
-			if len(fields) != 2 {
-				return kubecost.AllocationProperties{}, fmt.Errorf("illegal aggregate by annotation: %s", raw)
-			}
-			annotation := prom.SanitizeLabelName(strings.TrimSpace(fields[1]))
-			annotationMap[annotation] = ""
 		}
-
-	}
-
-	if len(labelMap) > 0 {
-		aggProps.Labels = labelMap
 	}
-
-	if len(annotationMap) > 0 {
-		aggProps.Annotations = annotationMap
-	}
-
-	return aggProps, nil
+	return AggregateBy, nil
 }
 
 // ComputeAllocationHandler computes an AllocationSetRange from the CostModel.
@@ -2223,7 +2188,7 @@ func (a *Accesses) ComputeAllocationHandler(w http.ResponseWriter, r *http.Reque
 	}
 
 	// Aggregate, if requested
-	if !aggregateBy.Equal(&kubecost.AllocationProperties{}) {
+	if len(aggregateBy) > 0 {
 		err = asr.AggregateBy(aggregateBy, nil)
 		if err != nil {
 			WriteError(w, InternalServerError(err.Error()))

pkg/costmodel/allocation.go

@@ -1002,7 +1002,9 @@ func applyLabels(podMap map[podKey]*Pod, namespaceLabels map[namespaceKey]map[st
 	for podKey, pod := range podMap {
 		for _, alloc := range pod.Allocations {
 			allocLabels := alloc.Properties.Labels
-
+			if allocLabels == nil {
+				allocLabels = make(map[string]string)
+			}
 			// Apply namespace labels first, then pod labels so that pod labels
 			// overwrite namespace labels.
 			nsKey := newNamespaceKey(podKey.Cluster, podKey.Namespace)
@@ -1026,7 +1028,9 @@ func applyAnnotations(podMap map[podKey]*Pod, namespaceAnnotations map[string]ma
 	for key, pod := range podMap {
 		for _, alloc := range pod.Allocations {
 			allocAnnotations := alloc.Properties.Annotations
-
+			if allocAnnotations == nil {
+				allocAnnotations = make(map[string]string)
+			}
 			// Apply namespace annotations first, then pod annotations so that
 			// pod labels overwrite namespace labels.
 			if labels, ok := namespaceAnnotations[key.Namespace]; ok {

pkg/kubecost/allocation.go

@@ -421,30 +421,8 @@ func (a *Allocation) add(that *Allocation) {
 		log.Warningf("Allocation.AggregateBy: trying to add a nil receiver")
 		return
 	}
-	if a.Properties != nil && that.Properties != nil {
-		aCluster := a.Properties.Cluster
-		thatCluster := that.Properties.Cluster
-		aNode := a.Properties.Node
-		thatNode := that.Properties.Node
-
-		// reset properties
-		a.Properties = nil
-
-		// ensure that we carry cluster ID and/or node over if they're the same
-		// required for idle/shared cost allocation
-		if aCluster == thatCluster {
-			a.Properties = &AllocationProperties{Cluster: aCluster}
-		}
-		if aNode == thatNode {
-			if a.Properties == nil {
-				a.Properties = &AllocationProperties{Node: aNode}
-			} else {
-				a.Properties.Node = aNode
-			}
-		}
-	} else {
-		a.Properties = nil
-	}
+	// Preserve string properties that are matching between the two allocations
+	a.Properties = a.Properties.Intersection(that.Properties)
 
 
 	// Expand the window to encompass both Allocations
@@ -556,7 +534,7 @@ type AllocationAggregationOptions struct {
 // AggregateBy aggregates the Allocations in the given AllocationSet by the given
 // AllocationProperty. This will only be legal if the AllocationSet is divisible by the
 // given AllocationProperty; e.g. Containers can be divided by Namespace, but not vice-a-versa.
-func (as *AllocationSet) AggregateBy(properties AllocationProperties, options *AllocationAggregationOptions) error {
+func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAggregationOptions) error {
 	// The order of operations for aggregating allocations is as follows:
 	//  1. Partition external, idle, and shared allocations into separate sets.
 	//     Also, create the aggSet into which the results will be aggregated.
@@ -703,7 +681,7 @@ func (as *AllocationSet) AggregateBy(properties AllocationProperties, options *A
 	// the shared allocations).
 	var idleCoefficients map[string]map[string]map[string]float64
 	if idleSet.Length() > 0 && options.ShareIdle != ShareNone {
-		idleCoefficients, err = computeIdleCoeffs(properties, options, as, shareSet)
+		idleCoefficients, err = computeIdleCoeffs(options, as, shareSet)
 		if err != nil {
 			log.Warningf("AllocationSet.AggregateBy: compute idle coeff: %s", err)
 			return fmt.Errorf("error computing idle coefficients: %s", err)
@@ -736,7 +714,7 @@ func (as *AllocationSet) AggregateBy(properties AllocationProperties, options *A
 	// need to track this on a per-cluster, per-allocation, per-resource basis.
 	var idleFiltrationCoefficients map[string]map[string]map[string]float64
 	if len(options.FilterFuncs) > 0 && options.ShareIdle == ShareNone {
-		idleFiltrationCoefficients, err = computeIdleCoeffs(properties, options, as, shareSet)
+		idleFiltrationCoefficients, err = computeIdleCoeffs(options, as, shareSet)
 		if err != nil {
 			return fmt.Errorf("error computing idle filtration coefficients: %s", err)
 		}
@@ -772,7 +750,7 @@ func (as *AllocationSet) AggregateBy(properties AllocationProperties, options *A
 	// of the main allocation set. See above for details and an example.
 	var shareCoefficients map[string]float64
 	if shareSet.Length() > 0 {
-		shareCoefficients, err = computeShareCoeffs(properties, options, as)
+		shareCoefficients, err = computeShareCoeffs(aggregateBy, options, as)
 		if err != nil {
 			return fmt.Errorf("error computing share coefficients: %s", err)
 		}
@@ -850,7 +828,7 @@ func (as *AllocationSet) AggregateBy(properties AllocationProperties, options *A
 		}
 
 		// (5) generate key to use for aggregation-by-key and allocation name
-		key := alloc.generateKey(properties)
+		key := alloc.generateKey(aggregateBy)
 
 		alloc.Name = key
 		if options.MergeUnallocated && alloc.IsUnallocated() {
@@ -983,7 +961,7 @@ func (as *AllocationSet) AggregateBy(properties AllocationProperties, options *A
 			}
 		}
 		if !skip {
-			key := alloc.generateKey(properties)
+			key := alloc.generateKey(aggregateBy)
 
 			alloc.Name = key
 			aggSet.Insert(alloc)
@@ -1004,7 +982,7 @@ func (as *AllocationSet) AggregateBy(properties AllocationProperties, options *A
 	return nil
 }
 
-func computeShareCoeffs(properties AllocationProperties, options *AllocationAggregationOptions, as *AllocationSet) (map[string]float64, error) {
+func computeShareCoeffs(aggregateBy []string, options *AllocationAggregationOptions, as *AllocationSet) (map[string]float64, error) {
 	// Compute coeffs by totalling per-allocation, then dividing by the total.
 	coeffs := map[string]float64{}
 
@@ -1024,7 +1002,7 @@ func computeShareCoeffs(properties AllocationProperties, options *AllocationAggr
 
 		// Determine the post-aggregation key under which the allocation will
 		// be shared.
-		name := alloc.generateKey(properties)
+		name := alloc.generateKey(aggregateBy)
 
 		// If the current allocation will be filtered out in step 3, contribute
 		// its share of the shared coefficient to a "__filtered__" bin, which
@@ -1069,7 +1047,7 @@ func computeShareCoeffs(properties AllocationProperties, options *AllocationAggr
 	return coeffs, nil
 }
 
-func computeIdleCoeffs(properties AllocationProperties, options *AllocationAggregationOptions, as *AllocationSet, shareSet *AllocationSet) (map[string]map[string]map[string]float64, error) {
+func computeIdleCoeffs(options *AllocationAggregationOptions, as *AllocationSet, shareSet *AllocationSet) (map[string]map[string]map[string]float64, error) {
 	types := []string{"cpu", "gpu", "ram"}
 
 	// Compute idle coefficients, then save them in AllocationAggregationOptions
@@ -1190,7 +1168,7 @@ func computeIdleCoeffs(properties AllocationProperties, options *AllocationAggre
 	return coeffs, nil
 }
 
-func (a *Allocation) generateKey(properties AllocationProperties) string {
+func (a *Allocation) generateKey(aggregateBy []string) string {
 	if a == nil {
 		return ""
 	}
@@ -1199,139 +1177,120 @@ func (a *Allocation) generateKey(properties AllocationProperties) string {
 	// identifies allocations.
 	names := []string{}
 
-	if properties.Cluster != "" {
-		cluster := a.Properties.Cluster
-		names = append(names, cluster)
-	}
-
-	if properties.Node != "" {
-		node := a.Properties.Node
-		names = append(names, node)
-	}
-
-	if properties.Namespace != ""  {
-		namespace := a.Properties.Namespace
-		names = append(names, namespace)
-	}
-
-	if properties.ControllerKind  != "" {
-		controllerKind := a.Properties.ControllerKind
-		if controllerKind == "" {
-			// Indicate that allocation has no controller
-			controllerKind = UnallocatedSuffix
-		}
-		// TODO find better way to pass controller kind filter
-		if prop := properties.ControllerKind; prop != "true" && prop != controllerKind {
-			// The allocation does not have the specified controller kind
-			controllerKind = UnallocatedSuffix
+	// Search for special case label for ETL conversion
+	aggControllerKind := ""
+	for _, agg := range aggregateBy {
+		if strings.HasPrefix(agg, "controllerKind:") {
+			aggControllerKind = strings.Split(agg, ":")[1]
 		}
-		names = append(names, controllerKind)
 	}
 
-	if properties.Controller != "" {
-		if properties.ControllerKind == "" {
+	for _, agg := range aggregateBy {
+		switch true {
+		case agg == string(AllocationClusterProp):
+			names = append(names, a.Properties.Cluster)
+		case agg == string(AllocationNodeProp):
+			names = append(names, a.Properties.Node)
+		case agg == string(AllocationNamespaceProp):
+			names = append(names, a.Properties.Namespace)
+		case agg == string(AllocationControllerKindProp):
 			controllerKind := a.Properties.ControllerKind
-			if controllerKind != "" {
-				names = append(names, controllerKind)
+			if controllerKind == "" {
+				// Indicate that allocation has no controller
+				controllerKind = UnallocatedSuffix
 			}
-		}
-
-		controller := a.Properties.Controller
-		if controller == "" {
-			// Indicate that allocation has no controller
-			controller = UnallocatedSuffix
-		}
-
-		names = append(names, controller)
-	}
-
-	if properties.Pod != "" {
-		pod := a.Properties.Pod
-		names = append(names, pod)
-	}
-
-	if properties.Container != "" {
-		container := a.Properties.Container
-		names = append(names, container)
-	}
-
-	if properties.Services != nil {
-		services := a.Properties.Services
-		if services == nil {
-			// Indicate that allocation has no services
-			names = append(names, UnallocatedSuffix)
-		} else {
-			if len(services) > 0 {
+			if aggControllerKind != "" && aggControllerKind != controllerKind {
+				// The allocation does not have the specified controller kind
+				controllerKind = UnallocatedSuffix
+			}
+			names = append(names, controllerKind)
+		case agg == string(AllocationControllerProp):
+			if indexOf(string(AllocationControllerKindProp), aggregateBy) == -1 &&
+				a.Properties.ControllerKind != "" {
+				names = append(names, a.Properties.ControllerKind)
+			}
+			controller := a.Properties.Controller
+			if controller == "" {
+				// Indicate that allocation has no controller
+				controller = UnallocatedSuffix
+			}
+			names = append(names, controller)
+		case agg == string(AllocationPodProp):
+			names = append(names, a.Properties.Pod)
+		case agg == string(AllocationContainerProp):
+			names = append(names, a.Properties.Container)
+		case agg == string(AllocationServiceProp):
+			services := a.Properties.Services
+			if services == nil || len(services) == 0  {
+				// Indicate that allocation has no services
+				names = append(names, UnallocatedSuffix)
+			} else {
+				// This just uses the first service
 				for _, service := range services {
 					names = append(names, service)
 					break
 				}
-			} else {
-				// Indicate that allocation has no services
-				names = append(names, UnallocatedSuffix)
 			}
-		}
-
-	}
-
-	if properties.Annotations != nil   {
-		annotations:= a.Properties.Annotations
-		if annotations == nil {
-			// Indicate that allocation has no annotations
-			names = append(names, UnallocatedSuffix)
-		} else {
-			annotationNames := []string{}
-			aggAnnotations := properties.Annotations
-			for annotationName := range aggAnnotations {
-				if val, ok := annotations[annotationName]; ok {
-					annotationNames = append(annotationNames, fmt.Sprintf("%s=%s", annotationName, val))
-				} else if indexOf(UnallocatedSuffix, annotationNames) == -1 { // if UnallocatedSuffix not already in names
-					annotationNames = append(annotationNames, UnallocatedSuffix)
+		case strings.HasPrefix(agg, "controllerKind:"):
+			continue
+		case strings.HasPrefix(agg, "label:"):
+			labels := a.Properties.Labels
+			if labels == nil {
+				// Indicate that allocation has no labels
+				names = append(names, UnallocatedSuffix)
+			} else {
+				labelNames := []string{}
+				aggLabels := strings.Split(strings.TrimPrefix(agg, "label:"), ";")
+				for _, labelName := range aggLabels {
+					if val, ok := labels[labelName]; ok {
+						labelNames = append(labelNames, fmt.Sprintf("%s=%s", labelName, val))
+					} else if indexOf(UnallocatedSuffix, labelNames) == -1 { // if UnallocatedSuffix not already in names
+						labelNames = append(labelNames, UnallocatedSuffix)
+					}
 				}
-			}
-			// resolve arbitrary ordering. e.g., app=app0/env=env0 is the same agg as env=env0/app=app0
-			if len(annotationNames) > 1 {
-				sort.Strings(annotationNames)
-			}
-			unallocatedSuffixIndex := indexOf(UnallocatedSuffix, annotationNames)
-			// suffix should be at index 0 if it exists b/c of underscores
-			if unallocatedSuffixIndex != -1 {
-				annotationNames = append(annotationNames[:unallocatedSuffixIndex], annotationNames[unallocatedSuffixIndex+1:]...)
-				annotationNames = append(annotationNames, UnallocatedSuffix) // append to end
-			}
-
-			names = append(names, annotationNames...)
-		}
-	}
-
-	if properties.Labels != nil {
-		labels := a.Properties.Labels
-		if labels == nil {
-			// Indicate that allocation has no labels
-			names = append(names, UnallocatedSuffix)
-		} else {
-			labelNames := []string{}
-			aggLabels := properties.Labels
-			for labelName := range aggLabels {
-				if val, ok := labels[labelName]; ok {
-					labelNames = append(labelNames, fmt.Sprintf("%s=%s", labelName, val))
-				} else if indexOf(UnallocatedSuffix, labelNames) == -1 { // if UnallocatedSuffix not already in names
-					labelNames = append(labelNames, UnallocatedSuffix)
+				// resolve arbitrary ordering. e.g., app=app0/env=env0 is the same agg as env=env0/app=app0
+				if len(labelNames) > 1 {
+					sort.Strings(labelNames)
 				}
+				unallocatedSuffixIndex := indexOf(UnallocatedSuffix, labelNames)
+				// suffix should be at index 0 if it exists b/c of underscores
+				if unallocatedSuffixIndex != -1 {
+					labelNames = append(labelNames[:unallocatedSuffixIndex], labelNames[unallocatedSuffixIndex+1:]...)
+					labelNames = append(labelNames, UnallocatedSuffix) // append to end
+				}
+
+				names = append(names, labelNames...)
 			}
-			// resolve arbitrary ordering. e.g., app=app0/env=env0 is the same agg as env=env0/app=app0
-			if len(labelNames) > 1 {
-				sort.Strings(labelNames)
-			}
-			unallocatedSuffixIndex := indexOf(UnallocatedSuffix, labelNames)
-			// suffix should be at index 0 if it exists b/c of underscores
-			if unallocatedSuffixIndex != -1 {
-				labelNames = append(labelNames[:unallocatedSuffixIndex], labelNames[unallocatedSuffixIndex+1:]...)
-				labelNames = append(labelNames, UnallocatedSuffix) // append to end
-			}
+		case strings.HasPrefix(agg, "annotation:"):
+			annotations:= a.Properties.Annotations
+			if annotations == nil {
+				// Indicate that allocation has no annotations
+				names = append(names, UnallocatedSuffix)
+			} else {
+				annotationNames := []string{}
+				aggAnnotations := strings.Split(strings.TrimPrefix(agg, "annotation:"), ";")
+				for _, annotationName := range aggAnnotations {
+					if val, ok := annotations[annotationName]; ok {
+						annotationNames = append(annotationNames, fmt.Sprintf("%s=%s", annotationName, val))
+					} else if indexOf(UnallocatedSuffix, annotationNames) == -1 { // if UnallocatedSuffix not already in names
+						annotationNames = append(annotationNames, UnallocatedSuffix)
+					}
+				}
+				// resolve arbitrary ordering. e.g., app=app0/env=env0 is the same agg as env=env0/app=app0
+				if len(annotationNames) > 1 {
+					sort.Strings(annotationNames)
+				}
+				unallocatedSuffixIndex := indexOf(UnallocatedSuffix, annotationNames)
+				// suffix should be at index 0 if it exists b/c of underscores
+				if unallocatedSuffixIndex != -1 {
+					annotationNames = append(annotationNames[:unallocatedSuffixIndex], annotationNames[unallocatedSuffixIndex+1:]...)
+					annotationNames = append(annotationNames, UnallocatedSuffix) // append to end
+				}
 
-			names = append(names, labelNames...)
+				names = append(names, annotationNames...)
+			}
 		}
+
 	}
 
 	return strings.Join(names, "/")
@@ -1870,14 +1829,14 @@ func (asr *AllocationSetRange) Accumulate() (*AllocationSet, error) {
 
 // AggregateBy aggregates each AllocationSet in the range by the given
 // properties and options.
-func (asr *AllocationSetRange) AggregateBy(properties AllocationProperties, options *AllocationAggregationOptions) error {
+func (asr *AllocationSetRange) AggregateBy(aggregateBy []string, options *AllocationAggregationOptions) error {
 	aggRange := &AllocationSetRange{allocations: []*AllocationSet{}}
 
 	asr.Lock()
 	defer asr.Unlock()
 
 	for _, as := range asr.allocations {
-		err := as.AggregateBy(properties, options)
+		err := as.AggregateBy(aggregateBy, options)
 		if err != nil {
 			return err
 		}

pkg/kubecost/allocation_test.go

@@ -464,8 +464,9 @@ func TestAllocationSet_generateKey(t *testing.T) {
 	var alloc *Allocation
 	var key string
 
-	props := AllocationProperties{}
-	props.Cluster = "true"
+	props := []string{
+		AllocationClusterProp.String(),
+	}
 
 	key = alloc.generateKey(props)
 	if key != "" {
@@ -486,8 +487,11 @@ func TestAllocationSet_generateKey(t *testing.T) {
 		t.Fatalf("generateKey: expected \"cluster1\"; actual \"%s\"", key)
 	}
 
-	props.Namespace = "true"
-	props.Labels = map[string]string{"app": ""}
+	props = []string{
+		AllocationClusterProp.String(),
+		AllocationNamespaceProp.String(),
+		"label:app",
+	}
 
 	key = alloc.generateKey(props)
 	if key != "cluster1//app=app1" {
@@ -860,7 +864,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 1a AggregationProperties=(Cluster)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Cluster: "true"}, nil)
+	err = as.AggregateBy([]string{string(AllocationClusterProp)}, nil)
 	assertAllocationSetTotals(t, as, "1a", err, numClusters+numIdle, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "1a", map[string]float64{
 		"cluster1": 46.00,
@@ -871,7 +875,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 1b AggregationProperties=(Namespace)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, nil)
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, nil)
 	assertAllocationSetTotals(t, as, "1b", err, numNamespaces+numIdle, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "1b", map[string]float64{
 		"namespace1": 28.00,
@@ -883,7 +887,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 1c AggregationProperties=(Pod)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Pod: "true"}, nil)
+	err = as.AggregateBy([]string{string(AllocationPodProp)}, nil)
 	assertAllocationSetTotals(t, as, "1c", err, numPods+numIdle, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "1c", map[string]float64{
 		"pod-jkl":  6.00,
@@ -901,7 +905,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 1d AggregationProperties=(Container)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Container: "true"}, nil)
+	err = as.AggregateBy([]string{string(AllocationContainerProp)}, nil)
 	assertAllocationSetTotals(t, as, "1d", err, numContainers+numIdle, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "1d", map[string]float64{
 		"container2": 6.00,
@@ -919,7 +923,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 1e AggregationProperties=(ControllerKind)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{ControllerKind: "true"}, nil)
+	err = as.AggregateBy([]string{string(AllocationControllerKindProp)}, nil)
 	assertAllocationSetTotals(t, as, "1e", err, numControllerKinds+numIdle+numUnallocated, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "1e", map[string]float64{
 		"daemonset":       12.00,
@@ -932,7 +936,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 1f AggregationProperties=(Controller)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Controller: "true"}, nil)
+	err = as.AggregateBy([]string{string(AllocationControllerProp)}, nil)
 	assertAllocationSetTotals(t, as, "1f", err, numControllers+numIdle+numUnallocated, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "1f", map[string]float64{
 		"deployment/deployment2":   24.00,
@@ -947,7 +951,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 1g AggregationProperties=(Service)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Services: []string{"true"}}, nil)
+	err = as.AggregateBy([]string{string(AllocationServiceProp)}, nil)
 	assertAllocationSetTotals(t, as, "1g", err, numServices+numIdle+numUnallocated, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "1g", map[string]float64{
 		"service1":        12.00,
@@ -958,7 +962,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 1h AggregationProperties=(Label:app)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Labels: map[string]string{"app": ""}}, nil)
+	err = as.AggregateBy([]string{"label:app"}, nil)
 	assertAllocationSetTotals(t, as, "1h", err, numLabelApps+numIdle+numUnallocated, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "1h", map[string]float64{
 		"app=app1":        16.00,
@@ -970,7 +974,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 1i AggregationProperties=(ControllerKind:deployment)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{ControllerKind: "deployment"}, nil)
+	err = as.AggregateBy([]string{string(AllocationControllerKindProp), "controllerKind:deployment" }, nil)
 	assertAllocationSetTotals(t, as, "1i", err, 1+numIdle+numUnallocated, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "1i", map[string]float64{
 		"deployment":      42.00,
@@ -981,7 +985,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 1j AggregationProperties=(Annotation:team)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Annotations: map[string]string{"team": ""}}, nil)
+	err = as.AggregateBy([]string{"annotation:team"}, nil)
 	assertAllocationSetTotals(t, as, "1j", err, 2+numIdle+numUnallocated, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "1j", map[string]float64{
 		"team=team1":      12.00,
@@ -999,7 +1003,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 2d AggregationProperties=(Label:app, Label:environment)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Labels: map[string]string{"app": "", "env": ""}}, nil)
+	err = as.AggregateBy([]string{"label:app;env"}, nil)
 	// sets should be {idle, unallocated, app1/env1, app2/env2, app2/unallocated}
 	assertAllocationSetTotals(t, as, "2d", err, numIdle+numUnallocated+3, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "2d", map[string]float64{
@@ -1012,7 +1016,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 2e AggregationProperties=(Cluster, Label:app, Label:environment)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Cluster: "true", Labels: map[string]string{"app": "", "env": ""}}, nil)
+	err = as.AggregateBy([]string{string(AllocationClusterProp), "label:app;env"}, nil)
 	assertAllocationSetTotals(t, as, "2e", err, 6, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "2e", map[string]float64{
 		"cluster1/app=app2/env=env2":             12.00,
@@ -1025,9 +1029,9 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 2f AggregationProperties=(annotation:team, pod)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Annotations: map[string]string{"team": ""}, Pod: "true"}, nil)
-	assertAllocationSetTotals(t, as, "2e", err, 11, activeTotalCost+idleTotalCost)
-	assertAllocationTotals(t, as, "2e", map[string]float64{
+	err = as.AggregateBy([]string{string(AllocationPodProp), "annotation:team"}, nil)
+	assertAllocationSetTotals(t, as, "2f", err, 11, activeTotalCost+idleTotalCost)
+	assertAllocationTotals(t, as, "2f", map[string]float64{
 		"pod-jkl/" + UnallocatedSuffix: 6.00,
 		"pod-stu/team=team1":           6.00,
 		"pod-abc/" + UnallocatedSuffix: 6.00,
@@ -1050,7 +1054,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// namespace2: 46.3125 = 36.00 + 5.0*(3.0/6.0) + 15.0*(3.0/16.0) + 5.0*(3.0/6.0) + 5.0*(3.0/6.0)
 	// namespace3: 23.0000 = 18.00 + 5.0*(3.0/6.0) + 5.0*(3.0/6.0)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{ShareIdle: ShareWeighted})
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{ShareIdle: ShareWeighted})
 	assertAllocationSetTotals(t, as, "3a", err, numNamespaces, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "3a", map[string]float64{
 		"namespace1": 42.69,
@@ -1064,7 +1068,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// namespace2: 51.0000 = 36.00 + 5.0*(1.0/2.0) + 15.0*(1.0/2.0) + 5.0*(1.0/2.0) + 5.0*(1.0/2.0)
 	// namespace3: 23.0000 = 18.00 + 5.0*(1.0/2.0) + 5.0*(1.0/2.0)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{ShareIdle: ShareEven})
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{ShareIdle: ShareEven})
 	assertAllocationSetTotals(t, as, "3a", err, numNamespaces, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "3a", map[string]float64{
 		"namespace1": 38.00,
@@ -1080,7 +1084,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// namespace2: 45.5000 = 36.00 + 18.00*(1.0/2.0)
 	// idle:       30.0000
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		ShareFuncs: []AllocationMatchFunc{isNamespace3},
 		ShareSplit: ShareEven,
 	})
@@ -1097,7 +1101,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// namespace2: 37.5000 =
 	// idle:       30.0000
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		ShareFuncs: []AllocationMatchFunc{isNamespace3},
 		ShareSplit: ShareWeighted,
 	})
@@ -1115,7 +1119,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// namespace3: 23.3333 = 18.00 + 16.00*(1.0/3.0)
 	// idle:       30.0000
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		ShareFuncs: []AllocationMatchFunc{isApp1},
 		ShareSplit: ShareEven,
 	})
@@ -1134,7 +1138,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// namespace3: 54.878 = 18.00 + (7.0*24.0)*(18.00/82.00)
 	// idle:       30.0000
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		SharedHourlyCosts: map[string]float64{"total": sharedOverheadHourlyCost},
 		ShareSplit:        ShareWeighted,
 	})
@@ -1165,7 +1169,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 5a Filter by cluster with separate idle
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Cluster: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationClusterProp)}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isCluster("cluster1")},
 		ShareIdle:   ShareNone,
 	})
@@ -1178,7 +1182,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 5b Filter by cluster with shared idle
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Cluster: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationClusterProp)}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isCluster("cluster1")},
 		ShareIdle:   ShareWeighted,
 	})
@@ -1190,7 +1194,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 5c Filter by cluster, agg by namespace, with separate idle
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isCluster("cluster1")},
 		ShareIdle:   ShareNone,
 	})
@@ -1204,7 +1208,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 5d Filter by namespace, agg by cluster, with separate idle
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Cluster: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationClusterProp)}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareIdle:   ShareNone,
 	})
@@ -1220,7 +1224,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 
 	// 6a SplitIdle
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{SplitIdle: true})
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{SplitIdle: true})
 	assertAllocationSetTotals(t, as, "6a", err, numNamespaces+numSplitIdle, activeTotalCost+idleTotalCost)
 	assertAllocationTotals(t, as, "6a", map[string]float64{
 		"namespace1":                           28.00,
@@ -1235,7 +1239,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// Should match values from unfiltered aggregation (3a)
 	// namespace2: 46.3125 = 36.00 + 5.0*(3.0/6.0) + 15.0*(3.0/16.0) + 5.0*(3.0/6.0) + 5.0*(3.0/6.0)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareIdle:   ShareWeighted,
 	})
@@ -1249,7 +1253,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// Should match values from unfiltered aggregation (3b)
 	// namespace2: 51.0000 = 36.00 + 5.0*(1.0/2.0) + 15.0*(1.0/2.0) + 5.0*(1.0/2.0) + 5.0*(1.0/2.0)
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareIdle:   ShareEven,
 	})
@@ -1266,7 +1270,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// idle:       30.0000
 	// Then namespace 2 is filtered.
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		FilterFuncs:       []AllocationMatchFunc{isNamespace("namespace2")},
 		SharedHourlyCosts: map[string]float64{"total": sharedOverheadHourlyCost},
 		ShareSplit:        ShareWeighted,
@@ -1287,7 +1291,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// namespace2: 54.667 = 36.00 + (28.00)*(36.00/54.00)
 	// idle:       30.0000
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareFuncs:  []AllocationMatchFunc{isNamespace("namespace1")},
 		ShareSplit:  ShareWeighted,
@@ -1335,7 +1339,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	//   shared cost    14.2292 = (42.6875)*(18.0/54.0)
 	//
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		ShareFuncs: []AllocationMatchFunc{isNamespace("namespace1")},
 		ShareSplit: ShareWeighted,
 		ShareIdle:  ShareWeighted,
@@ -1385,7 +1389,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	// Then, filter for namespace2: 74.7708
 	//
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		FilterFuncs: []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareFuncs:  []AllocationMatchFunc{isNamespace("namespace1")},
 		ShareSplit:  ShareWeighted,
@@ -1426,7 +1430,7 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	//
 	// Then namespace 2 is filtered.
 	as = generateAllocationSet(start)
-	err = as.AggregateBy(AllocationProperties{Namespace: "true"}, &AllocationAggregationOptions{
+	err = as.AggregateBy([]string{string(AllocationNamespaceProp)}, &AllocationAggregationOptions{
 		FilterFuncs:       []AllocationMatchFunc{isNamespace("namespace2")},
 		ShareSplit:        ShareWeighted,
 		ShareIdle:         ShareWeighted,

pkg/kubecost/allocationprops.go

@@ -33,7 +33,7 @@ func ParseProperty(text string) (AllocationProperty, error) {
 		return AllocationContainerProp, nil
 	case "controller":
 		return AllocationControllerProp, nil
-	case "controllerKind":
+	case "controllerkind":
 		return AllocationControllerKindProp, nil
 	case "namespace":
 		return AllocationNamespaceProp, nil
@@ -78,7 +78,6 @@ type AllocationLabels map[string]string
 // attributed to an Allocation
 type AllocationAnnotations map[string]string
 
-// TODO niko/etl make sure Services deep copy works correctly
 func (p *AllocationProperties) Clone() *AllocationProperties {
 	if p == nil {
 		return nil
@@ -93,9 +92,24 @@ func (p *AllocationProperties) Clone() *AllocationProperties {
 	clone.Namespace = p.Namespace
 	clone.Pod = p.Pod
 	clone.ProviderID = p.ProviderID
-	clone.Services = p.Services
-	clone.Labels = p.Labels
-	clone.Annotations = p.Annotations
+
+	var services []string
+	for _, s := range p.Services {
+		services = append(services, s)
+	}
+	clone.Services = services
+
+	labels := make(map[string]string)
+	for k, v := range p.Labels {
+		labels[k] = v
+	}
+	clone.Labels = labels
+
+	annotations := make(map[string]string)
+	for k, v := range p.Annotations {
+		annotations[k] = v
+	}
+	clone.Annotations = annotations
 
 	return clone
 }
@@ -178,6 +192,40 @@ func (p *AllocationProperties) Equal(that *AllocationProperties) bool {
 	return true
 }
 
+// Intersection returns an *AllocationProperties which contains all matching fields between the calling and parameter AllocationProperties
+// nillable slices and maps are left as nil
+func (p *AllocationProperties) Intersection(that *AllocationProperties) *AllocationProperties {
+	if p == nil || that == nil {
+		return nil
+	}
+	intersectionProps := &AllocationProperties{}
+	if p.Cluster == that.Cluster {
+		intersectionProps.Cluster = p.Cluster
+	}
+	if p.Node == that.Node {
+		intersectionProps.Node = p.Node
+	}
+	if p.Container == that.Container {
+		intersectionProps.Container = p.Container
+	}
+	if p.Controller == that.Controller {
+		intersectionProps.Controller = p.Controller
+	}
+	if p.ControllerKind == that.ControllerKind {
+		intersectionProps.ControllerKind = p.ControllerKind
+	}
+	if p.Namespace == that.Namespace {
+		intersectionProps.Namespace = p.Namespace
+	}
+	if p.Pod == that.Pod {
+		intersectionProps.Pod = p.Pod
+	}
+	if p.ProviderID == that.ProviderID {
+		intersectionProps.ProviderID = p.ProviderID
+	}
+	return intersectionProps
+}
+
 func (p *AllocationProperties) String() string {
 	if p == nil {
 		return "<nil>"