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

sdk: shut down span processors automatically #280

Merged
merged 10 commits into from
Nov 27, 2019
1 change: 0 additions & 1 deletion examples/trace/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,3 @@
tracer.add_span_processor(span_processor)

response = requests.get(url="http://127.0.0.1:5000/")
span_processor.shutdown()
1 change: 0 additions & 1 deletion examples/trace/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,3 @@ def hello():

if __name__ == "__main__":
app.run(debug=True)
span_processor.shutdown()
1 change: 0 additions & 1 deletion ext/opentelemetry-ext-azure-monitor/examples/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,3 @@
tracer.add_span_processor(span_processor)

response = requests.get(url="http://127.0.0.1:5000/")
span_processor.shutdown()
1 change: 0 additions & 1 deletion ext/opentelemetry-ext-azure-monitor/examples/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,3 @@ def hello():

if __name__ == "__main__":
app.run(debug=True)
span_processor.shutdown()
4 changes: 0 additions & 4 deletions ext/opentelemetry-ext-jaeger/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ gRPC is still not supported by this implementation.
with tracer.start_as_current_span('foo'):
print('Hello world!')

# shutdown the span processor
# TODO: this has to be improved so user doesn't need to call it manually
span_processor.shutdown()

The `examples <./examples>`_ folder contains more elaborated examples.

References
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,3 @@
time.sleep(0.2)

time.sleep(0.1)

# shutdown the span processor
# TODO: this has to be improved so user doesn't need to call it manually
span_processor.shutdown()
8 changes: 8 additions & 0 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.


import atexit
import logging
import random
import threading
Expand Down Expand Up @@ -333,6 +334,13 @@ def __init__(
self._current_span_slot = Context.register_slot(slot_name)
self._active_span_processor = MultiSpanProcessor()
self.sampler = sampler
self._atexit_hanlder = atexit.register(
Copy link
Contributor

Choose a reason for hiding this comment

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

My personal preference would be to not do this by default, but have it as something the user can plug himself, or at least make this optional.

Copy link
Member

@c24t c24t Nov 11, 2019

Choose a reason for hiding this comment

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

@carlosalberto why not do this by default? Because it would be surprising that calling del results in (blocking!) network calls?

I agree about making this configurable, but I think this is much better than the current default: call shutdown or lose unexported spans.

self._active_span_processor.shutdown
)

def __del__(self):
Copy link
Member

Choose a reason for hiding this comment

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

Why put this on Tracer instead of SpanProcessor?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that having it in Tracer makes it more general and other possible SpanProcessor implementations would not have to take care of this.

atexit.unregister(self._atexit_hanlder)
self._active_span_processor.shutdown()

def get_current_span(self):
"""See `opentelemetry.trace.Tracer.get_current_span`."""
Expand Down