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

Small clean up for Propagators. #577

Merged
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
971168e
Small clean up for Propagators.
carlosalberto Apr 27, 2020
445c105
Update specification/context/api-propagators.md
carlosalberto Apr 28, 2020
0dce0df
Update Format/Propagator relationship.
carlosalberto May 1, 2020
33673c5
Fix lint.
carlosalberto May 1, 2020
93e44dc
Do not use Format but Propagator type.
carlosalberto May 4, 2020
9bd2f0d
Update overview.
carlosalberto May 5, 2020
a2639e3
Amend style.
carlosalberto May 5, 2020
02764e4
Merge branch 'master' into clean_up_propagators
carlosalberto May 21, 2020
f99a88f
Add a small explanation on why getter/setters are separate entities.
carlosalberto May 22, 2020
809fa6c
Make getter/setter optional arguments.
carlosalberto Jun 19, 2020
65eebac
More love.
carlosalberto Jun 19, 2020
8b31eb5
More love.
carlosalberto Jul 9, 2020
250967c
Merge branch 'master' into clean_up_propagators
carlosalberto Jul 9, 2020
b44e83c
Update specification/context/api-propagators.md
carlosalberto Jul 9, 2020
46fd93c
Update specification/context/api-propagators.md
carlosalberto Jul 9, 2020
4c6dd1f
Update specification/trace/api.md
carlosalberto Jul 9, 2020
58af0eb
Update specification/context/api-propagators.md
carlosalberto Jul 9, 2020
bdb834f
Update specification/context/api-propagators.md
carlosalberto Jul 9, 2020
8d97361
Remove redundant sections.
carlosalberto Jul 10, 2020
48f3b06
Clarify Setter/Getter.
carlosalberto Jul 10, 2020
a390037
Small mention of what a field is.
carlosalberto Jul 10, 2020
dc659c9
Update specification/context/api-propagators.md
carlosalberto Jul 10, 2020
1297d08
Update CHANGELOG.
carlosalberto Jul 10, 2020
d4022d7
Remove the mention of default `Context` for Injection/Extraction.
carlosalberto Jul 14, 2020
b69ef73
Merge branch 'master' into clean_up_propagators
carlosalberto Jul 16, 2020
c4d4227
Merge branch 'master' into clean_up_propagators
carlosalberto Jul 18, 2020
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 @@ -15,6 +15,7 @@ Updates:

