-
Notifications
You must be signed in to change notification settings - Fork 118
feat(logging): add spdlog backend behind ICEBERG_SPDLOG (5/6) #726
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
9563778
e30d45f
e549ec2
8f7c826
7f2094e
dd952f0
4386cf7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| /* | ||
| * 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 | ||
|
|
||
| // Internal, build-generated configuration for the logging backend. | ||
| // This header is NOT installed and must only be included from .cc files | ||
| // (logger.cc, internal/spdlog_logger.cc) -- never from a public header. | ||
| // | ||
| // ICEBERG_HAS_SPDLOG is defined when the project is built with -DICEBERG_SPDLOG=ON | ||
| // and left undefined otherwise. Always test it with #ifdef / #ifndef, never #if | ||
| // (it carries no value). | ||
|
|
||
| #cmakedefine ICEBERG_HAS_SPDLOG |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| /* | ||
| * 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/internal/spdlog_logger.h" | ||
|
|
||
| #ifdef ICEBERG_HAS_SPDLOG | ||
|
|
||
| # include <memory> | ||
| # include <string> | ||
| # include <unordered_map> | ||
| # include <utility> | ||
|
|
||
| # include <spdlog/common.h> | ||
| # include <spdlog/sinks/stdout_color_sinks.h> | ||
|
|
||
| # include "iceberg/util/macros.h" | ||
|
|
||
| namespace iceberg::internal { | ||
|
|
||
| namespace { | ||
|
|
||
| spdlog::level::level_enum ToSpdLevel(LogLevel level) noexcept { | ||
| switch (level) { | ||
| case LogLevel::kTrace: | ||
| return spdlog::level::trace; | ||
| case LogLevel::kDebug: | ||
| return spdlog::level::debug; | ||
| case LogLevel::kInfo: | ||
| return spdlog::level::info; | ||
| case LogLevel::kWarn: | ||
| return spdlog::level::warn; | ||
| case LogLevel::kError: | ||
| return spdlog::level::err; | ||
| case LogLevel::kCritical: | ||
| case LogLevel::kFatal: | ||
| // spdlog has no "fatal"; the process abort is owned by the macro layer. | ||
| return spdlog::level::critical; | ||
| case LogLevel::kOff: | ||
| return spdlog::level::off; | ||
| } | ||
| return spdlog::level::off; | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| SpdLogger::SpdLogger(LogLevel level) | ||
| : SpdLogger(std::make_shared<spdlog::logger>( | ||
| "iceberg", std::make_shared<spdlog::sinks::stderr_color_sink_mt>()), | ||
| level) {} | ||
|
|
||
| Status SpdLogger::Initialize( | ||
| const std::unordered_map<std::string, std::string>& properties) { | ||
| if (auto it = properties.find(std::string(kPatternProperty)); it != properties.end()) { | ||
| logger_->set_pattern(it->second); | ||
| } | ||
| // Apply "level" via the base implementation. | ||
| return Logger::Initialize(properties); | ||
| } | ||
|
|
||
| SpdLogger::SpdLogger(std::shared_ptr<spdlog::logger> logger, LogLevel level) | ||
| : logger_(std::move(logger)), level_(level) { | ||
| // logger_ is a hard precondition: SpdLogger is not consumer-constructible (it is | ||
| // obtained via the default logger or the "spdlog" registry factory, both of which | ||
| // pass a real logger), so Initialize/Log/Flush may dereference it unconditionally. | ||
| ICEBERG_DCHECK(logger_ != nullptr, "SpdLogger requires a non-null spdlog::logger"); | ||
| logger_->set_level(spdlog::level::trace); // filtering is done by ShouldLog | ||
| } | ||
|
|
||
| void SpdLogger::Log(LogMessage&& message) noexcept { | ||
| try { | ||
| spdlog::source_loc loc{message.location.file_name(), | ||
| static_cast<int>(message.location.line()), | ||
| message.location.function_name()}; | ||
| // Pass the pre-formatted text as an argument ("{}") so any braces in the | ||
| // message are not re-interpreted as a format string. | ||
| logger_->log(loc, ToSpdLevel(message.level), "{}", message.message); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This re-formats an already formatted message through fmt for every emitted record, adding another full copy and possibly an allocation for long messages. Please call the raw-message overload with |
||
| } catch (...) { | ||
| // Logging must never throw. | ||
| } | ||
| } | ||
|
|
||
| void SpdLogger::Flush() noexcept { | ||
| try { | ||
| logger_->flush(); | ||
| } catch (...) { | ||
| } | ||
| } | ||
|
|
||
| } // namespace iceberg::internal | ||
|
|
||
| #endif // ICEBERG_HAS_SPDLOG | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| /* | ||
| * 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/internal/spdlog_logger.h | ||
| /// \brief spdlog-backed logging sink. | ||
| /// | ||
| /// INTERNAL, NOT INSTALLED. Included only from .cc files (logger.cc and | ||
| /// spdlog_logger.cc). It pulls in the build-generated config.h itself and gates | ||
| /// its entire body on ICEBERG_HAS_SPDLOG, so it compiles to nothing unless the | ||
| /// project was built with ICEBERG_SPDLOG=ON. SpdLogger is not a | ||
| /// consumer-constructible public type -- applications obtain it via the default | ||
| /// logger or the "logger-impl"="spdlog" registry factory. | ||
|
|
||
| #include "iceberg/logging/config.h" | ||
|
|
||
| #ifdef ICEBERG_HAS_SPDLOG | ||
|
|
||
| # include <atomic> | ||
| # include <memory> | ||
|
|
||
| # include <spdlog/logger.h> | ||
|
|
||
| # include "iceberg/logging/log_level.h" | ||
| # include "iceberg/logging/logger.h" | ||
|
|
||
| namespace iceberg::internal { | ||
|
|
||
| /// \brief Logger backed by spdlog (synchronous only in v1). | ||
| /// | ||
| /// Synchronous because spdlog::source_loc holds non-owning const char* that are | ||
| /// unsafe to forward into an async logger (spdlog #3227). | ||
| /// ICEBERG_EXPORT so the symbol is linkable from in-tree tests (and any | ||
| /// internal consumer) under -fvisibility=hidden / MSVC DLL builds. The header | ||
| /// is still not installed -- this is a binary-visibility detail, not public API. | ||
| class ICEBERG_EXPORT SpdLogger : public Logger { | ||
| public: | ||
| /// \brief Construct over a default stderr-backed spdlog logger. | ||
| explicit SpdLogger(LogLevel level = LogLevel::kInfo); | ||
|
|
||
| /// \brief Construct over a caller-provided spdlog logger. | ||
| /// | ||
| /// The logger MUST be synchronous. Log() forwards spdlog::source_loc, which | ||
| /// borrows the std::source_location's const char* pointers; an async spdlog | ||
| /// logger would queue them past their lifetime (spdlog #3227 -> UB). This is a | ||
| /// caller contract -- spdlog exposes no reliable sync/async query to assert on. | ||
| explicit SpdLogger(std::shared_ptr<spdlog::logger> logger, | ||
| LogLevel level = LogLevel::kInfo); | ||
|
|
||
| /// \brief Apply the "pattern" property (spdlog set_pattern), then "level". | ||
| Status Initialize( | ||
| const std::unordered_map<std::string, std::string>& properties) override; | ||
|
|
||
| bool ShouldLog(LogLevel level) const noexcept override { | ||
| return level >= level_.load(std::memory_order_relaxed); | ||
| } | ||
| void Log(LogMessage&& message) noexcept override; | ||
| void SetLevel(LogLevel level) noexcept override { | ||
| level_.store(level, std::memory_order_relaxed); | ||
| } | ||
| LogLevel level() const noexcept override { | ||
| return level_.load(std::memory_order_relaxed); | ||
| } | ||
| void Flush() noexcept override; | ||
|
|
||
| private: | ||
| std::shared_ptr<spdlog::logger> logger_; | ||
| std::atomic<LogLevel> level_; | ||
| }; | ||
|
|
||
| } // namespace iceberg::internal | ||
|
|
||
| #endif // ICEBERG_HAS_SPDLOG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ICEBERG_DCHECKdisappears underNDEBUG, and the constructor dereferenceslogger_immediately afterwards. Sinceshared_ptris nullable and the header only documents the synchronous requirement, an empty input becomes a release crash. Please reject null in a factory or encode the non-null requirement in the API.