opentelemetry-specification icon indicating copy to clipboard operation
opentelemetry-specification copied to clipboard

Support measurement processors in Metrics SDK

Open Blinkuu opened this issue 1 year ago • 11 comments

What are you trying to achieve?

I would like to be able to enrich my telemetry using members encoded inside the Baggage header. The idea is to enable an "end-to-end telemetry correlation" use case by leveraging Context and Baggage.

This is easy to do using the Trace SDK by leveraging the SpanProcessor. Concrete implementations exist, such as the one for Go.

Furthermore, a similar concept exists in the Logs SDK - the LogRecordProcessor. Similarly, there seem to exist concrete implementations.

What did you expect to see?

According to the Design Goals of Metrics Specification, enriching metrics attributes via Baggage and Context is a top priority:

Being able to connect metrics to other signals. For example, metrics and traces can be correlated via exemplars, and metrics attributes can be enriched via Baggage and Context. Additionally, Resource can be applied to logs/metrics/traces in a consistent way.

Based on the quote above, I expected to see a similar "processor" concept for Metrics SDK, but that doesn't seem to be the case.

Additional context.

Even though the "processor" concept isn't fully materialized, some evidence exists in the form of implementation, namely in the opentelemetry-java project. It looks like Views can accept a set of AttributesProcessors (see here). Moreover, there even exists an implementation of a processor that extracts baggage members and attaches them as attributes (see here). Nevertheless, this feature is marked as experimental.

Related Issues.

  • https://github.com/open-telemetry/opentelemetry-collector/discussions/9871

Blinkuu avatar Nov 18 '24 15:11 Blinkuu

https://github.com/open-telemetry/opentelemetry-specification/pull/1938 for reference.

cijothomas avatar Nov 18 '24 16:11 cijothomas

#1938 for reference.

From the SIG meeting notes:

View already covered most of the features, if we need something else, we should first look at how to improve View rather than introducing another concept at this moment.

pellared avatar Nov 19 '24 08:11 pellared

By @jmacd in https://github.com/open-telemetry/opentelemetry-specification/issues/4256#issuecomment-2486236831:

In the Lightstep metrics SDK, we have added a MeasurementProcessor API which has been discussed in the OpenTelemetry specification as far back as the OpenCensus origin. I would like to see OTel add this feature!

MeasurementProcessor absolutely uses the context to derive metric attributes. However, to conditionally enable or disable a metric instrument based on context would introduce an undetectable bias in the data. For example, if you only enable a metric instrument when the context is traced, then the metric instrument will under-count depending on sampling rate and the consumer of the data will have no way of knowing this. Metric instruments should strive to measure everything--users should let the context be used to adjust which attributes are used, not to conditionalize.

@jmacd, do you want to sponsor this or maybe even work on this or help someone who would like to address the issue?

pellared avatar Nov 19 '24 16:11 pellared

I think, we could consider also to allow views to read from Baggage to determine attributes.

bogdandrutu avatar Nov 19 '24 18:11 bogdandrutu

I like the measurement processor concept and am in favor of seeing it added. In addition to adding attributes from baggage, it would allow you to:

  • Filter out values that are outside of acceptable range.
  • Enrich measurements with other bits of data stored in context.
  • Perfect lossless unit translation. Unit translation of aggregated explicit bucket and exponential histogram aggregation is lossy.
  • Pipe raw measurements directly out of process.

There are definitely some performance implications, since changing (adding or removing) attributes on the hotpath is not free from a CPU or memory perspective. But users would be opting into this behavior and can make a decision on whether the behavior is worth the performance penalty.

jack-berg avatar Nov 19 '24 18:11 jack-berg

I think, we could consider also to allow views to read from Baggage to determine attributes.

This was in the specification at one point, perhaps it was removed?

jsuereth avatar Nov 26 '24 13:11 jsuereth

I think, we could consider also to allow views to read from Baggage to determine attributes.

This was in the specification at one point, perhaps it was removed?

yes. https://github.com/open-telemetry/opentelemetry-specification/pull/1730/files#diff-32d5f289beed96900fa1e6e69a3ee9cc0b6503ce8883672011c44bc772c0fa2aR126

cijothomas avatar Nov 26 '24 14:11 cijothomas

I can be the sponsor.

reyang avatar Nov 26 '24 16:11 reyang

I see adding an additional processing pipeline to the metric signal (in addition to views) will negatively impact the usability of the metric SDK APIs and impose performance global degradation across SDKs. It will add a competing stream processing configuration endpoint for users to understand and use in concert with all views and it will require all streams be passed through the new Metric processor (imposing non-zero overhead in the process) instead of just matching instruments (as would be the case with a view).

Instead, we should focus our efforts on improving views to support the desired use-cases.

As mentioned in the description, there is already a proof-of-concept for how to do this in Java. Additionally, there is a proof-of-concept for Go.

MrAlias avatar Jul 21 '25 20:07 MrAlias

Hi there, I want to ask whether this is something ready to be implemented (e.g. the AttributesProcessor in View) , I'd like to implement this in .NET

lapwingcloud avatar Oct 30 '25 09:10 lapwingcloud

Hi there, I want to ask whether this is something ready to be implemented (e.g. the AttributesProcessor in View) ,

There is nothing like that in this moment.

I'd like to implement this in .NET

Please go for it, having more prototypes and feedback is going to help move this issue forward.

pellared avatar Nov 03 '25 08:11 pellared