Merge pull request #839 from kubecost/niko/labels

Clean up external label configuration and querying

pkg/cloud/awsprovider.go

@@ -24,6 +24,7 @@ import (
 	"github.com/kubecost/cost-model/pkg/errors"
 	"github.com/kubecost/cost-model/pkg/log"
 	"github.com/kubecost/cost-model/pkg/util"
+	"github.com/kubecost/cost-model/pkg/util/cloudutil"
 	"github.com/kubecost/cost-model/pkg/util/json"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -1540,52 +1541,6 @@ func (a *AWS) GetDisks() ([]byte, error) {
 	})
 }
 
-// ConvertToGlueColumnFormat takes a string and runs through various regex
-// and string replacement statements to convert it to a format compatible
-// with AWS Glue and Athena column names.
-// Following guidance from AWS provided here ('Column Names' section):
-// https://docs.aws.amazon.com/awsaccountbilling/latest/aboutv2/run-athena-sql.html
-// It returns a string containing the column name in proper column name format and length.
-func ConvertToGlueColumnFormat(column_name string) string {
-	klog.V(5).Infof("Converting string \"%s\" to proper AWS Glue column name.", column_name)
-
-	// An underscore is added in front of uppercase letters
-	capital_underscore := regexp.MustCompile(`[A-Z]`)
-	final := capital_underscore.ReplaceAllString(column_name, `_$0`)
-
-	// Any non-alphanumeric characters are replaced with an underscore
-	no_space_punc := regexp.MustCompile(`[\s]{1,}|[^A-Za-z0-9]`)
-	final = no_space_punc.ReplaceAllString(final, "_")
-
-	// Duplicate underscores are removed
-	no_dup_underscore := regexp.MustCompile(`_{2,}`)
-	final = no_dup_underscore.ReplaceAllString(final, "_")
-
-	// Any leading and trailing underscores are removed
-	no_front_end_underscore := regexp.MustCompile(`(^\_|\_$)`)
-	final = no_front_end_underscore.ReplaceAllString(final, "")
-
-	// Uppercase to lowercase
-	final = strings.ToLower(final)
-
-	// Longer column name than expected - remove _ left to right
-	allowed_col_len := 128
-	undersc_to_remove := len(final) - allowed_col_len
-	if undersc_to_remove > 0 {
-		final = strings.Replace(final, "_", "", undersc_to_remove)
-	}
-
-	// If removing all of the underscores still didn't
-	// make the column name < 128 characters, trim it!
-	if len(final) > allowed_col_len {
-		final = final[:allowed_col_len]
-	}
-
-	klog.V(5).Infof("Column name being returned: \"%s\". Length: \"%d\".", final, len(final))
-
-	return final
-}
-
 func generateAWSGroupBy(lastIdx int) string {
 	sequence := []string{}
 	for i := 1; i < lastIdx+1; i++ {
@@ -1955,7 +1910,7 @@ func (a *AWS) ExternalAllocations(start string, end string, aggregators []string
 	formattedAggregators := []string{}
 	for _, agg := range aggregators {
 		aggregator_column_name := "resource_tags_user_" + agg
-		aggregator_column_name = ConvertToGlueColumnFormat(aggregator_column_name)
+		aggregator_column_name = cloudutil.ConvertToGlueColumnFormat(aggregator_column_name)
 		formattedAggregators = append(formattedAggregators, aggregator_column_name)
 	}
 	aggregatorNames := strings.Join(formattedAggregators, ",")
@@ -1963,7 +1918,7 @@ func (a *AWS) ExternalAllocations(start string, end string, aggregators []string
 	aggregatorOr = aggregatorOr + " <> ''"
 
 	filter_column_name := "resource_tags_user_" + filterType
-	filter_column_name = ConvertToGlueColumnFormat(filter_column_name)
+	filter_column_name = cloudutil.ConvertToGlueColumnFormat(filter_column_name)
 
 	var query string
 	var lastIdx int

pkg/kubecost/allocation.go

@@ -3,7 +3,6 @@ package kubecost
 import (
 	"bytes"
 	"fmt"
-	"sort"
 	"strings"
 	"sync"
 	"time"
@@ -715,13 +714,14 @@ func NewAllocationSet(start, end time.Time, allocs ...*Allocation) *AllocationSe
 // simple flag for sharing idle resources.
 type AllocationAggregationOptions struct {
 	FilterFuncs       []AllocationMatchFunc
-	SplitIdle         bool
 	IdleByNode        bool
+	LabelConfig       *LabelConfig
 	MergeUnallocated  bool
+	SharedHourlyCosts map[string]float64
 	ShareFuncs        []AllocationMatchFunc
 	ShareIdle         string
 	ShareSplit        string
-	SharedHourlyCosts map[string]float64
+	SplitIdle         bool
 }
 
 // AggregateBy aggregates the Allocations in the given AllocationSet by the given
@@ -754,11 +754,26 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 	// 10. If the merge idle option is enabled, merge any remaining idle
 	//     allocations into a single idle allocation
 
+	if as.IsEmpty() {
+		return nil
+	}
+
 	if options == nil {
 		options = &AllocationAggregationOptions{}
 	}
 
-	if as.IsEmpty() {
+	if options.LabelConfig == nil {
+		options.LabelConfig = NewLabelConfig()
+	}
+
+	// If aggregateBy is nil, we don't aggregate anything. On the other hand,
+	// an empty slice implies that we should aggregate everything. See
+	// generateKey for why that makes sense.
+	shouldAggregate := aggregateBy != nil
+	shouldFilter := len(options.FilterFuncs) > 0
+	shouldShare := len(options.SharedHourlyCosts) > 0 || len(options.ShareFuncs) > 0
+	if !shouldAggregate && !shouldFilter && !shouldShare {
+		// There is nothing for AggregateBy to do, so simply return nil
 		return nil
 	}
 
@@ -921,7 +936,7 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 			hours := as.Resolution().Hours()
 
 			// If set ends in the future, adjust hours accordingly
-			diff := time.Now().Sub(as.End())
+			diff := time.Since(as.End())
 			if diff < 0.0 {
 				hours += diff.Hours()
 			}
@@ -1022,7 +1037,7 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 		}
 
 		// (5) generate key to use for aggregation-by-key and allocation name
-		key := alloc.generateKey(aggregateBy)
+		key := alloc.generateKey(aggregateBy, options.LabelConfig)
 
 		alloc.Name = key
 		if options.MergeUnallocated && alloc.IsUnallocated() {
@@ -1153,7 +1168,7 @@ func (as *AllocationSet) AggregateBy(aggregateBy []string, options *AllocationAg
 			}
 		}
 		if !skip {
-			key := alloc.generateKey(aggregateBy)
+			key := alloc.generateKey(aggregateBy, options.LabelConfig)
 
 			alloc.Name = key
 			aggSet.Insert(alloc)
@@ -1198,7 +1213,7 @@ func computeShareCoeffs(aggregateBy []string, options *AllocationAggregationOpti
 
 		// Determine the post-aggregation key under which the allocation will
 		// be shared.
-		name := alloc.generateKey(aggregateBy)
+		name := alloc.generateKey(aggregateBy, options.LabelConfig)
 
 		// If the current allocation will be filtered out in step 3, contribute
 		// its share of the shared coefficient to a "__filtered__" bin, which
@@ -1383,11 +1398,15 @@ func (a *Allocation) getIdleId(options *AllocationAggregationOptions) (string, e
 	return idleId, nil
 }
 
-func (a *Allocation) generateKey(aggregateBy []string) string {
+func (a *Allocation) generateKey(aggregateBy []string, labelConfig *LabelConfig) string {
 	if a == nil {
 		return ""
 	}
 
+	if labelConfig == nil {
+		labelConfig = NewLabelConfig()
+	}
+
 	// Names will ultimately be joined into a single name, which uniquely
 	// identifies allocations.
 	names := []string{}
@@ -1429,7 +1448,7 @@ func (a *Allocation) generateKey(aggregateBy []string) string {
 			names = append(names, a.Properties.Container)
 		case agg == AllocationServiceProp:
 			services := a.Properties.Services
-			if services == nil || len(services) == 0 {
+			if len(services) == 0 {
 				// Indicate that allocation has no services
 				names = append(names, UnallocatedSuffix)
 			} else {
@@ -1442,61 +1461,93 @@ func (a *Allocation) generateKey(aggregateBy []string) string {
 		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(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
+				labelName := strings.TrimPrefix(agg, "label:")
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, fmt.Sprintf("%s=%s", labelName, labelValue))
+				} else {
+					names = append(names, UnallocatedSuffix)
 				}
-
-				names = append(names, labelNames...)
 			}
 		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)
-					}
+				annotationName := strings.TrimPrefix(agg, "annotation:")
+				if annotationValue, ok := annotations[annotationName]; ok {
+					names = append(names, fmt.Sprintf("%s=%s", annotationName, annotationValue))
+				} else {
+					names = append(names, 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)
+			}
+		case agg == AllocationDepartmentProp:
+			labels := a.Properties.Labels
+			if labels == nil {
+				names = append(names, UnallocatedSuffix)
+			} else {
+				labelName := labelConfig.DepartmentLabel
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, labelValue)
+				} else {
+					names = append(names, UnallocatedSuffix)
 				}
-				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
+			}
+		case agg == AllocationEnvironmentProp:
+			labels := a.Properties.Labels
+			if labels == nil {
+				names = append(names, UnallocatedSuffix)
+			} else {
+				labelName := labelConfig.EnvironmentLabel
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, labelValue)
+				} else {
+					names = append(names, UnallocatedSuffix)
+				}
+			}
+		case agg == AllocationOwnerProp:
+			labels := a.Properties.Labels
+			if labels == nil {
+				names = append(names, UnallocatedSuffix)
+			} else {
+				labelName := labelConfig.OwnerLabel
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, labelValue)
+				} else {
+					names = append(names, UnallocatedSuffix)
+				}
+			}
+		case agg == AllocationProductProp:
+			labels := a.Properties.Labels
+			if labels == nil {
+				names = append(names, UnallocatedSuffix)
+			} else {
+				labelName := labelConfig.ProductLabel
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, labelValue)
+				} else {
+					names = append(names, UnallocatedSuffix)
+				}
+			}
+		case agg == AllocationTeamProp:
+			labels := a.Properties.Labels
+			if labels == nil {
+				names = append(names, UnallocatedSuffix)
+			} else {
+				labelName := labelConfig.TeamLabel
+				if labelValue, ok := labels[labelName]; ok {
+					names = append(names, labelValue)
+				} else {
+					names = append(names, UnallocatedSuffix)
 				}
-
-				names = append(names, annotationNames...)
 			}
+		default:
+			// This case should never be reached, as input up until this point
+			// should be checked and rejected if invalid. But if we do get a
+			// value we don't recognize, log a warning.
+			log.Warningf("AggregateBy: illegal aggregation parameter: %s", agg)
 		}
