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

Add option in BatchSpanProcessor to support multiple pending exports #4280

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

trask
Copy link
Member

@trask trask commented Mar 20, 2022

Resolves #4264

This is a third option that I like better than both #4245 and #4246.

@trask trask requested a review from a user March 20, 2022 03:59
@trask trask requested a review from Oberon00 as a code owner March 20, 2022 03:59
@codecov
Copy link

codecov bot commented Mar 20, 2022

Codecov Report

Merging #4280 (1ccf507) into main (14ffacd) will increase coverage by 0.00%.
The diff coverage is 95.65%.

@@            Coverage Diff            @@
##               main    #4280   +/-   ##
=========================================
  Coverage     89.88%   89.88%           
- Complexity     4844     4847    +3     
=========================================
  Files           568      568           
  Lines         15063    15081   +18     
  Branches       1427     1429    +2     
=========================================
+ Hits          13539    13556   +17     
  Misses         1060     1060           
- Partials        464      465    +1     
Impacted Files Coverage Δ
...ry/sdk/trace/export/BatchSpanProcessorBuilder.java 94.73% <80.00%> (-2.24%) ⬇️
...telemetry/sdk/trace/export/BatchSpanProcessor.java 91.02% <100.00%> (+0.11%) ⬆️
...pentelemetry/sdk/common/CompletableResultCode.java 100.00% <0.00%> (+1.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14ffacd...1ccf507. Read the comment docs.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks good!

@anuraaga anuraaga marked this pull request as draft March 22, 2022 05:23
@trask
Copy link
Member Author

trask commented Mar 22, 2022

discussed with @anuraaga, I'm not in a rush for this change, will open a spec issue to discuss adding this setting to the spec

@trask trask changed the title Add option in BatchSpanProcessor to support multiple active exports Add option in BatchSpanProcessor to support multiple pending exports Mar 24, 2022
@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added the Stale label Jun 22, 2022
@trask
Copy link
Member Author

trask commented Jun 22, 2022

@open-telemetry/java-maintainers can you apply the prototype label to this (if you agree)?

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.

Non-blocking batch span processor?
2 participants