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

Centralize attributes definition #722

Merged
merged 8 commits into from
Jul 22, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
29 changes: 29 additions & 0 deletions specification/common/common.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Context
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved

<details>
<summary>
Table of Contents
</summary>

- [Attributes](#attribute)

</details>

## Attributes

Attributes represent a list with zero or more key:value pairs (`Attribute`). An `Attribute` is defined by the following properties:
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved

- (Required) The attribute key, which MUST be a non-`null` and non-empty string.
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
- (Required) The attribute value, which is either:
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
- A primitive type: string, boolean or double (IEEE 754-1985) and int64.
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
- An array of primitive type values. The array MUST be homogeneous,
i.e. it MUST NOT contain values of different types. For protocols that do
not natively support array values such values MUST be represented as JSON strings.
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved

`null` values within arrays MUST be preserved as-is (i.e., passed on to span
processors / exporters as `null`). If exporters do not support exporting `null`
values, they MAY replace those values by 0, `false`, or empty strings.
This is required for map/dictionary structures represented as two arrays with
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the required here a MUST overriding the previous MAY? I'm not sure how to interpret this sentence

Copy link
Member

Choose a reason for hiding this comment

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

This is a note that explains the rationale for both of the above together. It is meant to ban SDKs and exporters from just dropping the null values from the array (changing the index of following elements).

null values within arrays MUST be preserved as-is

Is what basically applies.

exporters [...] MAY replace those values

This is a relaxation for exporters. Notice how they MAY replace those values, but we do not allow them to drop them. So if they choose not to exercise the MAY, the "null values within arrays MUST be preserved as-is" applies.

That is how I read it, and I'm pretty sure that's how it was meant originally (CC @arminru). But it could be worded more clearly.

indices that are kept in sync (e.g., two attributes `header_keys` and `header_values`,
both containing an array of strings to represent a mapping
`header_keys[i] -> header_values[i]`).
11 changes: 3 additions & 8 deletions specification/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,8 @@ Each **Span** encapsulates the following state:

- An operation name
- A start and finish timestamp
- A set of zero or more key:value **Attributes**. The keys must be strings. The
values may be strings, bools, or numeric types.
- A set of zero or more **Events**, each of which is itself a key:value map
paired with a timestamp. The keys must be strings, though the values may be of
the same types as Span **Attributes**.
- [**Attributes**](./common/common.md#attributes)
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
- A set of zero or more **Events**, each of which is itself a tuple (timestamp, name, [**Attributes**](./common/common.md#attributes)). The name must be strings.
- Parent's **Span** identifier.
- [**Links**](#links-between-spans) to zero or more causally-related **Spans**
(via the **SpanContext** of those related **Spans**).
Expand Down Expand Up @@ -300,6 +297,4 @@ Span attributes.
* [Metrics Conventions](metrics/semantic_conventions/README.md)

The type of the attribute SHOULD be specified in the semantic convention
for that attribute. Array values are allowed for attributes. For
protocols that do not natively support array values such values MUST be
represented as JSON strings.
for that attribute. See more details about [Attributes](./common/common.md#attributes).
7 changes: 3 additions & 4 deletions specification/resource/sdk.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,13 @@ The SDK must support two ways to instantiate new resources. Those are:

### Create

The interface MUST provide a way to create a new resource, from a collection of
attributes. Examples include a factory method or a constructor for a resource
The interface MUST provide a way to create a new resource, from [`Attributes`](../common/common.md#attributes).
Examples include a factory method or a constructor for a resource
object. A factory method is recommended to enable support for cached objects.

Required parameters:

- a collection of name/value attributes, where name is a string and value can be one
of: string, int64, double, bool.
- [`Attributes`](../common/common.md#attributes)

### Merge

Expand Down
29 changes: 5 additions & 24 deletions specification/trace/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ the entire operation and, optionally, one or more sub-spans for its sub-operatio
- A [`SpanKind`](#spankind)
- A start timestamp
- An end timestamp
- [`Attribute`s](#set-attributes), a collection of key-value pairs
- [`Attributes`](../common/common.md#attributes)
- 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 @@ -281,8 +281,7 @@ The API MUST accept the following parameters:
See [Determining the Parent Span from a Context](#determining-the-parent-span-from-a-context)
for guidance on `Span` parenting from explicit and implicit `Context`s.
- [`SpanKind`](#spankind), default to `SpanKind.Internal` if not specified.
- `Attribute`s - A collection of key-value pairs, with the same semantics as
the ones settable with [Span::SetAttributes](#set-attributes). Additionally,
- [`Attributes`](../common/common.md#attributes). Additionally,
these attributes 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.
Expand Down Expand Up @@ -334,8 +333,7 @@ description](../overview.md#links-between-spans).
A `Link` is defined by the following properties:

- (Required) `SpanContext` of the `Span` to link to.
- (Optional) One or more `Attribute`s with the same restrictions as defined for
[Span Attributes](#set-attributes).
- (Optional) One or more `Attribute`s as defined [here](../).

The `Link` SHOULD be an immutable type.

Expand Down Expand Up @@ -393,15 +391,7 @@ propagators.

#### Set Attributes

A `Span` MUST have the ability to set attributes associated with it.

An `Attribute` is defined by the following properties:

- (Required) The attribute key, which MUST be a non-`null` and non-empty string.
- (Required) The attribute value, which is either:
- A primitive type: string, boolean or numeric.
- An array of primitive type values. The array MUST be homogeneous,
i.e. it MUST NOT contain values of different types.
A `Span` MUST have the ability to set [`Attributes`](../common/common.md#attributes) associated with it.

The Span interface MUST provide:

Expand All @@ -420,14 +410,6 @@ that `SetAttribute` call had never been made.
As an exception to this, if overwriting of values is supported, this results in
clearing the previous value and dropping the attribute key from the set of attributes.

`null` values within arrays MUST be preserved as-is (i.e., passed on to span
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
processors / exporters as `null`). If exporters do not support exporting `null`
values, they MAY replace those values by 0, `false`, or empty strings.
This is required for map/dictionary structures represented as two arrays with
indices that are kept in sync (e.g., two attributes `header_keys` and `header_values`,
both containing an array of strings to represent a mapping
`header_keys[i] -> header_values[i]`).

Note that the OpenTelemetry project documents certain ["standard
attributes"](semantic_conventions/README.md) that have prescribed semantic meanings.

Expand All @@ -439,8 +421,7 @@ with the moment when they are added to the `Span`.
An `Event` is defined by the following properties:

- (Required) Name of the event.
- (Optional) One or more `Attribute`s with the same restrictions as defined for
[Span Attributes](#set-attributes).
- (Optional) [`Attributes`](../common/common.md#attributes).
- (Optional) Timestamp for the event.

The `Event` SHOULD be an immutable type.
Expand Down