Add an option for multi-instrument callbacks

[jmacd]

Fixes #2280.
Fixes #2408.

Changes

Adds optional support for multi-instrument callbacks, which are a more efficient and practical way to report measurements from a single expensive method when more than one kind of measurements are involved. The "DIY" approach here is really difficult for programmers.

[jmacd]

Reviewers, take note. There is actually no language in the current specification that says callbacks should be associated with single-instruments. Before #2317 the specification stated that one callback was passed to each instrument constructor. The original OTel-Go prototype for batch observers allowed code like so, which I believe was already spec-compliant. This PR just makes it explicit.

The example spec-compliant code would read:

var inst1 AsyncInstrument
var inst2 AsyncInstrument
var callback = meter.RegisterBatchCallback(func() {
   inst1.Observe(...)
   inst2.Observe(...)
})
inst1 = meter.NewAsyncInstrument(...)
inst1 = meter.NewAsyncInstrument(...)

The only problem with this example was that the callback is registered before the instruments, so the callback has a race condition and needed some synchronization to correct. However, the specification wasn't broken here, IMO. There was one callback passed to each instrument constructor, and everything else is idiomatic and up to the API designer. Right?

[jmacd]

@open-telemetry/specs-metrics-approvers I consider this to be a high-priority gap in the API specification which is causing slowness in the OTel-Go repository. Please consider this an urgent request to update the API specification. This is independent of stabilizing the SDK specification, but one reason we haven't provided an OTel-Go SDK to help stabilize the SDK specification is that we're trying to find an API we're all happy with first. Please review this PR.

[bogdandrutu]

Do you have a prototype of this?

[jmacd]

Do you have a prototype of this?

The original OTel-Go prototype includes a batch-observer API. So does the proposed replacement OTel-Go API in open-telemetry/opentelemetry-go#2587.

I even think of the original prototype API as technically spec compliant, since a "single callback" is passed to each instrument constructor. Nothing in the current specification says that a callback can't observe multiple instruments. The spec just says "be idiomatic". I'm trying to clarify that "idiomatic" includes potentially the use of multiple instruments per callback.

[bogdandrutu]

LGTM, except I would like to see if we can just simply allow exactly same callbacks signature for both cases, and remove the limitation that these needs to happen after initialization.

[tsloughter]

Finally got around to reading this closely and what I was not expecting was code like:

        result.observe(self.usage, usage, {'property', self.property})

Simply put, I would have expected self.usage to be an argument to the callback:

def observe(self, result, i_usage, i_pressure):
      usage, pressure = expensive_system_call()
      result.observe(i_usage, usage, {'property', self.property})
      result.observe(i_pressure, pressure, {'property', self.property})

In the form this PR suggests I don't see the purpose of including the instruments in the callback registration since they are used through state of the Device object.

Am I missing something?

[tsloughter]

I realize the passing each instrument as an argument is not ideal, that was more to make a point about how the instrument is referenced. The type of callbacks I'd want to register to report multiple instruments would be potentially dealing with dozens of instruments.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[jmacd]

In the form this PR suggests I don't see the purpose of including the instruments in the callback registration since they are used through state of the Device object.

Am I missing something?

If a View does not select any of the instruments defined by a callback, for example, the SDK does not need to evaluate that callback. I believe there is a desire to support instruments that are collected on different intervals, that is also enabled by letting the SDK know which callbacks observe which instruments.

[jack-berg]

I've prototyped this in java here. I think this is a good addition which solves a valuable use case 👍 .