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

otlptracegrpc: Fix start/stop sync in mockCollector #3989

Merged
merged 3 commits into from
Apr 13, 2023

Conversation

pellared
Copy link
Member

@pellared pellared commented Apr 12, 2023

Noticed when looking at #2686

This PR is intended to get rid of <-time.After(number * time.Millisecond) and properly synchronize the mock collector startup and shutdown. It should make the tests more stable.

For me before this fix running go test -race -count=100 is almost hanging (240sec for me). After this fix I get:

$ go test -race -count=100
--- FAIL: TestExporterShutdown (0.02s)
    --- FAIL: TestExporterShutdown/testClientStopHonorsTimeout (0.01s)
        client.go:73: expected context DeadlineExceeded error, got <nil>
FAIL
exit status 1
FAIL    go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc 27.677s

$ go test -race -count=100
--- FAIL: TestExporterShutdown (0.03s)
    --- FAIL: TestExporterShutdown/testClientStopHonorsCancel (0.01s)
        client.go:92: expected context canceled error, got <nil>
FAIL
exit status 1
FAIL    go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc 27.049s

(before this fix the failure is after 241.189s).

So not only it makes the tests more trustworthy (stable) but also shortens the execution time by 10 times (on my PC).

@pellared pellared added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 12, 2023
@pellared pellared changed the title otlptracehttp: Fix start/stop sync in mockCollector otlptracegrpc: Fix start/stop sync in mockCollector Apr 12, 2023
@MrAlias MrAlias merged commit b24bcc1 into open-telemetry:main Apr 13, 2023
@pellared pellared changed the title otlptracegrpc: Fix start/stop sync in mockCollector otlptracegrpc: Refine start/stop sync in mockCollector Apr 13, 2023
@pellared pellared changed the title otlptracegrpc: Refine start/stop sync in mockCollector otlptracegrpc: Fix start/stop sync in mockCollector Apr 13, 2023
@pellared pellared deleted the sync-start-stop branch April 28, 2023 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants