[Metrics] Allow registering one callback for multiple instruments

[KJTsanaktsidis]

Fixes #3653

Changes

This commit adds a pair of methods Meter::RegisterCallback and Meter::DeregisterCallback which allow you to set up one callback which can record observations for multiple observable instruments.

The callback will be passed an object of type MultiObserverResult, which can be used to get an ObserverResultT for an instrument.

This allows a single expensive operation to yield multiple different observable metrics without the need to call said expensive operation multiple times.

There's a test demonstrating the usage, but the short version is

void callback(metrics::MultiObserverResult &result, void *state)
{
    auto instrument1 = reinterpret_cast<MyState *>(state)->instrument1;
    auto instrument2 = reinterpret_cast<MyState *>(state)->instrument2;

   result.ForInstrument<int64_t>(instrument1).Observe(32, {"some", "labels"});
   result.ForInstrument<double>(instrument2).Observe(3234.32);
}

auto instrument1 = meter->CreateInt64ObservableCounter(...);
auto instrument2 = meter->CreateDoubleObservableGauge(...);

MyState state{instrument1, instrument2};
std::array<opentelemetry::metrics::ObservableInstrument *, 2> reg_instruments{instrument1, instrument2};
meter->RegisterCallback(callback, state, reg_instruments);

Design notes

• I picked the names RegisterCallback based on the Go SDK, which exposes a RegisterCallback method on the Meter for this purpose. Is this the right name? Is Meter scope the right place to put this?
• The callback is required to have access to a opentelemetry::metrics::ObservableInstrument pointer somehow through state in order to record a measurement. This is in line with the interface sketched out in the spec - https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#multiple-instrument-callbacks
• I felt pretty silly defining the variant ObserverResultDirect = std::variant<ObserverResultT<double>, ObserverResultT<int64_t>> when we already have ObserverResult = std::variant<shared_ptr<ObserverResultT<double>>, shared_ptr<ObserverResultT<int64_t>> but it does get rid of a needless extra heap allocation in the multi instrument case so it's probably for the best?
• I don't love the fact that the MultiObserverResult has protected ForInstrumentDouble and ForInstrumentInt64; this is needed because you can't have a virtual template method, and I wanted the recording interface to look like res.ForInstrument<int64_t>(instrument).Observe(value). We could make ForInstrument return a variant, but that would require a) exposing ObserverResultDirect as part of the API, and b) requiring the calling code to std::get or std::visit that.
• There is in fact no actual reason why this interface requires you to pre-register the instruments to be observed ahead of time; the code could perfectly happily add them on the fly in calls to ForInstrument. However this seems to be what the spec describes, so I've stuck to the spec.

Very keen to hear what you think!

• CHANGELOG.md updated for non-trivial changes
• Unit tests have been added
• Changes in public API reviewed

[codecov]

Codecov Report

❌ Patch coverage is 80.91603% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.94%. Comparing base (05a6f4c) to head (79d6f7d).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
sdk/src/metrics/state/observable_registry.cc	76.72%	17 Missing ⚠️
sdk/src/metrics/multi_observer_result.cc	84.91%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3667      +/-   ##
==========================================
- Coverage   90.16%   89.94%   -0.22%     
==========================================
  Files         223      225       +2     
  Lines        7172     7273     +101     
==========================================
+ Hits         6466     6541      +75     
- Misses        706      732      +26     

Files with missing lines	Coverage Δ
api/include/opentelemetry/metrics/meter.h	100.00% <ø> (ø)
...lude/opentelemetry/metrics/multi_observer_result.h	100.00% <100.00%> (ø)
api/include/opentelemetry/metrics/noop.h	94.12% <ø> (ø)
sdk/include/opentelemetry/sdk/metrics/meter.h	57.15% <ø> (ø)
sdk/src/metrics/meter.cc	85.89% <ø> (ø)
sdk/src/metrics/multi_observer_result.cc	84.91% <84.91%> (ø)
sdk/src/metrics/state/observable_registry.cc	77.11% <76.72%> (-18.24%)	⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[owent]

Is it possiable to drop callbacks_ and allow constructing MultiObservableCallbackRecord from ObservableCallbackRecord? Maintaining multiple types is cumbersome for users.

[KJTsanaktsidis]

Is it possiable to drop callbacks_ and allow constructing MultiObservableCallbackRecord from ObservableCallbackRecord?

Is the ask here to essentially do...

class ObservableRegistry {
private:
#if OPENTELEMETRY_ABI_VERSION_NO >= 2
    std::unordered_map<uintptr_t, std::unique_ptr<MultiObservableCallbackRecord>> multi_callbacks_;
#else
  std::vector<std::unique_ptr<ObservableCallbackRecord>> callbacks_;
#endif
  std::mutex callbacks_m_;
}

i.e. to re-implement the existing single-observable callback functionality in terms of the multi-observable callback functionality? Yes, I can do this - let me know if I understood correctly

