Fix listFineGrainLoggers so that it lists loggers

source/common/common/fine_grain_logger.cc

@@ -82,6 +82,7 @@ spdlog::logger* FineGrainLogContext::initFineGrainLogger(absl::string_view file,
   }
 
   absl::WriterMutexLock l(fine_grain_log_lock_);
+  logger_keys_[key] = {std::string(file), std::string(name)};
   auto it = fine_grain_log_map_->find(key);
   spdlog::logger* target;
   if (it == fine_grain_log_map_->end()) {
@@ -302,7 +303,12 @@ level_enum FineGrainLogContext::getLogLevel(absl::string_view key) const {
 }
 
 std::pair<absl::string_view, absl::string_view>
-FineGrainLogContext::parseKey(absl::string_view key) {
+FineGrainLogContext::parseKey(absl::string_view key) const {
+  auto it = logger_keys_.find(key);
+  if (it != logger_keys_.end()) {
+    return {it->second.first, it->second.second};
+  }
+
   absl::string_view file = key;
   absl::string_view name;
   size_t colon = key.rfind(':');

source/common/common/fine_grain_logger.h

@@ -157,6 +157,7 @@ class FineGrainLogContext {
       ABSL_LOCKS_EXCLUDED(fine_grain_log_lock_) {
     absl::WriterMutexLock wl(fine_grain_log_lock_);
     fine_grain_log_map_->erase(key);
+    logger_keys_.erase(key);
   }
 
 private:
@@ -195,14 +196,22 @@ class FineGrainLogContext {
   /**
    * Parses a logger key into its file and name components.
    */
-  static std::pair<absl::string_view, absl::string_view> parseKey(absl::string_view key);
+  std::pair<absl::string_view, absl::string_view> parseKey(absl::string_view key) const
+      ABSL_SHARED_LOCKS_REQUIRED(fine_grain_log_lock_);
 
   /**
    * Map that stores <key, logger> pairs, key can be the file name.
    */
   FineGrainLogMapSharedPtr fine_grain_log_map_ ABSL_GUARDED_BY(fine_grain_log_lock_) =
       std::make_shared<FineGrainLogMap>();
 
+  /**
+   * Map that stores the metadata of registered loggers to reconstruct {file, name}
+   * components.
+   */
+  absl::flat_hash_map<std::string, std::pair<std::string, std::string>>
+      logger_keys_ ABSL_GUARDED_BY(fine_grain_log_lock_);
+
   /**
    * Vector that stores <update, level> pairs, key can be the file basename or glob expressions.
    * It will override the default verbosity log level.

test/common/common/log_verbosity_update_test.cc

@@ -357,6 +357,53 @@ TEST(FineGrainLog, listFineGrainLoggersExcludesGroups) {
   EXPECT_THAT(loggers, testing::Not(HasSubstr(group_key)));
 }
 
+TEST(FineGrainLog, listFineGrainLoggersWithColonsInPath) {
+  Logger::Context::enableFineGrainLogger();
+  std::atomic<spdlog::logger*> flogger{nullptr};
+  const std::string file_path = "path/with:colon/file.cc";
+
+  getFineGrainLogContext().initFineGrainLogger(file_path, "", flogger);
+
+  std::string loggers = getFineGrainLogContext().listFineGrainLoggers();
+  EXPECT_THAT(loggers, HasSubstr(file_path));
+
+  getFineGrainLogContext().removeFineGrainLogEntryForTest(file_path);
+}
+
+TEST(FineGrainLog, listFineGrainLoggersWithColonsAndGroups) {
+  Logger::Context::enableFineGrainLogger();
+  std::atomic<spdlog::logger*> flogger1{nullptr};
+  std::atomic<spdlog::logger*> flogger2{nullptr};
+  const std::string file_path = "path/with:colon/file.cc";
+  const std::string group_name = "my_group";
+  const std::string combined_key = "path/with:colon/file.cc:my_group";
+
+  getFineGrainLogContext().initFineGrainLogger(file_path, "", flogger1);
+  getFineGrainLogContext().initFineGrainLogger(file_path, group_name, flogger2);
+
+  std::string loggers = getFineGrainLogContext().listFineGrainLoggers();
+
+  // The base file logger should be listed
+  EXPECT_THAT(loggers, HasSubstr(file_path));
+  // The combined group logger key should NOT be listed
+  EXPECT_THAT(loggers, testing::Not(HasSubstr(combined_key)));
+
+  getFineGrainLogContext().removeFineGrainLogEntryForTest(file_path);
+  getFineGrainLogContext().removeFineGrainLogEntryForTest(combined_key);
+}
+
+TEST(FineGrainLog, parseKeyFallbackWithoutRegistry) {
+  Logger::Context::enableFineGrainLogger();
+  const std::string synthetic_key = "unregistered/file.cc:some_group";
+
+  // Setting verbosity using pattern matching triggers getLogLevel which relies on parseKey fallback
+  getFineGrainLogContext().updateVerbositySetting(
+      {{synthetic_key, static_cast<int>(spdlog::level::warn), false}});
+
+  // Verify updates didn't crash and were registered properly
+  getFineGrainLogContext().updateVerbositySetting({});
+}
+
 TEST(FineGrainLog, getFineGrainLogEntryForFlush) {
   Logger::Context::enableFineGrainLogger();