When using CreateInt64ObservableCounter I get "Error during observe.The metric storage is invalid"

[ashley-b]

Describe your environment opentelemetry-cpp: 1.8.1 gcc (GCC) 12.2.1 20221121 (Red Hat 12.2.1-4) Fedora release 37

Steps to reproduce Using a simple OStreamMetricExporter and observable counter meter I find if the counter fall out of scope before running it errors in debug or segfaults in release. I assume I am using it incorrectly, but I would assume when the counter goes out of scope it should clean up it's resource correctly.

int main()
{
    create_exporter();

    auto meterProvider = metrics_api::Provider::GetMeterProvider();
    auto meter = meterProvider->GetMeter("test", "");
    {
        auto counter = meter->CreateInt64ObservableCounter("counter");
        counter->AddCallback(&get_counter, nullptr);
    }
    std::this_thread::sleep_for(std::chrono::seconds(10));
    return 0;
}

What is the expected behavior? Not to see a Error message or segfault

What is the actual behavior? In debug build

[Error] File: opentelemetry-cpp/1.8.1/sdk/src/metrics/state/observable_registry.cc:55[ObservableRegistry::Observe] - Error during observe.The metric storage is invalid

In Release build it segfaults

Additional notes The life times and ownership of opentelemetry-cpp object is not clear from the documentation

[lalitb]

@ashley-b This is documented here - https://opentelemetry.io/docs/instrumentation/cpp/manual/#create-observable-counter

Create a Observable Counter Instrument from the Meter, and add a callback. The callback would be used to record the measurement during metrics collection. Ensure to keep the Instrument object active for the lifetime of collection.

This is requirement for the asynchronous instruments. There is no plan to change the behavior as of now.

[marcalff]

@lalitb

After AddCallback() is called, the ObservableRegistry contains a back pointer to the instrument.

I think that what is missing here, is cleanup in the ObservableInstrument destructor to remove back pointers from the registry, possibly printing a warning to complain about the missing call to RemoveCallback.

This will make the code more robust, and avoid crashes in cases of misuse.

The proper usage pattern is not changed, but the likelihood of users following the proper pattern will be much improved.

[lalitb]

I think that what is missing here, is cleanup in the ObservableInstrument destructor to remove back pointers from the registry, possibly printing a warning to complain about the missing call to RemoveCallback.

Agree, the destructor of ObservableInstrument can possibly delete all the callbacks registered for this particular instrument. I am assigning it to me, can fix it by end of this week, unless someone can fix it sooner :) Thanks @marcalff for the suggestion.

[ashley-b]

Thanks for the quick follow up and pointing me to the doc's

[github-actions[bot]]

This issue was marked as stale due to lack of activity.