Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
lalitb committed Sep 29, 2023
1 parent 9e34355 commit 4134c00
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 131 deletions.
8 changes: 2 additions & 6 deletions api/include/opentelemetry/metrics/noop.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,9 @@ class NoopGauge : public Gauge<T>
nostd::string_view /* description */,
nostd::string_view /* unit */) noexcept
{}
void Record(T /* value */, const context::Context & /* context */) noexcept override {}
void Record(T /* value */,
const common::KeyValueIterable & /* attributes */,
const context::Context & /* context */) noexcept override
{}
void Record(T value) noexcept override {}
void Record(T value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override
const common::KeyValueIterable * /* attributes */,
const context::Context * /* context */) noexcept override
{}
};
#endif
Expand Down
42 changes: 31 additions & 11 deletions api/include/opentelemetry/metrics/sync_instruments.h
Original file line number Diff line number Diff line change
Expand Up @@ -259,39 +259,59 @@ class Gauge : public SynchronousInstrument
{
public:
/**
* Record a value
* 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 Record(T value) noexcept = 0;
virtual void Record(T value,
const common::KeyValueIterable *attributes,
const context::Context *context) noexcept = 0;

/**
* Record a value
* 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 Record(T value, const context::Context &context) noexcept = 0;
void Record(T value,
const common::KeyValueIterable &attributes,
const context::Context &context) noexcept
{
this->Record(value, &attributes, &context);
}

/**
* Record a value with a set of attributes.
* Record a value
*
* @param value The increment amount. MUST be non-negative.
* @param attributes A set of attributes to associate with the value.
*/
virtual void Record(T value) noexcept { this->Record(value, nullptr, nullptr); }

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

/**
* 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 Record(T value,
const common::KeyValueIterable &attributes,
const context::Context &context) noexcept = 0;

void Record(T value, const common::KeyValueIterable &attributes) noexcept
{
this->Record(value, &attributes, nullptr);
}

template <class U,
nostd::enable_if_t<common::detail::is_key_value_iterable<U>::value> * = nullptr>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,7 @@ class DefaultAggregation
return AggregationType::kSum;
case InstrumentType::kHistogram:
return AggregationType::kHistogram;
#if OPENTELEMETRY_ABI_VERSION_NO >= 2
case InstrumentType::kGauge:
#endif
case InstrumentType::kObservableGauge:
return AggregationType::kLastValue;
default:
Expand Down
2 changes: 0 additions & 2 deletions sdk/include/opentelemetry/sdk/metrics/instruments.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ enum class InstrumentType
kCounter,
kHistogram,
kUpDownCounter,
#if OPENTELEMETRY_ABI_VERSION_NO >= 2
kGauge,
#endif
kObservableCounter,
kObservableGauge,
kObservableUpDownCounter
Expand Down
19 changes: 4 additions & 15 deletions sdk/include/opentelemetry/sdk/metrics/sync_instruments.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,19 +144,13 @@ class DoubleHistogram : public Synchronous, public opentelemetry::metrics::Histo

class LongGauge : public Synchronous, public opentelemetry::metrics::Gauge<uint64_t>
{

public:
LongGauge(InstrumentDescriptor instrument_descriptor,
std::unique_ptr<SyncWritableMetricStorage> storage);

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

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

class DoubleGauge : public Synchronous, public opentelemetry::metrics::Gauge<double>
Expand All @@ -166,13 +160,8 @@ class DoubleGauge : public Synchronous, public opentelemetry::metrics::Gauge<dou
std::unique_ptr<SyncWritableMetricStorage> storage);

void Record(double value,
const opentelemetry::common::KeyValueIterable &attributes) noexcept override;
void Record(double value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) noexcept override;

void Record(double value) noexcept override;
void Record(double value, const opentelemetry::context::Context &context) noexcept override;
const opentelemetry::common::KeyValueIterable *attributes,
const opentelemetry::context::Context *context) noexcept override;
};
#endif

Expand Down
101 changes: 6 additions & 95 deletions sdk/src/metrics/sync_instruments.cc
Original file line number Diff line number Diff line change
Expand Up @@ -453,52 +453,16 @@ LongGauge::LongGauge(InstrumentDescriptor instrument_descriptor,
}

void LongGauge::Record(uint64_t value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) noexcept
const opentelemetry::common::KeyValueIterable *attributes,
const opentelemetry::context::Context *context) noexcept
{
if (!storage_)
{
OTEL_INTERNAL_LOG_WARN("[LongGauge::Record(V,A,C)] Value not recorded - invalid storage for: "
<< instrument_descriptor_.name_);
return;
}
return storage_->RecordLong(value, attributes, context);
}

void LongGauge::Record(uint64_t value, const opentelemetry::context::Context &context) noexcept
{
if (!storage_)
{
OTEL_INTERNAL_LOG_WARN("[LongGauge::Record(V,C)] Value not recorded - invalid storage for: "
<< instrument_descriptor_.name_);
return;
}
return storage_->RecordLong(value, context);
}

void LongGauge::Record(uint64_t value,
const opentelemetry::common::KeyValueIterable &attributes) noexcept
{
if (!storage_)
{
OTEL_INTERNAL_LOG_WARN("[LongGauge::Record(V,A)] Value not recorded - invalid storage for: "
<< instrument_descriptor_.name_);
return;
}
auto context = opentelemetry::context::Context{};
return storage_->RecordLong(value, attributes, context);
}

void LongGauge::Record(uint64_t value) noexcept
{
if (!storage_)
{
OTEL_INTERNAL_LOG_WARN("[LongGauge::Record(V)] Value not recorded - invalid storage for: "
<< instrument_descriptor_.name_);
return;
}
auto context = opentelemetry::context::Context{};
return storage_->RecordLong(value, context);
return storage_->RecordLong(value, *attributes, *context);
}

DoubleGauge::DoubleGauge(InstrumentDescriptor instrument_descriptor,
Expand All @@ -514,8 +478,8 @@ DoubleGauge::DoubleGauge(InstrumentDescriptor instrument_descriptor,
}

void DoubleGauge::Record(double value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) noexcept
const opentelemetry::common::KeyValueIterable *attributes,
const opentelemetry::context::Context *context) noexcept
{
if (value < 0)
{
Expand All @@ -529,62 +493,9 @@ void DoubleGauge::Record(double value,
<< instrument_descriptor_.name_);
return;
}
return storage_->RecordDouble(value, attributes, context);
}

void DoubleGauge::Record(double value, const opentelemetry::context::Context &context) noexcept
{
if (value < 0)
{
OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,C)] Value not recorded - negative value for: "
<< instrument_descriptor_.name_);
return;
}
if (!storage_)
{
OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,C)] Value not recorded - invalid storage for: "
<< instrument_descriptor_.name_);
return;
}
return storage_->RecordDouble(value, context);
}

void DoubleGauge::Record(double value,
const opentelemetry::common::KeyValueIterable &attributes) noexcept
{
if (value < 0)
{
OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,A)] Value not recorded - negative value for: "
<< instrument_descriptor_.name_);
return;
}
if (!storage_)
{
OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V,A)] Value not recorded - invalid storage for: "
<< instrument_descriptor_.name_);
return;
}
auto context = opentelemetry::context::Context{};
return storage_->RecordDouble(value, attributes, context);
return storage_->RecordDouble(value, *attributes, *context);
}

void DoubleGauge::Record(double value) noexcept
{
if (value < 0)
{
OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V)] Value not recorded - negative value for: "
<< instrument_descriptor_.name_);
return;
}
if (!storage_)
{
OTEL_INTERNAL_LOG_WARN("[DoubleGauge::Record(V)] Value not recorded - invalid storage for: "
<< instrument_descriptor_.name_);
return;
}
auto context = opentelemetry::context::Context{};
return storage_->RecordDouble(value, context);
}
#endif

} // namespace metrics
Expand Down

0 comments on commit 4134c00

Please sign in to comment.