[BOOST LOG] Use correct type for severity logger (open-telemetry#460)

justin-mccann · Jul 4, 2024 · d953d32 · d953d32
1 parent 2badebf
commit d953d32
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 24 deletions.
diff --git a/instrumentation/boost_log/README.md b/instrumentation/boost_log/README.md
@@ -33,14 +33,14 @@ boost::log::core::get()->add_sink(sink);
 
 This will create a backend with the following assumptions about the attributes it collects, more precisely:
 
-| Keyword      | Type                        |
-|--------------|-----------------------------|
-| Severity     | int                         |
-| TimeStamp    | boost::posix_time::ptime    |
-| ThreadID     | boost::log::aux::thread::id |
-| FileName     | std::string                 |
-| FunctionName | std::string                 |
-| LineNumber   | int                         |
+| Keyword      | Type                                |
+|--------------|-------------------------------------|
+| Severity     | boost::log::trivial::severity_level |
+| TimeStamp    | boost::posix_time::ptime            |
+| ThreadID     | boost::log::aux::thread::id         |
+| FileName     | std::string                         |
+| FunctionName | std::string                         |
+| LineNumber   | int                                 |
 
 If, however, one or more of these attributes have a different name or type, it is possible to communicate this to the backend via the `ValueMappers` struct. It contains a function for each attribute described above, with their signatures as follows:
 

diff --git a/instrumentation/boost_log/example/main.cc b/instrumentation/boost_log/example/main.cc
@@ -5,6 +5,7 @@
 
 #include <boost/log/sinks/sync_frontend.hpp>
 #include <boost/log/sources/logger.hpp>
+#include <boost/log/trivial.hpp>
 #include <boost/log/utility/manipulators/add_value.hpp>
 #include <boost/log/utility/setup/common_attributes.hpp>
 #include <boost/smart_ptr/make_shared_object.hpp>
@@ -76,7 +77,7 @@ int main(int /* argc */, char ** /* argv */)
   boost::log::sources::logger simple;
   BOOST_LOG(simple) << "Test simplest message";
 
-  boost::log::sources::severity_logger<int> logger;
+  boost::log::sources::severity_logger<boost::log::trivial::severity_level> logger;
   BOOST_LOG_SEV(logger, boost::log::trivial::info) << "Test message with severity";
 
   BOOST_LOG_SEV(logger, boost::log::trivial::debug)

diff --git a/instrumentation/boost_log/include/opentelemetry/instrumentation/boost_log/sink.h b/instrumentation/boost_log/include/opentelemetry/instrumentation/boost_log/sink.h
@@ -57,7 +57,8 @@ class OpenTelemetrySinkBackend
     return kLibraryVersion;
   }
 
