OTLP Metric exporter configuration extends beyond the scope of its ability

[MrAlias]

trafficstars

#2619 added the configuration option of the OTLP exporter to configure the default aggregation. However, the SDK is defined with the Reader setting the default aggregation not the exporter:

Metric Exporters always have an associated MetricReader. The aggregation and temporality properties used by the OpenTelemetry Metric SDK are determined when registering Metric Exporters through their associated MetricReader.

There is an additional option allowed:

OpenTelemetry language implementations MAY support automatically configuring the MetricReader to use for an Exporter.

However, by defining a configuration option on the OTLP exporter as #2619 has done, it implicitly requires implementations to support this automatic configuration. This should not be the case.

[jack-berg]

#2619 extended the precedent established in #2404, where the OTLP exporter was given a configuration for temporality preference despite that technically being a reader concern.

Here's how I see things:

• MetricReader is an interface for reading metrics, and temporality and default aggregation are important configuration options when reading metrics.
• PeriodicMetricReader is a specific implementation of MetricReader which is always paired with a MetricExporter.
• PeriodicMetricReader's main concern is to read metrics on an interval and pass them to a MetricExporter. So while temporality and default aggregation are MetricReader concern, it doesn't make sense for sense for PeriodicMetricReader to be opinionated about them. Those are really MetricExporter concerns, and PeriodicMetricReader really just wants to defer to MetricExporter. This makes a lot of sense to me, since if PMR didn't defer to its MetricExporter, it'd be easy to configure a temporality and default aggregation which are incompatible with the MetricExporter. A user should just need choose an exporter, not worry about aligning the constraints of the exporter with its associated PMR.
• When I really think about it, temporality and default aggregation are actually MetricExporter concerns. However, the spec allows implementing pull based metric exporters as MetricReaders, which necessitates making temporality and default aggregation MetricReader level concerns.

[tsloughter]

PeriodicMetricReader is a specific implementation of MetricReader which is always paired with a MetricExporter.

Is it? I know the spec says an Exporter is always paired with a Reader but if a Reader isn't required to always have an Exporter then it would be aggregating with the wrong histogram before the Exporter is set for that Reader when the Exporter is created.

[jmacd]

I endorse @jack-berg's interpretation https://github.com/open-telemetry/opentelemetry-specification/issues/2810#issuecomment-1251651762.

However, we must address the question posed by @MrAlias regarding #2619 concerning the precedent set in #2404.

The key paragraph appears just above the edits in #2619, namely:

If a language provides a mechanism to automatically configure a MetricReader to pair with the associated Exporter (e.g., using the OTEL_METRICS_EXPORTER environment variable), then by default:

This means that the feature under discussion, both temporality and aggregation controls, are meant to apply to the default-configured OTLP exporter which comes from the environment (if the SDK has this support). In other words, this is not in-scope for the OTLP exporter in general, this is in-scope for the default-configured OTLP exporter. Somewhere the SDK has code to setup an OTLP exporter from the environment-- it is this only this MetricReader that is impacted by #2619 (and likewise it is only this MetricReader concerned with OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE).

[tsloughter]

If this is for setting up a single Reader/Exporter pair based on the environment then the name makes even less sense :). Am I understanding you right that this is more OTEL_DEFAULT_READER_EXPORTER_HISTOGRAM and OTEL_DEFAULT_READER_EXPORTER_TEMPORALITY_PREFERENCE (not actual proposals).

[rbailey7210]

Please discuss in the next spec call.

[jmacd]

This discussion was postponed until the 10/4 Spec SIG meeting.

[jmacd]

This is partly covered in #2854. I believe we can close this issue after a suitable issue describing the confusion over exporter-related environment variables.

[jmacd]

More in https://github.com/open-telemetry/opentelemetry-specification/issues/2746#issuecomment-1319259109

[carlosalberto]

Is this solved as #2854 was closed as well? @jmacd @MrAlias @jack-berg

[MrAlias]

Is this solved as #2854 was closed as well? @jmacd @MrAlias @jack-berg

#2854 was closed as it was a duplicate of https://github.com/open-telemetry/opentelemetry-specification/issues/2746.

I think we can close this and track the conversation in https://github.com/open-telemetry/opentelemetry-specification/issues/2746.