From 1ee2d2ee5c9b3cc5d19ca92bc4595c412da1d058 Mon Sep 17 00:00:00 2001 From: Kam Cheung Ting Date: Mon, 15 Jun 2026 10:47:10 +0000 Subject: [PATCH 1/2] feat(logging): add LogLevel severity enum (1/6) First block of the iceberg-cpp logging system. Introduces the severity level type the rest of the stack builds on: - LogLevel { kTrace, kDebug, kInfo, kWarn, kError, kCritical, kFatal, kOff }, ordered so `level >= threshold` is the enabled test; kOff is the max sentinel (disables all emission as a threshold, never an actual message level). - constexpr ToString() and case-insensitive LogLevelFromString() -> Result, mirroring the CounterUnit pattern in src/iceberg/metrics/. Wires the new logging/ subdirectory and a logging_test target into BOTH the CMake and Meson builds (header install + test), so the two build systems stay at parity from the first block. Co-authored-by: Isaac --- src/iceberg/CMakeLists.txt | 1 + src/iceberg/logging/CMakeLists.txt | 18 ++++++ src/iceberg/logging/log_level.h | 91 ++++++++++++++++++++++++++++++ src/iceberg/logging/meson.build | 18 ++++++ src/iceberg/meson.build | 2 + src/iceberg/test/CMakeLists.txt | 2 + src/iceberg/test/log_level_test.cc | 78 +++++++++++++++++++++++++ src/iceberg/test/meson.build | 5 ++ 8 files changed, 215 insertions(+) create mode 100644 src/iceberg/logging/CMakeLists.txt create mode 100644 src/iceberg/logging/log_level.h create mode 100644 src/iceberg/logging/meson.build create mode 100644 src/iceberg/test/log_level_test.cc diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index 0fe418d48..ba51b57cb 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -239,6 +239,7 @@ add_subdirectory(row) add_subdirectory(update) add_subdirectory(util) add_subdirectory(metrics) +add_subdirectory(logging) if(ICEBERG_BUILD_BUNDLE) set(ICEBERG_BUNDLE_SOURCES diff --git a/src/iceberg/logging/CMakeLists.txt b/src/iceberg/logging/CMakeLists.txt new file mode 100644 index 000000000..75b869908 --- /dev/null +++ b/src/iceberg/logging/CMakeLists.txt @@ -0,0 +1,18 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +iceberg_install_all_headers(iceberg/logging) diff --git a/src/iceberg/logging/log_level.h b/src/iceberg/logging/log_level.h new file mode 100644 index 000000000..65f174dd9 --- /dev/null +++ b/src/iceberg/logging/log_level.h @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +/// \file iceberg/logging/log_level.h +/// \brief Severity levels for the logging system. + +#include +#include + +#include "iceberg/iceberg_export.h" +#include "iceberg/result.h" +#include "iceberg/util/string_util.h" + +namespace iceberg { + +/// \brief Logging severity level, ordered from most to least verbose. +/// +/// Levels are ordered so that `level >= threshold` is the enabled test. +/// `kOff` is the maximum sentinel: as a threshold it disables all emission +/// (it is never the level of an actual message). +enum class LogLevel { + kTrace, + kDebug, + kInfo, + kWarn, + kError, + kCritical, + kFatal, + kOff, +}; + +/// \brief String representation of a LogLevel. +ICEBERG_EXPORT constexpr std::string_view ToString(LogLevel level) noexcept { + switch (level) { + case LogLevel::kTrace: + return "trace"; + case LogLevel::kDebug: + return "debug"; + case LogLevel::kInfo: + return "info"; + case LogLevel::kWarn: + return "warn"; + case LogLevel::kError: + return "error"; + case LogLevel::kCritical: + return "critical"; + case LogLevel::kFatal: + return "fatal"; + case LogLevel::kOff: + return "off"; + } + std::unreachable(); +} + +/// \brief Parse a LogLevel from a string (case-insensitive). +/// +/// \param s The string to parse ("trace", "debug", "info", "warn", "error", +/// "critical", "fatal", or "off"). +/// \return The LogLevel, or an InvalidArgument error if unrecognized. +ICEBERG_EXPORT inline Result LogLevelFromString(std::string_view s) { + auto level = StringUtils::ToLower(s); + if (level == "trace") return LogLevel::kTrace; + if (level == "debug") return LogLevel::kDebug; + if (level == "info") return LogLevel::kInfo; + if (level == "warn") return LogLevel::kWarn; + if (level == "error") return LogLevel::kError; + if (level == "critical") return LogLevel::kCritical; + if (level == "fatal") return LogLevel::kFatal; + if (level == "off") return LogLevel::kOff; + return InvalidArgument("Invalid log level: {}", s); +} + +} // namespace iceberg diff --git a/src/iceberg/logging/meson.build b/src/iceberg/logging/meson.build new file mode 100644 index 000000000..3c286a196 --- /dev/null +++ b/src/iceberg/logging/meson.build @@ -0,0 +1,18 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +install_headers(['log_level.h'], subdir: 'iceberg/logging') diff --git a/src/iceberg/meson.build b/src/iceberg/meson.build index 8c4b49e9b..edc470991 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -38,6 +38,8 @@ configure_file( install_dir: get_option('includedir') / 'iceberg', ) +subdir('logging') + iceberg_include_dir = include_directories('..') iceberg_sources = files( 'arrow_c_data_guard_internal.cc', diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index 0e8f03150..ad62042ce 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -101,6 +101,8 @@ add_iceberg_test(table_test table_test.cc table_update_test.cc) +add_iceberg_test(logging_test SOURCES log_level_test.cc) + add_iceberg_test(expression_test SOURCES aggregate_test.cc diff --git a/src/iceberg/test/log_level_test.cc b/src/iceberg/test/log_level_test.cc new file mode 100644 index 000000000..e4c25bf67 --- /dev/null +++ b/src/iceberg/test/log_level_test.cc @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/logging/log_level.h" + +#include +#include + +#include + +namespace iceberg { + +namespace { + +constexpr std::array kAllLevels = { + LogLevel::kTrace, LogLevel::kDebug, LogLevel::kInfo, LogLevel::kWarn, + LogLevel::kError, LogLevel::kCritical, LogLevel::kFatal, LogLevel::kOff}; + +} // namespace + +TEST(LogLevelTest, ToStringCoversEveryLevel) { + EXPECT_EQ(ToString(LogLevel::kTrace), "trace"); + EXPECT_EQ(ToString(LogLevel::kDebug), "debug"); + EXPECT_EQ(ToString(LogLevel::kInfo), "info"); + EXPECT_EQ(ToString(LogLevel::kWarn), "warn"); + EXPECT_EQ(ToString(LogLevel::kError), "error"); + EXPECT_EQ(ToString(LogLevel::kCritical), "critical"); + EXPECT_EQ(ToString(LogLevel::kFatal), "fatal"); + EXPECT_EQ(ToString(LogLevel::kOff), "off"); +} + +TEST(LogLevelTest, FromStringRoundTrips) { + for (LogLevel level : kAllLevels) { + auto parsed = LogLevelFromString(ToString(level)); + ASSERT_TRUE(parsed.has_value()) << "failed to parse " << ToString(level); + EXPECT_EQ(parsed.value(), level); + } +} + +TEST(LogLevelTest, FromStringIsCaseInsensitive) { + EXPECT_EQ(LogLevelFromString("WARN").value(), LogLevel::kWarn); + EXPECT_EQ(LogLevelFromString("Warn").value(), LogLevel::kWarn); + EXPECT_EQ(LogLevelFromString("CRITICAL").value(), LogLevel::kCritical); +} + +TEST(LogLevelTest, FromStringRejectsUnknown) { + auto parsed = LogLevelFromString("verbose"); + ASSERT_FALSE(parsed.has_value()); + EXPECT_EQ(parsed.error().kind, ErrorKind::kInvalidArgument); +} + +TEST(LogLevelTest, OrderingIsMonotonicWithOffAsMaximum) { + EXPECT_LT(LogLevel::kTrace, LogLevel::kDebug); + EXPECT_LT(LogLevel::kDebug, LogLevel::kInfo); + EXPECT_LT(LogLevel::kInfo, LogLevel::kWarn); + EXPECT_LT(LogLevel::kWarn, LogLevel::kError); + EXPECT_LT(LogLevel::kError, LogLevel::kCritical); + EXPECT_LT(LogLevel::kCritical, LogLevel::kFatal); + EXPECT_LT(LogLevel::kFatal, LogLevel::kOff); +} + +} // namespace iceberg diff --git a/src/iceberg/test/meson.build b/src/iceberg/test/meson.build index 03d9e1f6c..069994698 100644 --- a/src/iceberg/test/meson.build +++ b/src/iceberg/test/meson.build @@ -60,6 +60,11 @@ iceberg_tests = { 'table_update_test.cc', ), }, + 'logging_test': { + 'sources': files( + 'log_level_test.cc', + ), + }, 'expression_test': { 'sources': files( 'aggregate_test.cc', From 9fbff5914b4378428b59317729d4d8d2651135e8 Mon Sep 17 00:00:00 2001 From: Kam Cheung Ting Date: Mon, 15 Jun 2026 10:49:03 +0000 Subject: [PATCH 2/2] feat(logging): add Logger interface and process-global default (2/6) Second block: the backend-agnostic logging API and the swappable default logger. - Logger: pure-virtual sink (ShouldLog/Log/SetLevel/level/Flush/IsNoop), with ShouldLog() as the single authority for runtime filtering and a documented no-reentrancy / thread-safety / noexcept contract. Mirrors MetricsReporter. - LogMessage owns its formatted text (moved in) so a sink may retain records; reserves an owning attributes vector for future structured logging without an ABI break. logger.h is backend-agnostic -- it never includes the build config header nor references the spdlog feature macro, so consumers see one stable API. - Process-global default logger: GetDefaultLogger / SetDefaultLogger / SetDefaultLevel, plus a lock-free thread-local generation cache on the hot path (no lock or refcount churn in steady state; slot mutex only on swap/refresh). NoopLogger is the placeholder default here; CerrLogger and the spdlog backend arrive in the following blocks. Builds the source into both CMake and Meson and installs logger.h alongside log_level.h; logger_test covers the default-logger API, level lowering taking effect immediately, and concurrent swap/read safety. Co-authored-by: Isaac --- src/iceberg/CMakeLists.txt | 1 + src/iceberg/logging/logger.cc | 132 ++++++++++++++++++ src/iceberg/logging/logger.h | 171 ++++++++++++++++++++++++ src/iceberg/logging/meson.build | 2 +- src/iceberg/meson.build | 1 + src/iceberg/test/CMakeLists.txt | 2 +- src/iceberg/test/logger_test.cc | 114 ++++++++++++++++ src/iceberg/test/logging_test_helpers.h | 77 +++++++++++ src/iceberg/test/meson.build | 1 + 9 files changed, 499 insertions(+), 2 deletions(-) create mode 100644 src/iceberg/logging/logger.cc create mode 100644 src/iceberg/logging/logger.h create mode 100644 src/iceberg/test/logger_test.cc create mode 100644 src/iceberg/test/logging_test_helpers.h diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index ba51b57cb..7fead2080 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -44,6 +44,7 @@ set(ICEBERG_SOURCES inheritable_metadata.cc json_serde.cc location_provider.cc + logging/logger.cc manifest/manifest_adapter.cc manifest/manifest_entry.cc manifest/manifest_filter_manager.cc diff --git a/src/iceberg/logging/logger.cc b/src/iceberg/logging/logger.cc new file mode 100644 index 000000000..205e4bb20 --- /dev/null +++ b/src/iceberg/logging/logger.cc @@ -0,0 +1,132 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/logging/logger.h" + +#include +#include +#include +#include +#include + +namespace iceberg { + +namespace { + +/// \brief Logger that drops every record. +class NoopLogger final : public Logger { + public: + bool ShouldLog(LogLevel /*level*/) const override { return false; } + void Log(LogMessage&& /*message*/) noexcept override {} + void SetLevel(LogLevel /*level*/) override {} + LogLevel level() const override { return LogLevel::kOff; } + bool IsNoop() const override { return true; } +}; + +/// \brief Construct the process default logger for this build configuration. +/// +/// This block ships only the interface and the no-op logger; the concrete +/// std::cerr and spdlog sinks (and the build-config selection between them) +/// arrive in later blocks, which update this factory. +std::shared_ptr MakeDefaultLogger() { return std::make_shared(); } + +/// \brief The process-global default-logger slot. +struct DefaultSlot { + std::mutex mtx; + std::shared_ptr logger; + // Seeded to 1 so a fresh thread (tls_gen == 0) always refreshes on first use. + std::atomic gen{1}; + + DefaultSlot() : logger(MakeDefaultLogger()) {} +}; + +/// \brief Immortal (leaked, hence reachable -> LSan-clean) accessor for the slot. +DefaultSlot& Slot() { + static auto* slot = new DefaultSlot(); + return *slot; +} + +} // namespace + +std::shared_ptr Logger::Noop() { + // Intentionally leaked: reachable via the function-local static (LSan-clean) + // and never destroyed, so logging during static teardown stays safe. + static auto* instance = new std::shared_ptr(std::make_shared()); + return *instance; +} + +std::shared_ptr GetDefaultLogger() { + DefaultSlot& slot = Slot(); + std::lock_guard lock(slot.mtx); + return slot.logger; +} + +void SetDefaultLogger(std::shared_ptr logger) { + if (!logger) { + logger = Logger::Noop(); + } + DefaultSlot& slot = Slot(); + std::lock_guard lock(slot.mtx); + slot.logger = std::move(logger); + // Publish the swap; the mutex provides the happens-before, gen is a detector. + slot.gen.fetch_add(1, std::memory_order_relaxed); +} + +void SetDefaultLevel(LogLevel level) { + DefaultSlot& slot = Slot(); + std::lock_guard lock(slot.mtx); + slot.logger->SetLevel(level); +} + +namespace detail { + +const std::shared_ptr& CurrentLogger() noexcept { + static thread_local std::shared_ptr tls; + static thread_local uint64_t tls_gen = 0; + DefaultSlot& slot = Slot(); + uint64_t current = slot.gen.load(std::memory_order_relaxed); + if (current != tls_gen) { + std::lock_guard lock(slot.mtx); + tls = slot.logger; + tls_gen = current; + } + return tls; +} + +void Emit(Logger& logger, LogLevel level, const std::source_location& location, + std::string&& message) { + logger.Log(LogMessage{.level = level, + .message = std::move(message), + .location = location, + .attributes = {}}); +} + +void EmitFormatError(Logger& logger, LogLevel level, + const std::source_location& location) noexcept { + // Fixed short literal (<= 15 bytes, fits SSO on libstdc++/libc++/MSVC -> no heap + // allocation), no std::format, no retry. Cannot throw or recurse. + logger.Log(LogMessage{.level = level, + .message = std::string(""), + .location = location, + .attributes = {}}); +} + +} // namespace detail + +} // namespace iceberg diff --git a/src/iceberg/logging/logger.h b/src/iceberg/logging/logger.h new file mode 100644 index 000000000..f4806a074 --- /dev/null +++ b/src/iceberg/logging/logger.h @@ -0,0 +1,171 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +/// \file iceberg/logging/logger.h +/// \brief Pluggable logging interface, the process-global default logger, and +/// the logging macros. +/// +/// This header is backend-agnostic: it never includes the build-generated +/// backend configuration header and never references the spdlog feature macro, +/// so consumers see one stable API regardless of how the backend was configured. + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "iceberg/iceberg_export.h" +#include "iceberg/logging/log_level.h" +#include "iceberg/result.h" + +namespace iceberg { + +/// \brief A structured key/value attribute attached to a log record. +/// +/// Both key and value are owned so a sink may retain the record safely. +/// Unused in v1; reserved so structured logging can be added without an ABI +/// break to LogMessage. +struct LogAttribute { + std::string key; + std::string value; +}; + +/// \brief A single log record handed to a Logger. +/// +/// The formatted message is owned (moved in by the logging macros), so a sink +/// may safely retain the record beyond the Log() call. The member set must not +/// depend on the build's logging backend (the spdlog backend never appears here). +struct LogMessage { + LogLevel level; + std::string message; + std::source_location location; + std::vector attributes; +}; + +/// \brief Pluggable logging sink. +/// +/// ShouldLog() is the single authority for runtime filtering -- the macros call +/// it on every (compile-time-enabled) statement, so level changes by any path +/// take effect immediately. Implementations must be thread-safe and must not +/// throw. They must also obey: +/// - No reentrancy: Log()/Flush() must not call the logging macros or +/// GetDefaultLogger() (UB -- deadlock with mutex-based sinks). +/// - level() is an accessor consistent with ShouldLog (used by SetDefaultLevel +/// and introspection); ShouldLog may implement finer logic than a level compare. +class ICEBERG_EXPORT Logger { + public: + virtual ~Logger() = default; + + /// \brief Property-based setup, called by Loggers::Load() before first use. + /// + /// The default is a no-op. Recognized properties include "level" (parsed via + /// LogLevelFromString) and, for formatting sinks, "pattern". + virtual Status Initialize( + [[maybe_unused]] const std::unordered_map& properties) { + return {}; + } + + /// \brief Cheap check whether a record at \p level would be emitted. + virtual bool ShouldLog(LogLevel level) const = 0; + + /// \brief Emit one (already-formatted) record, taking ownership. Must not throw. + virtual void Log(LogMessage&& message) noexcept = 0; + + /// \brief Set the minimum level this logger emits. + virtual void SetLevel(LogLevel level) = 0; + + /// \brief Return the minimum level this logger emits. + virtual LogLevel level() const = 0; + + /// \brief Flush any buffered output. Must not throw; best-effort on the fatal path. + virtual void Flush() noexcept {} + + /// \brief Return true if this logger is a no-op. + virtual bool IsNoop() const { return false; } + + /// \brief Return a shared, immortal no-op logger singleton. + static std::shared_ptr Noop(); +}; + +/// \brief Return the process-global default logger (never null). +/// +/// Off the hot path -- acquires the slot lock and returns an owning copy. The +/// logging macros use the cheaper internal hot-path accessor instead. +ICEBERG_EXPORT std::shared_ptr GetDefaultLogger(); + +/// \brief Install a new process-global default logger. +/// +/// A null argument installs the no-op logger. Thread-safe; intended for +/// occasional (configuration-time) use rather than the hot path. +ICEBERG_EXPORT void SetDefaultLogger(std::shared_ptr logger); + +/// \brief Set the minimum level of the current default logger. +/// +/// Convenience for `GetDefaultLogger()->SetLevel(level)`. Filtering is always +/// decided by the logger's own ShouldLog(), so changing a logger's level by any +/// means (this, SetLevel on a held handle, or Initialize) takes effect immediately. +ICEBERG_EXPORT void SetDefaultLevel(LogLevel level); + +namespace detail { + +/// \brief Hot-path accessor for the default logger. +/// +/// Returns a reference to a thread-local cached shared_ptr that is refreshed +/// only when the default logger has changed (no lock / no refcount churn in +/// steady state). The reference is valid for the duration of the calling +/// statement. +ICEBERG_EXPORT const std::shared_ptr& CurrentLogger() noexcept; + +/// \brief Build a LogMessage from the already-formatted text and dispatch it. +/// +/// Declared ICEBERG_EXPORT because the logging macros expand into this call in +/// consumer translation units. +ICEBERG_EXPORT void Emit(Logger& logger, LogLevel level, + const std::source_location& location, std::string&& message); + +/// \brief Emit a fixed fallback record when formatting threw. +/// +/// noexcept, allocation-light (small/SSO literal), performs no std::format, and +/// does not recurse -- so the macro's "logging never throws" guarantee holds +/// even when a format argument throws. +ICEBERG_EXPORT void EmitFormatError(Logger& logger, LogLevel level, + const std::source_location& location) noexcept; + +/// \brief Runtime (non-literal) format-string helper. +/// +/// std::format requires a compile-time format string; this routes a runtime +/// string through std::vformat. Args are bound as named lvalues and the +/// arg-store is held in a named variable so it outlives the vformat call +/// (C++23 make_format_args rejects rvalues -- P2905 / LWG3631). +template +std::string VFormat(std::string_view fmt, Args&&... args) { + auto store = std::make_format_args(args...); + return std::vformat(fmt, store); +} + +} // namespace detail + +} // namespace iceberg diff --git a/src/iceberg/logging/meson.build b/src/iceberg/logging/meson.build index 3c286a196..3f7af4fec 100644 --- a/src/iceberg/logging/meson.build +++ b/src/iceberg/logging/meson.build @@ -15,4 +15,4 @@ # specific language governing permissions and limitations # under the License. -install_headers(['log_level.h'], subdir: 'iceberg/logging') +install_headers(['log_level.h', 'logger.h'], subdir: 'iceberg/logging') diff --git a/src/iceberg/meson.build b/src/iceberg/meson.build index edc470991..5521878ee 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -68,6 +68,7 @@ iceberg_sources = files( 'inheritable_metadata.cc', 'json_serde.cc', 'location_provider.cc', + 'logging/logger.cc', 'manifest/manifest_adapter.cc', 'manifest/manifest_entry.cc', 'manifest/manifest_filter_manager.cc', diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index ad62042ce..1563f9669 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -101,7 +101,7 @@ add_iceberg_test(table_test table_test.cc table_update_test.cc) -add_iceberg_test(logging_test SOURCES log_level_test.cc) +add_iceberg_test(logging_test SOURCES log_level_test.cc logger_test.cc) add_iceberg_test(expression_test SOURCES diff --git a/src/iceberg/test/logger_test.cc b/src/iceberg/test/logger_test.cc new file mode 100644 index 000000000..09794d66a --- /dev/null +++ b/src/iceberg/test/logger_test.cc @@ -0,0 +1,114 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/logging/logger.h" + +#include +#include +#include +#include + +#include + +#include "iceberg/logging/log_level.h" +#include "iceberg/test/logging_test_helpers.h" + +namespace iceberg { + +TEST(LoggerTest, NoopIsSharedImmortalAndSilent) { + auto noop = Logger::Noop(); + ASSERT_NE(noop, nullptr); + EXPECT_TRUE(noop->IsNoop()); + EXPECT_FALSE(noop->ShouldLog(LogLevel::kFatal)); + EXPECT_EQ(noop->level(), LogLevel::kOff); + // Same singleton instance every call. + EXPECT_EQ(noop.get(), Logger::Noop().get()); +} + +TEST(LoggerTest, DefaultLoggerIsNeverNull) { EXPECT_NE(GetDefaultLogger(), nullptr); } + +TEST(LoggerTest, SetAndGetDefaultLogger) { + auto capturing = std::make_shared(); + ScopedDefaultLogger guard(capturing); + EXPECT_EQ(GetDefaultLogger().get(), capturing.get()); + EXPECT_EQ(detail::CurrentLogger().get(), capturing.get()); +} + +TEST(LoggerTest, SetNullFallsBackToNoop) { + ScopedDefaultLogger guard(std::make_shared()); + SetDefaultLogger(nullptr); + EXPECT_TRUE(GetDefaultLogger()->IsNoop()); +} + +TEST(LoggerTest, CurrentLoggerTracksSwaps) { + auto first = std::make_shared(); + auto second = std::make_shared(); + ScopedDefaultLogger guard(first); + EXPECT_EQ(detail::CurrentLogger().get(), first.get()); + SetDefaultLogger(second); + // Generation bump must invalidate the thread-local cache. + EXPECT_EQ(detail::CurrentLogger().get(), second.get()); +} + +TEST(LoggerTest, SetDefaultLevelUpdatesLogger) { + auto capturing = std::make_shared(); + ScopedDefaultLogger guard(capturing); + SetDefaultLevel(LogLevel::kError); + EXPECT_EQ(capturing->level(), LogLevel::kError); +} + +// Filtering is decided by the logger's own ShouldLog (no separate cached gate), +// so lowering a logger's level out-of-band (not via SetDefaultLevel) takes effect +// immediately -- this is the regression guard for the dropped g_effective_level gate. +TEST(LoggerTest, OutOfBandLevelLoweringTakesEffect) { + auto capturing = std::make_shared(); + capturing->SetLevel(LogLevel::kError); + ScopedDefaultLogger guard(capturing); + EXPECT_FALSE(detail::CurrentLogger()->ShouldLog(LogLevel::kInfo)); + capturing->SetLevel(LogLevel::kTrace); // lowered directly on the handle + EXPECT_TRUE(detail::CurrentLogger()->ShouldLog(LogLevel::kInfo)); +} + +TEST(LoggerTest, ConcurrentSwapAndReadIsSafe) { + // Stress CurrentLogger()/GetDefaultLogger() against SetDefaultLogger() swaps. + // Run under TSan in CI; here it asserts no crash and a valid logger throughout. + auto a = std::make_shared(); + auto b = std::make_shared(); + ScopedDefaultLogger guard(a); + std::atomic stop{false}; + std::atomic saw_null{false}; + std::vector readers; + for (int i = 0; i < 6; ++i) { + readers.emplace_back([&stop, &saw_null] { + // ASSERT_* doesn't propagate from non-main threads; record via a flag. + while (!stop.load(std::memory_order_relaxed)) { + const auto& l = detail::CurrentLogger(); + if (!l) saw_null.store(true, std::memory_order_relaxed); + (void)l->ShouldLog(LogLevel::kError); + (void)GetDefaultLogger(); + } + }); + } + for (int i = 0; i < 2000; ++i) SetDefaultLogger((i & 1) ? a : b); + stop.store(true, std::memory_order_relaxed); + for (auto& t : readers) t.join(); + EXPECT_FALSE(saw_null.load()); // CurrentLogger() is never null across swaps +} + +} // namespace iceberg diff --git a/src/iceberg/test/logging_test_helpers.h b/src/iceberg/test/logging_test_helpers.h new file mode 100644 index 000000000..00a57cf82 --- /dev/null +++ b/src/iceberg/test/logging_test_helpers.h @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include +#include +#include + +#include "iceberg/logging/logger.h" + +namespace iceberg { + +/// \brief Test sink that records every emitted LogMessage under a mutex. +class CapturingLogger : public Logger { + public: + bool ShouldLog(LogLevel level) const override { return level >= level_; } + + void Log(LogMessage&& message) noexcept override { + std::lock_guard lock(mutex_); + records_.push_back(std::move(message)); + } + + void SetLevel(LogLevel level) override { level_ = level; } + LogLevel level() const override { return level_; } + + std::vector records() const { + std::lock_guard lock(mutex_); + return records_; + } + + std::size_t count() const { + std::lock_guard lock(mutex_); + return records_.size(); + } + + private: + mutable std::mutex mutex_; + LogLevel level_ = LogLevel::kTrace; + std::vector records_; +}; + +/// \brief RAII guard that restores the process default logger on scope exit, so +/// tests that swap the global default don't leak state into other tests. +class ScopedDefaultLogger { + public: + explicit ScopedDefaultLogger(std::shared_ptr logger) + : previous_(GetDefaultLogger()) { + SetDefaultLogger(std::move(logger)); + } + ~ScopedDefaultLogger() { SetDefaultLogger(previous_); } + + ScopedDefaultLogger(const ScopedDefaultLogger&) = delete; + ScopedDefaultLogger& operator=(const ScopedDefaultLogger&) = delete; + + private: + std::shared_ptr previous_; +}; + +} // namespace iceberg diff --git a/src/iceberg/test/meson.build b/src/iceberg/test/meson.build index 069994698..679464738 100644 --- a/src/iceberg/test/meson.build +++ b/src/iceberg/test/meson.build @@ -63,6 +63,7 @@ iceberg_tests = { 'logging_test': { 'sources': files( 'log_level_test.cc', + 'logger_test.cc', ), }, 'expression_test': {