Add single instrument callback and split metric instrument configuration

[MrAlias]

Add a WithCallback option for asynchronous instruments

Closes #3451

The OpenTelemetry specification states:

The API to construct asynchronous instruments MUST accept the following parameters:
[...]

• Zero or more callback functions, responsible for reporting Measurement values of the created instrument.

The API MUST support creation of asynchronous instruments by passing zero or more callback functions to be permanently registered to the newly created instrument.

This updates our implementation to conform with the specification.

Split instrument configuration specific number and async/sync combinations.

As discussed in #3451, the ideal callback design is...

• is cancel-able or has a way to receive timeout signals
• ensures only the instrument it is registered for is updated
• avoids having the user perform a type conversion

In order to achieve this using the native Go type system the additional Int64Option, Float64Option, Int64ObserverOption, and Float64ObserverOption types are added for their respective instruments. The instrument.Option remains to configure the unit and description of all instruments.

[codecov]

Codecov Report

Merging #3507 (25693fb) into main (0851690) will increase coverage by 0.2%.
The diff coverage is 82.8%.

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #3507     +/-   ##
=======================================
+ Coverage   78.7%   78.9%   +0.2%     
=======================================
  Files        165     169      +4     
  Lines      12277   12435    +158     
=======================================
+ Hits        9662    9814    +152     
+ Misses      2417    2413      -4     
- Partials     198     208     +10     

Impacted Files	Coverage Δ
metric/internal/global/instruments.go	54.3% <ø> (ø)
metric/noop.go	62.1% <50.0%> (ø)
sdk/metric/pipeline.go	92.3% <75.0%> (-1.7%)	⬇️
sdk/metric/meter.go	79.3% <77.1%> (-12.8%)	⬇️
metric/instrument/asyncfloat64.go	100.0% <100.0%> (ø)
metric/instrument/asyncint64.go	100.0% <100.0%> (ø)
metric/instrument/instrument.go	100.0% <100.0%> (ø)
metric/instrument/syncfloat64.go	100.0% <100.0%> (ø)
metric/instrument/syncint64.go	100.0% <100.0%> (ø)
metric/internal/global/meter.go	95.2% <100.0%> (ø)
... and 1 more

[MrAlias]

Holding this as a draft until release v0.34.0 is made.

[dashpole]

code changes LGTM
Should we move file and directory naming from sync/async to ""/observer as well?

[MrAlias]

code changes LGTM Should we move file and directory naming from sync/async to ""/observer as well?

I'm okay with that, but what should we rename the sync packages to?

[MrAlias]

I was also considering if we wanted to flatten the sync/async packages into the instrument package. Thoughts?

[dashpole]

I was also considering if we wanted to flatten the sync/async packages into the instrument package. Thoughts?

I'm +1 to flattening.

I'm okay with that, but what should we rename the sync packages to?

My best suggestion is int64observableinstrument and int64instrument? Basically Int64ObservableCounter() would return an int64observableinstrument.Counter, and Int64Counter() returns int64instrument.Counter. But I think it would be moot if we flattened.

[MrAlias]

@dashpole are you okay with flattening in a follow up PR?

[dashpole]

Yep, fine by me

[MadVikingGod]

We probably want to update the otel/metric asynchronous_single example to use the new WithCallback Option. (An issue is fine)

If we are going to return errors with the individual callback, should we also look at returning an error with the multi-callback (via RegisterCallback)? This would allow us to unify the logic under the hood to have a single collection of callbacks that need executing, because individual callbacks can be converted into multi-callbacks. (follow up is fine)

I'm also ok with flattening, even if this was counter to the original discussions we had a year ago that produced this design.

[MrAlias]

We probably want to update the otel/metric asynchronous_single example to use the new WithCallback Option. (An issue is fine)

If we are going to return errors with the individual callback, should we also look at returning an error with the multi-callback (via RegisterCallback)? This would allow us to unify the logic under the hood to have a single collection of callbacks that need executing, because individual callbacks can be converted into multi-callbacks. (follow up is fine)

I'm also ok with flattening, even if this was counter to the original discussions we had a year ago that produced this design.

👍

I've opened #3572 and #3573 to track the example update and the multi-callback refactor.