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

Metric SDK: add asynchronous instrument details; add export pipeline terminology details #1159

Closed
wants to merge 3 commits into from
Closed

Metric SDK: add asynchronous instrument details; add export pipeline terminology details #1159

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a374045
Add asynchronous instrument details; add export pipeline terminology …
jmacd Oct 30, 2020
cfb0411
Upstream
jmacd Nov 5, 2020
cc9c86f
Apply suggestions from code review
jmacd Nov 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
73 changes: 66 additions & 7 deletions specification/metrics/sdk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ collection cycle.
The Processor component is intended as the most customizable component
in the export pipeline. The Processor is responsible for selecting
Aggregators to use for specific instruments, via an independent
`AggregationSelector` interface, for reducing dimensionality, and for
`AggregatorSelector` interface, for reducing dimensionality, and for
conversion between DELTA and CUMULATIVE data point representation.
The Processor interface supports arbitrary protocol-independent data
transformation, and Processors can be chained together to form more
Expand Down Expand Up @@ -231,7 +231,7 @@ Accumulator, with detail shown for synchronous instruments.
For a synchronous instrument, the Accumulator will:

1. Map each active Label Set to a record, consisting of two instances of the same type Aggregator
2. Enter new records into the mapping, calling the AggregationSelector if needed
2. Enter new records into the mapping, calling the AggregatorSelector if needed
3. Update the current Aggregator instance, responding to concurrent API events
4. Call Aggregator.SynchronizedMove on the current Aggregator instance to: (a) copy its value into the snapshot Aggregator instance and (b) reset the current Aggregator to the zero state
5. Call Processor.Process for every resulting Accumulation (i.e., Instrument, Label Set, Resource, and Aggregator snapshot)
Expand All @@ -256,7 +256,10 @@ The Accumulator MUST implement a Collect method that builds and
processes current Accumulation values for active instruments, meaning
those that were updated since the prior collection. The Collect
method MUST call the Processor to process Accumulations corresponding
to all metric events that happened before the call.
to all metric events that happened since the prior collection.

Accumulations from a single instrument MUST be input to the Processor
during a single [Processor checkpoint interval](#processor-checkpoint-interval).

Accumulations MUST be computed during Collect using a _synchronized
move_ operation on the Aggregator. This operation, using some kind of synchronization, copies
Expand All @@ -266,12 +269,33 @@ collection period while the current one is processed. An Accumulation
is defined as the synchronously-copied Aggregator combined with the
Label Set, Resource, and metric Descriptor.

#### Accumulator: Asynchronous instrument state

The set of asynchronous instruments registered through a Meter
implementation each MUST be associated with single, non-null callback
with either a single- or batch-instrument calling signature.

The Accumulator MUST implement [single-value per label set restriction
specified by the API](api.md#asynchronous-calling-conventions) for
asynchronous instruments. The Accumulator MUST ensure the
last-observed value is the output value, after eliminating duplicate
values.

TODO: _Are there more Accumulator functional requirements?_
- e.g., do callbacks run with timeouts? what should SDKs do if they do not return?

### Processor

TODO _Processor functional requirements_

### Processor checkpoint interval

The Processor checkpoint interval brackets Accumulations into a single
unit of data. The Processor checkpoint interval starts and finishes
before and after calling Accumulator.Collect to process Acumulations
Comment on lines +294 to +295
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
unit of data. The Processor checkpoint interval starts and finishes
before and after calling Accumulator.Collect to process Acumulations
unit of data. The Processor checkpoint interval starts before and finishes
after calling Accumulator.Collect to process Acumulations

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, I'm very interested to see the Processor spec finished. I don't have a good idea of why "start" is necessary on the Processor and how its supposed to react to that call.

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 is a good point. Linking these two conversations together here: #1198 (comment)

@jkwatson you wrote:

why not have the controller pass this data to the processor, and decouple the accumulator from the processor?

See also open-telemetry/opentelemetry-go#1362, where this coupling makes trouble for setting up the SDK (FYI @seanschade). Thinking in terms of the OTel-Go implementation, it would make sense for Collect() from the Accumulator to write into a channel and for the Processor to read from a channel.

I can imagine in other languages it would be more natural to use an iterator pattern to iterate over the results of collection using a ForEach() pattern, the way exporters consume the output of the Processor (although it means executing asynchronous instrument callbacks during the iteration). It won't be easy to do such a refactoring in Go because of how map iteration works--I'll try to make this explanation allow more language-specific approach and encourage decoupling, then file issues to fix the OTel-Go implementation.

from one or more instruments. An `ExportRecordSet` is computed when
the Processor checkpoint interval is finished.

### Controller

TODO _Controller functional requirements_
Expand Down Expand Up @@ -371,7 +395,7 @@ Optional to this method:

- the instrumentation library version of the named Meter in use.

To help with instrumentat registration:
To help with instrument registration:

```go
// NewUniqueInstrumentMeterImpl returns a wrapped metric.MeterImpl with
Expand Down Expand Up @@ -452,9 +476,44 @@ considered language-specific details.

### Export pipeline detail

TODO: define AggregatorSelector, Aggregator, Accumulation, ExportKind,
ExportKindSelector, Aggregation, AggregationKind ExportRecord,
ExportRecordSet
#### Export pipeline data types

**Accumulation**: Passed from the Accumulator to the Processor, this
consists of the Metric Descriptor, the Label Set, the Resource, and a
snapshot of the Aggregator state (i.e., the Aggregation).

**Aggregation**: Output by an Aggregator, this represents state from
one or more metric events. These have various types (e.g., Sum,
Histogram).

**ExportRecord**: Passed from the Processor to the Exporter, this
consists of the same metadata as the Accumulation, with the
Aggregation corrected for ExportKind, with the associated start and
end timestamps.

**ExportRecordSet**: The set of ExportRecords computed during a
Processor checkpoint interval.

#### Export pipeline configuration

**AggregationKind**: The format which an Aggregator aggregates data into and ultimately sends it as.

**ExportKind**: This is determined by the ExportKindSelector and may
be cumulative or delta. Note that the term ExportKind is used in the
SDK to refer to this choice, while the same concept is called
AggregationTemporality when stored as a field in the OpenTelemetry
protocol. TODO: rename ExportKind to AggregationTemporality?
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea of consistency 👍


#### Export pipeline policies

**AggregatorSelector**: This selector is associated with the
Processor and called by the Accumulator to allocate new Aggregations.
This selector determines the kind of aggregate information available
to the exporter.

**ExportKindSelector**: This selector is assocaited with the Exporter
and called by the Processor to determine whether to allocate memory
for ExportKind conversion (e.g., delta to cumulative conversion).

### Processor Detail

Expand Down