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.

Sign up for GitHub

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

Jump to bottom

Add SpanExporter.ForceFlush. #1467

Merged
merged 17 commits into from
Mar 15, 2021
Merged

Add SpanExporter.ForceFlush. #1467

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e02d40c
Add SpanExporter.ForceFlush.
Oberon00 Feb 25, 2021
43f12aa
Add CHANGELOG, compliance matrix entries.
Oberon00 Feb 25, 2021
4be0529
Fill out PR#.
Oberon00 Feb 25, 2021
5f8f890
Apply suggestions from code review
Oberon00 Feb 25, 2021
2f27ddf
Update specification/trace/sdk.md
Oberon00 Feb 25, 2021
04e3e29
Tweak wording on SpanProcessor.ForceFlush
Oberon00 Feb 26, 2021
c74590f
Update specification/trace/sdk.md
Oberon00 Feb 26, 2021
a9b7c8c
Merge branch 'main' into exporter-forceflush
Oberon00 Mar 8, 2021
e2a36ff
Merge branch 'main' into exporter-forceflush
arminru Mar 11, 2021
a04f68e
Merge branch 'main' into exporter-forceflush
Oberon00 Mar 15, 2021
9be7b76
Merge branch 'main' into exporter-forceflush
arminru Mar 15, 2021
1d26e92
Update compliance matrix: Add row, add links
Oberon00 Mar 15, 2021
e71c9ef
Fix links
Oberon00 Mar 15, 2021
59a05e0
Fix link again
Oberon00 Mar 15, 2021
4b79b87
Fix links again (2)
Oberon00 Mar 15, 2021
a4e9209
Update spec-compliance-matrix.md
Oberon00 Mar 15, 2021
30af37a
Update spec-compliance-matrix.md
arminru Mar 15, 2021
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
View file
Open in desktop
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
View file
Open in desktop
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
32 changes: 30 additions & 2 deletions specification/trace/sdk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,14 +467,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 @@ -582,6 +592,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