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

Fix flaky test: TestExportSpansTimeoutHonored #2411

Merged
merged 1 commit into from
Nov 22, 2021
Merged

Fix flaky test: TestExportSpansTimeoutHonored #2411

merged 1 commit into from
Nov 22, 2021

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Nov 22, 2021 •
edited
Loading

Closing the export blocking channel and then reading the number of spans is a race between the collector export method and the getSpans method. Resulting in the periodic failure of the TestExportSpansTimeoutHonored test.

go test -timeout 60s -short ./exporters/otlp/otlptrace/otlptracegrpc/...
go: downloading go.uber.org/goleak v1.1.12
--- FAIL: TestExportSpansTimeoutHonored (0.16s)
    client_test.go:353: 
        	Error Trace:	client_test.go:353
        	Error:      	"[trace_id:"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"  span_id:"\x00\x00\x00\x00\x00\x00\x00\x00"  name:"Span 0"  start_time_unix_nano:11651379494838206464  end_time_unix_nano:11651379494838206464  status:{}]" should have 0 item(s), but has 1
        	Test:       	TestExportSpansTimeoutHonored

Remove the race by not asserting no spans are created. This test is intended to test the exporter error, and should not fail based on the mockCollector behavior.

@MrAlias MrAlias added bug Something isn't working Skip Changelog PRs that do not require a CHANGELOG.md entry labels Nov 22, 2021
@MrAlias MrAlias marked this pull request as ready for review November 22, 2021 17:05
@codecov
Copy link

codecov bot commented Nov 22, 2021 •
edited
Loading

Codecov Report

Merging #2411 (c8167ed) into main (94367b5) will increase coverage by 0.0%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #2411   +/-   ##
=====================================
  Coverage   73.3%   73.3%           
=====================================
  Files        174     174           
  Lines      12203   12203           
=====================================
+ Hits        8946    8954    +8     
+ Misses      3019    3014    -5     
+ Partials     238     235    -3
Impacted Files Coverage Δ
sdk/trace/batch_span_processor.go 84.2% <0.0%> (-1.1%) ⬇️
sdk/metric/sdk.go 81.5% <0.0%> (+1.4%) ⬆️
...s/otlp/otlptrace/internal/connection/connection.go 16.1% <0.0%> (+1.5%) ⬆️
sdk/metric/refcount_mapped.go 100.0% <0.0%> (+20.0%) ⬆️

@MrAlias
Copy link
Contributor Author

MrAlias commented Nov 22, 2021

/easycla

@MrAlias MrAlias merged commit da3fc12 into open-telemetry:main Nov 22, 2021
@MrAlias MrAlias deleted the otlptracegrpc-client-test-fix branch November 22, 2021 17:40
@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
Reviewers

@Aneurysm9 Aneurysm9 Aneurysm9 approved these changes

@dashpole dashpole Awaiting requested review from dashpole dashpole is a code owner

@evantorrie evantorrie Awaiting requested review from evantorrie

@jmacd jmacd Awaiting requested review from jmacd

@MadVikingGod MadVikingGod Awaiting requested review from MadVikingGod

@paivagustavo paivagustavo Awaiting requested review from paivagustavo

@pellared pellared Awaiting requested review from pellared pellared is a code owner

@XSAM XSAM Awaiting requested review from XSAM XSAM is a code owner

Assignees
No one assigned
Labels
bug Something isn't working Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Milestone
untracked
Development

Successfully merging this pull request may close these issues.
3 participants
@MrAlias @Aneurysm9 @pellared