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 5 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 @@ -10,6 +10,7 @@ release.
- Adds `none` as a possible value for OTEL_TRACES_EXPORTER and OTEL_METRICS_EXPORTER to disable export
([#1439](https://github.com/open-telemetry/opentelemetry-specification/pull/1439))
- Add [`ForceFlush`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#forceflush) to SDK's `TracerProvider` ([#1452](https://github.com/open-telemetry/opentelemetry-specification/pull/1452))
- Add `ForceFlush` to `Span Exporter` interface ([#1467](https://github.com/open-telemetry/opentelemetry-specification/pull/1467))

## v1.0.1 (2021-02-11)

Expand Down
2 changes: 2 additions & 0 deletions spec-compliance-matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ Note: Support for environment variables is optional.

| Feature | Optional | Go | Java | JS | Python | Ruby | Erlang | PHP | Rust | C++ | .Net | Swift |
|-------------------------------------------------------|----------|----|-----------------------------------------------------------------------|----|-------------------------------------------------------------------------|------|--------|-----|------|-----|------|-------|
| Exporter interface | | | + | | + | | | | | | + | |
| Exporter interface has `ForceFlush` | | | + | | | | | | | | | |
| Standard output (logging) | | + | + | + | + | + | + | - | + | + | + | + |
| In-memory (mock exporter) | | + | + | + | + | + | + | - | - | + | + | + |
| [OTLP](specification/protocol/otlp.md) | | | | | | | | | | | | |
Expand Down
28 changes: 26 additions & 2 deletions specification/trace/sdk.md
Original file line number Diff line number Diff line change
Expand Up @@ -467,14 +467,20 @@ 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 the `SpanProcessor` has any associated exporters, it MUST call
the exporter's `Export` with all spans for which this was not already done and
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
then invoke `ForceFlush` on it.

`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 @@ -582,6 +588,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` passed to the exporter prior to the
Oberon00 marked this conversation as resolved.
Show resolved Hide resolved
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