open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.76k stars 891 forks source link

Support measurement processors in Metrics SDK #4298

Open Blinkuu opened 4 days ago

Blinkuu commented 4 days ago

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.

cijothomas commented 4 days ago

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

pellared commented 3 days ago

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 commented 3 days ago

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?

bogdandrutu commented 2 days ago

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

jack-berg commented 2 days ago

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:

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.