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

Jump to bottom

Fix flaky test TestSimpleSpanProcessorShutdownHonorsContextCancel #2290

Merged
merged 9 commits into from Oct 19, 2021
Merged

Fix flaky test TestSimpleSpanProcessorShutdownHonorsContextCancel #2290

merged 9 commits into from Oct 19, 2021

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Oct 15, 2021

Resolves #2289

@MrAlias MrAlias added the bug Something isn't working label Oct 15, 2021
@codecov
Copy link

codecov bot commented Oct 15, 2021 •
edited

Codecov Report

Merging #2290 (4804c1d) into main (1f4b606) will increase coverage by 0.0%.
The diff coverage is 92.8%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #2290   +/-   ##
=====================================
  Coverage   72.6%   72.6%           
=====================================
  Files        172     172           
  Lines      11898   11911   +13     
=====================================
+ Hits        8643    8658   +15     
+ Misses      3021    3020    -1     
+ Partials     234     233    -1
Impacted Files Coverage Δ
sdk/trace/simple_span_processor.go 87.2% <92.8%> (+1.9%) ⬆️
...s/otlp/otlptrace/internal/connection/connection.go 16.2% <0.0%> (+1.5%) ⬆️

pellared
pellared reviewed Oct 15, 2021
View reviewed changes
Copy link
Member

@pellared pellared left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

I would love to remove this comment 😄

pellared
pellared previously approved these changes Oct 15, 2021
View reviewed changes
XSAM
XSAM previously approved these changes Oct 16, 2021
View reviewed changes
pellared
pellared reviewed Oct 16, 2021
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@pellared pellared dismissed their stale review October 16, 2021 07:43

I am not convinced that it is a bug. I prefer others to judge

pellared
pellared approved these changes Oct 18, 2021
View reviewed changes
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MrAlias MrAlias dismissed XSAM’s stale review October 18, 2021 19:06

Please take another look. The strategy has changed enough to need a new review.

@MrAlias MrAlias merged commit 23a3373 into open-telemetry:main Oct 19, 2021
@MrAlias MrAlias deleted the fix-2289 branch October 19, 2021 15:13
@MrAlias MrAlias mentioned this pull request Oct 26, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pellared pellared pellared approved these changes

@XSAM XSAM XSAM approved these changes

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

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

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

@jmacd jmacd Awaiting requested review from jmacd

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

@paivagustavo paivagustavo Awaiting requested review from paivagustavo

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Flaky test: TestSimpleSpanProcessorShutdownHonorsContextCancel
3 participants
@MrAlias @pellared @XSAM