Add MeasurementProcessor to Metrics SDK #4318
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,11 @@ linkTitle: SDK | |
| * [Instrument advisory parameters](#instrument-advisory-parameters) | ||
| * [Instrument enabled](#instrument-enabled) | ||
| - [Attribute limits](#attribute-limits) | ||
| - [MeasurementProcessor](#measurementprocessor) | ||
| * [MeasurementProcessor operations](#measurementprocessor-operations) | ||
| + [OnMeasure](#onmeasure) | ||
| * [Built-in processors](#built-in-processors) | ||
| + [DefaultProcessor](#defaultprocessor) | ||
| - [Exemplar](#exemplar) | ||
| * [ExemplarFilter](#exemplarfilter) | ||
| + [AlwaysOn](#alwayson) | ||
|
|
@@ -984,6 +989,66 @@ Attributes which belong to Metrics are exempt from the | |
| time. Attribute truncation or deletion could affect identity of metric time | ||
| series and the topic requires further analysis. | ||
|
|
||
| ## MeasurementProcessor | ||
|
|
||
| **Status**: [Development](../document-status.md) | ||
|
|
||
| `MeasurementProcessor` is an interface which allows hooks when a `Measurement` is recorded by an `Instrument`. | ||
Blinkuu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Built-in measurement processors are responsible for [Measurement Processing](#measurement-processing). | ||
|
|
||
| `MeasurementProcessors` can be registered directly on SDK `MeterProvider` and they are invoked in the same order as they were registered. | ||
cijothomas marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. It seems like it would be good to specify how these are meant to be registered, similar to what we have for span processors. Can they be provided when the MeterProvider is instantiated (similar to span processors)? Does it need to be possible to dynamically register/unregister them (similar to callbacks)? There was a problem hiding this comment. My suggestion is to use the same wording as we use for Provider configuration in general. There was a problem hiding this comment. I used the exact same wording we use for LogRecordProcessor and SpanProcessor. I think it should be consistent, it's the same concept. There was a problem hiding this comment. We should specify how MeterProviders are configured in the MeterProvider spec, not in the MeasurementProcessors spec. See this section of TracerProvider: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#configuration that @cijothomas quoted above. Make sure anything related to MeasurementProcessor is marked as Development status. |
||
|
|
||
| Each processor registered on the `MeterProvider` is part of a pipeline. | ||
jack-berg marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Did we consider making processors part of the View configuration? Many of the use-cases seem to apply to specific metrics (e.g. changing the unit of a metric), or aggregating specific attribute values together. It also fits nicely within the scope of what Views are meant to be able to do: Make pre-aggregation adjustments to metrics. There was a problem hiding this comment. Yes, it was considered. It looks similar, but in my opinion, Views are meant to work on a higher level of abstraction, not with individual measurements. The existing design is also deeply rooted in analysis of existing SDK implementations, where incorporating this concept into Views would be extremely difficult. There was a problem hiding this comment. Can you explain further? Attribute filters from views are applied very early in the SDK's handling of a measurement. They apply even prior to cardinality limit enforcement. Implementing both of those features post-aggregation (i.e. not on measurements) would defeat one of their primary purpose, which is to limit memory consumption of the SDK, so I would be very surprised if they were not applied to individual measurements. There was a problem hiding this comment. I'm not an expert on the Metrics SDKs, but after looking into the code when preparing this PR I concluded that supporting all the use cases we envisioned would require major overhaul of how Views work. Please see #4298 (comment) Perhaps I'm wrong, and there's a neat way to make Views fully programmable. Regarding performance - MeasurementProcessor, like LogRecordProcessor and SpanProcessor is an opt-in feature. If it's not used, the cost is a single if-statement, exactly the same as with https://github.com/MrAlias/opentelemetry-go/pull/1092/files#diff-2c6f5806e27a5ab81eeba6f9d6fff1a80ac35188c8e22deea1c97a77fdb04634 There was a problem hiding this comment. My suggestion would be to apply measurement processors immediately after the view's attribute filter, and prior to cardinality limit enforcement. The benefit of this would be that the measurement processor could reduce cardinality, and prevent hitting the limit by merging attribute values together, which is one of the intended uses. In terms of implementation, this means you would just keep the measurement processor very close to the attribute filter (e.g. pass them to the same functions). The existing attribute filter could be thought of as a measurement processor that always comes first, and has more limited capabilities. In go, we would probably extend, or add something similar to this function to add the measurement processor in addition to the existing filter. There was a problem hiding this comment.
The if statement you are referencing is in the build path. The hot-path performance has no additional branch points for the views implementation. |
||
|
|
||
| SDK MUST allow users to implement and configure custom processors. | ||
|
|
||
| The following diagram shows `MeasurementProcessor`'s relationship to other components in the SDK: | ||
|
There was a problem hiding this comment. Please include Views in this diagram. Importantly, is View configuration applied before the Measurement processor, or after, or both? We clearly want it to be applied before the aggregation portion of views is applied, but it would be nice if it was applied after the View's attribute filter so processors don't need to consider attributes that aren't going to be kept. There was a problem hiding this comment. Views are meant to be applied after Measurement Processors. They operate on a higher level of abstraction. There was a problem hiding this comment. I realized cardinality limits should probably also be included, but we can wait until the discussion below is resolved. |
||
|
|
||
| ```plaintext | ||
| +------------------+ | ||
| | MeterProvider | +----------------------+ +-----------------+ | ||
| | Meter A | Measurements... | | Measurements... | | | ||
| | Instrument X |-----------------> MeasurementProcessor +-----------------> In-memory state | | ||
| | Instrument Y + | | | | | ||
| | Meter B | +----------------------+ +-----------------+ | ||
| | Instrument Z | | ||
| | ... | +----------------------+ +-----------------+ | ||
| | ... | Measurements... | | Measurements... | | | ||
| | ... |-----------------> MeasurementProcessor +-----------------> In-memory state | | ||
| | ... | | | | | | ||
| | ... | +----------------------+ +-----------------+ | ||
| +------------------+ | ||
| ``` | ||
|
|
||
| ### MeasurementProcessor operations | ||
|
|
||
| #### OnMeasure | ||
|
|
||
| `OnMeasure` is called when a `Measurement` is recorded. This method is called synchronously on the thread that emitted the `Measurement`, therefore it SHOULD NOT block or throw exceptions. | ||
|
|
||
| **Parameters:** | ||
Blinkuu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| * `context` - the resolved `Context` (the explicitly passed `Context` or the current `Context`) | ||
reyang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * `measurement` - a [Measurement](./api.md#measurement) that was recorded | ||
Blinkuu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * `next` - a callback to invoke `OnMeasure` on the next processor in the chain. It MUST be callable without a reference to the next processor. | ||
Blinkuu marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| **Returns:** Void | ||
|
|
||
| For a `MeasurementProcessor` registered directly on SDK `MeterProvider`, the `measurement` mutations MUST be visible in next registered processors. | ||
|
|
||
jmacd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| A `MeasuremenetProcessor` MAY freely modify `measurement` for the duration of the `OnMeasure` call. | ||
pellared marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| A `MeasurementProcessor` SHOULD invoke `next`. A `MeasurementProcessor` MAY decide to drop the `Measurement` by not invoking the next processor. | ||
jmacd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ### Built-in processors | ||
|
|
||
| The standard OpenTelemetry SDK MUST implement default processor as described below. | ||
|
|
||
| #### DefaultProcessor | ||
|
|
||
| This is an implementation of `MeasurementProcessor` which calculates an in-memory state from incoming `Measurements`. | ||
reyang marked this conversation as resolved.
Show resolved
Hide resolved
Blinkuu marked this conversation as resolved.
Show resolved
Hide resolved
pellared marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ## Exemplar | ||
|
|
||
| **Status**: [Stable](../document-status.md) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.