Skip to content

Commit

Permalink
[Metrics] Make context optional for histogram instruments in Metrics …
Browse files Browse the repository at this point in the history 
…SDK (#2416)
  • Loading branch information
@ThomsonTan
ThomsonTan authored Nov 29, 2023
1 parent 71d0e50 commit 064fef0
Show file tree
Hide file tree
Showing 6 changed files with 375 additions and 139 deletions.
7 changes: 7 additions & 0 deletions api/include/opentelemetry/metrics/noop.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ class NoopHistogram : public Histogram<T>
const common::KeyValueIterable & /* attributes */,
const context::Context & /* context */) noexcept override
{}
#if OPENTELEMETRY_ABI_VERSION_NO >= 2
void Record(T /*value*/,
const opentelemetry::common::KeyValueIterable & /*attributes*/) noexcept override
{}

void Record(T /*value*/) noexcept override {}
#endif
};

template <class T>
Expand Down
99 changes: 76 additions & 23 deletions api/include/opentelemetry/metrics/sync_instruments.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,43 @@ class SynchronousInstrument
virtual ~SynchronousInstrument() = default;
};

/* A Counter instrument that adds values. */
template <class T>
class Counter : public SynchronousInstrument
{

public:
/**
* Add adds the value to the counter's sum
* Record a value
*
* @param value The increment amount. MUST be non-negative.
*/
virtual void Add(T value) noexcept = 0;

/**
* Record a value
*
* @param value The increment amount. MUST be non-negative.
* @param context The explicit context to associate with this measurement.
*/
virtual void Add(T value, const context::Context &context) noexcept = 0;

/**
* Add adds the value to the counter's sum. The attributes should contain
* the keys and values to be associated with this value. Counters only
* accept positive valued updates.
* Record a value with a set of attributes.
*
* @param value The increment amount. MUST be non-negative.
* @param attributes the set of attributes, as key-value pairs
* @param attributes A set of attributes to associate with the value.
*/

virtual void Add(T value, const common::KeyValueIterable &attributes) noexcept = 0;

/**
* Record a value with a set of attributes.
*
* @param value The increment amount. MUST be non-negative.
* @param attributes A set of attributes to associate with the value.
* @param context The explicit context to associate with this measurement.
*/
virtual void Add(T value,
const common::KeyValueIterable &attributes,
const context::Context &context) noexcept = 0;
Expand All @@ -55,8 +67,7 @@ class Counter : public SynchronousInstrument
nostd::enable_if_t<common::detail::is_key_value_iterable<U>::value> * = nullptr>
void Add(T value, const U &attributes) noexcept
{
auto context = context::Context{};
this->Add(value, common::KeyValueIterableView<U>{attributes}, context);
this->Add(value, common::KeyValueIterableView<U>{attributes});
}

template <class U,
Expand All @@ -70,11 +81,8 @@ class Counter : public SynchronousInstrument
std::initializer_list<std::pair<nostd::string_view, common::AttributeValue>>
attributes) noexcept
{
auto context = context::Context{};
this->Add(value,
nostd::span<const std::pair<nostd::string_view, common::AttributeValue>>{
attributes.begin(), attributes.end()},
context);
this->Add(value, nostd::span<const std::pair<nostd::string_view, common::AttributeValue>>{
attributes.begin(), attributes.end()});
}

