Skip to content

Commit

Permalink
Address comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
@zzhlogin
zzhlogin committed Dec 11, 2024
1 parent 75a5e47 commit 1806511
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 27 deletions.
2 changes: 1 addition & 1 deletion .chloggen/split-emf-log-when-buckets-larger-than-100.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ component: awsemfexporter
note: Split EMF log to multiple log splits when buckets larger than 100.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [242]
issues: [36771]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
Expand Down
6 changes: 3 additions & 3 deletions exporter/awsemfexporter/datapoint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2035,7 +2035,7 @@ func TestGetDataPoints(t *testing.T) {
rm := tc.metric.ResourceMetrics().At(0)
metrics := rm.ScopeMetrics().At(0).Metrics()
metric := metrics.At(metrics.Len() - 1)
metadata := generateTestMetricMetadata("namespace", time.Now().UnixNano()/int64(time.Millisecond), "log-group", "log-stream", "cloudwatch-otel", metric.Type())
metadata := generateTestMetricMetadata("namespace", time.Now().UnixNano()/int64(time.Millisecond), "log-group", "log-stream", "cloudwatch-otel", metric.Type(), 0)

t.Run(tc.name, func(t *testing.T) {
if tc.isPrometheusMetrics {
Expand Down Expand Up @@ -2095,7 +2095,7 @@ func TestGetDataPoints(t *testing.T) {
metric := pmetric.NewMetric()
metric.SetName("foo")
metric.SetUnit("Count")
metadata := generateTestMetricMetadata("namespace", time.Now().UnixNano()/int64(time.Millisecond), "log-group", "log-stream", "cloudwatch-otel", pmetric.MetricTypeEmpty)
metadata := generateTestMetricMetadata("namespace", time.Now().UnixNano()/int64(time.Millisecond), "log-group", "log-stream", "cloudwatch-otel", pmetric.MetricTypeEmpty, 0)
obs, logs := observer.New(zap.WarnLevel)
logger := zap.New(obs)

Expand Down Expand Up @@ -2139,7 +2139,7 @@ func benchmarkGetAndCalculateDeltaDataPoints(b *testing.B, bucketLength int) {
b.ResetTimer()
for n := 0; n < b.N; n++ {
for i := 0; i < metrics.Len(); i++ {
metadata := generateTestMetricMetadata("namespace", time.Now().UnixNano()/int64(time.Millisecond), "log-group", "log-stream", "cloudwatch-otel", metrics.At(i).Type())
metadata := generateTestMetricMetadata("namespace", time.Now().UnixNano()/int64(time.Millisecond), "log-group", "log-stream", "cloudwatch-otel", metrics.At(i).Type(), 0)
dps := getDataPoints(metrics.At(i), metadata, zap.NewNop())

for i := 0; i < dps.Len(); i++ {
Expand Down
43 changes: 20 additions & 23 deletions exporter/awsemfexporter/grouped_metric_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestAddToGroupedMetric(t *testing.T) {

for i := 0; i < metrics.Len(); i++ {
err := addToGroupedMetric(metrics.At(i), groupedMetrics,
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, metrics.At(i).Type()),
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, metrics.At(i).Type(), 0),
true,
nil,
testCfg,
Expand All @@ -121,7 +121,7 @@ func TestAddToGroupedMetric(t *testing.T) {
assert.Equal(t, len(tc.expectedMetricInfo), len(v.metrics))
assert.Equal(t, tc.expectedMetricInfo, v.metrics)
assert.Len(t, v.labels, 2)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, tc.expectedMetricType), v.metadata)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, tc.expectedMetricType, 0), v.metadata)
assert.Equal(t, tc.expectedLabels, v.labels)
}
})
Expand Down Expand Up @@ -150,7 +150,7 @@ func TestAddToGroupedMetric(t *testing.T) {
for i := 0; i < metrics.Len(); i++ {
err := addToGroupedMetric(metrics.At(i),
groupedMetrics,
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, metrics.At(i).Type()),
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, metrics.At(i).Type(), 0),
true,
nil,
testCfg,
Expand All @@ -165,19 +165,19 @@ func TestAddToGroupedMetric(t *testing.T) {
case "int-gauge", "double-gauge":
assert.Len(t, group.metrics, 2)
assert.Equal(t, "Count", metricInfo.unit)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeGauge), group.metadata)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeGauge, 0), group.metadata)
case "int-sum", "double-sum":
assert.Len(t, group.metrics, 2)
assert.Equal(t, "Count", metricInfo.unit)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeSum), group.metadata)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeSum, 0), group.metadata)
case "histogram":
assert.Len(t, group.metrics, 1)
assert.Equal(t, "Seconds", metricInfo.unit)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeHistogram), group.metadata)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeHistogram, 0), group.metadata)
case "summary":
assert.Len(t, group.metrics, 1)
assert.Equal(t, "Seconds", metricInfo.unit)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeSummary), group.metadata)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeSummary, 0), group.metadata)
default:
assert.Fail(t, fmt.Sprintf("Unhandled metric %s not expected", metricName))
}
Expand Down Expand Up @@ -222,7 +222,7 @@ func TestAddToGroupedMetric(t *testing.T) {
for i := 0; i < metrics.Len(); i++ {
err := addToGroupedMetric(metrics.At(i),
groupedMetrics,
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, metrics.At(i).Type()),
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, metrics.At(i).Type(), 0),
true,
nil,
testCfg,
Expand All @@ -237,19 +237,19 @@ func TestAddToGroupedMetric(t *testing.T) {
case "int-gauge", "double-gauge":
assert.Len(t, group.metrics, 2)
assert.Equal(t, "Count", metricInfo.unit)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeGauge), group.metadata)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeGauge, 0), group.metadata)
case "int-sum", "double-sum":
assert.Len(t, group.metrics, 2)
assert.Equal(t, "Count", metricInfo.unit)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeSum), group.metadata)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeSum, 0), group.metadata)
case "histogram":
assert.Len(t, group.metrics, 1)
assert.Equal(t, "Seconds", metricInfo.unit)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeHistogram), group.metadata)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeHistogram, 0), group.metadata)
case "summary":
assert.Len(t, group.metrics, 1)
assert.Equal(t, "Seconds", metricInfo.unit)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeSummary), group.metadata)
assert.Equal(t, generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeSummary, 0), group.metadata)
default:
assert.Fail(t, fmt.Sprintf("Unhandled metric %s not expected", metricName))
}
Expand All @@ -270,7 +270,7 @@ func TestAddToGroupedMetric(t *testing.T) {
ilms := otelMetrics.ResourceMetrics().At(0).ScopeMetrics()
metric := ilms.At(0).Metrics().At(0)

metricMetadata1 := generateTestMetricMetadata(namespace, timestamp, "log-group-1", logStreamName, instrumentationLibName, metric.Type())
metricMetadata1 := generateTestMetricMetadata(namespace, timestamp, "log-group-1", logStreamName, instrumentationLibName, metric.Type(), 0)
err := addToGroupedMetric(metric,
groupedMetrics,
metricMetadata1,
Expand All @@ -286,6 +286,7 @@ func TestAddToGroupedMetric(t *testing.T) {
logStreamName,
instrumentationLibName,
metric.Type(),
0,
)
err = addToGroupedMetric(metric, groupedMetrics, metricMetadata2, true, nil, testCfg, emfCalcs)
assert.NoError(t, err)
Expand Down Expand Up @@ -340,7 +341,7 @@ func TestAddToGroupedMetric(t *testing.T) {
for i := 0; i < metrics.Len(); i++ {
err := addToGroupedMetric(metrics.At(i),
groupedMetrics,
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, metrics.At(i).Type()),
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, metrics.At(i).Type(), 0),
true,
nil,
testCfg,
Expand Down Expand Up @@ -382,7 +383,7 @@ func TestAddToGroupedMetric(t *testing.T) {
testCfg.logger = zap.New(obs)
err := addToGroupedMetric(metric,
groupedMetrics,
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeEmpty),
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, pmetric.MetricTypeEmpty, 0),
true,
nil,
testCfg,
Expand Down Expand Up @@ -423,7 +424,7 @@ func TestAddToGroupedMetric(t *testing.T) {
for i := 0; i < metrics.Len(); i++ {
err := addToGroupedMetric(metrics.At(i),
groupedMetrics,
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, metrics.At(i).Type()),
generateTestMetricMetadata(namespace, timestamp, logGroup, logStreamName, instrumentationLibName, metrics.At(i).Type(), 0),
true,
nil,
testCfg,
Expand Down Expand Up @@ -500,7 +501,7 @@ func BenchmarkAddToGroupedMetric(b *testing.B) {
for n := 0; n < b.N; n++ {
groupedMetrics := make(map[any]*groupedMetric)
for i := 0; i < numMetrics; i++ {
metadata := generateTestMetricMetadata("namespace", int64(1596151098037), "log-group", "log-stream", "cloudwatch-otel", metrics.At(i).Type())
metadata := generateTestMetricMetadata("namespace", int64(1596151098037), "log-group", "log-stream", "cloudwatch-otel", metrics.At(i).Type(), 0)
err := addToGroupedMetric(metrics.At(i), groupedMetrics, metadata, true, nil, testCfg, emfCalcs)
assert.NoError(b, err)
}
Expand Down Expand Up @@ -550,11 +551,7 @@ func TestTranslateUnit(t *testing.T) {
assert.Equal(t, "Count", v)
}

func generateTestMetricMetadata(namespace string, timestamp int64, logGroup, logStreamName, instrumentationScopeName string, metricType pmetric.MetricType, batchIndex ...int) cWMetricMetadata {
mIndex := 0
if len(batchIndex) > 0 {
mIndex = batchIndex[0]
}
func generateTestMetricMetadata(namespace string, timestamp int64, logGroup, logStreamName, instrumentationScopeName string, metricType pmetric.MetricType, batchIndex int) cWMetricMetadata {
return cWMetricMetadata{
receiver: prometheusReceiver,
groupedMetricMetadata: groupedMetricMetadata{
Expand All @@ -563,7 +560,7 @@ func generateTestMetricMetadata(namespace string, timestamp int64, logGroup, log
logGroup: logGroup,
logStream: logStreamName,
metricDataType: metricType,
batchIndex: mIndex,
batchIndex: batchIndex,
},
instrumentationScopeName: instrumentationScopeName,
}
Expand Down

0 comments on commit 1806511

Please sign in to comment.