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

Small clean up for Propagators. #577

Merged
merged 26 commits into from
Jul 20, 2020
Merged

Small clean up for Propagators. #577

Show file tree
Hide file tree
Changes from 11 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
158 changes: 111 additions & 47 deletions specification/context/api-propagators.md
View file
Open in desktop
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,34 +29,88 @@ 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 library-specific request
interceptors, where the client-side injects values and the server-side extracts them.
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved

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 bound to a data type, in order to propagate in-band context data across process boundaries.
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved

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.
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved

### Carrier

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.

Carriers used at [Inject](#inject) are expected to be mutable.

### Operations

`Propagator`s MUST define `Inject` and `Extract` operations, in order to write
values to and read values from respectively. Each `Propagator` type MUST define the specific carrier type
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved
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:

- 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`
Copy link
Member

@yurishkuro yurishkuro Jun 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I would remove this sentence. There's nothing difficult in calling Propagator.inject(Context.current()) instead of expecting propagator to call Context.current(), and passing the context explicitly allows to decouple the propagator from the implementation details of the context, i.e. it would work with explicitly propagated context just as well, without depending on thread-local model.

Copy link
Member

@Oberon00 Oberon00 Jun 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

On the other hand, you would then couple all the instrumentations to the "implementation details" of the context layer which seems way worse.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's worse. From what we've seen in OpenTracing in Python world, which had a lot of different async frameworks, or even in Java when looking at more Rx-like space, it is pretty difficult to disentangle the instrumentation from specific context propagation framework being used. Thread-local is only one option among many, which would be forced onto everyone if the API allows not passing in the Context. Also, we can always build an additional abstraction above the context layer if we want instrumentation to be really agnostic, without coupling Propagators to thread-locals.

instance.
- The carrier that holds the propagation fields. For example, an outgoing message or HTTP request.

#### 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. It MUST store a value in the `Context`
that the implementation can recognize as a null or empty value.

Required arguments:

- `HTTPTextFormat` is used to inject values into and extract values from carriers as text that travel
in-band across process boundaries.
- A `Context`. For languages supporting current `Context` state this argument is OPTIONAL, defaulting
to the current `Context` instance.
- The carrier that holds the propagation fields. For example, an incoming message or http response.

A binary `Format` will be added in the future.
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.

## HTTP Text Format
If the extracted value is a `SpanContext`, its `IsRemote` property MUST be set to true.
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved

## HTTPText Propagator

`HTTPTextFormat` is a formatter that injects and extracts a cross-cutting concern
value as text into carriers that travel in-band across process boundaries.
`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.


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

Choose a reason for hiding this comment

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

When reading the next paragraph, what does this one add? It seems completely redundant, or I don't understand it's intended meaning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's remove it for now - we can add it later with a proper, longer explanation if needed.


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.

`HTTPTextFormat` MUST expose the APIs that injects values into carriers,
`HTTPTextPropagator` MUST expose the APIs that injects values into carriers,
Copy link
Member

Choose a reason for hiding this comment

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

This paragraph seems useless. If HttpTextPropagator is a propagator, that seems obvious.

and extracts values from carriers.

### Fields
Expand All @@ -68,65 +127,69 @@ 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 downstream. The required arguments are the same as defined by
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved
the base [Inject](#inject) operation.

Required arguments:
Optional arguments:

- 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.

- 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.
`Setter` is defined as a separate object from the carrier to avoid runtime allocations,
removing the need for additional objects wrapping the carrier that access its contents
through a given interface.

`Setter` MUST be stateless and allowed to be saved as a constant to avoid runtime allocations.

#### 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.
Extracts the value from an incoming request. The required arguments are the same as defined by
the base [Extract](#extract) operation.

If a value can not be parsed from the carrier for a cross-cutting concern,
the implementation MUST NOT throw an exception. It MUST store a value in the `Context`
that the implementation can recognize as a null or empty value.
Optional arguments:

Required arguments:
- 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.

- 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.
Returns a new `Context` derived from the `Context` passed as argument.

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.
`Getter` is defined as a separate object from the carrier to avoid runtime allocations,
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved
removing the need for additional objects wrapping the carrier that access its contents
through a given interface.

If the extracted value is a `SpanContext`, its `IsRemote` property MUST be set to true.
`Getter` MUST be stateless and allowed to be saved as a constant to avoid runtime allocations.

#### 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 +200,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. To improve compatibility with other text-based protocols, `HTTPTextPropagator`s 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`).
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved

## 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 +219,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 +256,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 +266,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
View file
Open in desktop
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