Skip to content

Commit 56fbf5e

Browse files
feat(logging): add spdlog backend behind ICEBERG_SPDLOG (5/6)
Fifth block: the default production backend and the build option that selects it. - SpdLogger wraps spdlog::logger (kCritical/kFatal -> spdlog critical, others 1:1), forwarding the pre-formatted message and source location. Synchronous only in v1 (spdlog's source_loc is a non-owning const char*, unsafe with async sinks). It lives in logging/internal/, is gated by #ifdef ICEBERG_HAS_SPDLOG, and is NOT installed -- consumers obtain it via the default logger or the registry, never by including spdlog headers. - New ICEBERG_SPDLOG CMake option (default ON). config.h is ALWAYS generated (only ICEBERG_HAS_SPDLOG's definedness varies) so logger.cc compiles in both configurations; MakeDefaultLogger() prefers SpdLogger when compiled in, else CerrLogger. - Critically, ICEBERG_SPDLOG=OFF now UNWIRES the previously-unconditional spdlog link (interface-lib lists + resolve_spdlog_dependency), not just the new source -- so an OFF build has no spdlog dependency at all. spdlog_logger_test (compiled only on the ON path) covers the level mapping including fatal->critical and source-location forwarding. Co-authored-by: Isaac
1 parent e549ec2 commit 56fbf5e

12 files changed

Lines changed: 387 additions & 9 deletions

File tree

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ option(ICEBERG_S3 "Build with S3 support" OFF)
5454
option(ICEBERG_SIGV4 "Build with SigV4 support" OFF)
5555
option(ICEBERG_BUNDLE_AWSSDK "Bundle AWS SDK for S3/SigV4 support" ON)
5656
option(ICEBERG_BUNDLE_THRIFT "Bundle Thrift (from Arrow) for Hive catalog" ON)
57+
option(ICEBERG_SPDLOG "Use spdlog as the default logging backend" ON)
5758
option(ICEBERG_ENABLE_ASAN "Enable Address Sanitizer" OFF)
5859
option(ICEBERG_ENABLE_UBSAN "Enable Undefined Behavior Sanitizer" OFF)
5960

cmake_modules/IcebergThirdpartyToolchain.cmake

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,9 @@ resolve_nanoarrow_dependency()
806806
resolve_croaring_dependency()
807807
resolve_utf8proc_dependency()
808808
resolve_nlohmann_json_dependency()
809-
resolve_spdlog_dependency()
809+
if(ICEBERG_SPDLOG)
810+
resolve_spdlog_dependency()
811+
endif()
810812

811813
if(ICEBERG_S3 OR ICEBERG_SIGV4)
812814
if(ICEBERG_SIGV4 AND NOT ICEBERG_BUILD_REST)

src/iceberg/CMakeLists.txt

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,18 @@
1717

