Address review comments

elastic · Sep 22, 2023 · 4eefdd9 · 4eefdd9
1 parent 0724a42
commit 4eefdd9
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 125 deletions.
diff --git a/x-pack/metricbeat/module/azure/app_insights/data.go b/x-pack/metricbeat/module/azure/app_insights/data.go
@@ -169,7 +169,9 @@ func groupMetricsByTime(metrics []MetricValue) map[metricTimeKey][]MetricValue {
 
 	for _, metric := range metrics {
 		// The start and end times are truncated to the nearest second.
-		// This is done to ensure that metrics that fall within the same second are grouped together, even if their actual times are slightly different.
+		// This is done to ensure that metrics that fall within the same
+		// second are grouped together, even if their actual time are
+		// slightly different.
 		timeKey := newMetricTimeKey(
 			metric.Start.Time.Truncate(time.Second),
 			metric.End.Time.Truncate(time.Second),
@@ -182,10 +184,13 @@ func groupMetricsByTime(metrics []MetricValue) map[metricTimeKey][]MetricValue {
 
 // groupMetricsByDimension groups the given metrics by their dimension keys.
 func groupMetricsByDimension(metrics []MetricValue) map[string][]MetricValue {
-	keys := make(map[string][]MetricValue)
-	var firstStart, firstEnd *date.Time
+	var (
+		keys                 = make(map[string][]MetricValue)
+		firstStart, firstEnd *date.Time
+		helper               func(metrics []MetricValue)
+	)
 
-	var helper func(metrics []MetricValue)
+	// Review comment: Can you add some more comments to this helper func?
 	helper = func(metrics []MetricValue) {
 		for _, metric := range metrics {
 			dimensionKey := getSortedKeys(metric.SegmentName)
@@ -222,9 +227,7 @@ func groupMetricsByDimension(metrics []MetricValue) map[string][]MetricValue {
 				}
 			} else if dimensionKey != "" {
 				m := metric
-				m.Start = firstStart
-				m.End = firstEnd
-
+				m.Start, m.End = firstStart, firstEnd
 				keys[dimensionKey] = append(keys[dimensionKey], m)
 			}
 		}

diff --git a/x-pack/metricbeat/module/azure/app_insights/data_test.go b/x-pack/metricbeat/module/azure/app_insights/data_test.go
@@ -17,99 +17,62 @@ import (
 	"github.com/elastic/elastic-agent-libs/mapstr"
 )
 
-func newMetricsTest(timestamp1, timestamp2, timestamp3 *date.Time) []MetricValue {
-	return []MetricValue{
+func newMetricsTest(ts ...*date.Time) []MetricValue {
+	type values struct {
+		SegmentName map[string]string
+		Value       map[string]interface{}
+		T           *date.Time
+	}
+
+	const numOfMetricValue = 3
+
+	if numOfMetricValue != len(ts) {
+		panic("number of arguments to newMetricsTest is not correct")
+	}
+
+	vals := [numOfMetricValue]values{
 		{
-			SegmentName: map[string]string{},
-			Value:       map[string]interface{}{},
-			Segments: []MetricValue{
-				{
-					SegmentName: map[string]string{},
-					Value:       map[string]interface{}{},
-					Segments: []MetricValue{
-						{
-							SegmentName: map[string]string{
-								"request_url_host": "",
-							},
-							Value: map[string]interface{}{
-								"users_count.unique": 44,
-							},
-							Segments: nil,
-							Interval: "",
-							Start:    nil,
-							End:      nil,
-						},
-					},
-					Interval: "",
-					Start:    nil,
-					End:      nil,
-				},
-			},
-			Interval: "P5M",
-			Start:    timestamp1,
-			End:      timestamp1,
+			SegmentName: map[string]string{"request_url_host": ""},
+			Value:       map[string]interface{}{"users_count.unique": 44},
+			T:           ts[0],
 		},
 		{
-			SegmentName: map[string]string{},
-			Value:       map[string]interface{}{},
-			Segments: []MetricValue{
-				{
-					SegmentName: map[string]string{},
-					Value:       map[string]interface{}{},
-					Segments: []MetricValue{
-						{
-							SegmentName: map[string]string{
-								"request_url_host": "",
-							},
-							Value: map[string]interface{}{
-								"sessions_count.unique": 44,
-							},
-							Segments: nil,
-							Interval: "",
-							Start:    nil,
-							End:      nil,
-						},
-					},
-					Interval: "",
-					Start:    nil,
-					End:      nil,
-				},
-			},
-			Interval: "P5M",
-			Start:    timestamp2,
-			End:      timestamp2,
+			SegmentName: map[string]string{"request_url_host": ""},
+			Value:       map[string]interface{}{"sessions_count.unique": 44},
+			T:           ts[1],
 		},
 		{
-			SegmentName: map[string]string{},
-			Value:       map[string]interface{}{},
-			Segments: []MetricValue{
-				{
-					SegmentName: map[string]string{},
-					Value:       map[string]interface{}{},
-					Segments: []MetricValue{
-						{
-							SegmentName: map[string]string{
-								"request_url_host": "localhost",
-							},
-							Value: map[string]interface{}{
-								"sessions_count.unique": 44,
+			SegmentName: map[string]string{"request_url_host": "localhost"},
+			Value:       map[string]interface{}{"sessions_count.unique": 44},
+			T:           ts[2],
+		},
+	}
+
+	mv := make([]MetricValue, 0, 3)
+	for i := range vals {
+		mv = append(mv,
+			MetricValue{
+				SegmentName: map[string]string{},
+				Value:       map[string]interface{}{},
+				Segments: []MetricValue{
+					{
+						SegmentName: map[string]string{},
+						Value:       map[string]interface{}{},
+						Segments: []MetricValue{
+							{
+								SegmentName: vals[i].SegmentName,
+								Value:       vals[i].Value,
 							},
-							Segments: nil,
-							Interval: "",
-							Start:    nil,
-							End:      nil,
 						},
 					},
-					Interval: "",
-					Start:    nil,
-					End:      nil,
 				},
+				Interval: "P5M",
+				Start:    vals[i].T, End: vals[i].T,
 			},
-			Interval: "P5M",
-			Start:    timestamp3,
-			End:      timestamp3,
-		},
+		)
 	}
+
+	return mv
 }
 
 func TestGroupMetrics(t *testing.T) {
@@ -128,10 +91,8 @@ func TestGroupMetrics(t *testing.T) {
 				Value: map[string]interface{}{
 					"users_count.unique": 44,
 				},
-				Segments: nil,
-				Interval: "",
-				Start:    timestamp1,
-				End:      timestamp1,
+				Start: timestamp1,
+				End:   timestamp1,
 			},
 			{
 				SegmentName: map[string]string{
@@ -140,10 +101,8 @@ func TestGroupMetrics(t *testing.T) {
 				Value: map[string]interface{}{
 					"sessions_count.unique": 44,
 				},
-				Segments: nil,
-				Interval: "",
-				Start:    timestamp2,
-				End:      timestamp2,
+				Start: timestamp2,
+				End:   timestamp2,
 			},
 		}
 
@@ -155,10 +114,8 @@ func TestGroupMetrics(t *testing.T) {
 				Value: map[string]interface{}{
 					"sessions_count.unique": 44,
 				},
-				Segments: nil,
-				Interval: "",
-				Start:    timestamp3,
-				End:      timestamp3,
+				Start: timestamp3,
+				End:   timestamp3,
 			},
 		}
 
@@ -205,10 +162,8 @@ func TestGroupMetrics(t *testing.T) {
 				Value: map[string]interface{}{
 					"users_count.unique": 44,
 				},
-				Segments: nil,
-				Interval: "",
-				Start:    timestamp1,
-				End:      timestamp1,
+				Start: timestamp1,
+				End:   timestamp1,
 			},
 			{
 				SegmentName: map[string]string{
@@ -217,10 +172,8 @@ func TestGroupMetrics(t *testing.T) {
 				Value: map[string]interface{}{
 					"sessions_count.unique": 44,
 				},
-				Segments: nil,
-				Interval: "",
-				Start:    timestamp2,
-				End:      timestamp2,
+				Start: timestamp2,
+				End:   timestamp2,
 			},
 		}
 
@@ -232,10 +185,8 @@ func TestGroupMetrics(t *testing.T) {
 				Value: map[string]interface{}{
 					"sessions_count.unique": 44,
 				},
-				Segments: nil,
-				Interval: "",
-				Start:    timestamp3,
-				End:      timestamp3,
+				Start: timestamp3,
+				End:   timestamp3,
 			},
 		}
 
@@ -247,10 +198,8 @@ func TestGroupMetrics(t *testing.T) {
 				Value: map[string]interface{}{
 					"users_count.unique": 44,
 				},
-				Segments: nil,
-				Interval: "",
-				Start:    timestamp1,
-				End:      timestamp1,
+				Start: timestamp1,
+				End:   timestamp1,
 			},
 		}
 
@@ -262,10 +211,8 @@ func TestGroupMetrics(t *testing.T) {
 				Value: map[string]interface{}{
 					"sessions_count.unique": 44,
 				},
-				Segments: nil,
-				Interval: "",
-				Start:    timestamp2,
-				End:      timestamp2,
+				Start: timestamp2,
+				End:   timestamp2,
 			},
 		}
 
@@ -277,10 +224,8 @@ func TestGroupMetrics(t *testing.T) {
 				Value: map[string]interface{}{
 					"sessions_count.unique": 44,
 				},
-				Segments: nil,
-				Interval: "",
-				Start:    timestamp3,
-				End:      timestamp3,
+				Start: timestamp3,
+				End:   timestamp3,
 			},
 		}