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

Add MeasurementProcessor specification to Metrics SDK #4318

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Add MeasurementProcessor specification to Metrics SDK #4318

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3f3186a
Add MeasurementProcessor specification to Metrics SDK
Blinkuu Dec 3, 2024
a8f5de3
Update TODO
Blinkuu Dec 3, 2024
1ce9e4d
Update specification/metrics/sdk.md
Blinkuu Dec 3, 2024
4b0a58d
Update specification/metrics/sdk.md
pellared Dec 3, 2024
449d2fb
Update specification/metrics/sdk.md
pellared Dec 3, 2024
60adbd3
Remove Shutdown and ForceFlush from MeasurementProcessor spec
Blinkuu Dec 6, 2024
9d60b12
Merge branch 'main' into add-measurement-processor-to-metrics-sdk-spec
Blinkuu Dec 6, 2024
17f650c
Merge branch 'main' into add-measurement-processor-to-metrics-sdk-spec
Blinkuu Dec 19, 2024
4cbc0ec
Merge branch 'main' into add-measurement-processor-to-metrics-sdk-spec
Blinkuu Jan 2, 2025
01bcc0a
feat: define built-in measurement processor
Blinkuu Jan 2, 2025
d28e993
Merge branch 'main' into add-measurement-processor-to-metrics-sdk-spec
Blinkuu Jan 7, 2025
90d331d
chore: update the wording
Blinkuu Jan 7, 2025
ee72e3f
chore: rename simple processor to default processor
Blinkuu Jan 7, 2025
225000a
feat: specify how we reference the next processor in the chain
Blinkuu Jan 7, 2025
8fbc341
Merge branch 'main' into add-measurement-processor-to-metrics-sdk-spec
Blinkuu Jan 22, 2025
f57fe00
Update specification of the `next` parameter and remove `NoopProcessor`
Blinkuu Jan 22, 2025
66c6926
Update specification/metrics/sdk.md
Blinkuu Jan 23, 2025
e585028
Update CHANGELOG.md
Blinkuu Jan 23, 2025
83a81a5
Update metrics/sdk.md
Blinkuu Jan 23, 2025
dbaffa1
Merge branch 'main' into add-measurement-processor-to-metrics-sdk-spec
Blinkuu Jan 23, 2025
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
Prev Previous commit
Next Next commit
feat: define built-in measurement processor
  • Loading branch information
@Blinkuu
Blinkuu committed Jan 2, 2025
commit 01bcc0abd7642a862964e0cf8202027b6d1edc15
14 changes: 14 additions & 0 deletions specification/metrics/sdk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ linkTitle: SDK
- [MeasurementProcessor](#measurementprocessor)
* [MeasurementProcessor operations](#measurementprocessor-operations)
+ [OnMeasure](#onmeasure)
* [Built-in processors](#built-in-processors)
+ [SimpleProcessor](#simpleprocessor)
- [Exemplar](#exemplar)
* [ExemplarFilter](#exemplarfilter)
+ [AlwaysOn](#alwayson)
Expand Down Expand Up @@ -993,6 +995,8 @@ series and the topic requires further analysis.

`MeasurementProcessor` is an interface which allows hooks when a `Measurement` is recorded by an `Instrument`.
Copy link
Member

Choose a reason for hiding this comment

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

In the context of tracing, a processor is somewhat of a different thing (it's not a filterer of data, it sits on top of exporters).

Should we name this differently than Processor to differenciate the two?
Or should we make both behaviors similar?

Copy link
Author

Choose a reason for hiding this comment

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

In the context of tracing, a processor is somewhat of a different thing (it's not a filterer of data, it sits on top of exporters).

The MeasurementProcessor is more than a filter of data. It can modify the measurement, decide to drop it, or even emit it multiple times.

Should we name this differently than Processor to differentiate the two?
Or should we make both behaviors similar?

To me, they are semantically similar. The difference is in the architecture of the Metrics SDK, which makes it hard to fit and connect the new processor pipeline directly with exporters. We could revisit this in the future and try to align the behavior between Logs, Traces, and Metrics SDKs. To keep an open door for such ideas, I'm purposefully specifying that:

Built-in measurement processors are responsible for Measurement Processing.

Where Measurement Processing is currently vaguely defined in this spec by using the word SHOULD.

Copy link
Contributor

Choose a reason for hiding this comment

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

I support @Blinkuu's reasoning above. In my thinking, we can use "processor" across OpenTelemetry when a component in a program receives and outputs the same type of data. Trace SDK processors input/output span data. Collector processors input/output pipeline data. Things that are not processors (e.g., Metric reader, Trace sampler, Exporters, Receivers) generally have different types of input and output.

Here, MeasurementProcessor is appropriate because the input and the output types are the same.

Copy link
Member

Choose a reason for hiding this comment

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

Here, MeasurementProcessor is appropriate because the input and the output types are the same.

+1


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

Choose a reason for hiding this comment

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

they are invoked in the same order as they were registered.

Unclear how is this part works with the later wording "The SDK MUST inject the OnMeasure function from the next processor in the chain into the current processor"

Who is invoking the MeasurementProcessors in the order? Is that MeterProvider/SDK ? Or is it the responsibility of each MeasurementProcessor to invoke the next one in the chain?

Copy link
Author

Choose a reason for hiding this comment

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

Unclear how is this part works with the later wording "The SDK MUST inject the OnMeasure function from the next processor in the chain into the current processor"

Who is invoking the MeasurementProcessors in the order? Is that MeterProvider/SDK ? Or is it the responsibility of each MeasurementProcessor to invoke the next one in the chain?

I have changed the wording around the next parameter, as the previous spec was confusing. The next parameter is a callback that MUST NOT require a reference to the next processor in the chain. On the implementation level, it will most likely end up as a closure. I see it as the SDK's responsibility to ensure processors are executed in the order they were configured on the MeterProvider.

That being said, every processor is able to decide whether it wants to invoke a chain of processors that follow it (by calling next(context, measurement)). Each processor can decide to call next either 0 (drop measurement), 1 (default flow), or N (custom use cases) times.


SDK MUST allow users to implement and configure custom processors.
Expand Down Expand Up @@ -1032,6 +1036,16 @@ For a `MeasurementProcessor` registered directly on SDK `MeterProvider`, the `me

A `MeasuremenetProcessor` may freely modify `measurement` for the duration of the `OnMeasure` call.
Blinkuu marked this conversation as resolved.
Show resolved Hide resolved

A `MeasurementProcessor` MUST invoke `OnMeasure` on the next registered processor.
Blinkuu marked this conversation as resolved.
Show resolved Hide resolved

### Built-in processors

The standard OpenTelemetry SDK MUST implement simple processor as described below.

#### SimpleProcessor

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

Choose a reason for hiding this comment

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

should the SDK ensure that this is the last processor in the chain?


## Exemplar

**Status**: [Stable](../document-status.md)
Expand Down
Loading