Skip to content

Commit

Permalink
[chore][exporter/elasticsearch] Fix flaky recorder assertions (#36255)
Browse files Browse the repository at this point in the history 
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

Incorrect rec.WaitItems call causes test to be flaky. Refactor to avoid
future occurrences.

<!-- Issue number (e.g. #1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes #35924

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->
  • Loading branch information
@carsonip
carsonip authored Nov 8, 2024
1 parent 8ec6180 commit f0c1736
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 31 deletions.
40 changes: 11 additions & 29 deletions exporter/elasticsearchexporter/exporter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,6 @@ func TestExporterLogs(t *testing.T) {
)
tc.body.CopyTo(logs.ResourceLogs().At(0).ScopeLogs().At(0).LogRecords().At(0).Body())
mustSendLogs(t, exporter, logs)
rec.WaitItems(1)

expected := []itemRequest{
{
Expand All @@ -514,7 +513,7 @@ func TestExporterLogs(t *testing.T) {
},
}

assertItemsEqual(t, expected, rec.Items(), false)
assertRecordedItems(t, expected, rec, false)
}

})
Expand Down Expand Up @@ -889,8 +888,6 @@ func TestExporterMetrics(t *testing.T) {

mustSendMetrics(t, exporter, metrics)

rec.WaitItems(8)

expected := []itemRequest{
{
Action: []byte(`{"create":{"_index":"metrics-generic-bar"}}`),
Expand Down Expand Up @@ -926,7 +923,7 @@ func TestExporterMetrics(t *testing.T) {
},
}

assertItemsEqual(t, expected, rec.Items(), false)
assertRecordedItems(t, expected, rec, false)
})