Maintaining multiple types is cumbersome for users.

This I didn't fully understand, because ObservableCallbackRecord and MultiObservableCallbackRecord are implementation details of the SDK ObservableRegistry; users shouldn't be touching these classes, I thought?

(In fact: the definition of MultiObservableCallbackRecord could be moved out of the header and into observable_registry.cc, by type-erasing std::unordered_map<uintptr_t, std::unique_ptr<MultiObservableCallbackRecord>> multi_callbacks_ with a shared_ptr or a unique_ptr with a deleter function instead. I think I'll do this now, it seems worthwhile not to expose types that aren't needed).

[KJTsanaktsidis]

Oh, one more thought; if ObservableRegistry can be thought of as a part of the internal API/ABI of the SDK, I could probably dispense with the ABI version guards in it (since symbols from ObservableRegistry would only be used inside the SDK library itself).

this would allow unconditionally reimplementing the existing single-observable callback in terms of the multi-observable implementation.

Reckon I should do this?

[lalitb]

Yes, it should be safe to do ABI breaking changes with ObservableRegistry.

[owent]

Is it possiable to drop callbacks_ and allow constructing MultiObservableCallbackRecord from ObservableCallbackRecord?

Is the ask here to essentially do...

class ObservableRegistry {
private:
#if OPENTELEMETRY_ABI_VERSION_NO >= 2
    std::unordered_map<uintptr_t, std::unique_ptr<MultiObservableCallbackRecord>> multi_callbacks_;
#else
  std::vector<std::unique_ptr<ObservableCallbackRecord>> callbacks_;
#endif
  std::mutex callbacks_m_;
}

i.e. to re-implement the existing single-observable callback functionality in terms of the multi-observable callback functionality? Yes, I can do this - let me know if I understood correctly

Yes, that's what I want.

Maintaining multiple types is cumbersome for users.

This I didn't fully understand, because ObservableCallbackRecord and MultiObservableCallbackRecord are implementation details of the SDK ObservableRegistry; users shouldn't be touching these classes, I thought?

We maintain cache of registered callbacks in our project, which allow us to do lazy registration, reload , cleanup and some others things, another group of types will make us to double the cache and APIs to maintain them.

BTW: It's OK to break ABI or API in v2 now.

[KJTsanaktsidis]

Yes, that's what I want.

OK, awesome - I think I've done that, hope this looks right to you 🙏

[Copilot]

Pull Request Overview

This PR adds support for registering a single callback function to observe multiple observable instruments, addressing the need to avoid redundant expensive operations when generating multiple related metrics. This implements the multi-instrument callback pattern described in the OpenTelemetry specification.

Key changes include:

• Implementation of Meter::RegisterCallback() and Meter::DeregisterCallback() methods that work with multiple instruments
• New MultiObserverResult API that allows callbacks to record observations for different instruments
• Backward-compatible changes that preserve existing single-instrument callback functionality

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
api/include/opentelemetry/metrics/meter.h	Adds RegisterCallback/DeregisterCallback method signatures for multi-instrument callbacks
api/include/opentelemetry/metrics/multi_observer_result.h	New API class defining the interface for recording observations across multiple instruments
api/include/opentelemetry/metrics/noop.h	No-op implementation of the new RegisterCallback/DeregisterCallback methods
sdk/include/opentelemetry/sdk/metrics/meter.h	SDK implementation declarations for the new callback methods
sdk/include/opentelemetry/sdk/metrics/multi_observer_result.h	SDK concrete implementation of MultiObserverResult
sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h	Updated registry interface to support multi-instrument callbacks
sdk/src/metrics/meter.cc	Implementation of RegisterCallback/DeregisterCallback in the SDK meter
sdk/src/metrics/multi_observer_result.cc	Implementation of the MultiObserverResult functionality
sdk/src/metrics/state/observable_registry.cc	Refactored registry to handle both single and multi-instrument callback patterns
sdk/src/metrics/CMakeLists.txt	Added multi_observer_result.cc to the build
sdk/test/metrics/multi_observer_test.cc	Comprehensive tests for the new multi-instrument callback functionality
sdk/test/metrics/CMakeLists.txt	Added multi_observer_test to the test suite
CHANGELOG.md	Documents the new feature

Comments suppressed due to low confidence (1)

api/include/opentelemetry/metrics/multi_observer_result.h:1

• Corrected spelling of 'unnescessary' to 'unnecessary'.

// Copyright The OpenTelemetry Authors

[marcalff]

Great PR, thanks for the contribution.

Please see some questions, approval to follow.

[KJTsanaktsidis]

Thanks, sorry I wasn't able to get to this last week - made the rest of the changes suggested.

[marcalff]

Excellent work. Thank you for the contribution.

[KJTsanaktsidis]

Thank you, a pleasure working with you.