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

filters/scheduler: wait for spans finishing in lifo errors test #1964

Merged
merged 1 commit into from
Feb 21, 2022

Conversation

AlexanderYastrebov
Copy link
Member

@AlexanderYastrebov AlexanderYastrebov commented Feb 21, 2022

The tests was introduced by #1944
It may happen that not all spans are finished by the time test verifies response results (because spans are closed after response has been served).
This change waits for all mock spans to finish and forces scheduler metrics update.

Signed-off-by: Alexander Yastrebov [email protected]

retry := time.NewTicker(100 * time.Millisecond)
defer retry.Stop()
for {
finished := t.MockTracer.FinishedSpans()
Copy link
Member Author

@AlexanderYastrebov AlexanderYastrebov Feb 21, 2022

Choose a reason for hiding this comment

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

opentracing/opentracing-go#239 adds UnfinishedSpans() but the library is deprecated opentracing/specification#163 so it seems there will be no new release

@aryszka
Copy link
Contributor

aryszka commented Feb 21, 2022

👍

@aryszka
Copy link
Contributor

aryszka commented Feb 21, 2022

I think this fix would fir the upstream well, because the use case is quite generic. I would open an upstream issue, and ask what the maintainers think about it.

@aryszka
Copy link
Contributor

aryszka commented Feb 21, 2022

noticed the mention of the deprecated state, sorry 😃

The tests was introduced by #1944
It may happen that not all spans are finished by the time test verifies
response results (because spans are closed after response has been
served).

This change waits for all mock spans to finish and forces scheduler
metrics update.

Signed-off-by: Alexander Yastrebov <[email protected]>
@szuecs
Copy link
Member

szuecs commented Feb 21, 2022

👍

1 similar comment
@AlexanderYastrebov
Copy link
Member Author

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants