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

sdk: implement multi-threading tests for BatchSpanProcessor #392

Closed
mauriciovasquezbernal opened this issue Feb 3, 2020 · 9 comments
Closed
Labels
backlog help wanted release:required-for-ga To be resolved before GA release sdk Affects the SDK package. tests

Comments

@mauriciovasquezbernal
Copy link
Member

As @Oberon00 suggested in #389 (comment), we are missing multi-threading tests for the batch span processor.
Those tests should check that shutdown, force_flush can be called concurrently without causing racing conditions.

@mauriciovasquezbernal mauriciovasquezbernal added sdk Affects the SDK package. tests labels Apr 3, 2020
@ocervell
Copy link

+1, this is a blocker for my customer - can we trust the BatchSpanProcessor to work across threads / processes ?

@aravinsiva
Copy link
Contributor

I'd be happy to look into this if help is still needed

@codeboten
Copy link
Contributor

Thanks @aravinsiva, assigning it to you

@aravinsiva
Copy link
Contributor

I just need a bit more clarification on this. I was under the impression that batch processors already run on a separate thread. So what exactly is meant by 'multi-threading' tests?

@lzchen
Copy link
Contributor

lzchen commented Jul 27, 2020

@aravinsiva
Have you taken a look at this comment? I believe they are referring to the behaviour for shutdown and force_flush when they are called at the same time in different threads (e.g. multiple BatchSpanProcessors)

@aabmass
Copy link
Member

aabmass commented Aug 25, 2020

@lzchen I'm confused how you can test this with "multiple BatchSpanProcessors" here? The issue says

Those tests should check that shutdown, force_flush can be called concurrently without causing racing conditions.

doesn't that mean calling those methods on a single BatchExportSpanProcessor instance concurrently from different threads?

@aravinsiva aravinsiva removed their assignment Aug 27, 2020
@aravinsiva
Copy link
Contributor

Please reassign ticket as internship is concluding.

@codeboten codeboten added help wanted release:required-for-ga To be resolved before GA release labels Sep 24, 2020
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this issue Nov 1, 2020
@codeboten codeboten removed the release:required-for-ga To be resolved before GA release label Nov 26, 2020
@lzchen lzchen added the release:required-for-ga To be resolved before GA release label Dec 18, 2020
@github-actions
Copy link

This issue was marked stale due to lack of activity. It will be closed in 30 days.

@github-actions
Copy link

Closed as inactive. Feel free to reopen if this issue needs resolving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog help wanted release:required-for-ga To be resolved before GA release sdk Affects the SDK package. tests
Projects
None yet
Development

No branches or pull requests

6 participants