wgtmac commented on code in PR #725:
URL: https://github.com/apache/iceberg-cpp/pull/725#discussion_r3473489213


##########
src/iceberg/logging/logger.h:
##########
@@ -371,3 +371,179 @@ void Log(Logger& logger, LogLevel level,
 }
 
 }  // namespace iceberg
+
+// ---------------------------------------------------------------------------
+// Logging macros.
+//
+// Every macro takes a std::format string followed by its arguments. The
+// rendered line depends on the active backend (see cerr_logger.h for the
+// std::cerr layout, or the spdlog pattern); the examples below show the call
+// site and, for the default CerrLogger, the line it produces.
+//
+//   ICEBERG_LOG_TRACE("entering scan for {}", table);
+//     2026-06-16T10:59:41.186Z trace [12345] table_scan.cc:88] entering scan 
for db.t
+//   ICEBERG_LOG_DEBUG("cache miss key={}", key);
+//     2026-06-16T10:59:41.186Z debug [12345] cache.cc:42] cache miss 
key=manifest-7
+//   ICEBERG_LOG_INFO("loaded {} manifests in {} ms", n, ms);
+//     2026-06-16T10:59:41.186Z info [12345] table_scan.cc:91] loaded 5 
manifests in 12 ms
+//   ICEBERG_LOG_WARN("retry {} after {}", attempt, err);
+//     2026-06-16T10:59:41.186Z warn [12345] io.cc:51] retry 2 after timeout
+//   ICEBERG_LOG_ERROR("commit failed: {}", status);
+//     2026-06-16T10:59:41.186Z error [12345] txn.cc:77] commit failed: 
conflict
+//   ICEBERG_LOG_CRITICAL("metadata unreadable at {}", path);
+//     2026-06-16T10:59:41.186Z critical [12345] meta.cc:30] metadata 
unreadable at
+//     s3://b/m.json
+//   ICEBERG_LOG_FATAL("unrecoverable: {}", reason);   // emits, flushes, then
+//   std::abort()
+//     2026-06-16T10:59:41.186Z fatal [12345] boot.cc:19] unrecoverable: bad 
config
+//
+// Less common forms:
+//   ICEBERG_LOG(level, "level chosen at runtime: {}", x);     // runtime 
severity
+//   ICEBERG_LOG_TO(logger, level, "to an explicit logger {}", y);
+//   ICEBERG_LOG_RUNTIME_FMT(level, fmt_string, args...);       // non-literal 
format
+//
+// With ICEBERG_LOG_SHORT_MACROS defined, bare aliases (LOG_INFO, ...) are also
+// available. A format string is mandatory; zero extra args is fine
+// (ICEBERG_LOG_INFO("done")).
+// ---------------------------------------------------------------------------
+
+/// \brief Compile-time severity floor: statements below this level are removed
+/// entirely from the build (their format call sites and source_location 
literals
+/// are never emitted). Defaults to keeping everything. ICEBERG_LOG_FATAL is 
never
+/// gated by this floor -- its abort is always compiled in.
+#ifndef ICEBERG_LOG_ACTIVE_LEVEL
+#  define ICEBERG_LOG_ACTIVE_LEVEL ::iceberg::LogLevel::kTrace
+#endif
+
+// Internal: fixed-severity emit with compile-time floor then the authoritative
+// Logger::ShouldLog (the single source of truth for runtime filtering), with
+// formatting only on the taken path, never throwing.
+#define ICEBERG_INTERNAL_LOG(level_, FMT_, ...)                                
      \
+  do {                                                                         
      \
+    if constexpr ((level_) >= ICEBERG_LOG_ACTIVE_LEVEL) {                      
      \
+      const auto& _ib_logger = ::iceberg::internal::CurrentLogger();           
      \
+      if (_ib_logger && _ib_logger->ShouldLog(level_)) {                       
      \
+        try {                                                                  
      \
+          ::iceberg::internal::Emit(*_ib_logger, (level_),                     
      \
+                                    ::std::source_location::current(),         
      \
+                                    ::std::format(FMT_ __VA_OPT__(, ) 
__VA_ARGS__)); \
+        } catch (...) {                                                        
      \
+          ::iceberg::internal::EmitFormatError(*_ib_logger, (level_),          
      \
+                                               
::std::source_location::current());   \
+        }                                                                      
      \
+      }                                                                        
      \
+    }                                                                          
      \
+  } while (0)
+
+#define ICEBERG_LOG_TRACE(...) \
+  ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kTrace, __VA_ARGS__)
+#define ICEBERG_LOG_DEBUG(...) \
+  ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kDebug, __VA_ARGS__)
+#define ICEBERG_LOG_INFO(...) \
+  ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kInfo, __VA_ARGS__)
+#define ICEBERG_LOG_WARN(...) \
+  ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kWarn, __VA_ARGS__)
+#define ICEBERG_LOG_ERROR(...) \
+  ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kError, __VA_ARGS__)
+#define ICEBERG_LOG_CRITICAL(...) \
+  ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kCritical, __VA_ARGS__)
+
+// FATAL: emit if enabled (never compile-stripped), then ALWAYS flush + abort.
+// Acquires the default logger ONCE and uses the same instance for emit and 
flush
+// so a concurrent SetDefaultLogger cannot flush a different logger than it 
emitted to.
+#define ICEBERG_LOG_FATAL(FMT_, ...)                                           
        \
+  do {                                                                         
        \
+    auto _ib_logger = ::iceberg::GetDefaultLogger();                           
        \

Review Comment:
   This should use the current scoped logger rather than the process default. 
`ScopedLogger` says `LOG_*` routes through the scoped binding, but this path 
would send a fatal record to the global default and then abort.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to