-
 	}
 
 	return strings.Join(names, "/")
@@ -2299,7 +2350,7 @@ func (asr *AllocationSetRange) Length() int {
 // MarshalJSON JSON-encodes the range
 func (asr *AllocationSetRange) MarshalJSON() ([]byte, error) {
 	asr.RLock()
-	asr.RUnlock()
+	defer asr.RUnlock()
 	return json.Marshal(asr.allocations)
 }
 

pkg/kubecost/allocation_test.go

@@ -435,7 +435,7 @@ func TestAllocationSet_generateKey(t *testing.T) {
 		AllocationClusterProp,
 	}
 
-	key = alloc.generateKey(props)
+	key = alloc.generateKey(props, nil)
 	if key != "" {
 		t.Fatalf("generateKey: expected \"\"; actual \"%s\"", key)
 	}
@@ -449,7 +449,7 @@ func TestAllocationSet_generateKey(t *testing.T) {
 		},
 	}
 
-	key = alloc.generateKey(props)
+	key = alloc.generateKey(props, nil)
 	if key != "cluster1" {
 		t.Fatalf("generateKey: expected \"cluster1\"; actual \"%s\"", key)
 	}
@@ -460,7 +460,7 @@ func TestAllocationSet_generateKey(t *testing.T) {
 		"label:app",
 	}
 
-	key = alloc.generateKey(props)
+	key = alloc.generateKey(props, nil)
 	if key != "cluster1//app=app1" {
 		t.Fatalf("generateKey: expected \"cluster1//app=app1\"; actual \"%s\"", key)
 	}
@@ -473,10 +473,36 @@ func TestAllocationSet_generateKey(t *testing.T) {
 			"env": "env1",
 		},
 	}
-	key = alloc.generateKey(props)
+	key = alloc.generateKey(props, nil)
 	if key != "cluster1/namespace1/app=app1" {
 		t.Fatalf("generateKey: expected \"cluster1/namespace1/app=app1\"; actual \"%s\"", key)
 	}
