search

open-telemetry / opentelemetry-ruby

OpenTelemetry Ruby API & SDK, and related gems
https://opentelemetry.io/
Apache License 2.0
484 stars 236 forks source link

Provide Ability to Perform Graceful Shutdown #1353

Closed arielvalentin closed 2 years ago

arielvalentin commented 2 years ago

I hesitate to register at_exit hooks in instrumentations and I think it would be better suited for the SDK package. There are other cases where the tracer provider should also be shutdown, e.g. Resque Workers and Unicorn that fork child process.

cc: @open-telemetry/ruby-maintainers

_Originally posted by @arielvalentin in https://github.com/open-telemetry/opentelemetry-ruby-contrib/pull/80#discussion_r933330202_

plantfansam commented 2 years ago

We discussed this in SIG on August 2. IIRC — we did not decide that this functionality should definitely exist in the SDK, but rather that if we want to provide this functionality, it would be in the SDK. Am I remembering that right @mwear @robertlaurin @arielvalentin @ericmustin?

fbogsany commented 2 years ago

The SDK provides the functions to call for graceful shutdown, but it cannot and should not anticipate every environment it might run in. Graceful shutdown is an application and/or application framework concern. IIRC we have a complicated set of Unicorn lifecycle hooks at Shopify, for example, and using at_exit can result in unpalatable situations like dropped spans or lengthy shutdown delays.

It is reasonable for Sidekiq instrumentation, for example, to optionally register job worker shutdown hooks that call the SDK's shutdown method. Whether this should default to "on" or "off" is probably framework-specific. Organizations that build their own SDK wrappers (e.g. Shopify and GitHub) have a better idea of the range of environments they need to support and can choose defaults that work for them.