-
Notifications
You must be signed in to change notification settings - Fork 192
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
Tracer references tests #759
Tracer references tests #759
Conversation
a recent bug highlighted that if there is no reference to a tracer provider, then the shared state will shutdown, even if that shared state is being used by active tracers. Adding a phpdoc comment explaining this behavior, and some tests to demonstrate it
Codecov Report
@@ Coverage Diff @@
## main #759 +/- ##
=========================================
Coverage 82.86% 82.86%
Complexity 1295 1295
=========================================
Files 150 150
Lines 3192 3192
=========================================
Hits 2645 2645
Misses 547 547
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
This documents and adds tests for the current behavior, but I did find the current behavior surprising. I think it's a fairly common practise to avoid creating intermediate objects, but in this case it has a side-effect.
The spec doesn't say that Another argument for changing the current behavior is that if I use DI to create a tracer (in my case, |
It depends on whether we want users to rely on the implicit shutdown behavior and expect them to not call
Moving shutdown handling to shared state might be the best option for now. |
I merged this because I figured moving shutdown handling to shared state could be handled in a subsequent PR. |
a recent bug in
examples/traces/demo
highlighted that if there is no reference to a tracer provider, then the shared state will shutdown, even if that shared state is being used by active tracers. Adding a phpdoc comment explaining this behavior, and some tests to demonstrate it