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

Allow adding links after span creation #2278

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ release.

### Traces

- Introduce the concept of Instrumentation Scope to replace/extend Instrumentation
Library. The Tracer is now associated with Instrumentation Scope
([#2276](https://github.com/open-telemetry/opentelemetry-specification/pull/2276)).
- Allow adding links after span creation, add new `AddLink` method.
([#2278](https://github.com/open-telemetry/opentelemetry-specification/pull/2278))

### Metrics

### Logs
Expand Down Expand Up @@ -178,6 +184,9 @@ release.
variable to point to the correct HTTP port and correct description of
`OTEL_TRACES_EXPORTER`.
([#2333](https://github.com/open-telemetry/opentelemetry-specification/pull/2333))
- Add `OTEL_EXPORTER_JAEGER_PROTOCOL` environment variable to select the protocol
used by the Jaeger exporter.
([#2341](https://github.com/open-telemetry/opentelemetry-specification/pull/2341))

### Metrics

Expand Down
4 changes: 2 additions & 2 deletions spec-compliance-matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ formats is required. Implementing more than one format is optional.
| Set active Span | | N/A | + | + | + | + | + | + | + | + | + | + |
| [Tracer](specification/trace/api.md#tracer-operations) | | | | | | | | | | | | |
| Create a new Span | | + | + | + | + | + | + | + | + | + | + | + |
| Documentation defines adding attributes at span creation as preferred | | | | | | | | | | | + | |
| Documentation defines adding attributes and links at span creation as preferred | | | | | | | | | | | | |
| Get active Span | | N/A | + | + | + | + | + | + | + | + | + | + |
| Mark Span active | | N/A | + | + | + | + | + | + | + | + | + | + |
| Safe for concurrent calls | | + | + | + | + | + | + | + | + | + | + | + |
Expand Down Expand Up @@ -64,7 +64,7 @@ formats is required. Implementing more than one format is optional.
| Array of primitives (homogeneous) | | + | + | + | + | + | + | + | + | + | + | + |
| `null` values documented as invalid/undefined | | + | + | + | + | + | N/A | + | | + | | N/A |
| Unicode support for keys and string values | | + | + | + | + | + | + | + | + | + | + | + |
| [Span linking](specification/trace/api.md#specifying-links) | | | | | | | | | | | | |
| [AddLink](specification/trace/api.md#add-links) | | | | | | | | | | | | |
| Links can be recorded on span creation | | + | + | | | + | + | + | + | + | + | |
| Links order is preserved | | + | + | | | + | + | + | + | + | + | |
| [Span events](specification/trace/api.md#add-events) | | | | | | | | | | | | |
Expand Down
2 changes: 1 addition & 1 deletion specification/common/common.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ at this time, as discussed in
[data points](../metrics/datamodel.md#metric-points),
[Spans](../trace/api.md#set-attributes), Span
[Events](../trace/api.md#add-events), Span
[Links](../trace/api.md#specifying-links) and
[Links](../trace/api.md#add-links) and
[Log Records](../logs/data-model.md) may contain a collection of attributes. The
keys in each such collection are unique, i.e. there MUST NOT exist more than one
key-value pair with the same key. The enforcement of uniqueness may be performed
Expand Down
2 changes: 1 addition & 1 deletion specification/compatibility/opencensus.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ using the OpenCensus <-> OpenTelemetry bridge.
This leads to some issues with OpenCensus APIs that allowed flexible
specification of parent spans post-initialization.
2. Links added to spans after the spans are created. This is [not supported in
OpenTelemetry](../trace/api.md#specifying-links), therefore OpenCensus spans
OpenTelemetry](../trace/api.md#add-links), therefore OpenCensus spans
that have links added to them after creation will be mapped to OpenTelemetry
spans without the links.
3. OpenTelemetry specifies that samplers are
Expand Down
2 changes: 1 addition & 1 deletion specification/compatibility/opentracing.md
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ OpenTracing defines two types of references:

OpenTelemetry does not define strict equivalent semantics for these
references. These reference types must not be confused with the
[Link](../trace/api.md#specifying-links) functionality. This information
[Link](../trace/api.md##add-links) functionality. This information
is however preserved as the `opentracing.ref_type` attribute.

## In process Propagation exceptions
Expand Down
68 changes: 41 additions & 27 deletions specification/trace/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@
- [Span](#span)
* [Span Creation](#span-creation)
+ [Determining the Parent Span from a Context](#determining-the-parent-span-from-a-context)
+ [Specifying links](#specifying-links)
* [Span operations](#span-operations)
+ [Get Context](#get-context)
+ [IsRecording](#isrecording)
+ [Set Attributes](#set-attributes)
+ [Add Links](#add-links)
+ [Add Events](#add-events)
+ [Set Status](#set-status)
+ [UpdateName](#updatename)
Expand Down Expand Up @@ -284,7 +284,7 @@ the entire operation and, optionally, one or more sub-spans for its sub-operatio
- A start timestamp
- An end timestamp
- [`Attributes`](../common/common.md#attributes)
- A list of [`Link`s](#specifying-links) to other `Span`s
- A list of [`Link`s](#add-links) to other `Span`s
- A list of timestamped [`Event`s](#add-events)
- A [`Status`](#set-status).

Expand Down Expand Up @@ -369,8 +369,14 @@ The API MUST accept the following parameters:
The API documentation MUST state that adding attributes at span creation is preferred
to calling `SetAttribute` later, as samplers can only consider information
already present during span creation.
- [`Link`s](../overview.md#links-between-spans) - an ordered sequence of links.
Additionally, these links may be used to make a sampling decision as
noted in [sampling description](sdk.md#sampling). An empty collection will be
assumed if not specified.

- `Link`s - an ordered sequence of Links, see API definition [here](#specifying-links).
The API documentation MUST state that adding links at span creation is
preferred to calling `AddLink` later, as samplers can only consider information
already present during span creation.
- `Start timestamp`, default to current time. This argument SHOULD only be set
when span creation time has already passed. If API is called at a moment of
a Span logical start, API user MUST NOT explicitly set this argument.
Expand Down Expand Up @@ -405,30 +411,6 @@ A `SpanContext` cannot be set as active in a `Context` directly, but by
[wrapping it into a Span](#wrapping-a-spancontext-in-a-span).
For example, a `Propagator` performing context extraction may need this.

#### Specifying links
Copy link
Member

Choose a reason for hiding this comment

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

Should we keep this section? I don't think that we can remove the existing API for specifying links at creation time. Instead, we should simply remove the last sentence of the first paragraph here:

Links cannot be added after Span creation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that we can remove the existing API for specifying links at creation time.

Right, the API for creating a span is still required to accept a list of links (see above). In addition, a method for adding links after span creation is added as a should.

I tried to structure this in a way similar to attributes, where the situation is similar (attributes can be added at creation time and after span creation). I'm open to change this, if it's clearer in another way.

Copy link

Choose a reason for hiding this comment

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

I was about to jump on the same train, but I've found that the diff is misleading. By reading the whole document (without diff context), the order of the document introduces the creation first and adding links comes later and is discouraged in the span creation section.

My only concern is that the "Link"s link in the "Spans encapsulate:" part jumps straight to the "Add Links" section which may be misleading. Perhaps we can create a separate "Links" subsection that describes what a link is (arguably the same could be done for Events however Events don't make sense at create time)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps we can create a separate "Links" subsection that describes what a link is (arguably the same could be done for Events however Events don't make sense at create time)

Any other opinions on this? Otherwise, I'll go ahead the change the PR according to the suggestion.


During `Span` creation, a user MUST have the ability to record links to other
`Span`s. Linked `Span`s can be from the same or a different trace -- see [Links
between spans](../overview.md#links-between-spans). `Link`s cannot be added after
Span creation.

A `Link` is structurally defined by the following properties:

- `SpanContext` of the `Span` to link to.
- Zero or more [`Attributes`](../common/common.md#attributes) further describing
the link.

The Span creation API MUST provide:

- An API to record a single `Link` where the `Link` properties are passed as
arguments. This MAY be called `AddLink`. This API takes the `SpanContext` of
the `Span` to link to and optional `Attributes`, either as individual
parameters or as an immutable object encapsulating them, whichever is most
appropriate for the language. Implementations MAY ignore links with an
[invalid](#isvalid) `SpanContext`.

Links SHOULD preserve the order in which they're set.

### Span operations

With the exception of the function to retrieve the `Span`'s `SpanContext` and
Expand Down Expand Up @@ -497,6 +479,38 @@ Note that [Samplers](sdk.md#sampler) can only consider information already
present during span creation. Any changes done later, including new or changed
attributes, cannot change their decisions.

#### Add Links

A `Span` SHOULD have the ability to add `Link`s associated with it. Linked
`Span`s can be from the same or a different trace (see
[Links between spans](../overview.md#links-between-spans)).

A `Link` is structurally defined by the following properties:

- `SpanContext` of the `Span` to link to.
- Zero or more [`Attributes`](../common/common.md#attributes) further describing
the link.

The Span interface SHOULD provide:

- An API to record a single `Link` where the `Link` properties are passed as
arguments. This MAY be called `AddLink`. This API takes the `SpanContext` of
the `Span` to link to and optional `Attributes`, either as individual
parameters or as an immutable object encapsulating them, whichever is most
appropriate for the language. Implementations MAY ignore links with an
[invalid](#isvalid) `SpanContext`.

The Span interface MAY provide:

- An API to set multiple `Link`s at once, where the `Link`s are passed in a
single method call.

Spans SHOULD preserve the order in which Links are set.

Note that [Samplers](sdk.md#sampler) can only consider information already
present during span creation. Any `Link`s added later cannot change their
decisions.
pyohannes marked this conversation as resolved.
Show resolved Hide resolved

#### Add Events

A `Span` MUST have the ability to add events. Events have a time associated
Expand Down