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

fix(tracing): batch span exports to prevent blocking #11364

Merged
merged 1 commit into from Jan 17, 2024

Conversation

milas
Copy link
Member

@milas milas commented Jan 17, 2024

What I did
This was a bad configuration (my fault) that meant each span was exported synchronously, as it ended. That can cause weird behavior such as stuttering/blocking.

There's really no reason to NOT use the batch processor, it's the recommended way to configure it. In the future, it might make sense to tune the intervals based on the fact that Compose is a CLI vs a long-running server app, but we handle flushing out on exit already, so it's not a huge deal.

Related issue
https://docker.atlassian.net/browse/COMP-387

(not mandatory) A picture of a cute animal, if possible in relation to what you did
hippo

This was a bad configuration (my fault) that meant each span was
exported synchronously, as it ended. That can cause weird behavior
such as stuttering/blocking.

There's really no reason to NOT use the batch processor, it's the
recommended way to configure it. In the future, it might make sense
to tune the intervals based on the fact that Compose is a CLI vs
a long-running server app, but we handle flushing out on exit
already, so it's not a huge deal.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

Yeah!!! Glad you catch it 👍

Copy link

codecov bot commented Jan 17, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (3c4593f) 56.55% compared to head (62fadb6) 56.61%.
Report is 1 commits behind head on main.

Files Patch % Lines
internal/tracing/tracing.go 0.00% 1 Missing ⚠️
pkg/compose/convert.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11364      +/-   ##
==========================================
+ Coverage   56.55%   56.61%   +0.06%     
==========================================
  Files         136      136              
  Lines       11541    11539       -2     
==========================================
+ Hits         6527     6533       +6     
+ Misses       4387     4381       -6     
+ Partials      627      625       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ndeloof ndeloof merged commit d688d3b into docker:main Jan 17, 2024
26 checks passed
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.

None yet

3 participants