Reduce uncessary pointer usage in getmetricdata code path (#1091)

prometheus-community · Aug 4, 2023 · b5423b4 · b5423b4
1 parent 651801a
commit b5423b4
Show file tree

Hide file tree

Showing 9 changed files with 34 additions and 30 deletions.
diff --git a/pkg/clients/cloudwatch/client.go b/pkg/clients/cloudwatch/client.go
@@ -17,16 +17,16 @@ type Client interface {
 
 	// GetMetricData returns the output of the GetMetricData CloudWatch API.
 	// Results pagination is handled automatically.
-	GetMetricData(ctx context.Context, logger logging.Logger, getMetricData []*model.CloudwatchData, namespace string, length int64, delay int64, configuredRoundingPeriod *int64) []*MetricDataResult
+	GetMetricData(ctx context.Context, logger logging.Logger, getMetricData []*model.CloudwatchData, namespace string, length int64, delay int64, configuredRoundingPeriod *int64) []MetricDataResult
 
 	// GetMetricStatistics returns the output of the GetMetricStatistics CloudWatch API.
 	GetMetricStatistics(ctx context.Context, logger logging.Logger, dimensions []*model.Dimension, namespace string, metric *config.Metric) []*model.Datapoint
 }
 
 type MetricDataResult struct {
-	ID        *string
-	Datapoint *float64
-	Timestamp *time.Time
+	ID        string
+	Datapoint float64
+	Timestamp time.Time
 }
 
 type limitedConcurrencyClient struct {
@@ -48,7 +48,7 @@ func (c limitedConcurrencyClient) GetMetricStatistics(ctx context.Context, logge
 	return res
 }
 
-func (c limitedConcurrencyClient) GetMetricData(ctx context.Context, logger logging.Logger, getMetricData []*model.CloudwatchData, namespace string, length int64, delay int64, configuredRoundingPeriod *int64) []*MetricDataResult {
+func (c limitedConcurrencyClient) GetMetricData(ctx context.Context, logger logging.Logger, getMetricData []*model.CloudwatchData, namespace string, length int64, delay int64, configuredRoundingPeriod *int64) []MetricDataResult {
 	c.sem <- struct{}{}
 	res := c.client.GetMetricData(ctx, logger, getMetricData, namespace, length, delay, configuredRoundingPeriod)
 	<-c.sem

diff --git a/pkg/clients/cloudwatch/v1/client.go b/pkg/clients/cloudwatch/v1/client.go
@@ -89,7 +89,7 @@ func toModelDimensions(dimensions []*cloudwatch.Dimension) []*model.Dimension {
 	return modelDimensions
 }
 
-func (c client) GetMetricData(ctx context.Context, logger logging.Logger, getMetricData []*model.CloudwatchData, namespace string, length int64, delay int64, configuredRoundingPeriod *int64) []*cloudwatch_client.MetricDataResult {
+func (c client) GetMetricData(ctx context.Context, logger logging.Logger, getMetricData []*model.CloudwatchData, namespace string, length int64, delay int64, configuredRoundingPeriod *int64) []cloudwatch_client.MetricDataResult {
 	var resp cloudwatch.GetMetricDataOutput
 	filter := createGetMetricDataInput(getMetricData, &namespace, length, delay, configuredRoundingPeriod, logger)
 	if c.logger.IsDebugEnabled() {
@@ -116,15 +116,15 @@ func (c client) GetMetricData(ctx context.Context, logger logging.Logger, getMet
 	return toMetricDataResult(resp)
 }
 
-func toMetricDataResult(resp cloudwatch.GetMetricDataOutput) []*cloudwatch_client.MetricDataResult {
-	output := make([]*cloudwatch_client.MetricDataResult, 0, len(resp.MetricDataResults))
+func toMetricDataResult(resp cloudwatch.GetMetricDataOutput) []cloudwatch_client.MetricDataResult {
+	output := make([]cloudwatch_client.MetricDataResult, 0, len(resp.MetricDataResults))
 	for _, metricDataResult := range resp.MetricDataResults {
-		mappedResult := cloudwatch_client.MetricDataResult{ID: metricDataResult.Id}
+		mappedResult := cloudwatch_client.MetricDataResult{ID: *metricDataResult.Id}
 		if len(metricDataResult.Values) > 0 {
-			mappedResult.Datapoint = metricDataResult.Values[0]
-			mappedResult.Timestamp = metricDataResult.Timestamps[0]
+			mappedResult.Datapoint = *metricDataResult.Values[0]
+			mappedResult.Timestamp = *metricDataResult.Timestamps[0]
 		}
-		output = append(output, &mappedResult)
+		output = append(output, mappedResult)
 	}
 	return output
 }

diff --git a/pkg/clients/cloudwatch/v2/client.go b/pkg/clients/cloudwatch/v2/client.go
@@ -90,7 +90,7 @@ func toModelDimensions(dimensions []types.Dimension) []*model.Dimension {
 	return modelDimensions
 }
 
-func (c client) GetMetricData(ctx context.Context, logger logging.Logger, getMetricData []*model.CloudwatchData, namespace string, length int64, delay int64, configuredRoundingPeriod *int64) []*cloudwatch_client.MetricDataResult {
+func (c client) GetMetricData(ctx context.Context, logger logging.Logger, getMetricData []*model.CloudwatchData, namespace string, length int64, delay int64, configuredRoundingPeriod *int64) []cloudwatch_client.MetricDataResult {
 	filter := createGetMetricDataInput(logger, getMetricData, &namespace, length, delay, configuredRoundingPeriod)
 	var resp cloudwatch.GetMetricDataOutput
 
@@ -120,15 +120,15 @@ func (c client) GetMetricData(ctx context.Context, logger logging.Logger, getMet
 	return toMetricDataResult(resp)
 }
 
-func toMetricDataResult(resp cloudwatch.GetMetricDataOutput) []*cloudwatch_client.MetricDataResult {
-	output := make([]*cloudwatch_client.MetricDataResult, 0, len(resp.MetricDataResults))
+func toMetricDataResult(resp cloudwatch.GetMetricDataOutput) []cloudwatch_client.MetricDataResult {
+	output := make([]cloudwatch_client.MetricDataResult, 0, len(resp.MetricDataResults))
 	for _, metricDataResult := range resp.MetricDataResults {
-		mappedResult := cloudwatch_client.MetricDataResult{ID: metricDataResult.Id}
+		mappedResult := cloudwatch_client.MetricDataResult{ID: *metricDataResult.Id}
 		if len(metricDataResult.Values) > 0 {
-			mappedResult.Datapoint = &metricDataResult.Values[0]
-			mappedResult.Timestamp = &metricDataResult.Timestamps[0]
+			mappedResult.Datapoint = metricDataResult.Values[0]
+			mappedResult.Timestamp = metricDataResult.Timestamps[0]
 		}
-		output = append(output, &mappedResult)
+		output = append(output, mappedResult)
 	}
 	return output
 }

diff --git a/pkg/clients/v2/factory_test.go b/pkg/clients/v2/factory_test.go
@@ -429,7 +429,7 @@ func (t testClient) ListMetrics(_ context.Context, _ string, _ *config.Metric, _
 	return nil, nil
 }
 
-func (t testClient) GetMetricData(_ context.Context, _ logging.Logger, _ []*model.CloudwatchData, _ string, _ int64, _ int64, _ *int64) []*cloudwatch_client.MetricDataResult {
+func (t testClient) GetMetricData(_ context.Context, _ logging.Logger, _ []*model.CloudwatchData, _ string, _ int64, _ int64, _ *int64) []cloudwatch_client.MetricDataResult {
 	return nil
 }
 

diff --git a/pkg/job/custom.go b/pkg/job/custom.go
@@ -55,9 +55,11 @@ func runCustomNamespaceJob(
 			if data != nil {
 				output := make([]*model.CloudwatchData, 0)
 				for _, result := range data {
-					getMetricData, err := findGetMetricDataByIDForCustomNamespace(input, *result.ID)
+					getMetricData, err := findGetMetricDataByIDForCustomNamespace(input, result.ID)
 					if err == nil {
-						getMetricData.GetMetricDataPoint = result.Datapoint
+						// Copy to avoid a loop closure bug
+						dataPoint := result.Datapoint
+						getMetricData.GetMetricDataPoint = &dataPoint
 						getMetricData.GetMetricDataTimestamps = result.Timestamp
 						output = append(output, getMetricData)
 					}

diff --git a/pkg/job/discovery.go b/pkg/job/discovery.go
@@ -71,7 +71,7 @@ func runDiscoveryJob(
 	g.SetLimit(concurrencyLimit)
 
 	mu := sync.Mutex{}
-	getMetricDataOutput := make([][]*cloudwatch.MetricDataResult, 0, partitionSize)
+	getMetricDataOutput := make([][]cloudwatch.MetricDataResult, 0, partitionSize)
 
 	count := 0
 	for i := 0; i < metricDataLength; i += maxMetricCount {
@@ -117,12 +117,14 @@ func runDiscoveryJob(
 			continue
 		}
 		for _, metricDataResult := range data {
-			idx := findGetMetricDataByID(getMetricDatas, *metricDataResult.ID)
+			idx := findGetMetricDataByID(getMetricDatas, metricDataResult.ID)
 			if idx == -1 {
-				logger.Warn("GetMetricData returned unknown metric ID", "metric_id", *metricDataResult.ID)
+				logger.Warn("GetMetricData returned unknown metric ID", "metric_id", metricDataResult.ID)
 				continue
 			}
-			getMetricDatas[idx].GetMetricDataPoint = metricDataResult.Datapoint
+			// Copy to avoid a loop closure bug
+			dataPoint := metricDataResult.Datapoint
+			getMetricDatas[idx].GetMetricDataPoint = &dataPoint
 			getMetricDatas[idx].GetMetricDataTimestamps = metricDataResult.Timestamp
 			getMetricDatas[idx].MetricID = nil // mark as processed
 		}

diff --git a/pkg/model/model.go b/pkg/model/model.go
@@ -67,7 +67,7 @@ type CloudwatchData struct {
 	Statistics              []string
 	Points                  []*Datapoint
 	GetMetricDataPoint      *float64
-	GetMetricDataTimestamps *time.Time
+	GetMetricDataTimestamps time.Time
 	NilToZero               *bool
 	AddCloudwatchTimestamp  *bool
 	CustomTags              []Tag

diff --git a/pkg/promutil/migrate.go b/pkg/promutil/migrate.go
@@ -106,7 +106,7 @@ func BuildMetrics(cwd []*model.CloudwatchData, labelsSnakeCase bool, logger logg
 
 func getDatapoint(cwd *model.CloudwatchData, statistic string) (*float64, time.Time, error) {
 	if cwd.GetMetricDataPoint != nil {
-		return cwd.GetMetricDataPoint, *cwd.GetMetricDataTimestamps, nil
+		return cwd.GetMetricDataPoint, cwd.GetMetricDataTimestamps, nil
 	}
 	var averageDataPoints []*model.Datapoint
 

diff --git a/pkg/promutil/migrate_test.go b/pkg/promutil/migrate_test.go
@@ -193,7 +193,7 @@ func TestBuildMetrics(t *testing.T) {
 					},
 					NilToZero:               aws.Bool(false),
 					GetMetricDataPoint:      aws.Float64(1),
-					GetMetricDataTimestamps: aws.Time(ts),
+					GetMetricDataTimestamps: ts,
 					ID:                      aws.String("arn:aws:elasticache:us-east-1:123456789012:cluster:redis-cluster"),
 					Region:                  aws.String("us-east-1"),
 					AccountID:               aws.String("123456789012"),
@@ -238,7 +238,7 @@ func TestBuildMetrics(t *testing.T) {
 					},
 					NilToZero:               aws.Bool(false),
 					GetMetricDataPoint:      aws.Float64(1),
-					GetMetricDataTimestamps: aws.Time(ts),
+					GetMetricDataTimestamps: ts,
 					ID:                      aws.String("arn:aws:elasticache:us-east-1:123456789012:cluster:redis-cluster"),
 					Region:                  aws.String("us-east-1"),
 					AccountID:               aws.String("123456789012"),