void Add(T value,
Expand All @@ -94,18 +102,54 @@ template <class T>
class Histogram : public SynchronousInstrument
{
public:
#if OPENTELEMETRY_ABI_VERSION_NO >= 2
/**
* @since ABI_VERSION 2
* Records a value.
*
* @param value The measurement value. MUST be non-negative.
*/
virtual void Record(T value) noexcept = 0;

/**
* @since ABI_VERSION 2
* Records a value with a set of attributes.
*
* @param value The measurement value. MUST be non-negative.
* @param attribute A set of attributes to associate with the value.
*/
virtual void Record(T value, const common::KeyValueIterable &attribute) noexcept = 0;

template <class U,
nostd::enable_if_t<common::detail::is_key_value_iterable<U>::value> * = nullptr>
void Record(T value, const U &attributes) noexcept
{
this->Record(value, common::KeyValueIterableView<U>{attributes});
}

void Record(T value,
std::initializer_list<std::pair<nostd::string_view, common::AttributeValue>>
attributes) noexcept
{
this->Record(value, nostd::span<const std::pair<nostd::string_view, common::AttributeValue>>{
attributes.begin(), attributes.end()});
}
#endif

/**
* Records a value.
*
* @param value The measurement value. MUST be non-negative.
* @param context The explicit context to associate with this measurement.
*/
virtual void Record(T value, const context::Context &context) noexcept = 0;

/**
* Records a value with a set of attributes.
*
* @param value The measurement value. MUST be non-negative.
* @param attributes A set of attributes to associate with the count.
* @param attributes A set of attributes to associate with the value..
* @param context The explicit context to associate with this measurement.
*/
virtual void Record(T value,
const common::KeyValueIterable &attributes,
Expand Down Expand Up @@ -137,22 +181,35 @@ class UpDownCounter : public SynchronousInstrument
{
public:
/**
* Adds a value.
* Record a value.
*
* @param value The amount of the measurement.
* @param value The increment amount. May be positive, negative or zero.
*/
virtual void Add(T value) noexcept = 0;

/**
* Record a value.
*
* @param value The increment amount. May be positive, negative or zero.
* @param context The explicit context to associate with this measurement.
*/
virtual void Add(T value, const context::Context &context) noexcept = 0;

/**
* Add a value with a set of attributes.
* Record a value with a set of attributes.
*
* @param value The increment amount. May be positive, negative or zero.
* @param attributes A set of attributes to associate with the count.
*/
virtual void Add(T value, const common::KeyValueIterable &attributes) noexcept = 0;

/**
* Record a value with a set of attributes.
*
* @param value The increment amount. May be positive, negative or zero.
* @param attributes A set of attributes to associate with the count.
* @param context The explicit context to associate with this measurement.
*/
virtual void Add(T value,
const common::KeyValueIterable &attributes,
const context::Context &context) noexcept = 0;
Expand All @@ -161,8 +218,7 @@ class UpDownCounter : public SynchronousInstrument
nostd::enable_if_t<common::detail::is_key_value_iterable<U>::value> * = nullptr>
void Add(T value, const U &attributes) noexcept
{
auto context = context::Context{};
this->Add(value, common::KeyValueIterableView<U>{attributes}, context);
this->Add(value, common::KeyValueIterableView<U>{attributes});
}

template <class U,
Expand All @@ -176,11 +232,8 @@ class UpDownCounter : public SynchronousInstrument
std::initializer_list<std::pair<nostd::string_view, common::AttributeValue>>
attributes) noexcept
{
auto context = context::Context{};
this->Add(value,
nostd::span<const std::pair<nostd::string_view, common::AttributeValue>>{
attributes.begin(), attributes.end()},
context);
this->Add(value, nostd::span<const std::pair<nostd::string_view, common::AttributeValue>>{
attributes.begin(), attributes.end()});
}

void Add(T value,
Expand Down
122 changes: 33 additions & 89 deletions sdk/include/opentelemetry/sdk/metrics/sync_instruments.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,61 +33,22 @@ class Synchronous
std::unique_ptr<SyncWritableMetricStorage> storage_;
};

template <typename T>
class LongCounter : public Synchronous, public opentelemetry::metrics::Counter<T>
class LongCounter : public Synchronous, public opentelemetry::metrics::Counter<uint64_t>
{
public:
LongCounter(InstrumentDescriptor instrument_descriptor,
std::unique_ptr<SyncWritableMetricStorage> storage)
: Synchronous(instrument_descriptor, std::move(storage))
{
if (!storage_)
{
OTEL_INTERNAL_LOG_ERROR("[LongCounter::LongCounter] - Error during constructing LongCounter."
<< "The metric storage is invalid"
<< "No value will be added");
}
}

void Add(T value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override
{
if (!storage_)
{
return;
}
auto context = opentelemetry::context::Context{};
return storage_->RecordLong(value, attributes, context);
}

void Add(T value,
std::unique_ptr<SyncWritableMetricStorage> storage);

void Add(uint64_t value,
const opentelemetry::common::KeyValueIterable &attributes) noexcept override;

void Add(uint64_t value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) noexcept override
{
if (!storage_)
{
return;
}
return storage_->RecordLong(value, attributes, context);
}

void Add(T value) noexcept override
{
auto context = opentelemetry::context::Context{};
if (!storage_)
{
return;
}
return storage_->RecordLong(value, context);
}

void Add(T value, const opentelemetry::context::Context &context) noexcept override
{
if (!storage_)
{
return;
}
return storage_->RecordLong(value, context);
}
const opentelemetry::context::Context &context) noexcept override;

void Add(uint64_t value) noexcept override;

void Add(uint64_t value, const opentelemetry::context::Context &context) noexcept override;
};

class DoubleCounter : public Synchronous, public opentelemetry::metrics::Counter<double>
Expand Down Expand Up @@ -139,48 +100,24 @@ class DoubleUpDownCounter : public Synchronous, public opentelemetry::metrics::U
void Add(double value, const opentelemetry::context::Context &context) noexcept override;
};

template <typename T>
class LongHistogram : public Synchronous, public opentelemetry::metrics::Histogram<T>
class LongHistogram : public Synchronous, public opentelemetry::metrics::Histogram<uint64_t>
{
public:
LongHistogram(InstrumentDescriptor instrument_descriptor,
std::unique_ptr<SyncWritableMetricStorage> storage)
: Synchronous(instrument_descriptor, std::move(storage))
{
if (!storage_)
{
OTEL_INTERNAL_LOG_ERROR(
"[LongHistogram::LongHistogram] - Error during constructing LongHistogram."
<< "The metric storage is invalid"
<< "No value will be added");
}
}

void Record(T value,
std::unique_ptr<SyncWritableMetricStorage> storage);

#if OPENTELEMETRY_ABI_VERSION_NO >= 2
void Record(uint64_t value,
const opentelemetry::common::KeyValueIterable &attributes) noexcept override;

void Record(uint64_t value) noexcept override;
#endif

void Record(uint64_t value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) noexcept override
{
if (value < 0)
{
OTEL_INTERNAL_LOG_WARN(
"[LongHistogram::Record(value, attributes)] negative value provided to histogram Name:"
<< instrument_descriptor_.name_ << " Value:" << value);
return;
}
return storage_->RecordLong(value, attributes, context);
}

void Record(T value, const opentelemetry::context::Context &context) noexcept override
{
if (value < 0)
{
OTEL_INTERNAL_LOG_WARN(
"[LongHistogram::Record(value)] negative value provided to histogram Name:"
<< instrument_descriptor_.name_ << " Value:" << value);
return;
}
return storage_->RecordLong(value, context);
}
const opentelemetry::context::Context &context) noexcept override;

void Record(uint64_t value, const opentelemetry::context::Context &context) noexcept override;
};

class DoubleHistogram : public Synchronous, public opentelemetry::metrics::Histogram<double>
Expand All @@ -189,6 +126,13 @@ class DoubleHistogram : public Synchronous, public opentelemetry::metrics::Histo
DoubleHistogram(InstrumentDescriptor instrument_descriptor,
std::unique_ptr<SyncWritableMetricStorage> storage);

#if OPENTELEMETRY_ABI_VERSION_NO >= 2
void Record(double value,
const opentelemetry::common::KeyValueIterable &attributes) noexcept override;

void Record(double value) noexcept override;
#endif

void Record(double value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) noexcept override;
Expand Down
4 changes: 2 additions & 2 deletions sdk/src/metrics/meter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ nostd::unique_ptr<metrics::Counter<uint64_t>> Meter::CreateUInt64Counter(
std::string{unit.data(), unit.size()}, InstrumentType::kCounter, InstrumentValueType::kLong};
auto storage = RegisterSyncMetricStorage(instrument_descriptor);
return nostd::unique_ptr<metrics::Counter<uint64_t>>(
new LongCounter<uint64_t>(instrument_descriptor, std::move(storage)));
new LongCounter(instrument_descriptor, std::move(storage)));
}

