diff --git a/.gitattributes b/.gitattributes index 03f42e5e1f..ca2467b474 100644 --- a/.gitattributes +++ b/.gitattributes @@ -41,3 +41,7 @@ LICENSE* text ## git files .gitignore text eol=lf .gitattributes text eol=lf + +## bazel files +WORKSPACE text eol=lf +BUILD text eol=lf diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a260fe8be..aa491b1afb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ Increment the: ## [Unreleased] +* [METRICS] Only record non-negative / finite / Non-NAN histogram values([#1427](https://github.com/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://github.com/open-telemetry/opentelemetry-cpp/pull/1383)) diff --git a/api/include/opentelemetry/trace/span_context.h b/api/include/opentelemetry/trace/span_context.h index ca13a8df60..e19bb99001 100644 --- a/api/include/opentelemetry/trace/span_context.h +++ b/api/include/opentelemetry/trace/span_context.h @@ -81,7 +81,7 @@ class SpanContext final bool IsRemote() const noexcept { return is_remote_; } - static SpanContext GetInvalid() { return SpanContext(false, false); } + static SpanContext GetInvalid() noexcept { return SpanContext(false, false); } bool IsSampled() const noexcept { return trace_flags_.IsSampled(); } diff --git a/exporters/etw/include/opentelemetry/exporters/etw/etw_logger_exporter.h b/exporters/etw/include/opentelemetry/exporters/etw/etw_logger_exporter.h index 936fe15ca5..9b96cb27a5 100644 --- a/exporters/etw/include/opentelemetry/exporters/etw/etw_logger_exporter.h +++ b/exporters/etw/include/opentelemetry/exporters/etw/etw_logger_exporter.h @@ -15,7 +15,7 @@ # include "opentelemetry/common/key_value_iterable_view.h" -# include "opentelemetry/logs/tracer_provider.h" +# include "opentelemetry/logs/logger_provider.h" # include "opentelemetry/trace/span_id.h" # include "opentelemetry/trace/trace_id.h" diff --git a/exporters/etw/include/opentelemetry/exporters/etw/etw_properties.h b/exporters/etw/include/opentelemetry/exporters/etw/etw_properties.h index 04ad86130f..3cf365c8a0 100644 --- a/exporters/etw/include/opentelemetry/exporters/etw/etw_properties.h +++ b/exporters/etw/include/opentelemetry/exporters/etw/etw_properties.h @@ -192,7 +192,7 @@ class PropertyValue : public PropertyVariant {} /** - * @brief Convert owning PropertyValue to non-owning common::AttributeValue + * @brief Convert non-owning common::AttributeValue to owning PropertyValue. * @return */ PropertyValue &FromAttributeValue(const common::AttributeValue &v) @@ -222,7 +222,8 @@ class PropertyValue : public PropertyVariant break; } case common::AttributeType::kTypeString: { - PropertyVariant::operator=(nostd::string_view(nostd::get(v)).data()); + PropertyVariant::operator= + (std::string{nostd::string_view(nostd::get(v)).data()}); break; } diff --git a/exporters/etw/test/etw_logger_test.cc b/exporters/etw/test/etw_logger_test.cc index 99d5b3a9b8..4e2210118f 100644 --- a/exporters/etw/test/etw_logger_test.cc +++ b/exporters/etw/test/etw_logger_test.cc @@ -8,7 +8,7 @@ # include # include -# include "opentelemetry/exporters/etw/etw_logger.h" +# include "opentelemetry/exporters/etw/etw_logger_exporter.h" # include "opentelemetry/sdk/trace/simple_processor.h" using namespace OPENTELEMETRY_NAMESPACE; diff --git a/exporters/jaeger/BUILD b/exporters/jaeger/BUILD index d373aa35b5..cc1babae06 100644 --- a/exporters/jaeger/BUILD +++ b/exporters/jaeger/BUILD @@ -180,6 +180,7 @@ cc_library( tags = ["jaeger"], deps = [ ":jaeger_exporter", + "//sdk/src/common:global_log_handler", ], ) diff --git a/sdk/include/opentelemetry/sdk/common/global_log_handler.h b/sdk/include/opentelemetry/sdk/common/global_log_handler.h index 612d21eca5..99cf48b820 100644 --- a/sdk/include/opentelemetry/sdk/common/global_log_handler.h +++ b/sdk/include/opentelemetry/sdk/common/global_log_handler.h @@ -55,7 +55,7 @@ inline std::string LevelToString(LogLevel level) class LogHandler { public: - virtual ~LogHandler() = default; + virtual ~LogHandler(); virtual void Handle(LogLevel level, const char *file, @@ -71,22 +71,7 @@ class DefaultLogHandler : public LogHandler const char *file, int line, const char *msg, - const sdk::common::AttributeMap &attributes) noexcept override - { - std::stringstream output_s; - output_s << "[" << LevelToString(level) << "] "; - if (file != nullptr) - { - output_s << "File: " << file << ":" << line; - } - if (msg != nullptr) - { - output_s << msg; - } - output_s << std::endl; - // TBD - print attributes - std::cout << output_s.str(); // thread safe. - } + const sdk::common::AttributeMap &attributes) noexcept override; }; class NoopLogHandler : public LogHandler @@ -96,10 +81,7 @@ class NoopLogHandler : public LogHandler const char *file, int line, const char *msg, - const sdk::common::AttributeMap &error_attributes) noexcept override - { - // ignore the log message - } + const sdk::common::AttributeMap &error_attributes) noexcept override; }; /** @@ -113,7 +95,7 @@ class GlobalLogHandler * * By default, a default LogHandler is returned. */ - static const nostd::shared_ptr &GetLogHandler() noexcept + static inline const nostd::shared_ptr &GetLogHandler() noexcept { return GetHandlerAndLevel().first; } @@ -123,7 +105,7 @@ class GlobalLogHandler * This should be called once at the start of application before creating any Provider * instance. */ - static void SetLogHandler(nostd::shared_ptr eh) noexcept + static inline void SetLogHandler(nostd::shared_ptr eh) noexcept { GetHandlerAndLevel().first = eh; } @@ -133,22 +115,17 @@ class GlobalLogHandler * * By default, a default log level is returned. */ - static LogLevel GetLogLevel() noexcept { return GetHandlerAndLevel().second; } + static inline LogLevel GetLogLevel() noexcept { return GetHandlerAndLevel().second; } /** * Changes the singleton Log level. * This should be called once at the start of application before creating any Provider * instance. */ - static void SetLogLevel(LogLevel level) noexcept { GetHandlerAndLevel().second = level; } + static inline void SetLogLevel(LogLevel level) noexcept { GetHandlerAndLevel().second = level; } private: - static std::pair, LogLevel> &GetHandlerAndLevel() noexcept - { - static std::pair, LogLevel> handler_and_level{ - nostd::shared_ptr(new DefaultLogHandler), LogLevel::Warning}; - return handler_and_level; - } + static std::pair, LogLevel> &GetHandlerAndLevel() noexcept; }; } // namespace internal_log diff --git a/sdk/src/common/BUILD b/sdk/src/common/BUILD index b0724c3810..b8369fc6f4 100644 --- a/sdk/src/common/BUILD +++ b/sdk/src/common/BUILD @@ -27,6 +27,18 @@ cc_library( include_prefix = "src/common", deps = [ "//api", + "//sdk:headers", "//sdk/src/common/platform:fork", ], ) + +cc_library( + name = "global_log_handler", + srcs = [ + "global_log_handler.cc", + ], + deps = [ + "//api", + "//sdk:headers", + ], +) diff --git a/sdk/src/common/CMakeLists.txt b/sdk/src/common/CMakeLists.txt index debffef495..d8674353b6 100644 --- a/sdk/src/common/CMakeLists.txt +++ b/sdk/src/common/CMakeLists.txt @@ -1,4 +1,4 @@ -set(COMMON_SRCS random.cc core.cc) +set(COMMON_SRCS random.cc core.cc global_log_handler.cc) if(WIN32) list(APPEND COMMON_SRCS platform/fork_windows.cc) else() diff --git a/sdk/src/common/global_log_handler.cc b/sdk/src/common/global_log_handler.cc index e69de29bb2..c86b652c70 100644 --- a/sdk/src/common/global_log_handler.cc +++ b/sdk/src/common/global_log_handler.cc @@ -0,0 +1,57 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include "opentelemetry/sdk/common/global_log_handler.h" + +#include +#include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace common +{ +namespace internal_log +{ + +LogHandler::~LogHandler() {} + +void DefaultLogHandler::Handle(LogLevel level, + const char *file, + int line, + const char *msg, + const sdk::common::AttributeMap &attributes) noexcept +{ + std::stringstream output_s; + output_s << "[" << LevelToString(level) << "] "; + if (file != nullptr) + { + output_s << "File: " << file << ":" << line; + } + if (msg != nullptr) + { + output_s << msg; + } + output_s << std::endl; + // TBD - print attributes + std::cout << output_s.str(); // thread safe. +} + +void NoopLogHandler::Handle(LogLevel, + const char *, + int, + const char *, + const sdk::common::AttributeMap &) noexcept +{} + +std::pair, LogLevel> &GlobalLogHandler::GetHandlerAndLevel() noexcept +{ + static std::pair, LogLevel> handler_and_level{ + nostd::shared_ptr(new DefaultLogHandler), LogLevel::Warning}; + return handler_and_level; +} + +} // namespace internal_log +} // namespace common +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/logs/BUILD b/sdk/src/logs/BUILD index ac51799125..53465cb8be 100644 --- a/sdk/src/logs/BUILD +++ b/sdk/src/logs/BUILD @@ -22,6 +22,7 @@ cc_library( deps = [ "//api", "//sdk:headers", + "//sdk/src/common:global_log_handler", "//sdk/src/resource", ], ) diff --git a/sdk/src/metrics/BUILD b/sdk/src/metrics/BUILD index d21daa765c..e75ba6a1ac 100644 --- a/sdk/src/metrics/BUILD +++ b/sdk/src/metrics/BUILD @@ -22,6 +22,7 @@ cc_library( deps = [ "//api", "//sdk:headers", + "//sdk/src/common:global_log_handler", "//sdk/src/resource", ], ) diff --git a/sdk/src/metrics/sync_instruments.cc b/sdk/src/metrics/sync_instruments.cc index 5c94ae6f24..7cf9034cdc 100644 --- a/sdk/src/metrics/sync_instruments.cc +++ b/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 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); } diff --git a/sdk/src/trace/BUILD b/sdk/src/trace/BUILD index 34ec5108d2..362680b744 100644 --- a/sdk/src/trace/BUILD +++ b/sdk/src/trace/BUILD @@ -22,6 +22,7 @@ cc_library( deps = [ "//api", "//sdk:headers", + "//sdk/src/common:global_log_handler", "//sdk/src/common:random", "//sdk/src/resource", ], diff --git a/sdk/test/common/BUILD b/sdk/test/common/BUILD index 8a98e5617d..91d56996fb 100644 --- a/sdk/test/common/BUILD +++ b/sdk/test/common/BUILD @@ -124,6 +124,7 @@ cc_test( deps = [ "//api", "//sdk:headers", + "//sdk/src/common:global_log_handler", "@com_google_googletest//:gtest_main", ], ) diff --git a/sdk/test/metrics/sync_instruments_test.cc b/sdk/test/metrics/sync_instruments_test.cc index e029821d41..1a590d8d26 100644 --- a/sdk/test/metrics/sync_instruments_test.cc +++ b/sdk/test/metrics/sync_instruments_test.cc @@ -9,6 +9,8 @@ # include "opentelemetry/sdk/metrics/state/multi_metric_storage.h" # include +# include +# include using namespace opentelemetry; using namespace opentelemetry::sdk::instrumentationlibrary; @@ -103,7 +105,7 @@ TEST(SyncInstruments, LongHistogram) std::unique_ptr 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({{"abc", "123"}, {"xyz", "456"}}), @@ -120,7 +122,11 @@ TEST(SyncInstruments, DoubleHistogram) std::unique_ptr 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::quiet_NaN(), + opentelemetry::context::Context{})); // This is ignored too + EXPECT_NO_THROW(counter.Record(std::numeric_limits::infinity(), + opentelemetry::context::Context{})); // This is ignored too EXPECT_NO_THROW(counter.Record( 10.10, opentelemetry::common::KeyValueIterableView({{"abc", "123"}, {"xyz", "456"}}),