Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Metrics API/SDK] Change Meter API/SDK to return nostd::unique_ptr for Sync Instruments #1707

Merged
merged 7 commits into from
Oct 25, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix
esigo committed Oct 22, 2022
commit b65512e478b74b16ec96c270c2ec4bfe3866bc55
3 changes: 1 addition & 2 deletions sdk/include/opentelemetry/sdk/metrics/async_instruments.h
Original file line number Diff line number Diff line change
@@ -17,8 +17,7 @@ namespace metrics

class AsyncWritableMetricStorage;

class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument,
public std::enable_shared_from_this<ObservableInstrument>
class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument
{
public:
ObservableInstrument(InstrumentDescriptor instrument_descriptor,
Original file line number Diff line number Diff line change
@@ -22,19 +22,19 @@ struct ObservableCallbackRecord
{
opentelemetry::metrics::ObservableCallbackPtr callback;
void *state;
std::shared_ptr<opentelemetry::metrics::ObservableInstrument> instrument;
opentelemetry::metrics::ObservableInstrument *instrument;
};

class ObservableRegistry
{
public:
void AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback,
void *state,
std::shared_ptr<opentelemetry::metrics::ObservableInstrument> instrument);
opentelemetry::metrics::ObservableInstrument *instrument);

void RemoveCallback(opentelemetry::metrics::ObservableCallbackPtr callback,
void *state,
std::shared_ptr<opentelemetry::metrics::ObservableInstrument> instrument);
opentelemetry::metrics::ObservableInstrument *instrument);

void Observe(opentelemetry::common::SystemTimestamp collection_ts);

4 changes: 2 additions & 2 deletions sdk/src/metrics/async_instruments.cc
Original file line number Diff line number Diff line change
@@ -26,13 +26,13 @@ ObservableInstrument::ObservableInstrument(InstrumentDescriptor instrument_descr
void ObservableInstrument::AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback,
void *state) noexcept
{
observable_registry_->AddCallback(callback, state, shared_from_this());
observable_registry_->AddCallback(callback, state, this);
}

void ObservableInstrument::RemoveCallback(opentelemetry::metrics::ObservableCallbackPtr callback,
void *state) noexcept
{
observable_registry_->RemoveCallback(callback, state, shared_from_this());
observable_registry_->RemoveCallback(callback, state, this);
}

const InstrumentDescriptor &ObservableInstrument::GetInstrumentDescriptor()
28 changes: 13 additions & 15 deletions sdk/src/metrics/state/observable_registry.cc
Original file line number Diff line number Diff line change
@@ -15,10 +15,9 @@ namespace sdk
namespace metrics
{

void ObservableRegistry::AddCallback(
opentelemetry::metrics::ObservableCallbackPtr callback,
void *state,
std::shared_ptr<opentelemetry::metrics::ObservableInstrument> instrument)
void ObservableRegistry::AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback,
void *state,
opentelemetry::metrics::ObservableInstrument *instrument)
{
// TBD - Check if existing
std::unique_ptr<ObservableCallbackRecord> record(
@@ -27,10 +26,9 @@ void ObservableRegistry::AddCallback(
callbacks_.push_back(std::move(record));
}

void ObservableRegistry::RemoveCallback(
opentelemetry::metrics::ObservableCallbackPtr callback,
void *state,
std::shared_ptr<opentelemetry::metrics::ObservableInstrument> instrument)
void ObservableRegistry::RemoveCallback(opentelemetry::metrics::ObservableCallbackPtr callback,
void *state,
opentelemetry::metrics::ObservableInstrument *instrument)
{
std::lock_guard<std::mutex> lock_guard{callbacks_m_};
auto new_end = std::remove_if(
@@ -47,13 +45,13 @@ void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collecti
std::lock_guard<std::mutex> lock_guard{callbacks_m_};
for (auto &callback_wrap : callbacks_)
{
auto value_type = std::static_pointer_cast<opentelemetry::sdk::metrics::ObservableInstrument>(
callback_wrap->instrument)
->GetInstrumentDescriptor()
.value_type_;
auto storage = std::static_pointer_cast<opentelemetry::sdk::metrics::ObservableInstrument>(
callback_wrap->instrument)
->GetMetricStorage();
auto value_type =
static_cast<opentelemetry::sdk::metrics::ObservableInstrument *>(callback_wrap->instrument)
->GetInstrumentDescriptor()
.value_type_;
auto storage =
static_cast<opentelemetry::sdk::metrics::ObservableInstrument *>(callback_wrap->instrument)
->GetMetricStorage();
if (!storage)
{
OTEL_INTERNAL_LOG_ERROR("[ObservableRegistry::Observe] - Error during observe."
30 changes: 30 additions & 0 deletions sdk/test/metrics/BUILD
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark")

cc_test(
name = "meter_test",
srcs = [
"meter_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"//sdk/src/metrics",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "meter_provider_sdk_test",
srcs = [
@@ -112,6 +127,21 @@ cc_test(
],
)

cc_test(
name = "async_instruments_test",
srcs = [
"async_instruments_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"//sdk/src/metrics",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "async_metric_storage_test",
srcs = [