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

Fix BatchSpanProcessor context done tests #1863

Merged
merged 2 commits into from
Apr 29, 2021

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Apr 29, 2021

#1860 added support for the BatchSpanProcessor to return export failures. This was added using a select statement.

select {
case err = <-wait:
case <-ctx.Done():
err = ctx.Err()
}

In Go select statements are random when multiple cases can be satisfied. This means that if the exporter is shutdown fast enough and the context has ended there is a chance that the exporter error value will be returned instead of the context error, causing test failures.

This updates the tests to have the exporter return the same context error, which a valid exporter should be doing, and resolves the bug.

@MrAlias MrAlias added bug Something isn't working pkg:SDK Related to an SDK package area:trace Part of OpenTelemetry tracing labels Apr 29, 2021
@Sawmi Sawmi mentioned this pull request Apr 29, 2021
@MrAlias MrAlias added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 29, 2021
@MrAlias
Copy link
Contributor Author

MrAlias commented Apr 29, 2021

Merging prior to 1 business day to avoid other test failures.

@MrAlias MrAlias merged commit ec75390 into open-telemetry:main Apr 29, 2021
@MrAlias MrAlias deleted the bsp-ctx-flush branch April 29, 2021 18:01
@pellared pellared added this to the untracked milestone Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:trace Part of OpenTelemetry tracing bug Something isn't working pkg:SDK Related to an SDK package Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants