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

Support independent, per-Reader Metric Views configuration

Open jmacd opened this issue 3 years ago • 3 comments

What are you trying to achieve?

Having implemented a mostly-complete Views implementation in https://github.com/open-telemetry/opentelemetry-go/pull/2525, I have the following feature request.

Make it possible to configure Views on a per-Reader basis.

Why? It seems like a shame to specify a powerful way to configure what gets exported from a metrics SDK and not let each Reader configure independent Views. Without a change of the specification, the user who wants to configure independent Views will be required to make a "Multi-MeterProvider" that are configured with one Reader/Views pair each. I view this as a poor outcome because the SDK will lose any opportunity to share work among the two readers, particularly for synchronous instruments.

I make this request after implementing the draft PR linked above, where it looks like only a slight change. The way that PR is structured, there's almost no difference between supporting independent views because an "intermediate" aggregation is computed for synchronous instruments. No matter how many Views are attached to the same synchronous instrument, only distinct aggregations need to be computed on the fly. It means that if you configure 1 Counter instrument and 2 different Views over different attribute sets (with different output names, of course), the runtime would track 1 intermediate aggregation (having Delta temporality) and then output into each View on collection. The SDK just knows that it has a number of destinations per intermediate aggregation; it doesn't matter how many readers there are, at least the way I've structured it there.

Example

The current specification has an example like this:

# metrics from X and Y (reported as Foo and Bar) will be exported
meter_provider
    .add_view("X")
    .add_view("Foo", instrument_name="Y")
    .add_view(
        "Bar",
        instrument_name="Y",
        aggregation=HistogramAggregation(buckets=[5.0, 10.0, 25.0, 50.0, 100.0]))
    .add_metric_reader(PeriodicExportingMetricReader(ConsoleExporter()))

With the proposed change, we could write examples like this:

# Instrument Foo is renamed differently for the two exporters 
meter_provider
    .add_metric_reader(PullExportingMetricReader(PrometheusExporter())
        .add_view("FooRenamedTheOtherWay", instrument_name="Foo")),
    .add_metric_reader(PeriodicExportingMetricReader(OTLPExporter())
        .add_view("FooRenamedOneWay", instrument_name="Foo"))

We could configure ExponentialHistograms for OTLP while configuring ExplicitBucketHistograms for Prometheus, for example, or limit the set of attributes differently for these two exporters.

jmacd avatar Jan 25 '22 01:01 jmacd

I'm curious what you think about the cost of maintaining two different histogram implementations inside the SDK.

I love the idea behind this, but I'm a bit worried about the overhead.

jsuereth avatar Jan 29 '22 00:01 jsuereth

I expected we would maintain two histogram implementations because we have two point kinds, and neither can exactly implement the other. However, I'd be glad to let explicit-boundary histograms be generated (somewhat imprecisely) by exponential histograms.

The same arguments would apply to other aggregations though. For OTLP push, I want to push full cardinality, whereas for Prometheus pull I want to export limited cardinality. It would be the same aggregators with different configurations.

jmacd avatar Feb 04 '22 21:02 jmacd

Should we have some clarification of what should happen to views that are registered outside of the scope of a MetricReader? For example, what should I expect if I do something like:

# What Metrics do I get from readers foo, and bar?
meter_provider
    .add_metric_reader(foo)
meter_provider
    .add_view("X")

 meter_provider
    .add_metric_reader(bar) 

What about:

# What Metrics do I get from reader foo?
meter_provider
    .add_view("*", aggregation=Drop)
    .add_metric_reader(foo) 
meter_provider
    .add_view("X")

Are the views attached to one reader applied to others?

# What Metrics do I get from readers foo and bar?
meter_provider
    .add_view("Foo", instrument_name="Y")
    .add_metric_reader(foo) 
meter_provider
    .add_view("Foo", instrument_name="X")
    .add_metric_reader(bar) 

MadVikingGod avatar Sep 29 '22 14:09 MadVikingGod

If this is accepted, the existing approach of configuring view(s) with the MeterProvider will need to be preserved. Stable implementations of the specification have already been released with this language.

Registering views with readers will need to be considered as an extension. The remaining question is if new language implementations will need to be required to also make their implementations accept view(s) to be registered with the MeterProviders?

MrAlias avatar Oct 19 '22 19:10 MrAlias

I feel there could be utility in per-reader views, personally. For example, suppose I had two metric readers - one paired with an OTLP push exporter, and the other with a Prometheus exporter. I might, potentially, wish to drop certain metrics or aggregate across certain labels for one of those exporters - but not the other. It would be difficult to implement this today without per-reader views.

ahayworth avatar Oct 20 '22 11:10 ahayworth