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

Add SpanExporter.ForceFlush. #1467

Merged
merged 17 commits into from
Mar 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ release.

### Traces

- Add `ForceFlush` to `Span Exporter` interface ([#1467](https://github.com/open-telemetry/opentelemetry-specification/pull/1467))
- Clarify the description for the `TraceIdRatioBased` sampler needs to include the sampler's sampling ratio. ([#1536](https://github.com/open-telemetry/opentelemetry-specification/pull/1536))

### Metrics
Expand Down
3 changes: 3 additions & 0 deletions spec-compliance-matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ status of the feature is not known.
| [New Span ID created also for non-recording Spans](specification/trace/sdk.md#sdk-span-creation) | | + | + | | + | + | + | | | | - | + |
| [IdGenerators](specification/trace/sdk.md#id-generators) | | | + | | | + | | | | | | + |
| [SpanLimits](specification/trace/sdk.md#span-limits) | X | | + | | | + | | | | | | + |
| [Built-in `SpanProcessor`s implement `ForceFlush` spec](specification/trace/sdk.md#forceflush-1) | | | | | | | | | | | | |

## Baggage

Expand Down Expand Up @@ -142,6 +143,8 @@ Note: Support for environment variables is optional.

| Feature | Optional | Go | Java | JS | Python | Ruby | Erlang | PHP | Rust | C++ | .Net | Swift |
|-------------------------------------------------------|----------|----|-----------------------------------------------------------------------|----|-------------------------------------------------------------------------|------|--------|-----|------|-----|------|-------|
| [Exporter interface](specification/trace/sdk.md#span-exporter) | | | + | | + | | | | | | + | |
| [Exporter interface has `ForceFlush`](specification/trace/sdk.md#forceflush-2) | | | + | | | | | | | | | |
| Standard output (logging) | | + | + | + | + | + | + | - | + | + | + | + |
| In-memory (mock exporter) | | + | + | + | + | + | + | - | - | + | + | + |
| [OTLP](specification/protocol/otlp.md) | | | | | | | | | | | | |
Expand Down
32 changes: 30 additions & 2 deletions specification/trace/sdk.md
Original file line number Diff line number Diff line change
Expand Up @@ -473,14 +473,24 @@ make the shutdown timeout configurable.

#### ForceFlush()

Exports all spans that have not yet been exported to the configured `Exporter`.
This is a hint to ensure that any tasks associated with `Spans` for which the
`SpanProcessor` had already received events prior to the call to `ForceFlush` SHOULD
be completed as soon as possible, preferably before returning from this method.

In particular, if any `SpanProcessor` has any associated exporter, it SHOULD
try to call the exporter's `Export` with all spans for which this was not
already done and then invoke `ForceFlush` on it.
The [built-in SpanProcessors](#built-in-span-processors) MUST do so.
Copy link
Member

Choose a reason for hiding this comment

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

@Oberon00 Please add an entry in the compliance matrix for that, otherwise it might be overlooked easily.

If a timeout is specified (see below), the SpanProcessor MUST prioritize honoring the timeout over
finishing all calls. It MAY skip or abort some or all Export or ForceFlush
calls it has made to achieve this goal.

`ForceFlush` SHOULD provide a way to let the caller know whether it succeeded,
failed or timed out.

`ForceFlush` SHOULD only be called in cases where it is absolutely necessary,
such as when using some FaaS providers that may suspend the process after an
invocation, but before the `Processor` exports the completed spans.
invocation, but before the `SpanProcessor` exports the completed spans.

`ForceFlush` SHOULD complete or abort within some timeout. `ForceFlush` can be
arminru marked this conversation as resolved.
Show resolved Hide resolved
implemented as a blocking API or an asynchronous API which notifies the caller
Expand Down Expand Up @@ -588,6 +598,24 @@ return a `Failure` result.
and the destination is unavailable). OpenTelemetry client authors can decide if they
want to make the shutdown timeout configurable.

#### `ForceFlush()`
Copy link
Member

Choose a reason for hiding this comment

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

I think we can say, in case of stable releases a "no-op" implementation can be offered as a default. I think that is the best we can do :)


This is a hint to ensure that the export of any `Spans` the exporter has received prior to the
call to `ForceFlush` SHOULD be completed as soon as possible, preferably before
arminru marked this conversation as resolved.
Show resolved Hide resolved
returning from this method.

`ForceFlush` SHOULD provide a way to let the caller know whether it succeeded,
failed or timed out.

`ForceFlush` SHOULD only be called in cases where it is absolutely necessary,
such as when using some FaaS providers that may suspend the process after an
invocation, but before the exporter exports the completed spans.

`ForceFlush` SHOULD complete or abort within some timeout. `ForceFlush` can be
arminru marked this conversation as resolved.
Show resolved Hide resolved
implemented as a blocking API or an asynchronous API which notifies the caller
via a callback or an event. OpenTelemetry client authors can decide if they want to
make the flush timeout configurable.
Copy link
Contributor

Choose a reason for hiding this comment

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

For blocking API, I think it is very hard to abort within some timeout. Like the implementation complies to the spec if it just does abort its entry point without doing any work. Should we say ForceFlush should return (complete or abort) as soon as possible if the given timeout is reached?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is copy & past from shutdown and the other ForceFlush. Please open an issue if you want tor reword this (should be consistent everywhere).


### Further Language Specialization

Based on the generic interface definition laid out above library authors must
Expand Down