feat(sdk-metrics): align MetricReader with specification and other language implementations

[pichlermarc]

Which problem is this PR solving?

The spec states that:

To construct a MetricReader when setting up an SDK, the caller SHOULD provide at least the following:

• The exporter to use, which is a MetricExporter instance.
• The default output aggregation (optional), a function of instrument kind. If not configured, the default aggregation SHOULD be used.
• The default output temporality (optional), a function of instrument kind. If not configured, the Cumulative temporarily SHOULD be used.

For the PeriodicExportingMetricReader the spec states configurable parameters as:

• exportIntervalMillis - the time interval in milliseconds between two consecutive exports. The default value is 60000 (milliseconds).
• exportTimeoutMillis - how long the export can run before it is cancelled. The default value is 30000 (milliseconds).

This PR:

• aligns the configuration options on the MetricReader with the spec by adding configurability of aggregationTemporalitySelector and aggregationSelector
  • see Python SDK
  • see .NET SDK (only temporality via public property, per-reader aggregation not implemented)
  • the Java SDK uses an alternative approach by internally allowing overrides of the MetricReader interface
• removes the ability to override the PeriodicExportingMetricReader's AggregationSelector
  • also not allowed in Java SDK
  • also not allowed in Python SDK
  • also not allowed in .NET SDK (per reader aggregation not implemented)
• adds optional configuration of the PeriodicExportingMetricReader's aggregationSelector via the Exporter
  • implemented in Python SDK
  • implemented in Java SDK
  • not implemented in .NET SDK
  • seems to be somewhat agreed upon (see spec issue comment)
• makes implementing selectAggregationTemporality optional for PushMetricExporters
  • the PeriodicExportingMetricReader will fall back to cumulative if not specified.

As the spec does not seem entirely clear, I added some links above with references to other language SDKs' implementations. 🙂

Fixes #3213
Related #3060

Short description of the changes

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

• Added unit tests
• Existing unit tests

Checklist:

• Followed the style guidelines of this project
• Unit tests have been added
• Documentation has been updated

[codecov]

Codecov Report

Merging #3225 (22d7455) into main (be24d69) will increase coverage by 0.00%.
The diff coverage is 95.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3225   +/-   ##
=======================================
  Coverage   93.30%   93.30%           
=======================================
  Files         202      203    +1     
  Lines        6599     6606    +7     
  Branches     1384     1389    +5     
=======================================
+ Hits         6157     6164    +7     
  Misses        442      442           

Impacted Files	Coverage Δ
...etrics/src/export/PeriodicExportingMetricReader.ts	95.65% <80.00%> (-0.43%)	⬇️
...etry-exporter-prometheus/src/PrometheusExporter.ts	92.40% <100.00%> (-0.19%)	⬇️
...etry-sdk-metrics/src/export/AggregationSelector.ts	100.00% <100.00%> (ø)
...entelemetry-sdk-metrics/src/export/MetricReader.ts	100.00% <100.00%> (ø)

[legendecas]

I think this can be a good approach to reduce the duplication on the aggregation selections.

[legendecas]

options.exporter.selectAggregation should either have their receiver to be bound to options.exporter, or maybe we should make the selector an interface, with methods like selectAggregation and selectAggregationTemporality, so that we can safely pass the selector around.

interface AggregationSelector {
  selectAggregation(instrumentType): Aggregation;
  selectTemporality(instrumentType): AggregationTemporality;
}

[pichlermarc]

Good catch, thanks! 🙂
I changed it to bind to the exporter in 2453614. 🙂

[legendecas]

LGTM. Thank you for working on this <3