1818
set(ICEBERG_INCLUDES "$<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/src>"
1919
"$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src>")
20+
21+
# Generate the logging backend config header. ALWAYS generated (not gated by
22+
# ICEBERG_SPDLOG) so logging/logger.cc can include it in both ON and OFF builds;
23+
# only the definedness of ICEBERG_HAS_SPDLOG varies. Generated into the build
24+
# tree (already on ICEBERG_INCLUDES), included as "iceberg/logging/config.h", and
25+
# NOT installed (it must never appear in a public/installed header).
26+
if(ICEBERG_SPDLOG)
27+
set(ICEBERG_HAS_SPDLOG ON)
28+
endif()
29+
configure_file("${CMAKE_CURRENT_SOURCE_DIR}/logging/config.h.in"
30+
"${CMAKE_CURRENT_BINARY_DIR}/logging/config.h")
31+
2032
set(ICEBERG_SOURCES
2133
arrow_c_data_guard_internal.cc
2234
arrow_c_data_util.cc
@@ -154,29 +166,37 @@ list(APPEND
154166
ICEBERG_STATIC_BUILD_INTERFACE_LIBS
155167
"$<IF:$<BOOL:${NANOARROW_VENDORED}>,nanoarrow::nanoarrow_static,$<IF:$<TARGET_EXISTS:nanoarrow::nanoarrow_static>,nanoarrow::nanoarrow_static,nanoarrow::nanoarrow_shared>>"
156168
nlohmann_json::nlohmann_json
157-
spdlog::spdlog
158169
utf8proc::utf8proc
159170
ZLIB::ZLIB)
160171
list(APPEND
161172
ICEBERG_SHARED_BUILD_INTERFACE_LIBS
162173
"$<IF:$<BOOL:${NANOARROW_VENDORED}>,nanoarrow::nanoarrow_static,$<IF:$<TARGET_EXISTS:nanoarrow::nanoarrow_shared>,nanoarrow::nanoarrow_shared,nanoarrow::nanoarrow_static>>"
163174
nlohmann_json::nlohmann_json
164-
spdlog::spdlog
165175
utf8proc::utf8proc
166176
ZLIB::ZLIB)
167177
list(APPEND
168178
ICEBERG_STATIC_INSTALL_INTERFACE_LIBS
169179
"$<IF:$<BOOL:${NANOARROW_VENDORED}>,iceberg::nanoarrow_static,$<IF:$<TARGET_EXISTS:nanoarrow::nanoarrow_static>,nanoarrow::nanoarrow_static,nanoarrow::nanoarrow_shared>>"
170180
"$<IF:$<BOOL:${NLOHMANN_JSON_VENDORED}>,iceberg::nlohmann_json,$<IF:$<TARGET_EXISTS:nlohmann_json::nlohmann_json>,nlohmann_json::nlohmann_json,nlohmann_json::nlohmann_json>>"
171-
"$<IF:$<BOOL:${SPDLOG_VENDORED}>,iceberg::spdlog,spdlog::spdlog>"
172181
"$<IF:$<BOOL:${UTF8PROC_VENDORED}>,iceberg::utf8proc,utf8proc::utf8proc>")
173182
list(APPEND
174183
ICEBERG_SHARED_INSTALL_INTERFACE_LIBS
175184
"$<IF:$<BOOL:${NANOARROW_VENDORED}>,iceberg::nanoarrow_static,$<IF:$<TARGET_EXISTS:nanoarrow::nanoarrow_shared>,nanoarrow::nanoarrow_shared,nanoarrow::nanoarrow_static>>"
176185
"$<IF:$<BOOL:${NLOHMANN_JSON_VENDORED}>,iceberg::nlohmann_json,$<IF:$<TARGET_EXISTS:nlohmann_json::nlohmann_json>,nlohmann_json::nlohmann_json,nlohmann_json::nlohmann_json>>"
177-
"$<IF:$<BOOL:${SPDLOG_VENDORED}>,iceberg::spdlog,spdlog::spdlog>"
178186
"$<IF:$<BOOL:${UTF8PROC_VENDORED}>,iceberg::utf8proc,utf8proc::utf8proc>")
179187

188+
# spdlog backend: linked and compiled only when ICEBERG_SPDLOG is ON. When OFF,
189+
# the core library has no spdlog dependency and CerrLogger is the default sink.
190+
if(ICEBERG_SPDLOG)
191+
list(APPEND ICEBERG_SOURCES logging/internal/spdlog_logger.cc)
192+
list(APPEND ICEBERG_STATIC_BUILD_INTERFACE_LIBS spdlog::spdlog)
193+
list(APPEND ICEBERG_SHARED_BUILD_INTERFACE_LIBS spdlog::spdlog)
194+
list(APPEND ICEBERG_STATIC_INSTALL_INTERFACE_LIBS
195+
"$<IF:$<BOOL:${SPDLOG_VENDORED}>,iceberg::spdlog,spdlog::spdlog>")
196+
list(APPEND ICEBERG_SHARED_INSTALL_INTERFACE_LIBS
197+
"$<IF:$<BOOL:${SPDLOG_VENDORED}>,iceberg::spdlog,spdlog::spdlog>")
198+
endif()
199+
180200
add_iceberg_lib(iceberg
181201
SOURCES
182202
${ICEBERG_SOURCES}

src/iceberg/logging/config.h.in

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#pragma once
21+
22+
// Internal, build-generated configuration for the logging backend.
23+
// This header is NOT installed and must only be included from .cc files
24+
// (logger.cc, internal/spdlog_logger.cc) -- never from a public header.
25+
//
26+
// ICEBERG_HAS_SPDLOG is defined when the project is built with -DICEBERG_SPDLOG=ON
27+
// and left undefined otherwise. Always test it with #ifdef / #ifndef, never #if
28+
// (it carries no value).
29+
30+
#cmakedefine ICEBERG_HAS_SPDLOG
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#include "iceberg/logging/internal/spdlog_logger.h"
21+
22+
#ifdef ICEBERG_HAS_SPDLOG
23+
24+
# include <memory>
25+
# include <string>
26+
# include <unordered_map>
27+
# include <utility>
28+
29+
# include <spdlog/common.h>
30+
# include <spdlog/sinks/stdout_color_sinks.h>
31+
32+
namespace iceberg::internal {
33+
34+
namespace {
35+
36+
spdlog::level::level_enum ToSpdLevel(LogLevel level) noexcept {
37+
switch (level) {
38+
case LogLevel::kTrace:
39+
return spdlog::level::trace;
40+
case LogLevel::kDebug:
41+
return spdlog::level::debug;
42+
case LogLevel::kInfo:
43+
return spdlog::level::info;
44+
case LogLevel::kWarn:
45+
return spdlog::level::warn;
46+
case LogLevel::kError:
47+
return spdlog::level::err;
48+
case LogLevel::kCritical:
49+
case LogLevel::kFatal:
50+
// spdlog has no "fatal"; the process abort is owned by the macro layer.
51+
return spdlog::level::critical;
52+
case LogLevel::kOff:
53+
return spdlog::level::off;
54+
}
55+
return spdlog::level::off;
56+
}
57+
58+
} // namespace
59+
60+
SpdLogger::SpdLogger(LogLevel level)
61+
: SpdLogger(std::make_shared<spdlog::logger>(
62+
"iceberg", std::make_shared<spdlog::sinks::stderr_color_sink_mt>()),
63+
level) {}
64+
65+
Status SpdLogger::Initialize(
66+
const std::unordered_map<std::string, std::string>& properties) {
67+
if (auto it = properties.find(std::string(kPatternProperty)); it != properties.end()) {
68+
logger_->set_pattern(it->second);
69+
}
70+
// Apply "level" via the base implementation.
71+
return Logger::Initialize(properties);
72+
}
73+
74+
SpdLogger::SpdLogger(std::shared_ptr<spdlog::logger> logger, LogLevel level)
75+
: logger_(std::move(logger)), level_(level) {
76+
if (logger_) {
77+
logger_->set_level(spdlog::level::trace); // filtering is done by ShouldLog
78+
}
79+
}
80+
81+
void SpdLogger::Log(LogMessage&& message) noexcept {
82+
try {
83+
spdlog::source_loc loc{message.location.file_name(),
84+
static_cast<int>(message.location.line()),
85+
message.location.function_name()};
86+
// Pass the pre-formatted text as an argument ("{}") so any braces in the
87+
// message are not re-interpreted as a format string.
88+
logger_->log(loc, ToSpdLevel(message.level), "{}", message.message);
89+
} catch (...) {
90+
// Logging must never throw.
91+
}
92+
}
93+
94+
void SpdLogger::Flush() noexcept {
95+
try {
96+
logger_->flush();
97+
} catch (...) {
98+
}
99+
}
100+
101+
} // namespace iceberg::internal
102+
103+
#endif // ICEBERG_HAS_SPDLOG
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#pragma once
21+
22+
/// \file iceberg/logging/internal/spdlog_logger.h
23+
/// \brief spdlog-backed logging sink.
24+
///
25+
/// INTERNAL, NOT INSTALLED. It is only included from .cc files (logger.cc and
26+
/// spdlog_logger.cc) after config.h, and only when the project is built with
27+
/// ICEBERG_SPDLOG=ON. SpdLogger is not a consumer-constructible public type --
28+
/// applications obtain it via the default logger or the "logger-impl"="spdlog"
29+
/// registry factory.
30+
31+
#include "iceberg/logging/config.h"
32+
33+
#ifdef ICEBERG_HAS_SPDLOG
34+
35+
# include <atomic>
36+
# include <memory>
37+
38+
# include <spdlog/logger.h>
39+
40+
# include "iceberg/logging/log_level.h"
41+
# include "iceberg/logging/logger.h"
42+
43+
namespace iceberg::internal {
44+
45+
/// \brief Logger backed by spdlog (synchronous only in v1).
46+
///
47+
/// Synchronous because spdlog::source_loc holds non-owning const char* that are
48+
/// unsafe to forward into an async logger (spdlog #3227).
49+
/// ICEBERG_EXPORT so the symbol is linkable from in-tree tests (and any
50+
/// internal consumer) under -fvisibility=hidden / MSVC DLL builds. The header
51+
/// is still not installed -- this is a binary-visibility detail, not public API.
52+
class ICEBERG_EXPORT SpdLogger : public Logger {
53+
public:
54+
/// \brief Construct over a default stderr-backed spdlog logger.
55+
explicit SpdLogger(LogLevel level = LogLevel::kInfo);
56+
57+
/// \brief Construct over a caller-provided spdlog logger.
58+
///
59+
/// The logger MUST be synchronous. Log() forwards spdlog::source_loc, which
60+
/// borrows the std::source_location's const char* pointers; an async spdlog
61+
/// logger would queue them past their lifetime (spdlog #3227 -> UB). This is a
62+
/// caller contract -- spdlog exposes no reliable sync/async query to assert on.
63+
explicit SpdLogger(std::shared_ptr<spdlog::logger> logger,
64+
LogLevel level = LogLevel::kInfo);
65+
66+
/// \brief Apply the "pattern" property (spdlog set_pattern), then "level".
67+
Status Initialize(
68+
const std::unordered_map<std::string, std::string>& properties) override;
69+
70+
bool ShouldLog(LogLevel level) const noexcept override {
71+
return level >= level_.load(std::memory_order_relaxed);
72+
}
73+
void Log(LogMessage&& message) noexcept override;
74+
void SetLevel(LogLevel level) noexcept override {
75+
level_.store(level, std::memory_order_relaxed);
76+
}
77+
LogLevel level() const noexcept override {
78+
return level_.load(std::memory_order_relaxed);
79+
}
80+
void Flush() noexcept override;
81+
82+
private:
83+
std::shared_ptr<spdlog::logger> logger_;
84+
std::atomic<LogLevel> level_;
85+
};
86+
87+
} // namespace iceberg::internal
88+
89+
#endif // ICEBERG_HAS_SPDLOG

src/iceberg/logging/logger.cc

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,13 @@
2626
#include <tuple>
2727
#include <utility>
2828

29+
// Build-generated, .cc-only (never from a public header). Defines
30+
// ICEBERG_HAS_SPDLOG when built with -DICEBERG_SPDLOG=ON; tested with #ifdef.
2931
#include "iceberg/logging/cerr_logger.h"
32+
#include "iceberg/logging/config.h"
33+
#ifdef ICEBERG_HAS_SPDLOG
34+
# include "iceberg/logging/internal/spdlog_logger.h"
35+
#endif
3036

3137
namespace iceberg {
3238

@@ -44,9 +50,15 @@ class NoopLogger final : public Logger {
4450

4551
/// \brief Construct the process default logger for this build configuration.
4652
///
47-
/// Uses the always-available std::cerr sink. The spdlog backend (preferred when
48-
/// compiled in) is wired into this factory in a later block.
49-
std::shared_ptr<Logger> MakeDefaultLogger() { return std::make_shared<CerrLogger>(); }
53+
/// Prefers the spdlog backend when compiled in; otherwise the always-available
54+
/// std::cerr logger.
55+
std::shared_ptr<Logger> MakeDefaultLogger() {
56+
#ifdef ICEBERG_HAS_SPDLOG
57+
return std::make_shared<internal::SpdLogger>();
58+
#else
59+
return std::make_shared<CerrLogger>();
60+
#endif
61+
}
5062

5163
/// \brief The process-global default-logger slot.
5264
struct DefaultSlot {

src/iceberg/logging/meson.build

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,16 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18+
# Generate the .cc-only logging backend config header. The meson build always
19+
# links spdlog, so ICEBERG_HAS_SPDLOG is always defined here. Generated into
20+
# build/src/iceberg/logging/config.h (resolved via include_directories('..'),
21+
# which exposes both the source and build trees); not installed.
22+
logging_config_data = configuration_data()
23+
logging_config_data.set('ICEBERG_HAS_SPDLOG', 1)
24+
configure_file(output: 'config.h', configuration: logging_config_data)
25+
26+
# Public logging headers. The build-generated config.h and the internal
27+
# SpdLogger header are intentionally NOT installed.
1828
install_headers(
1929
['cerr_logger.h', 'log_level.h', 'logger.h'],
2030
subdir: 'iceberg/logging',

src/iceberg/meson.build

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ configure_file(
6363
install_dir: get_option('includedir') / 'iceberg',
6464
)
6565

66+
# Generate iceberg/logging/config.h (must precede the library() that compiles
67+
# the logging sources which include it).
6668
subdir('logging')
6769

6870
iceberg_include_dir = include_directories('..')
@@ -100,6 +102,7 @@ iceberg_sources = files(
100102
'json_serde.cc',
101103
'location_provider.cc',
102104
'logging/cerr_logger.cc',
105+
'logging/internal/spdlog_logger.cc',
103106
'logging/logger.cc',
104107
'manifest/manifest_adapter.cc',
105108
'manifest/manifest_entry.cc',

src/iceberg/test/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ add_iceberg_test(logging_test
107107
log_level_test.cc
108108
logger_test.cc
109109
macros_active_level_test.cc
110-
macros_test.cc)
110+
macros_test.cc
111+
spdlog_logger_test.cc)
111112

112113
add_iceberg_test(expression_test
113114
SOURCES

0 commit comments

Comments
 (0)