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][extension/jaegerremotesampling] TestAutoUpdateStrategyWithFile is flaky on Windows #37275

Closed
pjanotti opened this issue Jan 16, 2025 · 6 comments · Fixed by #37289
Closed

[flaky test][extension/jaegerremotesampling] TestAutoUpdateStrategyWithFile is flaky on Windows #37275

pjanotti opened this issue Jan 16, 2025 · 6 comments · Fixed by #37289
Labels
extension/jaegerremotesampling flaky test a test is flaky os:windows

Comments

@pjanotti
Copy link
Contributor

Component(s)

extension/jaegerremotesampling

Describe the issue you're reporting

Multiple hits in different runs:

=== FAIL: internal/source/filesource TestAutoUpdateStrategyWithFile (0.03s)
    filesource_test.go:382: 
        	Error Trace:	D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/filesource/filesource_test.go:382
        	            				D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/filesource/filesource_test.go:422
        	Error:      	Received unexpected error:
        	            	remove C:\Users\RUNNER~1\AppData\Local\Temp\for_go_test_1706118286.json: The process cannot access the file because it is being used by another process.
        	Test:       	TestAutoUpdateStrategyWithFile

=== FAIL: internal/source/filesource TestAutoUpdateStrategyWithFile (re-run 1) (0.03s)
    filesource_test.go:382: 
        	Error Trace:	D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/filesource/filesource_test.go:382
        	            				D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/filesource/filesource_test.go:422
        	Error:      	Received unexpected error:
        	            	remove C:\Users\RUNNER~1\AppData\Local\Temp\for_go_test_3754742530.json: The process cannot access the file because it is being used by another process.
        	Test:       	TestAutoUpdateStrategyWithFile
@github-actions GitHub Actions
Copy link
Contributor

Pinging code owners for extension/jaegerremotesampling: @yurishkuro @frzifus. 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.

@github-actions GitHub Actions
Copy link
Contributor

Pinging code owners:

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

@yurishkuro
Copy link
Member

@ary82 please investigate, seems it started happening after your changes.

@pjanotti pjanotti removed the needs triage New item requiring triage label Jan 17, 2025
@pjanotti
Copy link
Contributor Author

@ary82 The issue is related to the fact that autoUpdateStrategies is launched in a goroutine here and while the routine is properly terminated it does so asynchronously from Close. So it can be reading the file while the test is cleaning the temporary folder and that fails on Windows. My suggestion would be to only return from Close when goroutine is out of the for / select loop.

@yurishkuro
Copy link
Member

+1, it should use a wait group

@ary82
Copy link
Contributor

ary82 commented Jan 17, 2025

Thanks for the help!

@ary82 ary82 mentioned this issue Jan 17, 2025
Merged
@github-actions github-actions bot mentioned this issue Jan 21, 2025
Open
chengchuanpeng pushed a commit to chengchuanpeng/opentelemetry-collector-contrib that referenced this issue Jan 26, 2025
@ary82 @yurishkuro @chengchuanpeng
[extension/jaegerremotesampling] fix: wait for autoUpdateStrategies i…
2a47c0f 
…n Close function (open-telemetry#37289)

#### Description
Add a waitGroup to strategyProvider struct that waits for
autoUpdateStrategies in the Close function

#### Link to tracking issue
Fixes open-telemetry#37275

---------

Signed-off-by: Aryan Goyal <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
extension/jaegerremotesampling flaky test a test is flaky os:windows
Projects
None yet
Milestone
No milestone
3 participants
@yurishkuro @pjanotti @ary82