Skip to content

Add the Context notion. #424

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

Merged
merged 85 commits into from
Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
85 commits
Select commit Hold shift + click to select a range
fb10415
Initial context integration.
carlosalberto Jan 22, 2020
8451596
Update api-propagators.md
carlosalberto Jan 24, 2020
5275f32
Remove the mention of binary propagators.
carlosalberto Jan 24, 2020
7c01d61
Mention that the tracing's propagation must happen internally.
carlosalberto Jan 24, 2020
dd834a7
Make context -> Context.
carlosalberto Jan 27, 2020
7f9c6b8
Rename api-context.md to context.md
carlosalberto Jan 27, 2020
f43b7db
Remove the notion of key.
carlosalberto Jan 27, 2020
548e3d0
Add a note on Context being a SDK API.
carlosalberto Jan 27, 2020
4942040
Note on the optional global operations.
carlosalberto Jan 27, 2020
dda30bb
Clean up pass.
carlosalberto Jan 27, 2020
59cf5f3
Remove context-prop from the *tracing* index.
carlosalberto Jan 27, 2020
278839b
Update specification/api-propagators.md
carlosalberto Jan 27, 2020
71f51e5
Update specification/api-propagators.md
carlosalberto Jan 27, 2020
a3502c1
Update specification/api-propagators.md
carlosalberto Jan 27, 2020
9be19e2
Update specification/context.md
carlosalberto Jan 27, 2020
ae1c052
First clean up pass over Context.
carlosalberto Jan 30, 2020
0a4f08d
Update specification/api-propagators.md
carlosalberto Jan 30, 2020
a491a5b
Overview and layout updates.
carlosalberto Jan 30, 2020
a36759c
Make the MD linter happy (long lines though).
carlosalberto Jan 30, 2020
58c6248
Update specification/context.md
carlosalberto Jan 30, 2020
ecdd80d
Update specification/context.md
carlosalberto Jan 30, 2020
95f7bd2
Update specification/context.md
carlosalberto Jan 30, 2020
656fe58
Update specification/context.md
carlosalberto Jan 30, 2020
9e8a3bd
Use MUST/SHOULD for clarifying propagators expectations.
carlosalberto Jan 30, 2020
086facb
Move the IsRemote deserialization note to the Extraction section.
carlosalberto Jan 30, 2020
63c40a6
Improve wording for Context handling in Extraction.
carlosalberto Jan 30, 2020
551a255
Clarify the protected global methods in context.md
carlosalberto Jan 30, 2020
bf398e1
Remove the extra explanation of Context in overview.md
carlosalberto Jan 30, 2020
5a9a679
Improve the wording for set/remove values in context.md
carlosalberto Jan 30, 2020
0f26bc7
Merge branch 'master' into context_aware
carlosalberto Jan 30, 2020
6b962ba
Update specification/api-propagators.md
carlosalberto Jan 30, 2020
60e22c0
Update specification/api-propagators.md
carlosalberto Jan 30, 2020
505c2f7
Update specification/api-propagators.md
carlosalberto Jan 30, 2020
e06c9b0
Update specification/api-propagators.md
carlosalberto Jan 30, 2020
af9287a
Fix style?
carlosalberto Jan 30, 2020
d38b6fc
Clarify the Context usage in Extract.
carlosalberto Jan 30, 2020
0e76724
Update specification/context.md
carlosalberto Jan 30, 2020
c697fef
Clarify IsRemote on Extract.
carlosalberto Jan 30, 2020
a4d588c
More style changes.
carlosalberto Jan 30, 2020
fdd79e1
OTel MUST provide Context impl if none exists.
carlosalberto Jan 30, 2020
48d98c2
Update specification/context.md
carlosalberto Jan 30, 2020
07398c1
Update specification/context.md
carlosalberto Jan 30, 2020
051f36f
Update specification/context.md
carlosalberto Jan 31, 2020
e307486
Reword the optionality of Context in propagators.
carlosalberto Jan 31, 2020
073a526
Clarify again the expected usage for implicit `Context`.
carlosalberto Jan 31, 2020
9745124
Update specification/context.md
carlosalberto Jan 31, 2020
4649574
Be more strict on the Context parameters.
carlosalberto Jan 31, 2020
c4b3423
Merge branch 'master' into context_aware
carlosalberto Feb 3, 2020
0b5c1f1
Clarify that SDK/OTel instrumentation only SHOULD use global Context.
carlosalberto Feb 4, 2020
3b58a29
Mention that Context will be used through concerns APIs.
carlosalberto Feb 4, 2020
3fe6238
Update specification/api-propagators.md
carlosalberto Feb 4, 2020
732f4e9
Update specification/api-propagators.md
carlosalberto Feb 4, 2020
01e0925
We do not use the Required word anymore.
carlosalberto Feb 4, 2020
a3e5134
Do not require extract to set null/empty upon errors.
carlosalberto Feb 5, 2020
13b1c02
Better wording for failed extraction values.
carlosalberto Feb 5, 2020
75ae4bc
Update specification/context.md
carlosalberto Feb 5, 2020
5f0b166
Clarify ONCE that Context is immutable.
carlosalberto Feb 5, 2020
7393c0c
Use "derived" instead of "child" for Context-Context relationships.
carlosalberto Feb 5, 2020
42adf76
Remove a misleading line.
carlosalberto Feb 5, 2020
eeb9afc
Mention tracing uses Context specifially.
carlosalberto Feb 5, 2020
0794250
Update specification/api-propagators.md
carlosalberto Feb 5, 2020
190da25
Update specification/api-propagators.md
carlosalberto Feb 6, 2020
c8bbf39
Merge branch 'master' into context_aware
carlosalberto Feb 6, 2020
f28568c
Clarify that Context keeps the untouched original values.
carlosalberto Feb 6, 2020
74d1084
Merge branch 'master' into context_aware
carlosalberto Feb 6, 2020
dc92999
Clarify a new Context is created only for write operations.
carlosalberto Feb 6, 2020
93a0cf7
Misc fixes.
carlosalberto Feb 10, 2020
e91a898
Set current Context should return a handle.
carlosalberto Feb 10, 2020
2db3f2e
Note on global Context operations.
carlosalberto Feb 11, 2020
4e02075
Remove the Remove operation on Context.
carlosalberto Feb 11, 2020
8bc320e
Clarify the usage of cross-cutting concern APIs.
carlosalberto Feb 11, 2020
96c26b9
Restore "Create a key" section.
carlosalberto Feb 11, 2020
85cbb5b
Merge branch 'master' into context_aware
carlosalberto Feb 11, 2020
f1431c8
Minor fix to a previous commit.
carlosalberto Feb 11, 2020
965f365
Add a detach operation.
carlosalberto Feb 11, 2020
a289dff
Update specification/api-propagators.md
carlosalberto Feb 12, 2020
27e701a
Update specification/api-propagators.md
carlosalberto Feb 12, 2020
980af1b
Update specification/context.md
carlosalberto Feb 12, 2020
c9c3cf1
Update specification/context.md
carlosalberto Feb 12, 2020
31af4d9
Update specification/context.md
carlosalberto Feb 12, 2020
a3567d9
Update specification/context.md
carlosalberto Feb 12, 2020
cbd3b71
Update specification/context.md
carlosalberto Feb 12, 2020
54735b8
Update specification/api-tracing.md
carlosalberto Feb 12, 2020
349af35
Use value instead of object for Token.
carlosalberto Feb 12, 2020
6ce4054
Merge branch 'master' into context_aware
bogdandrutu Feb 12, 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
62 changes: 19 additions & 43 deletions specification/api-propagators.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
Table of Contents
</summary>

