Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[flaky test][exporter/awsemf] TestAddToGroupedMetric/* is flaky #36727

Open
pjanotti opened this issue Dec 9, 2024 · 2 comments
Open

[flaky test][exporter/awsemf] TestAddToGroupedMetric/* is flaky #36727

pjanotti opened this issue Dec 9, 2024 · 2 comments
Assignees
@zzhlogin
Labels
exporter/awsemf awsemf exporter

Comments

@pjanotti
Copy link
Contributor

pjanotti commented Dec 9, 2024

Component(s)

exporter/awsemf

Describe the issue you're reporting

https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/12240079784/job/34142173462?pr=36726#step:9:148

=== FAIL: . TestAddToGroupedMetric/Duplicate_metric_names_with_different_metricIndex (0.00s)
    grouped_metric_test.go:440: 
        	Error Trace:	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/exporter/awsemfexporter/grouped_metric_test.go:440
        	Error:      	Not equal: 
        	            	expected: awsemfexporter.cWMetricMetadata{groupedMetricMetadata:awsemfexporter.groupedMetricMetadata{namespace:"namespace", timestampMs:1733762347215, logGroup:"logGroup", logStream:"logStream", metricDataType:4, batchIndex:0, retainInitialValueForDelta:false}, instrumentationScopeName:"cloudwatch-otel", receiver:"prometheus"}
        	            	actual  : awsemfexporter.cWMetricMetadata{groupedMetricMetadata:awsemfexporter.groupedMetricMetadata{namespace:"namespace", timestampMs:1733762347215, logGroup:"logGroup", logStream:"logStream", metricDataType:4, batchIndex:1, retainInitialValueForDelta:false}, instrumentationScopeName:"cloudwatch-otel", receiver:"prometheus"}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
make[2]: *** [../../Makefile.Common:137: test-with-cover] Error 1
make[1]: *** [Makefile:200: exporter/awsemfexporter] Error 2
make: *** [Makefile:136: gotest-with-cover] Error 2
        	            	@@ -7,3 +7,3 @@
        	            	   metricDataType: (pmetric.MetricType) 4,
        	            	-  batchIndex: (int) 0,
        	            	+  batchIndex: (int) 1,
        	            	   retainInitialValueForDelta: (bool) false
        	Test:       	TestAddToGroupedMetric/Duplicate_metric_names_with_different_metricIndex
    grouped_metric_test.go:440: 
        	Error Trace:	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/exporter/awsemfexporter/grouped_metric_test.go:440
        	Error:      	Not equal: 
        	            	expected: awsemfexporter.cWMetricMetadata{groupedMetricMetadata:awsemfexporter.groupedMetricMetadata{namespace:"namespace", timestampMs:1733762347215, logGroup:"logGroup", logStream:"logStream", metricDataType:4, batchIndex:1, retainInitialValueForDelta:false}, instrumentationScopeName:"cloudwatch-otel", receiver:"prometheus"}
        	            	actual  : awsemfexporter.cWMetricMetadata{groupedMetricMetadata:awsemfexporter.groupedMetricMetadata{namespace:"namespace", timestampMs:1733762347215, logGroup:"logGroup", logStream:"logStream", metricDataType:4, batchIndex:0, retainInitialValueForDelta:false}, instrumentationScopeName:"cloudwatch-otel", receiver:"prometheus"}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -7,3 +7,3 @@
        	            	   metricDataType: (pmetric.MetricType) 4,
        	            	-  batchIndex: (int) 1,
        	            	+  batchIndex: (int) 0,
        	            	   retainInitialValueForDelta: (bool) false
        	Test:       	TestAddToGroupedMetric/Duplicate_metric_names_with_different_metricIndex

=== FAIL: . TestAddToGroupedMetric (0.00s)

=== FAIL: . TestAddToGroupedMetric/Duplicate_metric_names_with_different_metricIndex (re-run 1) (0.00s)
    grouped_metric_test.go:440: 
        	Error Trace:	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/exporter/awsemfexporter/grouped_metric_test.go:440
        	Error:      	Not equal: 
        	            	expected: awsemfexporter.cWMetricMetadata{groupedMetricMetadata:awsemfexporter.groupedMetricMetadata{namespace:"namespace", timestampMs:1733762349535, logGroup:"logGroup", logStream:"logStream", metricDataType:4, batchIndex:0, retainInitialValueForDelta:false}, instrumentationScopeName:"cloudwatch-otel", receiver:"prometheus"}
        	            	actual  : awsemfexporter.cWMetricMetadata{groupedMetricMetadata:awsemfexporter.groupedMetricMetadata{namespace:"namespace", timestampMs:1733762349535, logGroup:"logGroup", logStream:"logStream", metricDataType:4, batchIndex:1, retainInitialValueForDelta:false}, instrumentationScopeName:"cloudwatch-otel", receiver:"prometheus"}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -7,3 +7,3 @@
        	            	   metricDataType: (pmetric.MetricType) 4,
        	            	-  batchIndex: (int) 0,
        	            	+  batchIndex: (int) 1,
        	            	   retainInitialValueForDelta: (bool) false
        	Test:       	TestAddToGroupedMetric/Duplicate_metric_names_with_different_metricIndex
    grouped_metric_test.go:440: 
        	Error Trace:	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/exporter/awsemfexporter/grouped_metric_test.go:440
        	Error:      	Not equal: 
        	            	expected: awsemfexporter.cWMetricMetadata{groupedMetricMetadata:awsemfexporter.groupedMetricMetadata{namespace:"namespace", timestampMs:1733762349535, logGroup:"logGroup", logStream:"logStream", metricDataType:4, batchIndex:1, retainInitialValueForDelta:false}, instrumentationScopeName:"cloudwatch-otel", receiver:"prometheus"}
        	            	actual  : awsemfexporter.cWMetricMetadata{groupedMetricMetadata:awsemfexporter.groupedMetricMetadata{namespace:"namespace", timestampMs:1733762349535, logGroup:"logGroup", logStream:"logStream", metricDataType:4, batchIndex:0, retainInitialValueForDelta:false}, instrumentationScopeName:"cloudwatch-otel", receiver:"prometheus"}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -7,3 +7,3 @@
        	            	   metricDataType: (pmetric.MetricType) 4,
        	            	-  batchIndex: (int) 1,
        	            	+  batchIndex: (int) 0,
        	            	   retainInitialValueForDelta: (bool) false
        	Test:       	TestAddToGroupedMetric/Duplicate_metric_names_with_different_metricIndex

=== FAIL: . TestAddToGroupedMetric (re-run 1) (0.00s)
@pjanotti pjanotti added the needs triage New item requiring triage label Dec 9, 2024
@github-actions github-actions bot added the exporter/awsemf awsemf exporter label Dec 9, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 9, 2024

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@pjanotti pjanotti removed the needs triage New item requiring triage label Dec 9, 2024
@github-actions github-actions bot mentioned this issue Dec 10, 2024
Closed
This was referenced Dec 10, 2024
Merged
Merged
mx-psi pushed a commit that referenced this issue Dec 10, 2024
@songy23
Revert "[exporter/awsemfexporter]Split EMF log with larger than 100 b…
ffd031a 
…uckets." (#36763)

Reverts #36336


leads to test failures, see
#36727
@zzhlogin zzhlogin mentioned this issue Dec 10, 2024
Merged
@zzhlogin
Copy link
Contributor

zzhlogin commented Dec 10, 2024 •
edited
Loading

The flaky test in TestAddToGroupedMetric/Duplicate_metric_names_with_different_metricIndex was caused by the fact that it was checking the batchIndex based on the order of the dps. However, Go’s map iteration order is randomized, so the iteration over dps doesn’t guarantee the order.
I have updated the test to ensure it always finds the correct batchIndex, and the tests are now passing consistently. No other change made to the code.
Fix PR: #36771

sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this issue Dec 17, 2024
@songy23 @sbylica-splunk
Revert "[exporter/awsemfexporter]Split EMF log with larger than 100 b…
51b99a0 
…uckets." (open-telemetry#36763)

Reverts open-telemetry#36336


leads to test failures, see
open-telemetry#36727
mx-psi pushed a commit that referenced this issue Dec 17, 2024
@zzhlogin
Add back "[exporter/awsemfexporter]Split EMF log with larger than 100…
a70fa09 
… buckets." (#36771)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
This PR fix the flaky unit test in previous PR:
#36336,
and add back the implementation of splitting the emf log logic.

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

#36727

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Unit test updated and passed with 10 count:
```
go test -run TestAddToGroupedMetric -count 10 -tags=always

PASS
ok  	github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsemfexporter	0.016s

```
mterhar pushed a commit to mterhar/opentelemetry-collector-contrib that referenced this issue Dec 19, 2024
@zzhlogin @mterhar
Add back "[exporter/awsemfexporter]Split EMF log with larger than 100…
97bca8e 
… buckets." (open-telemetry#36771)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
This PR fix the flaky unit test in previous PR:
open-telemetry#36336,
and add back the implementation of splitting the emf log logic.

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

open-telemetry#36727

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Unit test updated and passed with 10 count:
```
go test -run TestAddToGroupedMetric -count 10 -tags=always

PASS
ok  	github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsemfexporter	0.016s

```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zzhlogin zzhlogin

Labels
exporter/awsemf awsemf exporter
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pjanotti @zzhlogin