-  static inline opentelemetry::logs::Severity levelToSeverity(int level) noexcept
+  static inline opentelemetry::logs::Severity levelToSeverity(
+      boost::log::trivial::severity_level level) noexcept
   {
     using namespace boost::log::trivial;
     using opentelemetry::logs::Severity;

diff --git a/instrumentation/boost_log/src/sink.cc b/instrumentation/boost_log/src/sink.cc
@@ -68,7 +68,9 @@ OpenTelemetrySinkBackend::OpenTelemetrySinkBackend(const ValueMappers &mappers)
 {
   mappers_.ToSeverity =
       mappers.ToSeverity ? mappers.ToSeverity : [](const boost::log::record_view &record) {
-        return levelToSeverity(boost::log::extract_or_default<int>(record["Severity"], -1));
+        using boost::log::trivial::severity_level;
+        return levelToSeverity(boost::log::extract_or_default<severity_level>(
+            record["Severity"], static_cast<severity_level>(-1)));
       };
   mappers_.ToTimeStamp = mappers.ToTimeStamp ? mappers.ToTimeStamp : ToTimestampDefault;
   mappers_.ToThreadId  = mappers.ToThreadId ? mappers.ToThreadId : ToThreadIdDefault;

diff --git a/instrumentation/boost_log/test/sink_test.cc b/instrumentation/boost_log/test/sink_test.cc
@@ -91,23 +91,25 @@ class OpenTelemetrySinkTest : public testing::Test
 
 TEST_F(OpenTelemetrySinkTest, LevelToSeverity)
 {
-  namespace Level = boost::log::trivial;
+  using Level = boost::log::trivial::severity_level;
   using logs_api::Severity;
-  using ots = instr::boost_log::OpenTelemetrySinkBackend;
+  using ots          = instr::boost_log::OpenTelemetrySinkBackend;
+  const auto lowest  = std::numeric_limits<int>::lowest();
+  const auto highest = std::numeric_limits<int>::max();
 
   ASSERT_TRUE(Severity::kFatal == ots::levelToSeverity(Level::fatal));
   ASSERT_TRUE(Severity::kError == ots::levelToSeverity(Level::error));
   ASSERT_TRUE(Severity::kWarn == ots::levelToSeverity(Level::warning));
   ASSERT_TRUE(Severity::kInfo == ots::levelToSeverity(Level::info));
   ASSERT_TRUE(Severity::kDebug == ots::levelToSeverity(Level::debug));
   ASSERT_TRUE(Severity::kTrace == ots::levelToSeverity(Level::trace));
-  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(std::numeric_limits<int>::lowest()));
-  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(std::numeric_limits<int>::lowest() + 1));
-  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(-42));
-  ASSERT_TRUE(Severity::kTrace == ots::levelToSeverity(0));
-  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(42));
-  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(std::numeric_limits<int>::max() - 1));
-  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(std::numeric_limits<int>::max()));
+  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(static_cast<Level>(lowest)));
+  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(static_cast<Level>(lowest + 1)));
+  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(static_cast<Level>(-42)));
+  ASSERT_TRUE(Severity::kTrace == ots::levelToSeverity(static_cast<Level>(0)));
+  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(static_cast<Level>(42)));
+  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(static_cast<Level>(highest - 1)));
+  ASSERT_TRUE(Severity::kInvalid == ots::levelToSeverity(static_cast<Level>(highest)));
 }
 
 TEST_F(OpenTelemetrySinkTest, Log_Success)
@@ -119,7 +121,7 @@ TEST_F(OpenTelemetrySinkTest, Log_Success)
   auto logrecord_mock = new LogRecordMock();
   auto logrecord_ptr  = nostd::unique_ptr<logs_api::LogRecord>(logrecord_mock);
 
-  boost::log::sources::severity_logger<int> logger;
+  boost::log::sources::severity_logger<boost::log::trivial::severity_level> logger;
   auto pre_log = std::chrono::system_clock::now();
 
   EXPECT_CALL(*provider_mock, GetLogger(_, _, _, _, _))
@@ -182,7 +184,7 @@ TEST_F(OpenTelemetrySinkTest, Log_Failure)
   logs_api::Provider::SetLoggerProvider(nostd::shared_ptr<logs_api::LoggerProvider>(provider_mock));
   auto logger_mock = new LoggerMock();
   auto logger_ptr  = nostd::shared_ptr<logs_api::Logger>(logger_mock);
-  boost::log::sources::severity_logger<int> logger;
+  boost::log::sources::severity_logger<boost::log::trivial::severity_level> logger;
 
   EXPECT_CALL(*provider_mock, GetLogger(_, _, _, _, _))
       .WillOnce(DoAll(Invoke([](nostd::string_view logger_name, nostd::string_view library_name,
@@ -263,7 +265,7 @@ TEST_F(OpenTelemetrySinkTest, Multi_Threaded)
   std::vector<std::thread> threads;
   threads.reserve(count);
 
-  boost::log::sources::severity_logger<int> logger;
+  boost::log::sources::severity_logger<boost::log::trivial::severity_level> logger;
   const auto pre_log = std::chrono::system_clock::now().time_since_epoch().count();
 
   for (size_t index = 0; index < count; ++index)
@@ -360,7 +362,7 @@ TEST(OpenTelemetrySinkTestSuite, CustomMappers)
   auto logrecord_ptr  = nostd::unique_ptr<logs_api::LogRecord>(logrecord_mock);
 
   SetUpBackendWithDummyMappers();
-  boost::log::sources::severity_logger<int> logger;
+  boost::log::sources::severity_logger<boost::log::trivial::severity_level> logger;
 
   EXPECT_CALL(*provider_mock, GetLogger(_, _, _, _, _))
       .WillOnce(DoAll(Invoke([](nostd::string_view logger_name, nostd::string_view library_name,