t.Run("publish histogram", func(t *testing.T) {
Expand Down Expand Up @@ -959,8 +956,6 @@ func TestExporterMetrics(t *testing.T) {

mustSendMetrics(t, exporter, metrics)

rec.WaitItems(2)

expected := []itemRequest{
{
Action: []byte(`{"create":{"_index":"metrics-generic-default"}}`),
Expand All @@ -972,7 +967,7 @@ func TestExporterMetrics(t *testing.T) {
},
}

assertItemsEqual(t, expected, rec.Items(), false)
assertRecordedItems(t, expected, rec, false)
})

t.Run("publish exponential histogram", func(t *testing.T) {
Expand Down Expand Up @@ -1005,16 +1000,14 @@ func TestExporterMetrics(t *testing.T) {

mustSendMetrics(t, exporter, metrics)

rec.WaitItems(1)

expected := []itemRequest{
{
Action: []byte(`{"create":{"_index":"metrics-generic-default"}}`),
Document: []byte(`{"@timestamp":"1970-01-01T00:00:00.000000000Z","data_stream":{"dataset":"generic","namespace":"default","type":"metrics"},"metric":{"foo":{"counts":[1,1,2,1,1],"values":[-24.0,-3.0,0.0,6.0,12.0]}}}`),
},
}

assertItemsEqual(t, expected, rec.Items(), false)
assertRecordedItems(t, expected, rec, false)
})

t.Run("publish histogram cumulative temporality", func(t *testing.T) {
Expand Down Expand Up @@ -1113,8 +1106,6 @@ func TestExporterMetrics(t *testing.T) {
err := exporter.ConsumeMetrics(context.Background(), metrics)
assert.NoError(t, err)

rec.WaitItems(2)

expected := []itemRequest{
{
Action: []byte(`{"create":{"_index":"metrics-generic-default"}}`),
Expand All @@ -1126,7 +1117,7 @@ func TestExporterMetrics(t *testing.T) {
},
}

assertItemsEqual(t, expected, rec.Items(), false)
assertRecordedItems(t, expected, rec, false)
})

t.Run("otel mode", func(t *testing.T) {
Expand Down Expand Up @@ -1176,8 +1167,6 @@ func TestExporterMetrics(t *testing.T) {

mustSendMetrics(t, exporter, metrics)

rec.WaitItems(2)

expected := []itemRequest{
{
Action: []byte(`{"create":{"_index":"metrics-generic.otel-default","dynamic_templates":{"metrics.metric.foo":"histogram"}}}`),
Expand All @@ -1197,7 +1186,7 @@ func TestExporterMetrics(t *testing.T) {
},
}

assertItemsEqual(t, expected, rec.Items(), false)
assertRecordedItems(t, expected, rec, false)
})

t.Run("otel mode attribute array value", func(t *testing.T) {
Expand Down Expand Up @@ -1259,15 +1248,14 @@ func TestExporterMetrics(t *testing.T) {

mustSendMetrics(t, exporter, metrics)

rec.WaitItems(1)
expected := []itemRequest{
{
Action: []byte(`{"create":{"_index":"metrics-generic.otel-default","dynamic_templates":{"metrics.sum":"gauge_long","metrics.summary":"summary"}}}`),
Document: []byte(`{"@timestamp":"1970-01-01T00:00:00.000000000Z","_doc_count":10,"data_stream":{"dataset":"generic.otel","namespace":"default","type":"metrics"},"metrics":{"sum":0,"summary":{"sum":1.0,"value_count":10}},"resource":{"dropped_attributes_count":0},"scope":{"dropped_attributes_count":0}}`),
},
}

assertItemsEqual(t, expected, rec.Items(), false)
assertRecordedItems(t, expected, rec, false)
})

t.Run("otel mode aggregate_metric_double hint", func(t *testing.T) {
Expand Down Expand Up @@ -1310,7 +1298,6 @@ func TestExporterMetrics(t *testing.T) {

mustSendMetrics(t, exporter, metrics)

rec.WaitItems(1)
expected := []itemRequest{
{
Action: []byte(`{"create":{"_index":"metrics-generic.otel-default","dynamic_templates":{"metrics.histogram.summary":"summary"}}}`),
Expand All @@ -1322,7 +1309,7 @@ func TestExporterMetrics(t *testing.T) {
},
}

assertItemsEqual(t, expected, rec.Items(), false)
assertRecordedItems(t, expected, rec, false)
})

t.Run("otel mode metric name conflict", func(t *testing.T) {
Expand Down Expand Up @@ -1354,15 +1341,14 @@ func TestExporterMetrics(t *testing.T) {

mustSendMetrics(t, exporter, metrics)

rec.WaitItems(1)
expected := []itemRequest{
{
Action: []byte(`{"create":{"_index":"metrics-generic.otel-default","dynamic_templates":{"metrics.foo.bar":"gauge_long","metrics.foo":"gauge_long","metrics.foo.bar.baz":"gauge_long"}}}`),
Document: []byte(`{"@timestamp":"1970-01-01T00:00:00.000000000Z","data_stream":{"dataset":"generic.otel","namespace":"default","type":"metrics"},"metrics":{"foo":0,"foo.bar":0,"foo.bar.baz":0},"resource":{"dropped_attributes_count":0},"scope":{"dropped_attributes_count":0}}`),
},
}

assertItemsEqual(t, expected, rec.Items(), false)
assertRecordedItems(t, expected, rec, false)
})

t.Run("otel mode attribute key prefix conflict", func(t *testing.T) {
Expand Down Expand Up @@ -1422,8 +1408,6 @@ func TestExporterMetrics(t *testing.T) {

mustSendMetrics(t, exporter, metrics)

rec.WaitItems(2)

expected := []itemRequest{
{
Action: []byte(`{"create":{"_index":"metrics-generic-default"}}`),
Expand All @@ -1435,7 +1419,7 @@ func TestExporterMetrics(t *testing.T) {
},
}

assertItemsEqual(t, expected, rec.Items(), false)
assertRecordedItems(t, expected, rec, false)
})
}

Expand Down Expand Up @@ -1644,8 +1628,6 @@ func TestExporterTraces(t *testing.T) {

mustSendTraces(t, exporter, traces)

rec.WaitItems(2)

expected := []itemRequest{
{
Action: []byte(`{"create":{"_index":"traces-generic.otel-default"}}`),
Expand All @@ -1657,7 +1639,7 @@ func TestExporterTraces(t *testing.T) {
},
}

assertItemsEqual(t, expected, rec.Items(), false)
assertRecordedItems(t, expected, rec, false)
})

t.Run("otel mode attribute array value", func(t *testing.T) {
Expand Down
10 changes: 8 additions & 2 deletions exporter/elasticsearchexporter/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

"github.com/klauspost/compress/gzip"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/plog"
"go.opentelemetry.io/collector/pdata/pmetric"
Expand All @@ -37,7 +38,12 @@ func itemRequestsSortFunc(a, b itemRequest) int {
return comp
}

func assertItemsEqual(t *testing.T, expected, actual []itemRequest, assertOrder bool) { // nolint:unparam
func assertRecordedItems(t *testing.T, expected []itemRequest, recorder *bulkRecorder, assertOrder bool) { // nolint:unparam
recorder.WaitItems(len(expected))
assertItemRequests(t, expected, recorder.Items(), assertOrder)
}

func assertItemRequests(t *testing.T, expected, actual []itemRequest, assertOrder bool) { // nolint:unparam
expectedItems := expected
actualItems := actual
if !assertOrder {
Expand All @@ -50,7 +56,7 @@ func assertItemsEqual(t *testing.T, expected, actual []itemRequest, assertOrder
slices.SortFunc(actualItems, itemRequestsSortFunc)
}

assert.Equal(t, len(expectedItems), len(actualItems), "want %d items, got %d", len(expectedItems), len(actualItems))
require.Equal(t, len(expectedItems), len(actualItems), "want %d items, got %d", len(expectedItems), len(actualItems))
for i, want := range expectedItems {
got := actualItems[i]
assert.JSONEq(t, string(want.Action), string(got.Action), "item %d action", i)
Expand Down

0 comments on commit f0c1736

Please sign in to comment.