Skip to content

Commit

Permalink
Fix observable attributes drop (#2557)
Browse files Browse the repository at this point in the history
  • Loading branch information
lalitb authored Feb 27, 2024
1 parent 84e38b2 commit 32196a5
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
3 changes: 2 additions & 1 deletion examples/common/metrics_foo_library/foo_library.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ class MeasurementFetcher
{
double random_incr = (rand() % 5) + 1.1;
value_ += random_incr;
std::map<std::string, std::string> labels = get_random_attr();
nostd::get<nostd::shared_ptr<opentelemetry::metrics::ObserverResultT<double>>>(
observer_result)
->Observe(value_ /*, labelkv */);
->Observe(value_, labels);
}
}
static double value_;
Expand Down
8 changes: 6 additions & 2 deletions sdk/include/opentelemetry/sdk/metrics/observer_result.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,15 @@ class ObserverResultT final : public opentelemetry::metrics::ObserverResultT<T>

~ObserverResultT() override = default;

void Observe(T value) noexcept override { data_.insert({{}, value}); }
void Observe(T value) noexcept override
{
data_[MetricAttributes{{}, attributes_processor_}] = value;
}

void Observe(T value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override
{
data_.insert({MetricAttributes{attributes, attributes_processor_}, value});
data_[MetricAttributes{attributes, attributes_processor_}] =
value; // overwrites the previous value if present
}

const std::unordered_map<MetricAttributes, T, AttributeHashGenerator> &GetMeasurements()
Expand Down
5 changes: 3 additions & 2 deletions sdk/src/metrics/state/observable_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ void ObservableRegistry::CleanupCallback(opentelemetry::metrics::ObservableInstr

void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collection_ts)
{
static DefaultAttributesProcessor default_attribute_processor;
std::lock_guard<std::mutex> lock_guard{callbacks_m_};
for (auto &callback_wrap : callbacks_)
{
Expand All @@ -69,7 +70,7 @@ void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collecti
if (value_type == InstrumentValueType::kDouble)
{
nostd::shared_ptr<opentelemetry::metrics::ObserverResultT<double>> ob_res(
new opentelemetry::sdk::metrics::ObserverResultT<double>());
new opentelemetry::sdk::metrics::ObserverResultT<double>(&default_attribute_processor));
callback_wrap->callback(ob_res, callback_wrap->state);
storage->RecordDouble(
static_cast<opentelemetry::sdk::metrics::ObserverResultT<double> *>(ob_res.get())
Expand All @@ -79,7 +80,7 @@ void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collecti
else
{
nostd::shared_ptr<opentelemetry::metrics::ObserverResultT<int64_t>> ob_res(
new opentelemetry::sdk::metrics::ObserverResultT<int64_t>());
new opentelemetry::sdk::metrics::ObserverResultT<int64_t>(&default_attribute_processor));
callback_wrap->callback(ob_res, callback_wrap->state);
storage->RecordLong(
static_cast<opentelemetry::sdk::metrics::ObserverResultT<int64_t> *>(ob_res.get())
Expand Down

2 comments on commit 32196a5

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 32196a5 Previous: 84e38b2 Ratio
BM_ComputeIndex/0 11.47370332334472 ns/iter 5.581082372370041 ns/iter 2.06
BM_ComputeIndex/1 19.273231264504542 ns/iter 9.585992049955014 ns/iter 2.01
BM_BaselineBuffer/1 1722369.1940307617 ns/iter 853822.4697113037 ns/iter 2.02

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp api Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 32196a5 Previous: 84e38b2 Ratio
BM_SpinLockThrashing/1/process_time/real_time 0.37532316899932594 ms/iter 0.15675020217895508 ms/iter 2.39
BM_SpinLockThrashing/4/process_time/real_time 1.9933867454528809 ms/iter 0.6042412618403378 ms/iter 3.30
BM_ProcYieldSpinLockThrashing/1/process_time/real_time 0.4049951141441377 ms/iter 0.08553188245247274 ms/iter 4.74

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.