Skip to content

Commit

Permalink
Clarify behavior of Span.End, interaction with IsRecording (#1011)
Browse files Browse the repository at this point in the history
* Clarify behavior of Span.End.

* Add CHANGELOG.

* Update specification/trace/api.md

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

* Polish wording for end/timestamp more.

* End is kinda the only way to end a span.

* Tweak wording of IsRecording after SIG mtg.

* Add to compliance matrix.

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
  • Loading branch information
Oberon00 and yurishkuro committed Oct 12, 2020
1 parent 4b19e00 commit ec58e45
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ Updates:
([#995](https://github.com/open-telemetry/opentelemetry-specification/pull/995))
- Remove custom header name for Baggage, use official header
([#993](https://github.com/open-telemetry/opentelemetry-specification/pull/993))
- Trace API: Clarifications for `Span.End`, e.g. IsRecording becomes false after End
([#1011](https://github.com/open-telemetry/opentelemetry-specification/pull/1011))

## v0.6.0 (07-01-2020)

Expand Down
1 change: 1 addition & 0 deletions spec-compliance-matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ status of the feature is not known.
|End | + | + | + | + | + | + | + | + | + | + |
|End with timestamp | + | + | + | + | + | + | + | - | + | + |
|IsRecording | + | + | + | + | + | + | + | | + | + |
|IsRecording becomes false after End | | | | | | | | | | |
|Set status | + | + | + | + | + | + | + | + | + | + |
|Safe for concurrent calls | + | + | + | [-](https://github.com/open-telemetry/opentelemetry-python/issues/1157) | + | + | + | + | + | + |
|[Span attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#set-attributes)|
Expand Down
43 changes: 35 additions & 8 deletions specification/trace/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,12 @@ created in another process. Each propagators' deserialization must set
`IsRemote` to true on a parent `SpanContext` so `Span` creation knows if the
parent is remote.

Any span that is created MUST also be ended.
This is the responsibility of the user.
API implementations MAY leak memory or other resources
(including, for example, CPU time for periodic work that iterates all spans)
if the user forgot to end the span.

#### Determining the Parent Span from a Context

When a new `Span` is created from a `Context`, the `Context` may contain a `Span`
Expand Down Expand Up @@ -411,7 +417,12 @@ Returns true if this `Span` is recording information like events with the
`AddEvent` operation, attributes using `SetAttributes`, status with `SetStatus`,
etc.

There should be no parameter.
After a `Span` is ended, it usually becomes non-recording and thus
`IsRecording` SHOULD consequently return false for ended Spans.
Note: Streaming implementations, where it is not known if a span is ended,
are one expected case where `IsRecording` cannot change after ending a Span.

`IsRecording` SHOULD NOT take any parameters.

This flag SHOULD be used to avoid expensive computations of a Span attributes or
events in case when a Span is definitely not recorded. Note that any child
Expand Down Expand Up @@ -558,17 +569,31 @@ Required parameters:

#### End

Finish the `Span`. This call will take the current timestamp to set as `Span`'s
end time. Implementations MUST ignore all subsequent calls to `End` (there might
be exceptions when Tracer is streaming event and has no mutable state associated
with the `Span`).
Signals that the operation described by this span has
now (or at the time optionally specified) ended.

Implementations SHOULD ignore all subsequent calls to `End` and any other Span methods,
i.e. the Span becomes non-recording by being ended
(there might be exceptions when Tracer is streaming events
and has no mutable state associated with the `Span`).

Language SIGs MAY provide methods other than `End` in the API that also end the
span to support language-specific features like `with` statements in Python.
However, all API implementations of such methods MUST internally call the `End`
method and be documented to do so.

`End` MUST NOT have any effects on child spans.
Those may still be running and can be ended later.

Call to `End` of a `Span` MUST not have any effects on child spans. Those may
still be running and can be ended later.
`End` MUST NOT inactivate the `Span` in any `Context` it is active in.
It MUST still be possible to use an ended span as parent via a Context it is
contained in. Also, any mechanisms for putting the Span into a Context MUST
still work after the Span was ended.

Parameters:

- (Optional) Timestamp to explicitly set the end timestamp
- (Optional) Timestamp to explicitly set the end timestamp.
If omitted, this MUST be treated equivalent to passing the current time.

This API MUST be non-blocking.

Expand Down Expand Up @@ -621,6 +646,8 @@ The behavior is defined as follows:
are not being recorded, i.e. they are being dropped.

The remaining functionality of `Span` MUST be defined as no-op operations.
Note: This includes `End`, so as an exception from the general rule,
it is not required (or even helpful) to end a Propagated Span.

This functionality MUST be fully implemented in the API, and SHOULD NOT be overridable.

Expand Down

0 comments on commit ec58e45

Please sign in to comment.