nostd::unique_ptr<metrics::Counter<double>> Meter::CreateDoubleCounter(
Expand Down Expand Up @@ -138,7 +138,7 @@ nostd::unique_ptr<metrics::Histogram<uint64_t>> Meter::CreateUInt64Histogram(
InstrumentValueType::kLong};
auto storage = RegisterSyncMetricStorage(instrument_descriptor);
return nostd::unique_ptr<metrics::Histogram<uint64_t>>{
new LongHistogram<uint64_t>(instrument_descriptor, std::move(storage))};
new LongHistogram(instrument_descriptor, std::move(storage))};
}

nostd::unique_ptr<metrics::Histogram<double>> Meter::CreateDoubleHistogram(
Expand Down
Loading

1 comment on commit 064fef0

@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: 064fef0 Previous: 71d0e50 Ratio
BM_BaselineBuffer/2 13592340.94619751 ns/iter 2772278.07372867 ns/iter 4.90
BM_BaselineBuffer/4 15313642.024993896 ns/iter 2547808.103663947 ns/iter 6.01
BM_LockFreeBuffer/2 7467222.213745117 ns/iter 951385.9748840332 ns/iter 7.85
BM_LockFreeBuffer/4 10117330.551147461 ns/iter 1023276.2311785753 ns/iter 9.89

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

Please sign in to comment.