Collector Name Cleanup (#3183)

Signed-off-by: Matt Bolt <mbolt35@gmail.com>
Signed-off-by: Sean Holcomb <seanholcomb@gmail.com>
Co-authored-by: Sean Holcomb <seanholcomb@gmail.com>

modules/collector-source/pkg/collector/metricsquerier_test.go

@@ -1,7 +1,9 @@
 package collector
 
 import (
+	"cmp"
 	"reflect"
+	"slices"
 	"testing"
 	"time"
 
@@ -804,9 +806,15 @@ func Test_collectorMetricsQuerier_QueryNetInternetServiceGiB(t *testing.T) {
 			},
 		},
 	}
+
 	if len(res) != len(expected) {
 		t.Errorf("length of result was not as expected: got = %d, want %d", len(res), len(expected))
 	}
+
+	slices.SortFunc(res, func(a, b *source.NetInternetServiceGiBResult) int {
+		return cmp.Compare(a.Service, b.Service)
+	})
+
 	for i, got := range res {
 		if !reflect.DeepEqual(got, expected[i]) {
 			t.Errorf("result at index %d did not match: got = %v, want %v", i, got, expected[i])
@@ -989,6 +997,11 @@ func Test_collectorMetricsQuerier_QueryNetInternetServiceIngressGiB(t *testing.T
 	if len(res) != len(expected) {
 		t.Errorf("length of result was not as expected: got = %d, want %d", len(res), len(expected))
 	}
+
+	slices.SortFunc(res, func(a, b *source.NetInternetServiceIngressGiBResult) int {
+		return cmp.Compare(a.Service, b.Service)
+	})
+
 	for i, got := range res {
 		if !reflect.DeepEqual(got, expected[i]) {
 			t.Errorf("result at index %d did not match: got = %v, want %v", i, got, expected[i])

modules/collector-source/pkg/metric/aggregator/activeminutes.go

@@ -8,23 +8,17 @@ import (
 // activateMinutesAggregator is a MetricAggregator which records the first and last timestamp of updates called on it
 type activeMinutesAggregator struct {
 	lock        sync.Mutex
-	name        string
 	labelValues []string
 	start       *time.Time
 	end         *time.Time
 }
 
-func ActiveMinutes(name string, labelValues []string) MetricAggregator {
+func ActiveMinutes(labelValues []string) MetricAggregator {
 	return &activeMinutesAggregator{
-		name:        name,
 		labelValues: labelValues,
 	}
 }
 
-func (a *activeMinutesAggregator) Name() string {
-	return a.name
-}
-
 func (a *activeMinutesAggregator) AdditionInfo() map[string]string {
 	return nil
 }

modules/collector-source/pkg/metric/aggregator/aggregator.go

@@ -13,30 +13,36 @@ type MetricValue struct {
 	Timestamp *time.Time
 }
 
-// MetricResult contains a resulting metric name, the associated labels and label values, and a slice of
+// ToVector converts the MetricValue into a util.Vector (adapter for source.QueryResults).
+func (mv *MetricValue) ToVector() *util.Vector {
+	timestamp := 0.0
+	if mv.Timestamp != nil {
+		timestamp = float64(mv.Timestamp.Unix())
+	}
+	return &util.Vector{
+		Timestamp: timestamp,
+		Value:     mv.Value,
+	}
+}
+
+// MetricResult contains the metric result labels and label values, and a slice of
 // MetricValues.
 type MetricResult struct {
-	Name         string
 	MetricLabels map[string]string
 	Values       []MetricValue
 }
 
+// ToQueryResult converts the MetricResult into a source.QueryResult, which is the format used by
+// the data source to return query results.
 func (mr *MetricResult) ToQueryResult() *source.QueryResult {
-	metrics := map[string]any{}
+	metrics := make(map[string]any, len(mr.MetricLabels))
 	for key, value := range mr.MetricLabels {
 		metrics[key] = value
 	}
 
 	values := make([]*util.Vector, len(mr.Values))
 	for i, value := range mr.Values {
-		timestamp := 0.0
-		if value.Timestamp != nil {
-			timestamp = float64(value.Timestamp.Unix())
-		}
-		values[i] = &util.Vector{
-			Timestamp: timestamp,
-			Value:     value.Value,
-		}
+		values[i] = value.ToVector()
 	}
 
 	return source.NewQueryResult(metrics, values, nil)
@@ -47,7 +53,6 @@ func (mr *MetricResult) ToQueryResult() *source.QueryResult {
 // In this case, the `AverageOverTime` component is the MetricAggregator. It is the component responsible
 // for routing updates to metric values into their proper condensed form.
 type MetricAggregator interface {
-	Name() string
 	AdditionInfo() map[string]string
 	Update(value float64, timestamp time.Time, additionalInfo map[string]string)
 	Value() []MetricValue
@@ -56,4 +61,4 @@ type MetricAggregator interface {
 
 // MetricAggregatorFactory is a function that accepts a string name and returns a pointer to a MetricAggregator
 // implementation.
-type MetricAggregatorFactory func(name string, labelValues []string) MetricAggregator
+type MetricAggregatorFactory func(labelValues []string) MetricAggregator

modules/collector-source/pkg/metric/aggregator/aggregator_test.go

@@ -0,0 +1,40 @@
+package aggregator
+
+import (
+	"testing"
+	"time"
+)
+
+func TestMetricValueToVector(t *testing.T) {
+
+	t.Run("with timestamp", func(t *testing.T) {
+		timestamp := time.Now()
+		mv := &MetricValue{
+			Value:     42.0,
+			Timestamp: &timestamp,
+		}
+
+		vector := mv.ToVector()
+		if vector.Value != 42.0 {
+			t.Errorf("Expected value 42.0, got %f", vector.Value)
+		}
+		if vector.Timestamp != float64(timestamp.Unix()) {
+			t.Errorf("Expected timestamp %f, got %f", float64(timestamp.Unix()), vector.Timestamp)
+		}
+	})
+
+	t.Run("without timestamp", func(t *testing.T) {
+		mv := &MetricValue{
+			Value:     42.0,
+			Timestamp: nil,
+		}
+
+		vector := mv.ToVector()
+		if vector.Value != 42.0 {
+			t.Errorf("Expected value 42.0, got %f", vector.Value)
+		}
+		if vector.Timestamp != 0.0 {
+			t.Errorf("Expected timestamp 0.0, got %f", vector.Timestamp)
+		}
+	})
+}

modules/collector-source/pkg/metric/aggregator/avgovertime.go

@@ -9,24 +9,18 @@ import (
 // total of all values by the count of unique timestamps
 type averageOverTimeAggregator struct {
 	lock        sync.Mutex
-	name        string
 	labelValues []string
 	total       float64
 	count       int
 	currentTime *time.Time
 }
 
-func AverageOverTime(name string, labelValues []string) MetricAggregator {
+func AverageOverTime(labelValues []string) MetricAggregator {
 	return &averageOverTimeAggregator{
-		name:        name,
 		labelValues: labelValues,
 	}
 }
 
-func (a *averageOverTimeAggregator) Name() string {
-	return a.name
-}
-
 func (a *averageOverTimeAggregator) AdditionInfo() map[string]string {
 	return nil
 }

modules/collector-source/pkg/metric/aggregator/increase.go

@@ -7,7 +7,6 @@ import (
 
 type increaseAggregator struct {
 	lock        sync.Mutex
-	name        string
 	labelValues []string
 	initialized bool
 	initialTime time.Time
@@ -16,17 +15,12 @@ type increaseAggregator struct {
 	current     float64
 }
 
-func Increase(name string, labelValues []string) MetricAggregator {
+func Increase(labelValues []string) MetricAggregator {
 	return &increaseAggregator{
-		name:        name,
 		labelValues: labelValues,
 	}
 }
 
-func (a *increaseAggregator) Name() string {
-	return a.name
-}
-
 func (a *increaseAggregator) AdditionInfo() map[string]string {
 	return nil
 }

modules/collector-source/pkg/metric/aggregator/info.go

@@ -9,22 +9,16 @@ import (
 // infoAggregator is MetricAggregator meant to record label values and addition information
 type infoAggregator struct {
 	lock           sync.RWMutex
-	name           string
 	labelValues    []string
 	additionalInfo map[string]string
 }
 
-func Info(name string, labelValues []string) MetricAggregator {
+func Info(labelValues []string) MetricAggregator {
 	return &infoAggregator{
-		name:        name,
 		labelValues: labelValues,
 	}
 }
 
-func (a *infoAggregator) Name() string {
-	return a.name
-}
-
 func (a *infoAggregator) AdditionInfo() map[string]string {
 	a.lock.Lock()
 	defer a.lock.Unlock()

modules/collector-source/pkg/metric/aggregator/iratemax.go

@@ -19,17 +19,12 @@ type iRateMaxAggregator struct {
 	max          float64
 }
 
-func IRateMax(name string, labelValues []string) MetricAggregator {
+func IRateMax(labelValues []string) MetricAggregator {
 	return &iRateMaxAggregator{
-		name:        name,
 		labelValues: labelValues,
 	}
 }
 
-func (a *iRateMaxAggregator) Name() string {
-	return a.name
-}
-
 func (a *iRateMaxAggregator) AdditionInfo() map[string]string {
 	return nil
 }

modules/collector-source/pkg/metric/aggregator/maxovertime.go

@@ -8,22 +8,16 @@ import (
 // maxOverTimeAggregator is a MetricAggregator which returns the max value passed to it through the Update function
 type maxOverTimeAggregator struct {
 	lock        sync.Mutex
-	name        string
 	labelValues []string
 	max         float64
 }
 
-func MaxOverTime(name string, labelValues []string) MetricAggregator {
+func MaxOverTime(labelValues []string) MetricAggregator {
 	return &maxOverTimeAggregator{
-		name:        name,
 		labelValues: labelValues,
 	}
 }
 
-func (a *maxOverTimeAggregator) Name() string {
-	return a.name
-}
-
 func (a *maxOverTimeAggregator) AdditionInfo() map[string]string {
 	return nil
 }

modules/collector-source/pkg/metric/aggregator/rate.go

@@ -9,7 +9,6 @@ import (
 // to function properly calls to Update must have a timestamp greater than or equal to the last call to update.
 type rateAggregator struct {
 	lock        sync.Mutex
-	name        string
 	labelValues []string
 	initialized bool
 	initialTime time.Time
@@ -18,17 +17,12 @@ type rateAggregator struct {
 	current     float64
 }
 
-func Rate(name string, labelValues []string) MetricAggregator {
+func Rate(labelValues []string) MetricAggregator {
 	return &rateAggregator{
-		name:        name,
 		labelValues: labelValues,
 	}
 }
 
-func (a *rateAggregator) Name() string {
-	return a.name
-}
-
 func (a *rateAggregator) AdditionInfo() map[string]string {
 	return nil
 }

modules/collector-source/pkg/metric/collector.go

@@ -2,7 +2,6 @@ package metric
 
 import (
 	"maps"
-	"sort"
 	"time"
 
 	"github.com/opencost/opencost/modules/collector-source/pkg/metric/aggregator"
@@ -125,8 +124,7 @@ func (mi *MetricCollector) Update(labels map[string]string, value float64, times
 	}
 	key := util.Hash(labelValues)
 	if mi.metrics[key] == nil {
-		mi.metrics[key] = mi.aggregatorFactory(
-			util.MetricNameFor(mi.metricName, mi.labels, labelValues), labelValues)
+		mi.metrics[key] = mi.aggregatorFactory(labelValues)
 	}
 
 	mi.metrics[key].Update(value, timestamp, additionalInfo)
@@ -138,7 +136,6 @@ func (mi *MetricCollector) Get() []*aggregator.MetricResult {
 		labels := util.ToMap(mi.labels, metric.LabelValues())
 		maps.Copy(labels, metric.AdditionInfo())
 		mr := &aggregator.MetricResult{
-			Name:         metric.Name(),
 			MetricLabels: labels,
 			Values:       metric.Value(),
 		}
@@ -146,10 +143,6 @@ func (mi *MetricCollector) Get() []*aggregator.MetricResult {
 		results = append(results, mr)
 	}
 
-	sort.Slice(results, func(i, j int) bool {
-		return results[i].Name < results[j].Name
-	})
-	
 	return results
 }