opentelemetry-cpp
opentelemetry-cpp copied to clipboard
[Metrics SDK] Add support for Pull Metric Exporter
The MeterProvider::AddMetricReader interface does not make it easy to create pull-based readers. For example, my unit tests look something like this:
// Attempt at Pull-based test reader
class TestReader : public opentelemetry::v1::sdk::metrics::MetricReader {
void OnInitialized() noexcept override {}
bool OnForceFlush(std::chrono::microseconds timeout) noexcept override {
return true;
}
bool OnShutDown(std::chrono::microseconds timeout) noexcept override {
return true;
}
};
BOOST_AUTO_TEST_CASE(TestMetrics) {
opentelemetry::v1::sdk::metrics::MeterProvider meter_provider;
auto meter = meter_provider.GetMeter("unit_test_meter", "0.0.0");
auto reader = std::make_unique<TestReader>();
auto* readerPtr = reader.get(); // Work-around for funky OTel SDK
meter_provider.AddMetricReader(std::move(reader));
// ... generate metric data ...
readerPtr->Collect([&](auto& records) {
BOOST_TEST(has(records, "my.test.meter", {{"att1", "value1"}}, 12345L));
return true;
});
}
Notice the raw pointer reference to object owned by the std::unique_ptr (yuck)
I would suggest two options for an alternative:
The simplest change would be to change the AddMetricReader method:
void AddMetricReader(std::shared_ptr<MetricReader> reader) noexcept;
To allow for shared ownership.
A more complex (and somewhat nicer IMHO) fix would be to abstract away the MetricReader class entirely:
void AddPeriodicPushMetricExporter(std::unique_ptr<MetricExporter> exporter, std::chrono::duration interval, std::chrono::duration timeout);
std::unique_ptr<MetricReader> AddPullMetricExporter(std::unique_ptr<MetricExporter> exporter, std::chrono::duration timeout);
Where the returned MetricReader interface provides the Collect() method that can be called to trigger MetricExporter::Collect
Both options follow the SDK documentation as well as I understand them.
Thanks for raising the issue. I like the second approach, though it needs some design discussion. As we are nearing the Release Candidate, have added it for the next milestone (GA).