[Metrics SDK] Only record non-negative / finite / Non-NAN histogram v…

…alues (#1427)

* validate histogram value

* handle Nan

* add changelog

* divide by 0 error on windows

* fix markdown lint

CHANGELOG.md

@@ -15,6 +15,8 @@ Increment the:
 
 ## [Unreleased]
 
+* [METRICS] Only record non-negative / finite / Non-NAN histogram values([#1427](https:/open-telemetry/opentelemetry-cpp/pull/1427))
+
 ## [1.4.0] 2022-05-17
 
 * [API SDK] Upgrade proto to v0.17.0, update log data model ([#1383](https:/open-telemetry/opentelemetry-cpp/pull/1383))

sdk/src/metrics/sync_instruments.cc

@@ -4,6 +4,9 @@
 #ifndef ENABLE_METRICS_PREVIEW
 # include "opentelemetry/sdk/metrics/sync_instruments.h"
 # include "opentelemetry/sdk/metrics/state/metric_storage.h"
+# include "opentelemetry/sdk_config.h"
+
+# include <cmath>
 
 OPENTELEMETRY_BEGIN_NAMESPACE
 namespace sdk
@@ -139,11 +142,25 @@ void LongHistogram::Record(long value,
  const opentelemetry::common::KeyValueIterable &attributes,
  const opentelemetry::context::Context &context) noexcept
 {
+ 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 LongHistogram::Record(long value, const opentelemetry::context::Context &context) noexcept
 {
+ 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);
 }
 
@@ -156,11 +173,26 @@ void DoubleHistogram::Record(double value,
  const opentelemetry::common::KeyValueIterable &attributes,
  const opentelemetry::context::Context &context) noexcept
 {
+ if (value < 0 || std::isnan(value) || std::isinf(value))
+ {
+ OTEL_INTERNAL_LOG_WARN(
+ "[DoubleHistogram::Record(value, attributes)] negative/nan/infinite value provided to "
+ "histogram Name:"
+ << instrument_descriptor_.name_);
+ return;
+ }
  return storage_->RecordDouble(value, attributes, context);
 }
 
 void DoubleHistogram::Record(double value, const opentelemetry::context::Context &context) noexcept
 {
+ if (value < 0 || std::isnan(value) || std::isinf(value))
+ {
+ OTEL_INTERNAL_LOG_WARN(
+ "[DoubleHistogram::Record(value)] negative/nan/infinite value provided to histogram Name:"
+ << instrument_descriptor_.name_);
+ return;
+ }
  return storage_->RecordDouble(value, context);
 }
 

sdk/test/metrics/sync_instruments_test.cc

@@ -9,6 +9,8 @@
 # include "opentelemetry/sdk/metrics/state/multi_metric_storage.h"
 
 # include <gtest/gtest.h>
+# include <cmath>
+# include <limits>
 
 using namespace opentelemetry;
 using namespace opentelemetry::sdk::instrumentationlibrary;
@@ -103,7 +105,7 @@ TEST(SyncInstruments, LongHistogram)
  std::unique_ptr<WritableMetricStorage> metric_storage(new MultiMetricStorage());
  LongHistogram counter(instrument_descriptor, std::move(metric_storage));
  EXPECT_NO_THROW(counter.Record(10l, opentelemetry::context::Context{}));
- EXPECT_NO_THROW(counter.Record(10l, opentelemetry::context::Context{}));
+ EXPECT_NO_THROW(counter.Record(-10l, opentelemetry::context::Context{})); // This is ignored
 
  EXPECT_NO_THROW(counter.Record(
  10l, opentelemetry::common::KeyValueIterableView<M>({{"abc", "123"}, {"xyz", "456"}}),
@@ -120,7 +122,11 @@ TEST(SyncInstruments, DoubleHistogram)
  std::unique_ptr<WritableMetricStorage> metric_storage(new MultiMetricStorage());
  DoubleHistogram counter(instrument_descriptor, std::move(metric_storage));
  EXPECT_NO_THROW(counter.Record(10.10, opentelemetry::context::Context{}));
- EXPECT_NO_THROW(counter.Record(10.10, opentelemetry::context::Context{}));
+ EXPECT_NO_THROW(counter.Record(-10.10, opentelemetry::context::Context{})); // This is ignored.
+ EXPECT_NO_THROW(counter.Record(std::numeric_limits<double>::quiet_NaN(),
+ opentelemetry::context::Context{})); // This is ignored too
+ EXPECT_NO_THROW(counter.Record(std::numeric_limits<double>::infinity(),
+ opentelemetry::context::Context{})); // This is ignored too
 
  EXPECT_NO_THROW(counter.Record(
  10.10, opentelemetry::common::KeyValueIterableView<M>({{"abc", "123"}, {"xyz", "456"}}),