opentracing / specification

A place to document (and discuss) the OpenTracing specification. 🛑 This project is DEPRECATED! https://github.com/opentracing/specification/issues/163
http://opentracing.io/spec
Apache License 2.0
1.17k stars 182 forks source link

RFC: Closeable Tracers #128

Closed austinlparker closed 5 years ago

austinlparker commented 5 years ago

This proposal builds off of discussions primarily in the Java API surrounding the inclusion of a Close method in that library. The primary goal of adding this to the specification is to provide a clean decoupling of OpenTracing Tracers from their concrete implementations in a consistent way across platforms and languages.

See https://github.com/opentracing/opentracing-java/issues/250 for further recent discussion.

yurishkuro commented 5 years ago

What happens to spans started or even finished after the tracer is closed is a good question. We have seen cases where async callbacks were executing and finishing spans even after the tracer was closed, so sometime the application does not have full control over that. Ie we were seeing crashes when the app was just shutting down.

There's a large number of possible states we need to consider.

austinlparker commented 5 years ago

The case of 'what happens if I try to start a span after closing a tracer' seems straightforward enough, the tracer should check to see if it's been closed. If it has been closed, an error occurs and the span is not created.

I don't really have a good idea about what happens to spans that are in-flight during the tracer being closed, however. Couple options -

There's probably others I haven't thought of.

yurishkuro commented 5 years ago

If it has been closed, an error occurs and the span is not created.

that's not possible - methods for creating a span are not allowed to fail. You can return a noop-equivalent of a span.

what happens to spans that are in-flight

I think the required guarantee is that spans that have been Finished() prior to tracer.close() call must be flushed. Spans created or finished after tracer.close() may or may not be flushed, but should not be throwing errors and messing up the application.

yurishkuro commented 5 years ago

lgtm

tedsuo commented 5 years ago

LGTM. Some details:

austinlparker commented 5 years ago

@tedsuo, are you ok with deferring your question about if close should be synchronous so we can get this merged?

tedsuo commented 5 years ago

@austinlparker: yes, the RFC basically says that close is synchronous right now; I would just like that to be explicit. I doubt this is a contentious proposal.

austinlparker commented 5 years ago

Alright, I called that out specifically. Thanks!