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.

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

[pkg/stanza] Flaky test TestMatcher/Numeric_Sorting_with_grouping #36623

Closed
dmitryax opened this issue Dec 2, 2024 · 4 comments · Fixed by #36640
Closed

[pkg/stanza] Flaky test TestMatcher/Numeric_Sorting_with_grouping #36623

dmitryax opened this issue Dec 2, 2024 · 4 comments · Fixed by #36640
Assignees
Labels
bug Something isn't working flaky test a test is flaky help wanted Extra attention is needed pkg/stanza

Comments

@dmitryax
Copy link
Member

dmitryax commented Dec 2, 2024

Seen in https://github.com/open-telemetry/opentelemetry-collector/actions/runs/12127649470/job/33812375774?pr=11637

✖  fileconsumer/matcher (17ms)

=== Failed
=== FAIL: fileconsumer/matcher TestMatcher/Numeric_Sorting_with_grouping (0.00s)
    matcher_test.go:856: 
        	Error Trace:	/tmp/opentelemetry-collector-contrib/pkg/stanza/fileconsumer/matcher/matcher_test.go:856
        	Error:      	Not equal: 
        	            	expected: []string{"err.a.123456789.log", "err.a.123456788.log", "err.a.123456787.log", "err.a.123456786.log", "err.b.123456789.log", "err.b.123456788.log"}
        	            	actual  : []string{"err.b.123456789.log", "err.b.123456788.log", "err.a.123456789.log", "err.a.123456788.log", "err.a.123456787.log", "err.a.123456786.log"}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,2 +1,4 @@
        	            	 ([]string) (len=6) {
        	            	+ (string) (len=19) "err.b.123456789.log",
        	            	+ (string) (len=19) "err.b.123456788.log",
        	            	  (string) (len=19) "err.a.123456789.log",
        	            	@@ -4,5 +6,3 @@
        	            	  (string) (len=19) "err.a.123456787.log",
        	            	- (string) (len=19) "err.a.123456786.log",
        	            	- (string) (len=19) "err.b.123456789.log",
        	            	- (string) (len=19) "err.b.123456788.log"
        	            	+ (string) (len=19) "err.a.123456786.log"
        	            	 }
        	Test:       	TestMatcher/Numeric_Sorting_with_grouping

=== FAIL: fileconsumer/matcher TestMatcher (0.04s)

=== FAIL: fileconsumer/matcher TestMatcher/Numeric_Sorting_with_grouping (re-run 1) (0.00s)
    matcher_test.go:856: 
        	Error Trace:	/tmp/opentelemetry-collector-contrib/pkg/stanza/fileconsumer/matcher/matcher_test.go:856
        	Error:      	Not equal: 
        	            	expected: []string{"err.a.123456789.log", "err.a.123456788.log", "err.a.123456787.log", "err.a.123456786.log", "err.b.123456789.log", "err.b.123456788.log"}
        	            	actual  : []string{"err.b.123456789.log", "err.b.123456788.log", "err.a.123456789.log", "err.a.123456788.log", "err.a.123456787.log", "err.a.123456786.log"}
        	            	
        	            	Diff:
        	            	--- Expected
make[3]: *** [../../Makefile.Common:132: test] Error 1
make[2]: *** [Makefile:200: pkg/stanza] Error 2
make[1]: *** [Makefile:132: gotest] Error 2
make: *** [Makefile:268: check-contrib] Error 2
        	            	+++ Actual
        	            	@@ -1,2 +1,4 @@
        	            	 ([]string) (len=6) {
        	            	+ (string) (len=19) "err.b.123456789.log",
        	            	+ (string) (len=19) "err.b.123456788.log",
        	            	  (string) (len=19) "err.a.123456789.log",
        	            	@@ -4,5 +6,3 @@
        	            	  (string) (len=19) "err.a.123456787.log",
        	            	- (string) (len=19) "err.a.123456786.log",
        	            	- (string) (len=19) "err.b.123456789.log",
        	            	- (string) (len=19) "err.b.123456788.log"
        	            	+ (string) (len=19) "err.a.123456786.log"
        	            	 }
        	Test:       	TestMatcher/Numeric_Sorting_with_grouping

=== FAIL: fileconsumer/matcher TestMatcher (re-run 1) (0.01s)
@dmitryax dmitryax added bug Something isn't working flaky test a test is flaky pkg/stanza labels Dec 2, 2024
Copy link
Contributor

github-actions bot commented Dec 2, 2024

Pinging code owners for pkg/stanza: @djaglowski. See Adding Labels via Comments if you do not have permissions to add labels yourself. For example, comment '/label priority:p2 -needs-triaged' to set the priority and remove the needs-triaged label.

Copy link
Contributor

github-actions bot commented Dec 2, 2024

Pinging code owners:

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

@dmitryax dmitryax added the help wanted Extra attention is needed label Dec 3, 2024
@VihasMakwana
Copy link
Contributor

VihasMakwana commented Dec 3, 2024

Filed a fix #36640

@VihasMakwana VihasMakwana self-assigned this Dec 3, 2024
dmitryax pushed a commit that referenced this issue Dec 3, 2024
Fixes
#36623

We're using a map to store groups and we then do a `for .. range` over
keys.

https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/6119d51a5a85565c409bb225027d6d1ab353aecc/pkg/stanza/fileconsumer/matcher/matcher.go#L200-L206

For maps, the ordering of keys is not guaranteed. Hence, the check
fails.
We should instead check with `assert.ElementsMatch` (which was
previously the case, but
#36518
changed it to `assert.Equal`)
shivanthzen pushed a commit to shivanthzen/opentelemetry-collector-contrib that referenced this issue Dec 5, 2024
Fixes
open-telemetry#36623

We're using a map to store groups and we then do a `for .. range` over
keys.

https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/6119d51a5a85565c409bb225027d6d1ab353aecc/pkg/stanza/fileconsumer/matcher/matcher.go#L200-L206

For maps, the ordering of keys is not guaranteed. Hence, the check
fails.
We should instead check with `assert.ElementsMatch` (which was
previously the case, but
open-telemetry#36518
changed it to `assert.Equal`)
ZenoCC-Peng pushed a commit to ZenoCC-Peng/opentelemetry-collector-contrib that referenced this issue Dec 6, 2024
Fixes
open-telemetry#36623

We're using a map to store groups and we then do a `for .. range` over
keys.

https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/6119d51a5a85565c409bb225027d6d1ab353aecc/pkg/stanza/fileconsumer/matcher/matcher.go#L200-L206

For maps, the ordering of keys is not guaranteed. Hence, the check
fails.
We should instead check with `assert.ElementsMatch` (which was
previously the case, but
open-telemetry#36518
changed it to `assert.Equal`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working flaky test a test is flaky help wanted Extra attention is needed pkg/stanza
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants