Clarify SDK/MetricReader/MetricExporter boundaries

[bogdandrutu]

What are you trying to achieve?

Clear design and terminology used. What is the "SDK", what are the components that an SDK communicates with, external boundaries, configurations, etc.

Additional context.

https:/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#temporality-override-rules

It is very unclear if an SDK "knows" about the MetricExporter interface or just about the "MetricReader" interface. From all the examples and text in the https:/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md it seems that the SDK only exposes one "external point" which is the MetricReader interface, and some implementations on the MetricReader may expose a "MetricExporter" interface. If that is the case the "SDK" never sees directly an Exporter and does not know about temporality from there. So the SDK cannot and should not resolve any temporality conflict.

[aabmass]

Not related to temporality but I also don't understand the boundary between exporters and readers for shutdown. An exporter is always tied to one MetricReader but it is MeterProvider's job to call shutdown on them separately:

Shutdown MUST be implemented at least by invoking Shutdown on all registered MetricReader and MetricExporter instances.

There is an specific order needed for shutdown, though. For push exporters, you need to shutdown the reader (to flush the SDK) and then the shutdown the exporter. For pull exporters, you need to shutdown the exporter and then the reader, otherwise the exporter will call collect() on a shutdown reader which is disallowed.

How does the MeterProvider infer the correct dependencies between all of the exporters and readers?

[reyang]

How does the MeterProvider infer the correct dependencies between all of the exporters and readers?

MeterProvider doesn't see exporters, it only sees readers. Certain language implementation might choose to model Prometheus Exporter (or any pull exporter) as a MetricReader instead of MetricExporter, according to the SDK spec.

[reyang]

There is an specific order needed for shutdown, though. For push exporters, you need to shutdown the reader (to flush the SDK) and then the shutdown the exporter. For pull exporters, you need to shutdown the exporter and then the reader, otherwise the exporter will call collect() on a shutdown reader which is disallowed.

Probably it could be simpler, I can give one possible way of implementing this (it's just one of the many possible ways of implementing this). Note that I only give the higher level ideas, there are many devils in the details (e.g. how to properly handle synchronization in a performant way).

For push exporter:

• Reader shutdown getting called
• Reader shutdown does necessary job (e.g. collection, calling exporter, etc.), then call exporter shutdown
• Reader shutdown returns AFTER exporter shutdown returned (or timed out)

For pull exporter:

• Reader shutdown getting called
• Reader marked it's internal state as "shutdown in-progress"
• Reader rejects any exporter pull by returning false (because it is shutting down), and Reader waits for any in-flight Collect call to finish
• Reader calls exporter shutdown
• Reader shutdown returns AFTER exporter shutdown returned (or timed out)

[aabmass]

@reyang this got closed by #2132, should I open a separate issue to continue this discussion or can your re-open?