- Add semantic convention for NGINX custom HTTP 499 status code.
- Adapt semantic conventions for the span name of messaging systems ([#690](https://github.com/open-telemetry/opentelemetry-specification/pull/690))
- Clean up api-propagators.md, by extending documentation and removing redundant sections ([#577](https://github.com/open-telemetry/opentelemetry-specification/pull/577))

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

Expand Down
155 changes: 103 additions & 52 deletions specification/context/api-propagators.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@ Table of Contents
</summary>

- [Overview](#overview)
- [HTTP Text Format](#http-text-format)
- [Propagator Types](#propagator-types)
- [Carrier](#carrier)
- [Operations](#operations)
- [Inject](#inject)
- [Extract](#extract)
- [HTTPText Propagator](#httptext-propagator)
- [Fields](#fields)
- [Inject](#inject)
- [Inject](#inject-1)
- [Setter argument](#setter)
- [Set](#set)
- [Extract](#extract)
- [Extract](#extract-1)
- [Getter argument](#getter)
- [Get](#get)
- [Composite Propagator](#composite-propagator)
Expand All @@ -24,41 +29,94 @@ Table of Contents
Cross-cutting concerns send their state to the next process using
`Propagator`s, which are defined as objects used to read and write
context data to and from messages exchanged by the applications.
Each concern creates a set of `Propagator`s for every supported `Format`.
Each concern creates a set of `Propagator`s for every supported
`Propagator` type.

Propagators leverage the `Context` to inject and extract data for each
`Propagator`s leverage the `Context` to inject and extract data for each
cross-cutting concern, such as traces and correlation context.

Propagation is usually implemented via a cooperation of library-specific request
interceptors and `Propagators`, where the interceptors detect incoming and outgoing requests and use the `Propagator`'s extract and inject operations respectively.

The Propagators API is expected to be leveraged by users writing
instrumentation libraries.

The Propagators API currently consists of one `Format`:
## Propagator Types

A `Propagator` type defines the restrictions imposed by a specific transport
and is bound to a data type, in order to propagate in-band context data across process boundaries.
Copy link
Member

Choose a reason for hiding this comment

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


The Propagators API currently defines one `Propagator` type:

- `HTTPTextPropagator` is a type that inject values into and extracts values
from carriers as string key/value pairs.

A binary `Propagator` type will be added in the future (see [#437](https://github.com/open-telemetry/opentelemetry-specification/issues/437)).

### Carrier

- `HTTPTextFormat` is used to inject values into and extract values from carriers as text that travel
in-band across process boundaries.
A carrier is the medium used by `Propagator`s to read values from and write values to.
Each specific `Propagator` type defines its expected carrier type, such as a string map
or a byte array.

A binary `Format` will be added in the future.
Carriers used at [Inject](#inject) are expected to be mutable.

## HTTP Text Format
### Operations

`Propagator`s MUST define `Inject` and `Extract` operations, in order to write
values to and read values from carriers respectively. Each `Propagator` type MUST define the specific carrier type
and CAN define additional parameters.
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved

#### Inject

Injects the value into a carrier. For example, into the headers of an HTTP request.

Required arguments:

`HTTPTextFormat` is a formatter that injects and extracts a cross-cutting concern
value as text into carriers that travel in-band across process boundaries.
- A `Context`. The Propagator MUST retrieve the appropriate value from the `Context` first, such as
`SpanContext`, `CorrelationContext` or another cross-cutting concern context.
- The carrier that holds the propagation fields. For example, an outgoing message or HTTP request.

Encoding is expected to conform to the HTTP Header Field semantics. Values are often encoded as
RPC/HTTP request headers.
#### Extract

Extracts the value from an incoming request. For example, from the headers of an HTTP request.

If a value can not be parsed from the carrier for a cross-cutting concern,
the implementation MUST NOT throw an exception and MUST NOT store a new value in the `Context`,
in order to preserve any previously existing valid value.

Required arguments:

- A `Context`.
- The carrier that holds the propagation fields. For example, an incoming message or http response.

Returns a new `Context` derived from the `Context` passed as argument,
containing the extracted value, which can be a `SpanContext`,
`CorrelationContext` or another cross-cutting concern context.

## HTTPText Propagator

`HTTPTextPropagator` performs the injection and extraction of a cross-cutting concern
value as string key/values pairs into carriers that travel in-band across process boundaries.
Comment on lines +99 to +100
Copy link
Member

Choose a reason for hiding this comment

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

This has a lot of redundancy. I would simply state:

Suggested change
`HTTPTextPropagator` performs the injection and extraction of a cross-cutting concern
value as string key/values pairs into carriers that travel in-band across process boundaries.
`HTTPTextPropagator` is a `Propagator` whose carrier type is a list of string key value pairs.

or instead of "list of string key value pairs", we could use "multimap from string to string".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The carrier itself might not be a list of string/key pairs (even if it can be deserialized as such). Also, using key-value pairs keep things more generic, as opposed to introducing the notion of a map.


The carrier of propagated data on both the client (injector) and server (extractor) side is
usually an http request. Propagation is usually implemented via library-specific request
interceptors, where the client-side injects values and the server-side extracts them.
usually an HTTP request.

`Getter` and `Setter` are optional helper components used for extraction and injection respectively,
and are defined as separate objects from the carrier to avoid runtime allocations,
by removing the need for additional interface-implementing-objects wrapping the carrier in order
to access its contents.

`HTTPTextFormat` MUST expose the APIs that injects values into carriers,
and extracts values from carriers.
`Getter` and `Setter` MUST be stateless and allowed to be saved as constants, in order to effectively
avoid runtime allocations.

### Fields

The propagation fields defined. If your carrier is reused, you should delete the fields here
before calling [inject](#inject).

Fields are defined as string keys identifying format-specific components in a carrier.
Copy link
Member

@Oberon00 Oberon00 Jul 14, 2020

Choose a reason for hiding this comment

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

Maybe it would be more clear to define fields as the key-value pairs and then use field key or field name below.

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'd rather so that as a follow up in order to make progress and merge this PR ;)

Trying to clarify: you are suggesting we only change the terms used (and update the document with them), not the actual return value (a list of strings), right?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I was just trying to clarify terminology, not suggesting an actual content change.


For example, if the carrier is a single-use or immutable request object, you don't need to
clear fields as they couldn't have been set before. If it is a mutable, retryable object,
successive calls should clear these fields first.
Expand All @@ -68,65 +126,57 @@ The use cases of this are:
- allow pre-allocation of fields, especially in systems like gRPC Metadata
- allow a single-pass over an iterator

Returns list of fields that will be used by this formatter.
Returns list of fields that will be used by the `HttpTextPropagator`.
Oberon00 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

I find this Fields section very confusing and not well-worded (e.g. what does "Returns..." refer to?) Let's revisit it in another PR. #712


### Inject

Injects the value downstream. For example, as http headers.
Injects the value into a carrier. The required arguments are the same as defined by
the base [Inject](#inject) operation.

Required arguments:
Optional arguments:

- A `Context`. The Propagator MUST retrieve the appropriate value from the `Context` first, such as `SpanContext`, `CorrelationContext` or another cross-cutting concern context. For languages supporting current `Context` state, this argument is OPTIONAL, defaulting to the current `Context` instance.
- the carrier that holds propagation fields. For example, an outgoing message or http request.
- the `Setter` invoked for each propagation key to add or remove.
- A `Setter` invoked for each propagation key to add or remove. This is an additional
argument that languages are free to define to help inject data into the carrier.

#### Setter argument

Setter is an argument in `Inject` that sets value into given field.
Setter is an argument in `Inject` that sets values into given fields.

`Setter` allows a `HTTPTextFormat` to set propagated fields into a carrier.
`Setter` allows a `HttpTextPropagator` to set propagated fields into a carrier.

`Setter` MUST be stateless and allowed to be saved as a constant to avoid runtime allocations. One of the ways to implement it is `Setter` class with `Set` method as described below.
One of the ways to implement it is `Setter` class with `Set` method as described below.

##### Set

Replaces a propagated field with the given value.

Required arguments:

- the carrier holds propagation fields. For example, an outgoing message or http request.
- the carrier holding the propagation fields. For example, an outgoing message or an HTTP request.
- the key of the field.
- the value of the field.

The implemenation SHOULD preserve casing (e.g. it should not transform `Content-Type` to `content-type`) if the used protocol is case insensitive, otherwise it MUST preserve casing.
The implementation SHOULD preserve casing (e.g. it should not transform `Content-Type` to `content-type`) if the used protocol is case insensitive, otherwise it MUST preserve casing.

### Extract

Extracts the value from an incoming request. For example, from the headers of an HTTP request.

If a value can not be parsed from the carrier for a cross-cutting concern,
the implementation MUST NOT throw an exception and MUST NOT store a new value in the `Context`,
in order to preserve any previously existing valid value.

Required arguments:
Extracts the value from an incoming request. The required arguments are the same as defined by
the base [Extract](#extract) operation.

- A `Context`. For languages supporting current `Context` state this argument is OPTIONAL, defaulting to the current `Context` instance.
- the carrier holds propagation fields. For example, an outgoing message or http request.
- the instance of `Getter` invoked for each propagation key to get.
Optional arguments:

Returns a new `Context` derived from the `Context` passed as argument,
containing the extracted value, which can be a `SpanContext`,
`CorrelationContext` or another cross-cutting concern context.
- A `Getter` invoked for each propagation key to get. This is an additional
argument that languages are free to define to help extract data from the carrier.

If the extracted value is a `SpanContext`, its `IsRemote` property MUST be set to true.
Returns a new `Context` derived from the `Context` passed as argument.

#### Getter argument

Getter is an argument in `Extract` that get value from given field

`Getter` allows a `HttpTextFormat` to read propagated fields from a carrier.
`Getter` allows a `HttpTextPropagator` to read propagated fields from a carrier.

`Getter` MUST be stateless and allowed to be saved as a constant to avoid runtime allocations. One of the ways to implement it is `Getter` class with `Get` method as described below.
One of the ways to implement it is `Getter` class with `Get` method as described below.

##### Get
Copy link
Member

Choose a reason for hiding this comment

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

Jaeger won't be able to implement its propagation format if only Get(key) is available. Should this include the iterator over keys as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be covered by future work done as part of #433

Copy link
Member

Choose a reason for hiding this comment

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

#433 is about multi-valued headers, different from iterator. I booked #713.


Expand All @@ -137,11 +187,11 @@ Required arguments:
- the carrier of propagation fields, such as an HTTP request.
- the key of the field.

The Get function is responsible for handling case sensitivity. If the getter is intended to work with a HTTP request object, the getter MUST be case insensitive. To improve compatibility with other text-based protocols, text `Format` implementions MUST ensure to always use the canonical casing for their attributes. NOTE: Canonical casing for HTTP headers is usually title case (e.g. `Content-Type` instead of `content-type`).
The Get function is responsible for handling case sensitivity. If the getter is intended to work with a HTTP request object, the getter MUST be case insensitive.

## Injectors and Extractors as Separate Interfaces

Languages can choose to implement a `Propagator` for a format as a single object
Languages can choose to implement a `Propagator` type as a single object
exposing `Inject` and `Extract` methods, or they can opt to divide the
responsibilities further into individual `Injector`s and `Extractor`s. A
`Propagator` can be implemented by composing individual `Injector`s and
Expand All @@ -156,9 +206,10 @@ single entity.
A composite propagator can be built from a list of propagators, or a list of
injectors and extractors. The resulting composite `Propagator` will invoke the `Propagator`s, `Injector`s, or `Extractor`s, in the order they were specified.

Each composite `Propagator` will be bound to a specific `Format`, such
as `HttpTextFormat`, as different `Format`s will likely operate on different
Each composite `Propagator` will implement a specific `Propagator` type, such
as `HttpTextPropagator`, as different `Propagator` types will likely operate on different
data types.

There MUST be functions to accomplish the following operations.

- Create a composite propagator
Expand Down Expand Up @@ -192,7 +243,7 @@ Required arguments:
## Global Propagators

Implementations MAY provide global `Propagator`s for
each supported `Format`.
each supported `Propagator` type.

If offered, the global `Propagator`s should default to a composite `Propagator`
containing the W3C Trace Context Propagator and the Correlation Context `Propagator`
Expand All @@ -202,13 +253,13 @@ OpenTelemetry implementations.

### Get Global Propagator

This method MUST exist for each supported `Format`.
This method MUST exist for each supported `Propagator` type.

Returns a global `Propagator`. This usually will be composite instance.

### Set Global Propagator

This method MUST exist for each supported `Format`.
This method MUST exist for each supported `Propagator` type.

Sets the global `Propagator` instance.

Expand Down
9 changes: 5 additions & 4 deletions specification/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,12 @@ See the [Context](context/context.md)
## Propagators

OpenTelemetry uses `Propagators` to serialize and deserialize cross-cutting concern values
such as `SpanContext` and `CorrelationContext` into a `Format`. Currently there is one
type of propagator:
such as `SpanContext` and `CorrelationContext`. Different `Propagator` types define the restrictions
imposed by a specific transport and bound to a data type.

- `HTTPTextFormat` which is used to inject and extract a value as text into carriers that travel
in-band across process boundaries.
The Propagators API currently defines one `Propagator` type:

- `HTTPTextPropagator` injects values into and extracts values from carriers as text.
Oberon00 marked this conversation as resolved.
Show resolved Hide resolved

## Collector

Expand Down
3 changes: 2 additions & 1 deletion specification/trace/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ TraceID and a non-zero SpanID.

`IsRemote` is a boolean flag which returns true if the SpanContext was propagated
from a remote parent.
When creating children from remote spans, their IsRemote flag MUST be set to false.
When extracting a `SpanContext` through the Propagators API, its `IsRemote` flag MUST
be set to true, whereas the SpanContext of any child spans MUST have it set to false.

Please review the W3C specification for details on the [Tracestate
field](https://www.w3.org/TR/trace-context/#tracestate-field).
Expand Down