feat: bind controller with wrapped meter provider

[legendecas]

Which problem is this PR solving?

One controller now binds with a membraned MeterProvider and can collect all metrics registered with the MeterProvider and its subsidiary Meters. Also, it is possible to create a pulling controller to properly control proactive metrics collections.

Short description of the changes

• Added MeterProvider.addController to register a controller.

This PR superseded controller part changes of #1017.

Illustration of usage:

const { MeterProvider, PushController, ConsoleExporter } = require('@opentelemetry/metrics');

const meterProvider = new MeterProvider();
meterProvider.addController(new PushController({
  exporter: new ConsoleExporter(),
  interval: 3000,
}));
const meter = meterProvider.getMeter('example-observer');

[codecov]

Codecov Report

Merging #1333 into master will increase coverage by 0.05%.
The diff coverage is 92.59%.

@@            Coverage Diff             @@
##           master    #1333      +/-   ##
==========================================
+ Coverage   94.11%   94.17%   +0.05%     
==========================================
  Files         145      144       -1     
  Lines        4335     4308      -27     
  Branches      883      875       -8     
==========================================
- Hits         4080     4057      -23     
+ Misses        255      251       -4     

Impacted Files	Coverage Δ
packages/opentelemetry-metrics/src/Meter.ts	95.69% <ø> (-0.22%)	⬇️
...s/opentelemetry-metrics/src/export/NoopExporter.ts	60.00% <0.00%> (+26.66%)	⬆️
packages/opentelemetry-metrics/src/types.ts	100.00% <ø> (ø)
...y-metrics/src/export/controllers/PushController.ts	90.00% <90.00%> (ø)
...ackages/opentelemetry-metrics/src/MeterProvider.ts	100.00% <100.00%> (ø)
...ntelemetry-metrics/src/export/controllers/index.ts	100.00% <100.00%> (ø)
packages/opentelemetry-sdk-node/src/sdk.ts	77.58% <100.00%> (+2.58%)	⬆️
packages/opentelemetry-plugin-https/src/utils.ts
packages/opentelemetry-plugin-https/src/https.ts
... and 3 more

[dyladan]

Maybe this is a silly question, but what is membraned?

[legendecas]

@dyladan No, its a fairly good question. I was also wondering if it's required to expose the collect method from MeterProvider since no spec requiring this. So currently, I'm exposing the collect method by a wrapper object (the membrane), which is compliant with the interface MetricsCollector.

interface MetricsCollector {
  collect(): Promise<MetricRecord[]>;
}

The metrics collector interface that the controller have access to, doesn't expose the internal state of meter providers. And the controller is receiving a thin object that delegates the collect method to the meter providers private _collect.

[dyladan]

Is there some reason you decided to make this its own collector interface instead of just adding it to the existing meter provider?

[legendecas]

The collect method is not required by the spec. So I've tried not to expose the method on meter provider publicly but with restricted access from controllers.

[dyladan]

You can avoid the extra function call and associated overhead if you make the collect method public and do this.

controller.registerMetricsCollector(this);

[dyladan]

Or you could just inline the collect function implementation since it is only called in one place.

[legendecas]

Fixed by inlining them.

[dyladan]

should this also shut down the associated exporter? If I create a system like this:

const provider = new MeterProvider(new Controller(new Exporter()));

I would typically expect calling provider.shutdown(), the provider would shut down the controller, and the controller would shut down the exporter. This is just my gut feeling though.

[legendecas]

Fixed, it's true that controller shall shutdown exporters too.

[legendecas]

@dyladan PR updated with the nodejs sdk. PTAL :D

[legendecas]

This PR does a lot of changes on the API of the metric SDK. However, it is recommended to follow the metric SDK specification so that there will not be significant changes regarding the API while the metric SDK spec is been drafted. So this PR will be blocked until there is a resolution on the metric SDK specification regarding the controllers and accumulators.

[legendecas]

#1528 Need updates according to the latest spec.