Tracing: Consider adding a flush API.

[songy23]

Proposed by @reyang :

I propose to have a top level Tracing.flush() instead. The reasons are:

1. shutdown hooks are nondeterministic, if there are other shutdown hooks trying to use OpenCensus, their trace data might get dropped (due to the order of shutdown hooks execution).

2. shutdown hook requires RuntimePermission("shutdownHooks") which could be a problem for certain scenarios.

3. give user explicit control so they can flush the trace when appropriate.

/cc @bogdandrutu

[sebright]

Related: census-instrumentation/opencensus-java#1141

[semistrict]

Go does not have shutdown hooks. Most of the exporters already support a Flush method, but not having this in an interface means that we need always downcast in generic code. Would be nice to see this concept formalized.

While we are at it, does it make sense to make the concept of an internal buffer API-visible? Like being able to control the sizing and auto-flushing policy?

[songy23]

While we are at it, does it make sense to make the concept of an internal buffer API-visible?

I'm not sure if this is necessary, given that users already have control over the exporting interval.

Like being able to control the sizing and auto-flushing policy?

Say some user has a config of buffer size 10 and exporting every 10 seconds, what happened when the buffer was full at 3rd second? Do we reset the exporting time?

[reyang]

If the buffer was full before the exporting period, I suggest we discard the incoming traces/metrics until the buffer got flushed. And we might need a log entry indicating there are missing entries (with start time and recovery time).

[semistrict]

Say some user has a config of buffer size 10 and exporting every 10 seconds, what happened when the buffer was full at 3rd second? Do we reset the exporting time?

Not exactly sure but probably. Most of the Go exporters use google.golang.org/api/support/bundler under the hood.

[songy23]

If the buffer was full before the exporting period, I suggest we discard the incoming traces/metrics until the buffer got flushed.

We're trying to avoid down-sampling. As @Ramonza mentioned, looks like the current logic is to push the buffered data and reset start time once buffer is full.

Not exactly sure but probably. Most of the Go exporters use google.golang.org/api/support/bundler under the hood.

I see, then I think we should warn user that if they have a small buffer but yet huge amount of data at some point of time, the exporting interval would be much shorter and they need to be aware of the increasing traffic.

[semistrict]

Another question that occurred to me regarding the proposed Tracing.flush(): why would this be part of the tracing API and not just an Exporter method?

[reyang]

@Ramonza I'm fine if we decided to make it an Exporter method instead of Tracing.flush(). Having it in Tracing seems natural to me personally. (e.g. most people could tell what Tracing.flush() or Trace.flush() mean literally)

[reyang]

@songy23 if there are too much data going into the buffer, will we eventually block the writers? I'm concerned about the performance impact.

[songy23]

@reyang I don't think we would block the writer currently. In Java the buffer will just keep growing until next export.

[semistrict]

The tracing API is targeted to instrumenting code (libraries or applications). Instrumentation shouldn't care about explicit flushing. Flushing has to do with exporting, so I think it logically belongs there.

If we make this part of the exporter API, I think it's more of a language-specific concern rather than a cross-language specs issue.

[reyang]

I believe we'll support the model where people manipulating exporters from the runtime configuration instead of hard-coding. In this case, having to call exporter API to flush seems to be a very strange design.
Technically I understand that flush belongs to exporting, my point is that having flush at Tracing level makes more sense to the user. Take one example, we have file.flush instead of disk.flush/network.flush.

[semistrict]

I think this is language-specific but it doesn't seem strange to me for exporters to have a flush method. Indeed, most of them in Go already do (it just isn't required by the Exporter interface).

So there's a question of whether, in addition to this per-Exporter flush, we need some kind of global flush that flushes all the registered exporters. In this case, we probably want to also flush stats exporters (since they have exactly the same requirements for buffered items to be submitted before program termination).

[briantq]

I ran into issues in #1391 that I cannot flush traces at will. In a multi threaded environment (new Thread per request), I want to be able to write any span data before the thread is killed. Leaving a Thread hung for 5 seconds defeats the purpose of performance. This one issue alone prevents me from using OpenCensus, as I have to choose between getting output or killing my apps performance I have worked hard to optimize.

My vote is to add flush to the exporter method (Tracing.getExportComponent().flush()), though I am fine with it anywhere as long as I can flush my data prior to losing it.