Merge pull request #8680 from The-OpenROAD-Project-staging/secure-add-warning-metrics

Added new metrics for warnings.

Author: maliberty

src/utl/include/utl/Logger.h

@@ -279,6 +279,7 @@ class Logger
            const Args&... args)
   {
     assert(id >= 0 && id <= max_message_id);
+    message_levels_[tool][id].store(level, std::memory_order_relaxed);
     auto& counter = message_counters_[tool][id];
     auto count = counter++;
     if (count < max_message_print) {
@@ -319,6 +320,9 @@ class Logger
 
   void flushMetrics();
   void finalizeMetrics();
+  // Add new metrics for non-zero warnings. It also counts the number of
+  // unique warning types.
+  void addWarningMetrics();
 
   void setRedirectSink(std::ostream& sink_stream, bool keep_sinks = false);
   void restoreFromRedirect();
@@ -359,6 +363,9 @@ class Logger
   // from multiple threads without locks.
   using MessageCounter = std::array<std::atomic_int16_t, max_message_id + 1>;
   std::array<MessageCounter, ToolId::SIZE> message_counters_;
+  using MessageLevel
+      = std::array<std::atomic<spdlog::level::level_enum>, max_message_id + 1>;
+  std::array<MessageLevel, ToolId::SIZE> message_levels_;
   std::array<DebugGroups, ToolId::SIZE> debug_group_level_;
   bool debug_on_{false};
   std::atomic_int warning_count_{0};

src/utl/src/Logger.cpp

@@ -4,6 +4,7 @@
 #include "utl/Logger.h"
 
 #include <algorithm>
+#include <atomic>
 #include <cstdint>
 #include <cstring>
 #include <fstream>
@@ -21,6 +22,7 @@
 #else
 #include "spdlog/pattern_formatter.h"
 #endif
+#include "spdlog/common.h"
 #include "spdlog/sinks/basic_file_sink.h"
 #include "spdlog/sinks/ostream_sink.h"
 #include "spdlog/sinks/stdout_color_sinks.h"
@@ -58,6 +60,12 @@ Logger::Logger(const char* log_filename, const char* metrics_filename)
     }
   }
 
+  for (auto& levels : message_levels_) {
+    for (auto& level : levels) {
+      level.store(spdlog::level::off, std::memory_order_relaxed);
+    }
+  }
+
   prometheus_registry_ = std::make_shared<PrometheusRegistry>();
 }
 
@@ -178,11 +186,34 @@ void Logger::flushMetrics()
   }
 }
 
+void Logger::addWarningMetrics()
+{
+  // Add metrics for non-zero warnings
+  int warning_type_cnt = 0;
+  for (int i = 0; i < ToolId::SIZE; ++i) {
+    for (int j = 0; j <= max_message_id; ++j) {
+      if (message_counters_[i][j] > 0
+          && message_levels_[i][j] == spdlog::level::warn) {
+        warning_type_cnt++;
+        log_metric(
+            // NOLINTNEXTLINE(misc-include-cleaner)
+            fmt::format("flow__warnings__count:{}-{}", tool_names_[i], j),
+            std::to_string(message_counters_[i][j]));
+      }
+    }
+  }
+
+  // Add a metric to report the number of unique warning types
+  log_metric("flow__warnings__type_count", std::to_string(warning_type_cnt));
+}
+
 void Logger::finalizeMetrics()
 {
   log_metric("flow__warnings__count", std::to_string(warning_count_));
   log_metric("flow__errors__count", std::to_string(error_count_));
 
+  addWarningMetrics();
+
   for (MetricsPolicy policy : metrics_policies_) {
     policy.applyPolicy(metrics_entries_);
   }

src/utl/test/BUILD

@@ -86,3 +86,15 @@ cc_test(
         "@googletest//:gtest_main",
     ],
 )
+
+cc_test(
+    name = "TestMetrics",
+    srcs = [
+        "cpp/TestMetrics.cpp",
+    ],
+    deps = [
+        "//src/utl",
+        "@googletest//:gtest",
+        "@googletest//:gtest_main",
+    ],
+)

src/utl/test/cpp/CMakeLists.txt

@@ -18,3 +18,16 @@ gtest_discover_tests(TestCFileUtils
 add_dependencies(build_and_test
   TestCFileUtils
 )
+
+
+add_executable(TestMetrics TestMetrics.cpp)
+
+target_link_libraries(TestMetrics ${TEST_LIBS})
+
+gtest_discover_tests(TestMetrics
+  WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+)
+
+add_dependencies(build_and_test
+  TestMetrics
+)

src/utl/test/cpp/TestMetrics.cpp

@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: BSD-3-Clause
+// Copyright (c) 2024, The OpenROAD Authors
+
+#include <cstdio>
+#include <fstream>
+#include <iterator>
+#include <string>
+
+#include "gtest/gtest.h"
+#include "utl/Logger.h"
+
+namespace utl {
+
+TEST(Utl, WarningMetrics)
+{
+  // ARRANGE
+  std::string metrics_filename = "test_metrics_warning_metrics.rpt";
+  // The logger must be scoped to trigger the destructor which writes the
+  // metrics
+  {
+    utl::Logger logger;
+    logger.addMetricsSink(metrics_filename.c_str());
+
+    // ACTION
+    for (int i = 0; i < 10; i++) {
+      logger.warn(utl::UTL, 20, "Test warning UTL-20.");
+    }
+    logger.warn(utl::UTL, 21, "Another test warning UTL-21.");
+  }
+
+  // ASSERT
+  std::ifstream metrics_file(metrics_filename);
+  ASSERT_TRUE(metrics_file.good());
+  std::string content((std::istreambuf_iterator<char>(metrics_file)),
+                      (std::istreambuf_iterator<char>()));
+
+  // The output is a JSON string, which may be multi-line.
+  // For simplicity, we'll just check for substrings.
+  EXPECT_NE(content.find("\"flow__warnings__count\": 11"), std::string::npos);
+  EXPECT_NE(content.find("\"flow__errors__count\": 0"), std::string::npos);
+  EXPECT_NE(content.find("\"flow__warnings__count:UTL-20\": 10"),
+            std::string::npos);
+  EXPECT_NE(content.find("\"flow__warnings__count:UTL-21\": 1"),
+            std::string::npos);
+  EXPECT_NE(content.find("\"flow__warnings__type_count\": 2"),
+            std::string::npos);
+
+  // Clean up
+  std::remove(metrics_filename.c_str());
+}
+
+}  // namespace utl