Remove automatic ::shutdown(), add ShutdownHandler::register()

[Nevay]

See #759, removes automatic shutdown handling. Users should keep a reference to the TracerProvider and call ::shutdown() explicitely. ShutdownHandler::register([$tracerProvider, 'shutdown']) can be used to protect against process timeout / exit, but also requires users to keep a reference to the TracerProvider.

The tracer provider should be in scope anyways to be globally accessible in the current execution context (somewhat depends on how instrumentation registration is solved?).

[codecov]

Codecov Report

Merging #760 (e9ab294) into main (37a8c8e) will decrease coverage by 1.16%.
The diff coverage is 30.30%.

@@             Coverage Diff              @@
##               main     #760      +/-   ##
============================================
- Coverage     79.59%   78.43%   -1.17%     
- Complexity     1295     1331      +36     
============================================
  Files           150      153       +3     
  Lines          3195     3278      +83     
============================================
+ Hits           2543     2571      +28     
- Misses          652      707      +55     

Flag	Coverage Δ
7.4	78.43% <30.61%> (-1.15%)	⬇️
8.0	78.50% <30.61%> (-1.15%)	⬇️
8.1	78.50% <30.61%> (-1.15%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/SDK/Common/Util/WeakMap.php	0.00% <0.00%> (ø)
src/SDK/Trace/TracerProvider.php	100.00% <ø> (+35.00%)	⬆️
src/SDK/Common/Util/ShutdownHandler.php	63.63% <63.63%> (ø)
src/SDK/Common/Util/functions.php	100.00% <100.00%> (ø)

---

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 37a8c8e...e9ab294. Read the comment docs.

[Nevay]

shutdown is called if no new traces can be created

This approach won't work for metrics; the metrics API allows registration of permanent callbacks. Asynchronous Instrument API:

The API MUST support creation of asynchronous instruments by passing zero or more callback functions to be permanently registered to the newly created instrument.

I see three options:

1. shutdown tracer provider after no new traces can be created, shut down meter provider after last synchronous writer (and non-permanent asynchronous callback?) goes out of scope, which might be unexpected if the user registers only permanent callbacks
2. keep the current behavior and shut down when tracer provider / meter provider are destroyed
3. remove automatic shutdown behavior (at least on __destruct(), keep register_shutdown_function() handler?)

We should also consider that ::shutdown() is a blocking operation, triggering this at an arbitrary point might lead to an unexpected delay for the application.

[brettmc]

I see three options ....

I'm inclined to think option 3: if you want to shut down a tracer provider, you must keep a reference to it and explicitly call shutdown(). Does that resurrect any tracer provider leaks that you fixed up in the earlier PR?

[Nevay]

Does that resurrect any tracer provider leaks that you fixed up in the earlier PR?

No, __destruct() could simply call ::unregisterShutdownFunction() instead of ::shutdown().

Converting this to draft for now; I've decided to create a POC that allows users to opt-in instead of enabling it by default.

ShutdownHandler::register($tracerProvider->shutdown(...)); // Closure::fromCallable([$provider, 'shutdown']) if php < 8.1

[Nevay]

This is also very useful for the metrics API; we can add a $weak parameter to asynchronous instruments ::observe() method that would allow users to provide normal callbacks with bound $this instead of forcing them to take care of weakening the $this reference themselves.

[brettmc]

@Nevay let us know if it's ok to merge this now.

[Nevay]

Yes, this can be merged.