- [Binary Format](#binary-format)
- [ToBytes](#tobytes)
- [FromBytes](#frombytes)
- [Overview] (#overview)
- [HTTP Text Format](#http-text-format)
- [Fields](#fields)
- [Inject](#inject)
Expand All @@ -19,43 +17,15 @@ Table of Contents

</details>

Propagators API consists of two main formats:
## Overview

- `BinaryFormat` is used to serialize and deserialize a value into a binary representation.
- `HTTPTextFormat` is used to inject and extract a value as text into carriers that travel
in-band across process boundaries.

Deserializing must set `IsRemote` to true on the returned `SpanContext`.

## Binary Format

`BinaryFormat` is a formatter to serialize and deserialize a value into a binary format.

`BinaryFormat` MUST expose the APIs that serializes values into bytes,
and deserializes values from bytes.

### ToBytes

Serializes the given value into the on-the-wire representation.

Required arguments:

- the value to serialize, can be `SpanContext` or `DistributedContext`.
Propagators leverage the `Context` to inject and extract
data for each cross-cutting concern, such as traces and metrics.

Returns the on-the-wire byte representation of the value.
The Propagators API consists of the following formats:

### FromBytes

Creates a value from the given on-the-wire encoded representation.

If the value could not be parsed, the underlying implementation SHOULD decide to return ether
an empty value, an invalid value, or a valid value.

Required arguments:

- on-the-wire byte representation of the value.

Returns a value deserialized from bytes.
- `HTTPTextFormat` is used to inject values into and extract values from carriers as text that travel
in-band across process boundaries.

## HTTP Text Format

Expand Down Expand Up @@ -94,7 +64,7 @@ Injects the value downstream. For example, as http headers.

Required arguments:

- the value to be injected, can be `SpanContext` or `DistributedContext`.
- A `Context`. The Propagator MUST retrieve the appropiate value from the `Context` first, which can be `SpanContext`, `DistributedContext` 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.

Expand All @@ -120,18 +90,24 @@ The implemenation SHOULD preserve casing (e.g. it should not transform `Content-

### Extract

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

If the value could not be parsed, the underlying implementation will decide to return an
object representing either an empty value, an invalid value, or a valid value. Implementations
MUST not return null.
If a cross-cutting concern value could not be parsed, the implementation MUST set a value
it deems appropiate, and it MUST NOT throw any exception.

Required arguments:

- 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 the non-null extracted value.
Returns a new `Context` as child of the `Context` passed as argument,
containing the extracted value, which can be a `SpanContext`,
`DistributedContext` or another cross-cutting concern context.

The `Context` passed as an argument MUST NOT be modified.

If the extracted value is a `SpanContext`, its `IsRemote` property MUST be set to true.

#### Getter argument

Expand Down
20 changes: 7 additions & 13 deletions specification/api-tracing.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ Table of Contents
* [GetCurrentSpan](#getcurrentspan)
* [WithSpan](#withspan)
* [SpanBuilder](#spanbuilder)
* [GetBinaryFormat](#getbinaryformat)
* [GetHttpTextFormat](#gethttptextformat)
* [SpanContext](#spancontext)
* [Span](#span)
* [Span creation](#span-creation)
Expand Down Expand Up @@ -127,14 +125,15 @@ mechanism, for instance the `ServiceLoader` class in Java.

The `Tracer` MUST provide methods to:

- Get the currently active `Span`
- Create a new `Span`

The `Tracer` SHOULD provide methods to:

- Get the currently active `Span`
- Make a given `Span` as active

The `Tracer` SHOULD allow end users to configure other tracing components that
control how `Span`s are passed across process boundaries, including the binary
and text format `Propagator`s used to serialize `Span`s created by the
`Tracer`.
The `Tracer` MUST internally leverage the context layer in order to handle the
current `Span` state and how `Span`s are passed across process boundaries.

When getting the current span, the `Tracer` MUST return a placeholder `Span`
with an invalid `SpanContext` if there is no currently active `Span`.
Expand All @@ -145,18 +144,13 @@ SHOULD create each new `Span` as a child of its active `Span` unless an
explicit parent is provided or the option to create a span without a parent is
selected, or the current active `Span` is invalid.

The `Tracer` MUST provide a way to update its active `Span`, and MAY provide
The `Tracer` SHOULD provide a way to update its active `Span` and MAY provide
convenience methods to manage a `Span`'s lifetime and the scope in which a
`Span` is active. When an active `Span` is made inactive, the previously-active
`Span` SHOULD be made active. A `Span` maybe finished (i.e. have a non-null end
Copy link
Member

Choose a reason for hiding this comment

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

s/maybe/may be/

Is "finished" accurate? I wish we kept OT naming here, "span ended" sounds weird.

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 will update this line in another (tiny) PR.

time) but stil active. A `Span` may be active on one thread after it has been
made inactive on another.

The implementation MUST provide no-op binary and text `Propagator`s, which the
`Tracer` SHOULD use by default if other propagators are not configured. SDKs
SHOULD use the W3C HTTP Trace Context as the default text format. For more
details, see [trace-context](https://github.com/w3c/trace-context).

## SpanContext

A `SpanContext` represents the portion of a `Span` which must be serialized and
Expand Down
96 changes: 96 additions & 0 deletions specification/context.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Context

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

- [Overview](#overview)
- [Get value](#get-value)
- [Set value](#set-value)
- [Remove value](#remove-value)
- [Optional operations](#optional-operations)
- [Get current Context](#get-current-context)
- [Set current Context](#set-current-context)

</details>

## Overview

`Context` is a propagation mechanism which carries execution-scoped values
across API boundaries and between logically associated execution units.
Cross-cutting concerns access their data in-process using the same shared
`Context` object.

Languages are expected to use the single, widely used `Context` implementation
if one exists for them. In the cases where an extremely clear, pre-existing
option is not available, OpenTelemetry MUST provide its own `Context`
implementation. Depending on the language, its usage may be either explicit
or implicit.

`Context` can be considered as an implementation detail of the SDK.
Users will manipulate `Context` through the cross-cutting concerns APIs
rather than directly.

Users writing instrumentation in languages that
use `Context` implicitly are discouraged to use the `Context` API directly.

`Context` is expected to have the following operations, with their
respective language differences:

## Get value

Concerns can access their local state in the current execution state
represented by a `Context`.

The API MUST accept the following parameters:

- The `Context`.
- The concern identifier.

The API MUST return the value in the `Context` for the specified concern
identifier.

## Set value

Concerns can record their local state in the current execution state
represented by a `Context`.

The API MUST accept the following parameters:

- The `Context`.
- The concern identifier.
- The value to be set.

The API MUST return a new `Context` containing the new value.
The `Context` passed as parameter MUST NOT be modified.

## Remove value

Concerns can clear their local state in a specified `Context`.

The API MUST accept the following parameters:

- The `Context`.
- The concern identifier.

The API MUST return a new `Context` with the value cleared.
The `Context` passed as parameter MUST NOT be modified.

## Optional Global operations

These operations are optional, and SHOULD only be used to
implement automatic scope switching and define higher level APIs
by SDK components and OpenTelemetry instrumentation libraries.

### Get current Context

The API MUST return the `Context` associated with the caller's current execution unit.

### Set current Context

Associates a `Context` with the caller's current execution unit.

The API MUST accept the following parameters:

- The `Context`.
Copy link
Member

Choose a reason for hiding this comment

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

We should encourage to have this return a Token that is used to restore the Context. See Java (grpc) and https://docs.python.org/3/library/contextvars.html#contextvars.ContextVar.set which does the same at a value level because set makes that value current.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added another minor note as well.

Copy link
Member

Choose a reason for hiding this comment

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

We probably need a restore where we use the handle to restore to the previous Context.

PS: attach/detach are probably better names for these 2 methods.

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 leave Restore out for now as well (for simplicity purposes - lets add it when we add "Remove" from Context to have a more complete set of operations).

And you mean:

Set current Context -> Attach Context
Restore Context -> Detach Context

?

To me that sounds great, but probably too Java-ish ;)

Copy link
Member

@bogdandrutu bogdandrutu Feb 11, 2020

Choose a reason for hiding this comment

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

Restore/Detach is not equivalent to Remove. You need to have code like this:

Token token = Context.attach(newContext);
try {
  // your code;
} finally {
  Context.detach(token);
}

So detach is very important compared with Remove, I don't see how a Context would work without detach, because you will set a new Context and that will stay active even after you are out of scope. Another option is to have Context.attach(newContext) returning a Scope that when it goes out of scope restores the previous Context. See example:

try (Scope scope = Context.attach(newContext)) {
  // your code;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the way, added a small section on the Detach Context part. Please review.

Copy link
Contributor

Choose a reason for hiding this comment

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

My question around this change is that since the Context API already provides a mechanism to explicitly set and get a Context, how will using a token to restore a previous Context differ from getting a previous Context then setting it again when wanting to restore?

Copy link
Member

Choose a reason for hiding this comment

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

@codeboten this is a very interesting point about why not just simply having one set is not enough, because as you said it can be used to restore the previous Context.

There are few reasons:

  1. Checking correctness - by having a restore that accepts a Token or just simply the previous Context you can check if the given value is the expected one (in other words you can see the calls to set as push to a stack and restore as pop where you check that the element you intend to pop is the expected one. This is critical and I've seen cases where auth tokens were leaked because of this issue (I don't think I can share too many things about this issue, but the root cause were people calling set without a paired restore).
  2. This approach guarantees (at least tries to enforce because mistakes can still happen) that the Context is not "modified" by calling a function:
void bar() {
  Context current = Context.current();
  foo();
  // This should always be the case, otherwise for examples
  // credentials can be leaked here.
  assert(current == Context.current());
}

It can still happen if a Set is not followed by a Restore, but by having the Restore users can identify very soon this wrong behavior because the next Restore call will give a signal to the users (log, crash, based on a config).

So this API helps users use the Context API in an expected way.

Copy link
Member

Choose a reason for hiding this comment

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

Can this reasoning be added to the spec? It became a topic for discussion without any sort of information in the merged spec itself to help substantiate: open-telemetry/opentelemetry-python#429 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added #490

14 changes: 12 additions & 2 deletions specification/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,22 @@ OpenTelemetry
[proto](https://github.com/open-telemetry/opentelemetry-proto/blob/a46c815aa5e85a52deb6cb35b8bc182fb3ca86a0/src/opentelemetry/proto/agent/common/v1/common.proto#L28-L96)
for an example.

## Context Propagation

All of OpenTelemetry cross-cutting concerns, such as traces and metrics,
share an underlying `Context` mechanism for storing state and
accessing data across the lifespan of a distributed transaction.

See the [Context](context.md)

## Propagators

OpenTelemetry uses `Propagators` to serialize and deserialize `SpanContext` and `DistributedContext`
into a binary or text format. Currently there are two types of propagators:
into any of the supported formats. Observe that `Propagators` leverage the underlying
`Context` to both access and record cross-cutting concerns data.

Currently there is one type of propagators:

- `BinaryFormat` which is used to serialize and deserialize a value into a binary representation.
- `HTTPTextFormat` which is used to inject and extract a value as text into carriers that travel
in-band across process boundaries.

Expand Down