+
+	props = []string{
+		AllocationDepartmentProp,
+		AllocationEnvironmentProp,
+		AllocationOwnerProp,
+		AllocationProductProp,
+		AllocationTeamProp,
+	}
+
+	labelConfig := NewLabelConfig()
+
+	alloc.Properties = &AllocationProperties{
+		Cluster:   "cluster1",
+		Namespace: "namespace1",
+		Labels: map[string]string{
+			labelConfig.DepartmentLabel:  "dept1",
+			labelConfig.EnvironmentLabel: "envt1",
+			labelConfig.OwnerLabel:       "ownr1",
+			labelConfig.ProductLabel:     "prod1",
+			labelConfig.TeamLabel:        "team1",
+		},
+	}
+	key = alloc.generateKey(props, nil)
+	if key != "dept1/envt1/ownr1/prod1/team1" {
+		t.Fatalf("generateKey: expected \"dept1/envt1/ownr1/prod1/team1\"; actual \"%s\"", key)
+	}
 }
 
 func TestNewAllocationSet(t *testing.T) {
@@ -920,17 +946,17 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 		// 2d AggregationProperties=(Label:app, Label:environment)
 		"2d": {
 			start:      start,
-			aggBy:      []string{"label:app;env"},
+			aggBy:      []string{"label:app", "label:env"},
 			aggOpts:    nil,
 			numResults: 3 + numIdle + numUnallocated,
 			totalCost:  activeTotalCost + idleTotalCost,
 			// sets should be {idle, unallocated, app1/env1, app2/env2, app2/unallocated}
 			results: map[string]float64{
-				"app=app1/env=env1":             16.00,
-				"app=app2/env=env2":             12.00,
-				"app=app2/" + UnallocatedSuffix: 12.00,
-				IdleSuffix:                      30.00,
-				UnallocatedSuffix:               42.00,
+				"app=app1/env=env1":                         16.00,
+				"app=app2/env=env2":                         12.00,
+				"app=app2/" + UnallocatedSuffix:             12.00,
+				IdleSuffix:                                  30.00,
+				UnallocatedSuffix + "/" + UnallocatedSuffix: 42.00,
 			},
 			windowStart: startYesterday,
 			windowEnd:   endYesterday,
@@ -939,17 +965,17 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 		// 2e AggregationProperties=(Cluster, Label:app, Label:environment)
 		"2e": {
 			start:      start,
-			aggBy:      []string{AllocationClusterProp, "label:app;env"},
+			aggBy:      []string{AllocationClusterProp, "label:app", "label:env"},
 			aggOpts:    nil,
 			numResults: 6,
 			totalCost:  activeTotalCost + idleTotalCost,
 			results: map[string]float64{
-				"cluster1/app=app2/env=env2":             12.00,
-				"__idle__":                               30.00,
-				"cluster1/app=app1/env=env1":             16.00,
-				"cluster1/" + UnallocatedSuffix:          18.00,
-				"cluster2/app=app2/" + UnallocatedSuffix: 12.00,
-				"cluster2/" + UnallocatedSuffix:          24.00,
+				"cluster1/app=app2/env=env2": 12.00,
+				"__idle__":                   30.00,
+				"cluster1/app=app1/env=env1": 16.00,
+				"cluster1/" + UnallocatedSuffix + "/" + UnallocatedSuffix: 18.00,
+				"cluster2/app=app2/" + UnallocatedSuffix:                  12.00,
+				"cluster2/" + UnallocatedSuffix + "/" + UnallocatedSuffix: 24.00,
 			},
 			windowStart: startYesterday,
 			windowEnd:   endYesterday,

pkg/kubecost/allocationprops.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+
+	"github.com/kubecost/cost-model/pkg/prom"
 )
 
 const (
@@ -23,6 +25,11 @@ const (
 	AllocationStatefulSetProp    string = "statefulset"
 	AllocationDaemonSetProp      string = "daemonset"
 	AllocationJobProp            string = "job"
+	AllocationDepartmentProp     string = "department"
+	AllocationEnvironmentProp    string = "environment"
+	AllocationOwnerProp          string = "owner"
+	AllocationProductProp        string = "product"
+	AllocationTeamProp           string = "team"
 )
 
 func ParseProperty(text string) (string, error) {
@@ -57,7 +64,28 @@ func ParseProperty(text string) (string, error) {
 		return AllocationStatefulSetProp, nil
 	case "job":
 		return AllocationJobProp, nil
+	case "department":
+		return AllocationDepartmentProp, nil
+	case "environment":
+		return AllocationEnvironmentProp, nil
+	case "owner":
+		return AllocationOwnerProp, nil
+	case "product":
+		return AllocationProductProp, nil
+	case "team":
+		return AllocationTeamProp, nil
+	}
+
+	if strings.HasPrefix(text, "label:") {
+		label := prom.SanitizeLabelName(strings.TrimSpace(strings.TrimPrefix(text, "label:")))
+		return fmt.Sprintf("label:%s", label), nil
 	}
+
+	if strings.HasPrefix(text, "annotation:") {
+		annotation := prom.SanitizeLabelName(strings.TrimSpace(strings.TrimPrefix(text, "annotation:")))
+		return fmt.Sprintf("annotation:%s", annotation), nil
+	}
+
 	return AllocationNilProp, fmt.Errorf("invalid allocation property: %s", text)
 }
 
@@ -72,8 +100,8 @@ type AllocationProperties struct {
 	Pod            string                `json:"pod,omitempty"`
 	Services       []string              `json:"services,omitempty"`
 	ProviderID     string                `json:"providerID,omitempty"`
-	Labels         AllocationLabels      `json:"allocationLabels,omitempty"`
-	Annotations    AllocationAnnotations `json:"allocationAnnotations,omitempty"`
+	Labels         AllocationLabels      `json:"labels,omitempty"`
+	Annotations    AllocationAnnotations `json:"annotations,omitempty"`
 }
 
 // AllocationLabels is a schema-free mapping of key/value pairs that can be

pkg/kubecost/asset.go

@@ -121,11 +121,16 @@ type Asset interface {
 //   => nil, err
 //
 // (See asset_test.go for assertions of these examples and more.)
-func AssetToExternalAllocation(asset Asset, aggregateBy []string, externalLabelsCfg map[string]string) (*Allocation, error) {
+func AssetToExternalAllocation(asset Asset, aggregateBy []string, labelConfig *LabelConfig) (*Allocation, error) {
 	if asset == nil {
 		return nil, fmt.Errorf("asset is nil")
 	}
 
+	// Use default label config if one is not provided.
+	if labelConfig == nil {
+		labelConfig = NewLabelConfig()
+	}
+
 	// names will collect the slash-separated names accrued by iterating over
 	// aggregateBy and checking the relevant labels.
 	names := []string{}
@@ -137,83 +142,67 @@ func AssetToExternalAllocation(asset Asset, aggregateBy []string, externalLabels
 	// props records the relevant Properties to set on the resultant Allocation
 	props := AllocationProperties{}
 
+	// For each aggregation parameter, try to find a match in the asset's
+	// labels, using the labelConfig to translate. For an aggregation parameter
+	// defined by a label (e.g. "label:app") this is simple: look for the label
+	// and use it (e.g. if "app" is a defined label on the asset, then use its
+	// value). For an aggregation parameter defined by a non-label property
+	// (e.g. "namespace") this requires using the labelConfig to look up the
+	// label name associated with that property and to use the value under that
+	// label, if set (e.g. if the aggregation property is "namespace" and the
+	// labelConfig is configured with "namespace_external_label" => "kubens"
+	// and the asset has label "kubens":"kubecost", then file the asset as an
+	// external cost under "kubecost").
 	for _, aggBy := range aggregateBy {
-		// labelName should be derived from the mapping of properties to
-		// label names, unless the aggBy is explicitly a label, in which
-		// case we should pull the label name from the aggBy string.
-		// Unless this matches a special aggregation, as we have that mapping already transformed...
-		labelName := aggBy
-		agglName := aggBy
-		if strings.HasPrefix(aggBy, "label:") {
-			labelName = strings.TrimPrefix(aggBy, "label:")
-			agglName = labelName
-			if v, ok := externalLabelsCfg[labelName]; ok {
-				agglName = v
-			}
-		}
-
-		if labelName == "" {
+		name := labelConfig.GetExternalAllocationName(asset.Labels(), aggBy)
+		if name == "" {
 			// No matching label has been defined in the cost-analyzer label config
 			// relating to the given aggregateBy property.
 			names = append(names, UnallocatedSuffix)
 			continue
-		}
-
-		if value := asset.Labels()[agglName]; value != "" {
-			// Valid label value was found for one of the aggregation properties,
-			// so add it to the name.
-			if strings.HasPrefix(aggBy, "label:") {
-				// Use naming convention labelName=labelValue for labels
-				// e.g. aggBy="label:env", value="prod" => "env=prod"
-				names = append(names, fmt.Sprintf("%s=%s", strings.TrimPrefix(aggBy, "label:"), value))
-				match = true
-
-				// Set the corresponding label in props
-				labels := props.Labels
-				if labels == nil {
-					labels = map[string]string{}
-				}
-
-				labels[labelName] = value
-				props.Labels = labels
-			} else {
-				names = append(names, value)
-				match = true
-
-				// Set the corresponding property on props
-				switch aggBy {
-				case AllocationClusterProp:
-					props.Cluster = value
-				case AllocationNodeProp:
-					props.Node = value
-				case AllocationNamespaceProp:
-					props.Namespace = value
-				case AllocationControllerKindProp:
-					props.ControllerKind = value
-				case AllocationControllerProp:
-					props.Controller = value
-				case AllocationPodProp:
-					props.Pod = value
-				case AllocationContainerProp:
-					props.Container = value
-				case AllocationServiceProp:
-					// TODO: external allocation: how to do this? multi-service?
-					props.Services = []string{value}
+		} else {
+			names = append(names, name)
+			match = true
+
+			// Set the corresponding property on props
+			switch aggBy {
+			case AllocationClusterProp:
+				props.Cluster = name
+			case AllocationNodeProp:
+				props.Node = name
+			case AllocationNamespaceProp:
+				props.Namespace = name
+			case AllocationControllerKindProp:
+				props.ControllerKind = name
+			case AllocationControllerProp:
+				props.Controller = name
+			case AllocationPodProp:
+				props.Pod = name
+			case AllocationContainerProp:
+				props.Container = name
+			case AllocationServiceProp:
+				props.Services = []string{name}
+			default:
+				if strings.HasPrefix(aggBy, "label:") {
+					// Set the corresponding label in props
+					if props.Labels == nil {
+						props.Labels = map[string]string{}
+					}
+					labelName := strings.TrimPrefix(aggBy, "label:")
+					labelValue := strings.TrimPrefix(name, labelName+"=")
+					props.Labels[labelName] = labelValue
 				}
 			}
-		} else {
-			// No value label value was found on the Asset; consider it
-			// unallocated. Note that this case is only truly relevant if at
-			// least one other property matches (e.g. case 3 in the examples)
-			// because if there are no matches, then an error is returned.
-			names = append(names, UnallocatedSuffix)
 		}
 	}
 
+	// If not a signle aggregation property generated a matching label property,
+	// then consider the asset ineligible to be treated as an external allocation.
 	if !match {
 		return nil, fmt.Errorf("asset does not qualify as an external allocation")
 	}
 
+	// Use naming to label as an external allocation. See IsExternal() for more.
 	names = append(names, ExternalSuffix)
 
 	// TODO: external allocation: efficiency?

pkg/kubecost/asset_test.go

@@ -923,9 +923,9 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	var alloc *Allocation
 	var err error
 
-	alloc, err = AssetToExternalAllocation(asset, []string{"namespace"}, map[string]string{})
+	_, err = AssetToExternalAllocation(asset, []string{"namespace"}, nil)
 	if err == nil {
-		t.Fatalf("expected error due to nil asset")
+		t.Fatalf("expected error due to nil asset; no error returned")
 	}
 
 	// Consider this Asset:
@@ -938,20 +938,20 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	//   }
 	cloud := NewCloud(ComputeCategory, "abc123", start1, start2, windows[0])
 	cloud.SetLabels(map[string]string{
-		"namespace": "monitoring",
-		"env":       "prod",
-		"product":   "cost-analyzer",
+		"kubernetes_namespace": "monitoring",
+		"env":                  "prod",
+		"app":                  "cost-analyzer",
 	})
 	cloud.Cost = 10.00
 	asset = cloud
 
-	alloc, err = AssetToExternalAllocation(asset, []string{"namespace"}, map[string]string{})
+	_, err = AssetToExternalAllocation(asset, []string{"namespace"}, nil)
 	if err != nil {
-		t.Fatalf("expected to not error")
+		t.Fatalf("unexpected error: %s", err)
 	}
-	alloc, err = AssetToExternalAllocation(asset, nil, map[string]string{})
+	_, err = AssetToExternalAllocation(asset, nil, nil)
 	if err == nil {
-		t.Fatalf("expected error due to nil aggregateBy")
+		t.Fatalf("expected error due to nil aggregateBy; no error returned")
 	}
 
 	// Given the following parameters, we expect to return:
@@ -977,7 +977,7 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	//   => nil, err
 
 	// 1) single-prop full match
-	alloc, err = AssetToExternalAllocation(asset, []string{"namespace"}, map[string]string{})
+	alloc, err = AssetToExternalAllocation(asset, []string{"namespace"}, nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
@@ -985,7 +985,7 @@ func TestAssetToExternalAllocation(t *testing.T) {
 		t.Fatalf("expected external allocation with name '%s'; got '%s'", "monitoring/__external__", alloc.Name)
 	}
 	if ns := alloc.Properties.Namespace; ns != "monitoring" {
-		t.Fatalf("expected external allocation with AllocationProperties.Namespace '%s'; got '%s' (%s)", "monitoring", ns, err)
+		t.Fatalf("expected external allocation with AllocationProperties.Namespace '%s'; got '%s'", "monitoring", ns)
 	}
 	if alloc.ExternalCost != 10.00 {
 		t.Fatalf("expected external allocation with ExternalCost %f; got %f", 10.00, alloc.ExternalCost)
@@ -995,7 +995,7 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	}
 
 	// 2) multi-prop full match
-	alloc, err = AssetToExternalAllocation(asset, []string{"namespace", "label:env"}, map[string]string{})
+	alloc, err = AssetToExternalAllocation(asset, []string{"namespace", "label:env"}, nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
@@ -1016,7 +1016,7 @@ func TestAssetToExternalAllocation(t *testing.T) {
 	}
 
 	// 3) multi-prop partial match
-	alloc, err = AssetToExternalAllocation(asset, []string{"namespace", "label:foo"}, map[string]string{})
+	alloc, err = AssetToExternalAllocation(asset, []string{"namespace", "label:foo"}, nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
@@ -1033,134 +1033,17 @@ func TestAssetToExternalAllocation(t *testing.T) {
 		t.Fatalf("expected external allocation with TotalCost %f; got %f", 10.00, alloc.TotalCost())
 	}
 
-	// 3) no match
-	alloc, err = AssetToExternalAllocation(asset, []string{"cluster"}, map[string]string{})
+	// 4) no match
+	alloc, err = AssetToExternalAllocation(asset, []string{"cluster"}, nil)
 	if err == nil {
 		t.Fatalf("expected 'no match' error")
 	}
 
-	alloc, err = AssetToExternalAllocation(asset, []string{"namespace", "label:app"}, map[string]string{"app": "product"})
+	alloc, err = AssetToExternalAllocation(asset, []string{"namespace", "label:app"}, nil)
 	if alloc.ExternalCost != 10.00 {
 		t.Fatalf("expected external allocation with ExternalCost %f; got %f", 10.00, alloc.ExternalCost)
 	}
 	if alloc.TotalCost() != 10.00 {
 		t.Fatalf("expected external allocation with TotalCost %f; got %f", 10.00, alloc.TotalCost())
 	}
-
 }
-
-// TODO merge conflict had this:
-
-// as.Each(func(key string, a Asset) {
-// 	if exp, ok := exps[key]; ok {
-// 		if math.Round(a.TotalCost()*100) != math.Round(exp*100) {
-// 			t.Fatalf("AssetSet.AggregateBy[%s]: key %s expected total cost %.2f, actual %.2f", msg, key, exp, a.TotalCost())
-// 		}
-// 		if !a.Window().Equal(window) {
-// 			t.Fatalf("AssetSet.AggregateBy[%s]: key %s expected window %s, actual %s", msg, key, window, as.Window)
-// 		}
-// 	} else {
-// 		t.Fatalf("AssetSet.AggregateBy[%s]: unexpected asset: %s", msg, key)
-// 	}
-// })
-// }
-
-// // GenerateMockAssetSet generates the following topology:
-// //
-// // | Asset                        | Cost |  Adj |
-// // +------------------------------+------+------+
-// //   cluster1:
-// //     node1:                        6.00   1.00
-// //     node2:                        4.00   1.50
-// //     node3:                        7.00  -0.50
-// //     disk1:                        2.50   0.00
-// //     disk2:                        1.50   0.00
-// //     clusterManagement1:           3.00   0.00
-// // +------------------------------+------+------+
-// //   cluster1 subtotal              24.00   2.00
-// // +------------------------------+------+------+
-// //   cluster2:
-// //     node4:                       12.00  -1.00
-// //     disk3:                        2.50   0.00
-// //     disk4:                        1.50   0.00
-// //     clusterManagement2:           0.00   0.00
-// // +------------------------------+------+------+
-// //   cluster2 subtotal              16.00  -1.00
-// // +------------------------------+------+------+
-// //   cluster3:
-// //     node5:                       17.00   2.00
-// // +------------------------------+------+------+
-// //   cluster3 subtotal              17.00   2.00
-// // +------------------------------+------+------+
-// //   total                          57.00   3.00
-// // +------------------------------+------+------+
-// func GenerateMockAssetSet(start time.Time) *AssetSet {
-// end := start.Add(day)
-// window := NewWindow(&start, &end)
-
-// hours := window.Duration().Hours()
-
-// node1 := NewNode("node1", "cluster1", "gcp-node1", *window.Clone().start, *window.Clone().end, window.Clone())
-// node1.CPUCost = 4.0
-// node1.RAMCost = 4.0
-// node1.GPUCost = 2.0
-// node1.Discount = 0.5
-// node1.CPUCoreHours = 2.0 * hours
-// node1.RAMByteHours = 4.0 * gb * hours
-// node1.SetAdjustment(1.0)
-// node1.SetLabels(map[string]string{"test": "test"})
-
-// node2 := NewNode("node2", "cluster1", "gcp-node2", *window.Clone().start, *window.Clone().end, window.Clone())
-// node2.CPUCost = 4.0
-// node2.RAMCost = 4.0
-// node2.GPUCost = 0.0
-// node2.Discount = 0.5
-// node2.CPUCoreHours = 2.0 * hours
-// node2.RAMByteHours = 4.0 * gb * hours
-// node2.SetAdjustment(1.5)
-
-// node3 := NewNode("node3", "cluster1", "gcp-node3", *window.Clone().start, *window.Clone().end, window.Clone())
-// node3.CPUCost = 4.0
-// node3.RAMCost = 4.0
-// node3.GPUCost = 3.0
-// node3.Discount = 0.5
-// node3.CPUCoreHours = 2.0 * hours
-// node3.RAMByteHours = 4.0 * gb * hours
-// node3.SetAdjustment(-0.5)
-
-// node4 := NewNode("node4", "cluster2", "gcp-node4", *window.Clone().start, *window.Clone().end, window.Clone())
-// node4.CPUCost = 10.0
-// node4.RAMCost = 6.0
-// node4.GPUCost = 0.0
-// node4.Discount = 0.25
-// node4.CPUCoreHours = 4.0 * hours
-// node4.RAMByteHours = 12.0 * gb * hours
-// node4.SetAdjustment(-1.0)
-
-// node5 := NewNode("node5", "cluster3", "aws-node5", *window.Clone().start, *window.Clone().end, window.Clone())
-// node5.CPUCost = 10.0
-// node5.RAMCost = 7.0
-// node5.GPUCost = 0.0
-// node5.Discount = 0.0
-// node5.CPUCoreHours = 8.0 * hours
-// node5.RAMByteHours = 24.0 * gb * hours
-// node5.SetAdjustment(2.0)
-
-// disk1 := NewDisk("disk1", "cluster1", "gcp-disk1", *window.Clone().start, *window.Clone().end, window.Clone())
-// disk1.Cost = 2.5
-// disk1.ByteHours = 100 * gb * hours
-
-// disk2 := NewDisk("disk2", "cluster1", "gcp-disk2", *window.Clone().start, *window.Clone().end, window.Clone())
-// disk2.Cost = 1.5
-// disk2.ByteHours = 60 * gb * hours
-
-// disk3 := NewDisk("disk3", "cluster2", "gcp-disk3", *window.Clone().start, *window.Clone().end, window.Clone())
-// disk3.Cost = 2.5
-// disk3.ByteHours = 100 * gb * hours
-
-// disk4 := NewDisk("disk4", "cluster2", "gcp-disk4", *window.Clone().start, *window.Clone().end, window.Clone())
-// disk4.Cost = 1.5
-// disk4.ByteHours = 100 * gb * hours
-
-// cm1 := NewClusterManagement("gcp", "cluster1", window.Clone())
-// cm1.Cost = 3.0

pkg/kubecost/assetprops.go

@@ -65,20 +65,6 @@ func ParseAssetProperty(text string) (AssetProperty, error) {
 	return AssetNilProp, fmt.Errorf("invalid asset property: %s", text)
 }
 
-func propsEqual(p1, p2 []AssetProperty) bool {
-	if len(p1) != len(p2) {
-		return false
-	}
-
-	for _, p := range p1 {
-		if !hasProp(p2, p) {
-			return false
-		}
-	}
-
-	return true
-}
-
 // Category options
 
 // ComputeCategory signifies the Compute Category

pkg/kubecost/config.go

@@ -3,6 +3,9 @@ package kubecost
 import (
 	"fmt"
 	"strings"
+
+	"github.com/kubecost/cost-model/pkg/prom"
+	"github.com/kubecost/cost-model/pkg/util/cloudutil"
 )
 
 // LabelConfig is a port of type AnalyzerConfig. We need to be more thoughtful
@@ -29,6 +32,30 @@ type LabelConfig struct {
 	TeamExternalLabel        string `json:"team_external_label"`
 }
 
+// NewLabelConfig creates a new LabelConfig instance with default values.
+func NewLabelConfig() *LabelConfig {
+	return &LabelConfig{
+		DepartmentLabel:          "department",
+		EnvironmentLabel:         "env",
+		OwnerLabel:               "owner",
+		ProductLabel:             "app",
+		TeamLabel:                "team",
+		ClusterExternalLabel:     "kubernetes_cluster",
+		NamespaceExternalLabel:   "kubernetes_namespace",
+		ControllerExternalLabel:  "kubernetes_controller",
+		DaemonsetExternalLabel:   "kubernetes_daemonset",
+		DeploymentExternalLabel:  "kubernetes_deployment",
+		StatefulsetExternalLabel: "kubernetes_statefulset",
+		ServiceExternalLabel:     "kubernetes_service",
+		PodExternalLabel:         "kubernetes_pod",
+		DepartmentExternalLabel:  "kubernetes_label_department",
+		EnvironmentExternalLabel: "kubernetes_label_env",
+		OwnerExternalLabel:       "kubernetes_label_owner",
+		ProductExternalLabel:     "kubernetes_label_app",
+		TeamExternalLabel:        "kubernetes_label_team",
+	}
+}
+
 // Map returns the config as a basic string map, with default values if not set
 func (lc *LabelConfig) Map() map[string]string {
 	// Start with default values
@@ -142,57 +169,104 @@ func (lc *LabelConfig) Map() map[string]string {
 	return m
 }
 
-// ExternalQueryLabels returns the config's external labels as a mapping of the
-// query column to the label it should set;
-// e.g. if the config stores "statefulset_external_label": "kubernetes_sset",
-//      then this would return "kubernetes_sset": "statefulset"
-func (lc *LabelConfig) ExternalQueryLabels() map[string]string {
-	queryLabels := map[string]string{}
+// GetExternalAllocationName derives an external allocation name from a set of
+// labels, given an aggregation property. If the aggregation property is,
+// itself, a label (e.g. label:app) then this function looks for a
+// corresponding value under "app" and returns "app=thatvalue". If the
+// aggregation property is not a label but a Kubernetes concept
+// (e.g. namespace) then this function first finds the "external label"
+// configured to represent it (e.g. NamespaceExternalLabel: "kubens") and uses
+// that label to determine an external allocation name. If no label value can
+// be found, return an empty string.
+func (lc *LabelConfig) GetExternalAllocationName(labels map[string]string, aggregateBy string) string {
+	labelNames := []string{}
+	aggByLabel := false
+
+	// Determine if the aggregation property is, itself, a label or not. If
+	// not, determine the label associated with the given aggregation property.
+	if strings.HasPrefix(aggregateBy, "label:") {
+		labelNames = append(labelNames, prom.SanitizeLabelName(strings.TrimPrefix(aggregateBy, "label:")))
+		aggByLabel = true
+	} else {
+		// If lc is nil, use a default LabelConfig to do a best-effort match
+		if lc == nil {
+			lc = NewLabelConfig()
+		}
+
+		switch strings.ToLower(aggregateBy) {
+		case AllocationClusterProp:
+			labelNames = strings.Split(lc.ClusterExternalLabel, ",")
+		case AllocationControllerProp:
+			labelNames = strings.Split(lc.ControllerExternalLabel, ",")
+		case AllocationNamespaceProp:
+			labelNames = strings.Split(lc.NamespaceExternalLabel, ",")
+		case AllocationPodProp:
+			labelNames = strings.Split(lc.PodExternalLabel, ",")
+		case AllocationServiceProp:
+			labelNames = strings.Split(lc.ServiceExternalLabel, ",")
+		case AllocationDeploymentProp:
+			labelNames = strings.Split(lc.DeploymentExternalLabel, ",")
+		case AllocationStatefulSetProp:
+			labelNames = strings.Split(lc.StatefulsetExternalLabel, ",")
+		case AllocationDaemonSetProp:
+			labelNames = strings.Split(lc.DaemonsetExternalLabel, ",")
+		case AllocationDepartmentProp:
+			labelNames = strings.Split(lc.DepartmentExternalLabel, ",")
+		case AllocationEnvironmentProp:
+			labelNames = strings.Split(lc.EnvironmentExternalLabel, ",")
+		case AllocationOwnerProp:
+			labelNames = strings.Split(lc.OwnerExternalLabel, ",")
+		case AllocationProductProp:
+			labelNames = strings.Split(lc.ProductExternalLabel, ",")
+		case AllocationTeamProp:
+			labelNames = strings.Split(lc.TeamExternalLabel, ",")
+		}
 
-	for label, query := range lc.Map() {
-		if strings.HasSuffix(label, "external_label") && query != "" {
-			queryLabels[query] = label
+		for i, labelName := range labelNames {
+			labelNames[i] = prom.SanitizeLabelName(strings.TrimSpace(labelName))
 		}
 	}
 
-	return queryLabels
-}
+	// No label is set for the given aggregation property.
+	if len(labelNames) == 0 {
+		return ""
+	}
 
-// AllocationPropertyLabels returns the config's external resource labels
-// as a mapping from k8s resource-to-label name.
-// e.g. if the config stores "statefulset_external_label": "kubernetes_sset",
-//      then this would return "statefulset": "kubernetes_sset"
-// e.g. if the config stores "owner_label": "product_owner",
-//      then this would return "label:product_owner": "product_owner"
-func (lc *LabelConfig) AllocationPropertyLabels() map[string]string {
-	labels := map[string]string{}
-
-	for labelKind, labelName := range lc.Map() {
-		if labelName != "" {
-			switch labelKind {
-			case "namespace_external_label":
-				labels["namespace"] = labelName
-			case "cluster_external_label":
-				labels["cluster"] = labelName
-			case "controller_external_label":
-				labels["controller"] = labelName
-			case "product_external_label":
-				labels["product"] = labelName
-			case "service_external_label":
-				labels["service"] = labelName
-			case "deployment_external_label":
-				labels["deployment"] = labelName
-			case "statefulset_external_label":
-				labels["statefulset"] = labelName
-			case "daemonset_external_label":
-				labels["daemonset"] = labelName
-			case "pod_external_label":
-				labels["pod"] = labelName
-			default:
-				labels[fmt.Sprintf("label:%s", labelName)] = labelName
+	// The relevant label is not present in the set of labels provided.
+	labelName := ""
+	labelValue := ""
+	for _, ln := range labelNames {
+		if lv, ok := labels[ln]; ok {
+			// Match found for given label
+			labelName = ln
+			labelValue = lv
+			break
+		} else {
+			// Convert the label name to a format compatible with AWS Glue and
+			// Athena column naming and check again. If not found after that,
+			// then consider the label not present.
+			ln = cloudutil.ConvertToGlueColumnFormat(ln)
+			if lv, ok = labels[ln]; ok {
+				// Match found for given label after converting to AWS format
+				labelName = ln
+				labelValue = lv
+				break
 			}
 		}
 	}
 
-	return labels
+	// No match found
+	if labelName == "" {
+		return ""
+	}
+
+	// When aggregating by some label (i.e. not by a Kubernetes concept),
+	// prepend the label value with the label name (e.g. "app=cost-analyzer").
+	// This step is not necessary for Kubernetes concepts (e.g. for namespace,
+	// we do not need "namespace=kubecost"; just "kubecost" will do).
+	if aggByLabel {
+		return fmt.Sprintf("%s=%s", labelName, labelValue)
+	}
+
+	return labelValue
 }

pkg/kubecost/config_test.go

@@ -1,6 +1,10 @@
 package kubecost
 
-import "testing"
+import (
+	"testing"
+
+	"github.com/kubecost/cost-model/pkg/util/cloudutil"
+)
 
 func TestLabelConfig_Map(t *testing.T) {
 	var m map[string]string
@@ -32,63 +36,97 @@ func TestLabelConfig_Map(t *testing.T) {
 	}
 }
 
-func TestLabelConfig_ExternalQueryLabels(t *testing.T) {
-	var qls map[string]string
-	var lc *LabelConfig
+func TestLabelConfig_GetExternalAllocationName(t *testing.T) {
+	// Make sure that AWS's Glue/Athena column formatting is supported
+	glueFormattedLabel := cloudutil.ConvertToGlueColumnFormat("Non__GlueFormattedLabel")
 
-	qls = lc.ExternalQueryLabels()
-	if len(qls) != 13 {
-		t.Fatalf("ExternalQueryLabels: expected length %d; got length %d", 13, len(qls))
-	}
-	if val, ok := qls["kubernetes_deployment"]; !ok || val != "deployment_external_label" {
-		t.Fatalf("ExternalQueryLabels: expected %s; got %s", "deployment_external_label", val)
-	}
-	if val, ok := qls["kubernetes_namespace"]; !ok || val != "namespace_external_label" {
-		t.Fatalf("ExternalQueryLabels: expected %s; got %s", "namespace_external_label", val)
+	labels := map[string]string{
+		"kubens":                      "kubecost-staging",
+		"kubeowner":                   "kubecost-owner",
+		"env":                         "env1",
+		"app":                         "app1",
+		"team":                        "team1",
+		glueFormattedLabel:            "glue",
+		"prom_sanitization_test":      "pass",
+		"kubernetes_cluster":          "cluster-one",
+		"kubernetes_namespace":        "kubecost",
+		"kubernetes_controller":       "kubecost-controller",
+		"kubernetes_daemonset":        "kubecost-daemonset",
+		"kubernetes_deployment":       "kubecost-deployment",
+		"kubernetes_statefulset":      "kubecost-statefulset",
+		"kubernetes_service":          "kubecost-service",
+		"kubernetes_pod":              "kubecost-cost-analyzer-abc123",
+		"kubernetes_label_department": "kubecost-department",
+		"kubernetes_label_env":        "kubecost-env",
+		"kubernetes_label_owner":      "kubecost-owner",
+		"kubernetes_label_app":        "kubecost-app",
+		"kubernetes_label_team":       "kubecost-team",
 	}
 
-	lc = &LabelConfig{
-		DaemonsetExternalLabel: "kubernetes_ds",
-	}
-	qls = lc.ExternalQueryLabels()
-	if len(qls) != 13 {
-		t.Fatalf("ExternalQueryLabels: expected length %d; got length %d", 13, len(qls))
-	}
-	if val, ok := qls["kubernetes_ds"]; !ok || val != "daemonset_external_label" {
-		t.Fatalf("ExternalQueryLabels: expected %s; got %s", "daemonset_external_label", val)
-	}
-	if val, ok := qls["kubernetes_namespace"]; !ok || val != "namespace_external_label" {
-		t.Fatalf("ExternalQueryLabels: expected %s; got %s", "namespace_external_label", val)
+	testCases := []struct {
+		aggBy    string
+		expected string
+	}{
+		{"label:env", "env=env1"},
+		{"label:app", "app=app1"},
+		{"label:Non__GlueFormattedLabel", "non_glue_formatted_label=glue"},
+		{"cluster", "cluster-one"},
+		{"namespace", "kubecost"},
+		{"controller", "kubecost-controller"},
+		{"daemonset", "kubecost-daemonset"},
+		{"deployment", "kubecost-deployment"},
+		{"statefulset", "kubecost-statefulset"},
+		{"service", "kubecost-service"},
+		{"pod", "kubecost-cost-analyzer-abc123"},
+		{"pod", "kubecost-cost-analyzer-abc123"},
+		{"notathing", ""},
+		{"", ""},
 	}
-}
 
-func TestTestLabelConfig_AllocationPropertyLabels(t *testing.T) {
-	var labels map[string]string
 	var lc *LabelConfig
 
-	labels = lc.AllocationPropertyLabels()
-	if len(labels) != 18 {
-		t.Fatalf("AllocationPropertyLabels: expected length %d; got length %d", 18, len(labels))
-	}
-	if val, ok := labels["namespace"]; !ok || val != "kubernetes_namespace" {
-		t.Fatalf("AllocationPropertyLabels: expected %s; got %s", "kubernetes_namespace", val)
-	}
-	if val, ok := labels["label:env"]; !ok || val != "env" {
-		t.Fatalf("AllocationPropertyLabels: expected %s; got %s", "env", val)
+	// If lc is nil, everything should still work off of defaults.
+	for _, tc := range testCases {
+		actual := lc.GetExternalAllocationName(labels, tc.aggBy)
+		if actual != tc.expected {
+			t.Fatalf("GetExternalAllocationName failed; expected '%s'; got '%s'", tc.expected, actual)
+		}
 	}
 
-	lc = &LabelConfig{
-		NamespaceExternalLabel: "kubens",
-		EnvironmentLabel:       "kubeenv",
-	}
-	labels = lc.AllocationPropertyLabels()
-	if len(labels) != 18 {
-		t.Fatalf("AllocationPropertyLabels: expected length %d; got length %d", 18, len(labels))
+	// If lc is default, everything should work, just like the nil.
+	lc = NewLabelConfig()
+	for _, tc := range testCases {
+		actual := lc.GetExternalAllocationName(labels, tc.aggBy)
+		if actual != tc.expected {
+			t.Fatalf("GetExternalAllocationName failed; expected '%s'; got '%s'", tc.expected, actual)
+		}
 	}
-	if val, ok := labels["namespace"]; !ok || val != "kubens" {
-		t.Fatalf("AllocationPropertyLabels: expected %s; got %s", "kubens", val)
-	}
-	if val, ok := labels["label:kubeenv"]; !ok || val != "kubeenv" {
-		t.Fatalf("AllocationPropertyLabels: expected %s; got %s", "kubeenv", val)
+
+	// Change the external label for namespace and confirm it still works
+	lc.NamespaceExternalLabel = "kubens"
+	lc.ServiceExternalLabel = "prom-sanitization-test"
+	lc.PodExternalLabel = "Non__GlueFormattedLabel"
+	lc.OwnerExternalLabel = "kubeowner"
+	lc.DepartmentExternalLabel = "doesntexist,env"
+	lc.TeamExternalLabel = "team,env"
+
+	// TODO how is e.g. OwnerExternalLabel supposed to work?
+
+	testCases = []struct {
+		aggBy    string
+		expected string
+	}{
+		{"namespace", "kubecost-staging"},
+		{"service", "pass"},
+		{"pod", "glue"},
+		{"owner", "kubecost-owner"},
+		{"department", "env1"},
+		{"team", "team1"},
+	}
+	for _, tc := range testCases {
+		actual := lc.GetExternalAllocationName(labels, tc.aggBy)
+		if actual != tc.expected {
+			t.Fatalf("GetExternalAllocationName failed; expected '%s'; got '%s'", tc.expected, actual)
+		}
 	}
 }

pkg/kubecost/window.go

@@ -572,7 +572,7 @@ func (w Window) DurationOffset() (time.Duration, time.Duration, error) {
 	}
 
 	duration := w.Duration()
-	offset := time.Now().Sub(*w.End())
+	offset := time.Since(*w.End())
 
 	return duration, offset, nil
 }

pkg/kubecost/window_test.go

@@ -598,7 +598,7 @@ func TestWindow_DurationOffsetStrings(t *testing.T) {
 	if err != nil {
 		t.Fatalf(`unexpected error parsing "1589448338,1589534798": %s`, err)
 	}
-	dur, off = w.DurationOffsetStrings()
+	dur, _ = w.DurationOffsetStrings()
 	if dur != "1d" {
 		t.Fatalf(`expect: window to be "1d"; actual: "%s"`, dur)
 	}

pkg/log/log.go

@@ -63,7 +63,7 @@ func Profilef(format string, a ...interface{}) {
 }
 
 func Debugf(format string, a ...interface{}) {
-	klog.V(4).Infof(fmt.Sprintf("[Debug] %s", format), a...)
+	klog.V(5).Infof(fmt.Sprintf("[Debug] %s", format), a...)
 }
 
 func Profile(start time.Time, name string) {

pkg/util/cloudutil/aws.go

@@ -0,0 +1,54 @@
+package cloudutil
+
+import (
+	"regexp"
+	"strings"
+
+	"github.com/kubecost/cost-model/pkg/log"
+)
+
+// ConvertToGlueColumnFormat takes a string and runs through various regex
+// and string replacement statements to convert it to a format compatible
+// with AWS Glue and Athena column names.
+// Following guidance from AWS provided here ('Column Names' section):
+// https://docs.aws.amazon.com/awsaccountbilling/latest/aboutv2/run-athena-sql.html
+// It returns a string containing the column name in proper column name format and length.
+func ConvertToGlueColumnFormat(columnName string) string {
+	log.Debugf("Converting string \"%s\" to proper AWS Glue column name.", columnName)
+
+	// An underscore is added in front of uppercase letters
+	capitalUnderscore := regexp.MustCompile(`[A-Z]`)
+	final := capitalUnderscore.ReplaceAllString(columnName, `_$0`)
+
+	// Any non-alphanumeric characters are replaced with an underscore
+	noSpacePunc := regexp.MustCompile(`[\s]{1,}|[^A-Za-z0-9]`)
+	final = noSpacePunc.ReplaceAllString(final, "_")
+
+	// Duplicate underscores are removed
+	noDupUnderscore := regexp.MustCompile(`_{2,}`)
+	final = noDupUnderscore.ReplaceAllString(final, "_")
+
+	// Any leading and trailing underscores are removed
+	noFrontUnderscore := regexp.MustCompile(`(^\_|\_$)`)
+	final = noFrontUnderscore.ReplaceAllString(final, "")
+
+	// Uppercase to lowercase
+	final = strings.ToLower(final)
+
+	// Longer column name than expected - remove _ left to right
+	allowedColLen := 128
+	underscoreToRemove := len(final) - allowedColLen
+	if underscoreToRemove > 0 {
+		final = strings.Replace(final, "_", "", underscoreToRemove)
+	}
+
+	// If removing all of the underscores still didn't
+	// make the column name < 128 characters, trim it!
+	if len(final) > allowedColLen {
+		final = final[:allowedColLen]
+	}
+
+	log.Debugf("Column name being returned: \"%s\". Length: \"%d\".", final, len(final))
+
+	return final
+}