Skip to content

Commit

Permalink
[Metrics] Switch to explicit 64 bit integers (#1686)
Browse files Browse the repository at this point in the history
  • Loading branch information
esigo authored Oct 25, 2022
1 parent 2ed104c commit 7fd76ee
Show file tree
Hide file tree
Showing 57 changed files with 430 additions and 437 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ Increment the:

## [Unreleased]

* [Metrics] Switch to explicit 64 bit integers [#1686](https://github.com/open-telemetry/opentelemetry-cpp/pull/1686)
which includes breaking change in the Metrics api and sdk.
* [Metrics SDK] Add support for Pull Metric Exporter [#1701](https://github.com/open-telemetry/opentelemetry-cpp/pull/1701)
which includes breaking change in the Metrics api.
* [BUILD] Add CMake OTELCPP_MAINTAINER_MODE [#1650](https://github.com/open-telemetry/opentelemetry-cpp/pull/1650)
Expand Down
12 changes: 6 additions & 6 deletions api/include/opentelemetry/metrics/meter.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Meter
* @return a shared pointer to the created Counter.
*/

virtual nostd::shared_ptr<Counter<long>> CreateLongCounter(
virtual nostd::shared_ptr<Counter<uint64_t>> CreateUInt64Counter(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept = 0;
Expand All @@ -54,7 +54,7 @@ class Meter
* @param description a brief description of what the Observable Counter is used for.
* @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html.
*/
virtual nostd::shared_ptr<ObservableInstrument> CreateLongObservableCounter(
virtual nostd::shared_ptr<ObservableInstrument> CreateInt64ObservableCounter(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept = 0;
Expand All @@ -72,7 +72,7 @@ class Meter
* @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html.
* @return a shared pointer to the created Histogram.
*/
virtual nostd::shared_ptr<Histogram<long>> CreateLongHistogram(
virtual nostd::shared_ptr<Histogram<uint64_t>> CreateUInt64Histogram(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept = 0;
Expand All @@ -90,7 +90,7 @@ class Meter
* @param description a brief description of what the Observable Gauge is used for.
* @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html.
*/
virtual nostd::shared_ptr<ObservableInstrument> CreateLongObservableGauge(
virtual nostd::shared_ptr<ObservableInstrument> CreateInt64ObservableGauge(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept = 0;
Expand All @@ -109,7 +109,7 @@ class Meter
* @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html.
* @return a shared pointer to the created UpDownCounter.
*/
virtual nostd::shared_ptr<UpDownCounter<long>> CreateLongUpDownCounter(
virtual nostd::shared_ptr<UpDownCounter<int64_t>> CreateInt64UpDownCounter(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept = 0;
Expand All @@ -127,7 +127,7 @@ class Meter
* @param description a brief description of what the Observable UpDownCounter is used for.
* @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html.
*/
virtual nostd::shared_ptr<ObservableInstrument> CreateLongObservableUpDownCounter(
virtual nostd::shared_ptr<ObservableInstrument> CreateInt64ObservableUpDownCounter(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept = 0;
Expand Down
26 changes: 14 additions & 12 deletions api/include/opentelemetry/metrics/noop.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,12 @@ class NoopObservableInstrument : public ObservableInstrument
class NoopMeter final : public Meter
{
public:
nostd::shared_ptr<Counter<long>> CreateLongCounter(nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept override
nostd::shared_ptr<Counter<uint64_t>> CreateUInt64Counter(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept override
{
return nostd::shared_ptr<Counter<long>>{new NoopCounter<long>(name, description, unit)};
return nostd::shared_ptr<Counter<uint64_t>>{new NoopCounter<uint64_t>(name, description, unit)};
}

nostd::shared_ptr<Counter<double>> CreateDoubleCounter(
Expand All @@ -102,7 +103,7 @@ class NoopMeter final : public Meter
return nostd::shared_ptr<Counter<double>>{new NoopCounter<double>(name, description, unit)};
}

nostd::shared_ptr<ObservableInstrument> CreateLongObservableCounter(
nostd::shared_ptr<ObservableInstrument> CreateInt64ObservableCounter(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept override
Expand All @@ -120,12 +121,13 @@ class NoopMeter final : public Meter
new NoopObservableInstrument(name, description, unit));
}

nostd::shared_ptr<Histogram<long>> CreateLongHistogram(
nostd::shared_ptr<Histogram<uint64_t>> CreateUInt64Histogram(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept override
{
return nostd::shared_ptr<Histogram<long>>{new NoopHistogram<long>(name, description, unit)};
return nostd::shared_ptr<Histogram<uint64_t>>{
new NoopHistogram<uint64_t>(name, description, unit)};
}

nostd::shared_ptr<Histogram<double>> CreateDoubleHistogram(
Expand All @@ -136,7 +138,7 @@ class NoopMeter final : public Meter
return nostd::shared_ptr<Histogram<double>>{new NoopHistogram<double>(name, description, unit)};
}

nostd::shared_ptr<ObservableInstrument> CreateLongObservableGauge(
nostd::shared_ptr<ObservableInstrument> CreateInt64ObservableGauge(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept override
Expand All @@ -154,13 +156,13 @@ class NoopMeter final : public Meter
new NoopObservableInstrument(name, description, unit));
}

nostd::shared_ptr<UpDownCounter<long>> CreateLongUpDownCounter(
nostd::shared_ptr<UpDownCounter<int64_t>> CreateInt64UpDownCounter(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept override
{
return nostd::shared_ptr<UpDownCounter<long>>{
new NoopUpDownCounter<long>(name, description, unit)};
return nostd::shared_ptr<UpDownCounter<int64_t>>{
new NoopUpDownCounter<int64_t>(name, description, unit)};
}

nostd::shared_ptr<UpDownCounter<double>> CreateDoubleUpDownCounter(
Expand All @@ -172,7 +174,7 @@ class NoopMeter final : public Meter
new NoopUpDownCounter<double>(name, description, unit)};
}

nostd::shared_ptr<ObservableInstrument> CreateLongObservableUpDownCounter(
nostd::shared_ptr<ObservableInstrument> CreateInt64ObservableUpDownCounter(
nostd::string_view name,
nostd::string_view description = "",
nostd::string_view unit = "") noexcept override
Expand Down
2 changes: 1 addition & 1 deletion api/include/opentelemetry/metrics/observer_result.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class ObserverResultT
}
};

using ObserverResult = nostd::variant<nostd::shared_ptr<ObserverResultT<long>>,
using ObserverResult = nostd::variant<nostd::shared_ptr<ObserverResultT<int64_t>>,
nostd::shared_ptr<ObserverResultT<double>>>;

} // namespace metrics
Expand Down
42 changes: 21 additions & 21 deletions api/test/metrics/noop_sync_instrument_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,42 +9,42 @@

TEST(Counter, Add)
{
std::shared_ptr<opentelemetry::metrics::Counter<long>> counter{
new opentelemetry::metrics::NoopCounter<long>("test", "none", "unitless")};
std::shared_ptr<opentelemetry::metrics::Counter<uint64_t>> counter{
new opentelemetry::metrics::NoopCounter<uint64_t>("test", "none", "unitless")};

std::map<std::string, std::string> labels = {{"k1", "v1"}};
counter->Add(10l, labels);
counter->Add(10l, labels, opentelemetry::context::Context{});
counter->Add(2l);
counter->Add(2l, opentelemetry::context::Context{});
counter->Add(10l, {{"k1", "1"}, {"k2", 2}});
counter->Add(10l, {{"k1", "1"}, {"k2", 2}}, opentelemetry::context::Context{});
counter->Add(10, labels);
counter->Add(10, labels, opentelemetry::context::Context{});
counter->Add(2);
counter->Add(2, opentelemetry::context::Context{});
counter->Add(10, {{"k1", "1"}, {"k2", 2}});
counter->Add(10, {{"k1", "1"}, {"k2", 2}}, opentelemetry::context::Context{});
}

TEST(histogram, Record)
{
std::shared_ptr<opentelemetry::metrics::Histogram<long>> counter{
new opentelemetry::metrics::NoopHistogram<long>("test", "none", "unitless")};
std::shared_ptr<opentelemetry::metrics::Histogram<uint64_t>> histogram{
new opentelemetry::metrics::NoopHistogram<uint64_t>("test", "none", "unitless")};

std::map<std::string, std::string> labels = {{"k1", "v1"}};
counter->Record(10l, labels, opentelemetry::context::Context{});
counter->Record(2l, opentelemetry::context::Context{});
histogram->Record(10, labels, opentelemetry::context::Context{});
histogram->Record(2, opentelemetry::context::Context{});

counter->Record(10l, {{"k1", "1"}, {"k2", 2}}, opentelemetry::context::Context{});
histogram->Record(10, {{"k1", "1"}, {"k2", 2}}, opentelemetry::context::Context{});
}

TEST(UpDownCountr, Record)
{
std::shared_ptr<opentelemetry::metrics::UpDownCounter<long>> counter{
new opentelemetry::metrics::NoopUpDownCounter<long>("test", "none", "unitless")};
std::shared_ptr<opentelemetry::metrics::UpDownCounter<int64_t>> counter{
new opentelemetry::metrics::NoopUpDownCounter<int64_t>("test", "none", "unitless")};

std::map<std::string, std::string> labels = {{"k1", "v1"}};
counter->Add(10l, labels);
counter->Add(10l, labels, opentelemetry::context::Context{});
counter->Add(2l);
counter->Add(2l, opentelemetry::context::Context{});
counter->Add(10l, {{"k1", "1"}, {"k2", 2}});
counter->Add(10l, {{"k1", "1"}, {"k2", 2}}, opentelemetry::context::Context{});
counter->Add(10, labels);
counter->Add(10, labels, opentelemetry::context::Context{});
counter->Add(2);
counter->Add(2, opentelemetry::context::Context{});
counter->Add(10, {{"k1", "1"}, {"k2", 2}});
counter->Add(10, {{"k1", "1"}, {"k2", 2}}, opentelemetry::context::Context{});
}

#endif
20 changes: 10 additions & 10 deletions exporters/ostream/src/metric_exporter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po
{
sout_ << nostd::get<double>(sum_point_data.value_);
}
else if (nostd::holds_alternative<long>(sum_point_data.value_))
else if (nostd::holds_alternative<int64_t>(sum_point_data.value_))
{
sout_ << nostd::get<long>(sum_point_data.value_);
sout_ << nostd::get<int64_t>(sum_point_data.value_);
}
}
else if (nostd::holds_alternative<sdk::metrics::HistogramPointData>(point_data))
Expand All @@ -179,24 +179,24 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po
{
sout_ << nostd::get<double>(histogram_point_data.sum_);
}
else if (nostd::holds_alternative<long>(histogram_point_data.sum_))
else if (nostd::holds_alternative<int64_t>(histogram_point_data.sum_))
{
sout_ << nostd::get<long>(histogram_point_data.sum_);
sout_ << nostd::get<int64_t>(histogram_point_data.sum_);
}

if (histogram_point_data.record_min_max_)
{
if (nostd::holds_alternative<long>(histogram_point_data.min_))
if (nostd::holds_alternative<int64_t>(histogram_point_data.min_))
{
sout_ << "\n min : " << nostd::get<long>(histogram_point_data.min_);
sout_ << "\n min : " << nostd::get<int64_t>(histogram_point_data.min_);
}
else if (nostd::holds_alternative<double>(histogram_point_data.min_))
{
sout_ << "\n min : " << nostd::get<double>(histogram_point_data.min_);
}
if (nostd::holds_alternative<long>(histogram_point_data.max_))
if (nostd::holds_alternative<int64_t>(histogram_point_data.max_))
{
sout_ << "\n max : " << nostd::get<long>(histogram_point_data.max_);
sout_ << "\n max : " << nostd::get<int64_t>(histogram_point_data.max_);
}
if (nostd::holds_alternative<double>(histogram_point_data.max_))
{
Expand All @@ -222,9 +222,9 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po
{
sout_ << nostd::get<double>(last_point_data.value_);
}
else if (nostd::holds_alternative<long>(last_point_data.value_))
else if (nostd::holds_alternative<int64_t>(last_point_data.value_))
{
sout_ << nostd::get<long>(last_point_data.value_);
sout_ << nostd::get<int64_t>(last_point_data.value_);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions exporters/ostream/test/ostream_metric_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ TEST(OStreamMetricsExporter, ExportHistogramPointData)
histogram_point_data2.boundaries_ = std::list<double>{10.0, 20.0, 30.0};
histogram_point_data2.count_ = 3;
histogram_point_data2.counts_ = {200, 300, 400, 500};
histogram_point_data2.sum_ = 900l;
histogram_point_data2.sum_ = (int64_t)900;
metric_sdk::ResourceMetrics data;
auto resource = opentelemetry::sdk::resource::Resource::Create(
opentelemetry::sdk::resource::ResourceAttributes{});
Expand Down Expand Up @@ -190,7 +190,7 @@ TEST(OStreamMetricsExporter, ExportLastValuePointData)
last_value_point_data.is_lastvalue_valid_ = true;
last_value_point_data.sample_ts_ = opentelemetry::common::SystemTimestamp{};
metric_sdk::LastValuePointData last_value_point_data2{};
last_value_point_data2.value_ = 20l;
last_value_point_data2.value_ = (int64_t)20;
last_value_point_data2.is_lastvalue_valid_ = true;
last_value_point_data2.sample_ts_ = opentelemetry::common::SystemTimestamp{};
metric_sdk::MetricData metric_data{
Expand Down
20 changes: 10 additions & 10 deletions exporters/otlp/src/otlp_metric_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ void OtlpMetricUtils::ConvertSumMetric(const metric_sdk::MetricData &metric_data
proto_sum_point_data->set_time_unix_nano(ts);
auto sum_data = nostd::get<sdk::metrics::SumPointData>(point_data_with_attributes.point_data);

if ((nostd::holds_alternative<long>(sum_data.value_)))
if ((nostd::holds_alternative<int64_t>(sum_data.value_)))
{
proto_sum_point_data->set_as_int(nostd::get<long>(sum_data.value_));
proto_sum_point_data->set_as_int(nostd::get<int64_t>(sum_data.value_));
}
else
{
Expand Down Expand Up @@ -96,9 +96,9 @@ void OtlpMetricUtils::ConvertHistogramMetric(
auto histogram_data =
nostd::get<sdk::metrics::HistogramPointData>(point_data_with_attributes.point_data);
// sum
if ((nostd::holds_alternative<long>(histogram_data.sum_)))
if ((nostd::holds_alternative<int64_t>(histogram_data.sum_)))
{
proto_histogram_point_data->set_sum(nostd::get<long>(histogram_data.sum_));
proto_histogram_point_data->set_sum(nostd::get<int64_t>(histogram_data.sum_));
}
else
{
Expand All @@ -108,17 +108,17 @@ void OtlpMetricUtils::ConvertHistogramMetric(
proto_histogram_point_data->set_count(histogram_data.count_);
if (histogram_data.record_min_max_)
{
if (nostd::holds_alternative<long>(histogram_data.min_))
if (nostd::holds_alternative<int64_t>(histogram_data.min_))
{
proto_histogram_point_data->set_min(nostd::get<long>(histogram_data.min_));
proto_histogram_point_data->set_min(nostd::get<int64_t>(histogram_data.min_));
}
else
{
proto_histogram_point_data->set_min(nostd::get<double>(histogram_data.min_));
}
if (nostd::holds_alternative<long>(histogram_data.max_))
if (nostd::holds_alternative<int64_t>(histogram_data.max_))
{
proto_histogram_point_data->set_min(nostd::get<long>(histogram_data.max_));
proto_histogram_point_data->set_min(nostd::get<int64_t>(histogram_data.max_));
}
else
{
Expand Down Expand Up @@ -158,9 +158,9 @@ void OtlpMetricUtils::ConvertGaugeMetric(const opentelemetry::sdk::metrics::Metr
auto gauge_data =
nostd::get<sdk::metrics::LastValuePointData>(point_data_with_attributes.point_data);

if ((nostd::holds_alternative<long>(gauge_data.value_)))
if ((nostd::holds_alternative<int64_t>(gauge_data.value_)))
{
proto_gauge_point_data->set_as_int(nostd::get<long>(gauge_data.value_));
proto_gauge_point_data->set_as_int(nostd::get<int64_t>(gauge_data.value_));
}
else
{
Expand Down
8 changes: 4 additions & 4 deletions exporters/otlp/test/otlp_http_metric_exporter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ class OtlpHttpMetricExporterTestPeer : public ::testing::Test
last_value_point_data.is_lastvalue_valid_ = true;
last_value_point_data.sample_ts_ = opentelemetry::common::SystemTimestamp{};
opentelemetry::sdk::metrics::LastValuePointData last_value_point_data2{};
last_value_point_data2.value_ = 20l;
last_value_point_data2.value_ = (int64_t)20;
last_value_point_data2.is_lastvalue_valid_ = true;
last_value_point_data2.sample_ts_ = opentelemetry::common::SystemTimestamp{};
opentelemetry::sdk::metrics::MetricData metric_data{
Expand Down Expand Up @@ -392,7 +392,7 @@ class OtlpHttpMetricExporterTestPeer : public ::testing::Test
last_value_point_data.is_lastvalue_valid_ = true;
last_value_point_data.sample_ts_ = opentelemetry::common::SystemTimestamp{};
opentelemetry::sdk::metrics::LastValuePointData last_value_point_data2{};
last_value_point_data2.value_ = 20l;
last_value_point_data2.value_ = (int64_t)20;
last_value_point_data2.is_lastvalue_valid_ = true;
last_value_point_data2.sample_ts_ = opentelemetry::common::SystemTimestamp{};
opentelemetry::sdk::metrics::MetricData metric_data{
Expand Down Expand Up @@ -492,7 +492,7 @@ class OtlpHttpMetricExporterTestPeer : public ::testing::Test
histogram_point_data2.boundaries_ = {10.0, 20.0, 30.0};
histogram_point_data2.count_ = 3;
histogram_point_data2.counts_ = {200, 300, 400, 500};
histogram_point_data2.sum_ = 900l;
histogram_point_data2.sum_ = (int64_t)900;

opentelemetry::sdk::metrics::MetricData metric_data{
opentelemetry::sdk::metrics::InstrumentDescriptor{
Expand Down Expand Up @@ -627,7 +627,7 @@ class OtlpHttpMetricExporterTestPeer : public ::testing::Test
histogram_point_data2.boundaries_ = {10.0, 20.0, 30.0};
histogram_point_data2.count_ = 3;
histogram_point_data2.counts_ = {200, 300, 400, 500};
histogram_point_data2.sum_ = 900l;
histogram_point_data2.sum_ = (int64_t)900;

opentelemetry::sdk::metrics::MetricData metric_data{
opentelemetry::sdk::metrics::InstrumentDescriptor{
Expand Down
Loading

1 comment on commit 7fd76ee

@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: 7fd76ee Previous: 2ed104c Ratio
BM_LockFreeBuffer/1 2951510.190963745 ns/iter 407284.6960757609 ns/iter 7.25
BM_LockFreeBuffer/2 3238791.7964302716 ns/iter 1282601.8333435059 ns/iter 2.53

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

Please sign in to comment.