feat(sdk-metrics-base): hoist async instrument callback invocations

[legendecas]

Which problem is this PR solving?

This PR add support for the spe changes introduced in open-telemetry/opentelemetry-specification#2361.

Blocked on:

• refactor(sdk-metrics-base): meter shared states #2821
• Clarify how should SDK invokes async instrument callbacks opentelemetry-specification#2408

Fixes #2782

Short description of the changes

• Add ObservableRegistry to call all async callbacks at once.
• All async callbacks are called before collecting synchronous instruments.
• Add support for registering multiple async callbacks to one single async instrument.
• Fix callbacks associated with one instrument may be invoked multiple times when the instrument is mapped with views to multiple metrics.
• MetricStorage.collect no longer invokes async callbacks so it is marked as synchronous methods.

Type of change

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

• MeterSharedState.collect on async instruments

Checklist:

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

[codecov]

Codecov Report

Merging #2822 (482a74f) into main (d98548b) will increase coverage by 0.12%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##             main    #2822      +/-   ##
==========================================
+ Coverage   92.88%   93.01%   +0.12%     
==========================================
  Files         182      183       +1     
  Lines        5906     5909       +3     
  Branches     1253     1254       +1     
==========================================
+ Hits         5486     5496      +10     
+ Misses        420      413       -7     

Impacted Files	Coverage Δ
...emetry-sdk-metrics-base/src/state/MetricStorage.ts	100.00% <ø> (ø)
...ry-sdk-metrics-base/src/state/SyncMetricStorage.ts	100.00% <ø> (+21.05%)	⬆️
...try-sdk-metrics-base/src/state/MeterSharedState.ts	97.50% <88.88%> (-2.50%)	⬇️
...y-sdk-metrics-base/src/state/AsyncMetricStorage.ts	100.00% <100.00%> (+14.81%)	⬆️
...y-sdk-metrics-base/src/state/ObservableRegistry.ts	100.00% <100.00%> (ø)

[dyladan]

Does this depend on another PR?

[legendecas]

@dyladan: Does this depend on another PR?

It depends on one statement in open-telemetry/opentelemetry-specification#2363:

Every currently registered Callback associated with an instrument MUST
be evaluted exactly once during collection prior to reading data for
that instrument.

[legendecas]

However, I think the depended statement in open-telemetry/opentelemetry-specification#2363 is unlikely to change (as the spec PR is mainly focused on multi-instrument callbacks). To get async instruments to work correctly with views, the problem that callbacks associated with one instrument are invoked multiple times during a single collection is outstanding and needs to be fixed by this PR.

Maybe we should proceed with this PR?

[dyladan]

However, I think the depended statement in open-telemetry/opentelemetry-specification#2363 is unlikely to change (as the spec PR is mainly focused on multi-instrument callbacks). To get async instruments to work correctly with views, the problem that callbacks associated with one instrument are invoked multiple times during a single collection is outstanding and needs to be fixed by this PR.

Maybe we should proceed with this PR?

I would proceed with it. I agree the statement is unlikely to change. Even if it does change, it is unlikely to become a complete contradiction and they would likely only tweak wording.

[legendecas]

@open-telemetry/javascript-approvers this is ready for review, PTAL, thank you :D

[vmarchaud]

lgtm, do you expect to tackle the timeout part afterwards ? Since its not a hard requirement on the spec part

[legendecas]

@vmarchaud I'm not sure which timeout you are referring to. #2742 is expected to provide async observer timeout support properly.

[vmarchaud]

I'm not sure which timeout you are referring to. #2742 is expected to provide async observer timeout support properly.

I didn't see this PR sorry

[dyladan]

LGTM