Skip to content

Commit e9308ff

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 ab08386 commit e9308ff

13 files changed

Lines changed: 495 additions & 17 deletions

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
@@ -869,7 +869,9 @@ resolve_nanoarrow_dependency()
869869
resolve_croaring_dependency()
870870
resolve_utf8proc_dependency()
871871
resolve_nlohmann_json_dependency()
872-
resolve_spdlog_dependency()
872+
if(ICEBERG_SPDLOG)
873+
resolve_spdlog_dependency()
874+
endif()
873875

874876
if(ICEBERG_S3 OR ICEBERG_SIGV4)
875877
if(ICEBERG_SIGV4 AND NOT ICEBERG_BUILD_REST)

meson.options

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,13 @@ option(
5151
value: 'disabled',
5252
)
5353

54+
option(
55+
'spdlog',
56+
type: 'feature',
57+
description: 'Use spdlog as the default logging backend (CMake: ICEBERG_SPDLOG)',
58+
value: 'enabled',
59+
)
60+
5461
option('tests', type: 'feature', description: 'Build tests', value: 'enabled')
5562

5663
option(

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
@@ -157,29 +169,37 @@ list(APPEND
157169
ICEBERG_STATIC_BUILD_INTERFACE_LIBS
158170
"$<IF:$<BOOL:${NANOARROW_VENDORED}>,nanoarrow::nanoarrow_static,$<IF:$<TARGET_EXISTS:nanoarrow::nanoarrow_static>,nanoarrow::nanoarrow_static,nanoarrow::nanoarrow_shared>>"
159171
nlohmann_json::nlohmann_json
160-
spdlog::spdlog
161172
utf8proc::utf8proc
162173
ZLIB::ZLIB)
163174
list(APPEND
164175
ICEBERG_SHARED_BUILD_INTERFACE_LIBS
165176
"$<IF:$<BOOL:${NANOARROW_VENDORED}>,nanoarrow::nanoarrow_static,$<IF:$<TARGET_EXISTS:nanoarrow::nanoarrow_shared>,nanoarrow::nanoarrow_shared,nanoarrow::nanoarrow_static>>"
166177
nlohmann_json::nlohmann_json
167-
spdlog::spdlog
168178
utf8proc::utf8proc
169179
ZLIB::ZLIB)
170180
list(APPEND
171181
ICEBERG_STATIC_INSTALL_INTERFACE_LIBS
172182
"$<IF:$<BOOL:${NANOARROW_VENDORED}>,iceberg::nanoarrow_static,$<IF:$<TARGET_EXISTS:nanoarrow::nanoarrow_static>,nanoarrow::nanoarrow_static,nanoarrow::nanoarrow_shared>>"
173183
"$<IF:$<BOOL:${NLOHMANN_JSON_VENDORED}>,iceberg::nlohmann_json,$<IF:$<TARGET_EXISTS:nlohmann_json::nlohmann_json>,nlohmann_json::nlohmann_json,nlohmann_json::nlohmann_json>>"
174-
"$<IF:$<BOOL:${SPDLOG_VENDORED}>,iceberg::spdlog,spdlog::spdlog>"
175184
"$<IF:$<BOOL:${UTF8PROC_VENDORED}>,iceberg::utf8proc,utf8proc::utf8proc>")
176185
list(APPEND
177186
ICEBERG_SHARED_INSTALL_INTERFACE_LIBS
178187
"$<IF:$<BOOL:${NANOARROW_VENDORED}>,iceberg::nanoarrow_static,$<IF:$<TARGET_EXISTS:nanoarrow::nanoarrow_shared>,nanoarrow::nanoarrow_shared,nanoarrow::nanoarrow_static>>"
179188
"$<IF:$<BOOL:${NLOHMANN_JSON_VENDORED}>,iceberg::nlohmann_json,$<IF:$<TARGET_EXISTS:nlohmann_json::nlohmann_json>,nlohmann_json::nlohmann_json,nlohmann_json::nlohmann_json>>"
180-
"$<IF:$<BOOL:${SPDLOG_VENDORED}>,iceberg::spdlog,spdlog::spdlog>"
181189
"$<IF:$<BOOL:${UTF8PROC_VENDORED}>,iceberg::utf8proc,utf8proc::utf8proc>")
182190

191+
# spdlog backend: linked and compiled only when ICEBERG_SPDLOG is ON. When OFF,
192+
# the core library has no spdlog dependency and CerrLogger is the default sink.
193+
if(ICEBERG_SPDLOG)
194+
list(APPEND ICEBERG_SOURCES logging/internal/spdlog_logger.cc)
195+
list(APPEND ICEBERG_STATIC_BUILD_INTERFACE_LIBS spdlog::spdlog)
196+
list(APPEND ICEBERG_SHARED_BUILD_INTERFACE_LIBS spdlog::spdlog)
197+
list(APPEND ICEBERG_STATIC_INSTALL_INTERFACE_LIBS
198+
"$<IF:$<BOOL:${SPDLOG_VENDORED}>,iceberg::spdlog,spdlog::spdlog>")
199+
list(APPEND ICEBERG_SHARED_INSTALL_INTERFACE_LIBS
200+
"$<IF:$<BOOL:${SPDLOG_VENDORED}>,iceberg::spdlog,spdlog::spdlog>")
201+
endif()
202+
183203
add_iceberg_lib(iceberg
184204
SOURCES
185205
${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: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
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+
/// \brief The built-in sink: a color stderr spdlog logger.
59+
std::shared_ptr<spdlog::logger> MakeDefaultSpdLogger() {
60+
return std::make_shared<spdlog::logger>(
61+
"iceberg", std::make_shared<spdlog::sinks::stderr_color_sink_mt>());
62+
}
63+
64+
} // namespace
65+
66+
SpdLogger::SpdLogger(LogLevel level) : SpdLogger(MakeDefaultSpdLogger(), level) {}
67+
68+
Status SpdLogger::Initialize(
69+
const std::unordered_map<std::string, std::string>& properties) {
70+
if (auto it = properties.find(std::string(kPatternProperty)); it != properties.end()) {
71+
logger_->set_pattern(it->second);
72+
}
73+
// Apply "level" via the base implementation.
74+
return Logger::Initialize(properties);
75+
}
76+
77+
SpdLogger::SpdLogger(std::shared_ptr<spdlog::logger> logger, LogLevel level)
78+
: logger_(std::move(logger)), level_(level) {
79+
// logger_ is non-null for the rest of this object's life, so Initialize/Log/Flush
80+
// may dereference it unconditionally. Enforced by substitution rather than an
81+
// assertion, which would vanish under NDEBUG and leave a release-build crash: a
82+
// null argument falls back to the same stderr-backed logger the default
83+
// constructor builds, so a caller mistake degrades to the default sink.
84+
if (!logger_) {
85+
logger_ = MakeDefaultSpdLogger();
86+
}
87+
logger_->set_level(spdlog::level::trace); // filtering is done by ShouldLog
88+
}
89+
90+
void SpdLogger::Log(LogMessage&& message) noexcept {
91+
try {
92+
spdlog::source_loc loc{message.location.file_name(),
93+
static_cast<int>(message.location.line()),
94+
message.location.function_name()};
95+
// Raw-message overload: the text is already formatted, so hand spdlog the bytes
96+
// directly instead of running them back through fmt (which would re-parse and
97+
// copy the whole message, allocating for long ones). It also means braces in the
98+
// message can never be interpreted as format placeholders.
99+
logger_->log(loc, ToSpdLevel(message.level),
100+
spdlog::string_view_t{message.message.data(), message.message.size()});
101+
} catch (...) {
102+
// Logging must never throw.
103+
}
104+
}
105+
106+
void SpdLogger::Flush() noexcept {
107+
try {
108+
logger_->flush();
109+
} catch (...) {
110+
}
111+
}
112+
113+
} // namespace iceberg::internal
114+
115+
#endif // ICEBERG_HAS_SPDLOG
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
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. Included only from .cc files (logger.cc and
26+
/// spdlog_logger.cc). It pulls in the build-generated config.h itself and gates
27+
/// its entire body on ICEBERG_HAS_SPDLOG, so it compiles to nothing unless the
28+
/// project was built with ICEBERG_SPDLOG=ON. SpdLogger is not a
29+
/// consumer-constructible public type -- applications obtain it via the default
30+
/// logger or the "logger-impl"="spdlog" registry factory.
31+
32+
#include "iceberg/logging/config.h"
33+
34+
#ifdef ICEBERG_HAS_SPDLOG
35+
36+
# include <atomic>
37+
# include <memory>
38+
39+
# include <spdlog/logger.h>
40+
41+
# include "iceberg/logging/log_level.h"
42+
# include "iceberg/logging/logger.h"
43+
44+
namespace iceberg::internal {
45+
46+
/// \brief Logger backed by spdlog (synchronous only in v1).
47+
///
48+
/// Synchronous because spdlog::source_loc holds non-owning const char* that are
49+
/// unsafe to forward into an async logger (spdlog #3227).
50+
/// ICEBERG_EXPORT so the symbol is linkable from in-tree tests (and any
51+
/// internal consumer) under -fvisibility=hidden / MSVC DLL builds. The header
52+
/// is still not installed -- this is a binary-visibility detail, not public API.
53+
class ICEBERG_EXPORT SpdLogger : public Logger {
54+
public:
55+
/// \brief Construct over a default stderr-backed spdlog logger.
56+
explicit SpdLogger(LogLevel level = LogLevel::kInfo);
57+
58+
/// \brief Construct over a caller-provided spdlog logger.
59+
///
60+
/// The logger MUST be synchronous. Log() forwards spdlog::source_loc, which
61+
/// borrows the std::source_location's const char* pointers; an async spdlog
62+
/// logger would queue them past their lifetime (spdlog #3227 -> UB). This is a
63+
/// caller contract -- spdlog exposes no reliable sync/async query to assert on.
64+
explicit SpdLogger(std::shared_ptr<spdlog::logger> logger,
65+
LogLevel level = LogLevel::kInfo);
66+
67+
/// \brief Apply the "pattern" property (spdlog set_pattern), then "level".
68+
Status Initialize(
69+
const std::unordered_map<std::string, std::string>& properties) override;
70+
71+
bool ShouldLog(LogLevel level) const noexcept override {
72+
return level >= level_.load(std::memory_order_relaxed);
73+
}
74+
void Log(LogMessage&& message) noexcept override;
75+
void SetLevel(LogLevel level) noexcept override {
76+
level_.store(level, std::memory_order_relaxed);
77+
}
78+
LogLevel level() const noexcept override {
79+
return level_.load(std::memory_order_relaxed);
80+
}
81+
void Flush() noexcept override;
82+
83+
private:
84+
std::shared_ptr<spdlog::logger> logger_;
85+
std::atomic<LogLevel> level_;
86+
};
87+
88+
} // namespace iceberg::internal
89+
90+
#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: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,21 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18+
# Generate the .cc-only logging backend config header. ALWAYS generated (logger.cc
19+
# includes it in both configurations); only the definedness of ICEBERG_HAS_SPDLOG
20+
# varies with the `spdlog` feature option -- mirroring CMake's ICEBERG_SPDLOG. When
21+
# disabled, CerrLogger is the default sink and spdlog is neither compiled nor
22+
# linked. Generated into build/src/iceberg/logging/config.h (resolved via
23+
# include_directories('..'), which exposes both the source and build trees); not
24+
# installed.
25+
logging_config_data = configuration_data()
26+
if spdlog_enabled
27+
logging_config_data.set('ICEBERG_HAS_SPDLOG', 1)
28+
endif
29+
configure_file(output: 'config.h', configuration: logging_config_data)
30+
31+
# Public logging headers. The build-generated config.h and the internal
32+
# SpdLogger header are intentionally NOT installed.
1833
install_headers(
1934
[
2035
'cerr_logger.h',

0 commit comments

Comments
 (0)