diff --git a/test/support/src/helpers.h b/test/support/src/helpers.h index 3aecbc6f817..bf179ff434a 100644 --- a/test/support/src/helpers.h +++ b/test/support/src/helpers.h @@ -33,7 +33,6 @@ #ifndef TILEDB_TEST_HELPERS_H #define TILEDB_TEST_HELPERS_H -#include #include "test/support/src/coords_workaround.h" #include "test/support/src/mem_helpers.h" #include "tiledb.h" diff --git a/tiledb/common/logger.cc b/tiledb/common/logger.cc index ce2e8d769f5..2c7e5235662 100644 --- a/tiledb/common/logger.cc +++ b/tiledb/common/logger.cc @@ -34,6 +34,8 @@ #include #include #include +#include + #ifndef _WIN32 #include #endif @@ -42,6 +44,24 @@ namespace tiledb::common { +spdlog::level::level_enum to_spdlog_level(Logger::Level lvl) { + switch (lvl) { + case Logger::Level::FATAL: + return spdlog::level::critical; + case Logger::Level::ERR: + return spdlog::level::err; + case Logger::Level::WARN: + return spdlog::level::warn; + case Logger::Level::INFO: + return spdlog::level::info; + case Logger::Level::DBG: + return spdlog::level::debug; + default: + assert(lvl == Logger::Level::TRACE); + return spdlog::level::trace; + } +} + /* ********************************* */ /* CONSTRUCTORS & DESTRUCTORS */ /* ********************************* */ @@ -189,6 +209,10 @@ void Logger::fatal(const std::stringstream& msg) { exit(1); } +bool Logger::should_log(Logger::Level lvl) { + return logger_->should_log(to_spdlog_level(lvl)); +} + void Logger::set_level(Logger::Level lvl) { switch (lvl) { case Logger::Level::FATAL: diff --git a/tiledb/common/logger.h b/tiledb/common/logger.h index da7a3d9733c..f0de1df2a6c 100644 --- a/tiledb/common/logger.h +++ b/tiledb/common/logger.h @@ -29,29 +29,14 @@ * * This file defines class Logger, which is implemented as a wrapper around * `spdlog`. By policy `spdlog` must remain encapsulated as an implementation - * and not be exposed as a dependency of the TileDB library. Accordingly, this - * header should not be included as a header in any other header file. For - * inclusion in a header (notably for use within the definition of - * template-dependent functions), include the header `logger_public.h`. - * - * The reason for this restriction is a technical limitation in template - * instantiation. Part of the interface to `spdlog` consists of template - * functions with variadic template arguments. Instantiation of such function - * does not instantiate a variadic function (for exmaple `printf`) but rather a - * function with a fixed number of arguments that depend upon the argument list. - * Such variadic template argument lists cannot be forwarded across the - * boundaries of compilation units, so exposing variadic template arguments - * necessarily exposes the dependency upon `spdlog`. Thus this file `logger.h`, - * which does have such arguments, must remain entirely within the library, but - * `logger_public.h`, which does not have such arguments, may be exposed without - * creating an additional external dependency. + * and not be exposed as a dependency of the TileDB library. */ #pragma once #ifndef TILEDB_LOGGER_H #define TILEDB_LOGGER_H -#include +#include #include #include @@ -59,6 +44,10 @@ #include "tiledb/common/heap_memory.h" #include "tiledb/common/status.h" +namespace spdlog { +class logger; +} + namespace tiledb { namespace common { @@ -123,12 +112,15 @@ class Logger { * * @param fmt A fmtlib format string, see http://fmtlib.net/latest/ for * details. - * @param arg positional argument to format. - * @param args optional additional positional arguments to format. + * @param args positional arguments to format. */ - template - void trace(std::string_view fmt, const Arg1& arg1, const Args&... args) { - logger_->trace(fmt::runtime(fmt), arg1, args...); + template + void trace(fmt::format_string fmt, Args&&... args) { + // Check that this level is enabled to avoid needlessly formatting the + // string. + if (!should_log(Level::TRACE)) + return; + trace(fmt::format(fmt, std::forward(args)...)); } /** @@ -157,12 +149,15 @@ class Logger { * * @param fmt A fmtlib format string, see http://fmtlib.net/latest/ for * details. - * @param arg positional argument to format. - * @param args optional additional positional arguments to format. + * @param args positional arguments to format. */ - template - void debug(std::string_view fmt, const Arg1& arg1, const Args&... args) { - logger_->debug(fmt::runtime(fmt), arg1, args...); + template + void debug(fmt::format_string fmt, Args&&... args) { + // Check that this level is enabled to avoid needlessly formatting the + // string. + if (!should_log(Level::DBG)) + return; + debug(fmt::format(fmt, std::forward(args)...)); } /** @@ -191,12 +186,15 @@ class Logger { * * @param fmt A fmtlib format string, see http://fmtlib.net/latest/ for * details. - * @param arg positional argument to format. - * @param args optional additional positional arguments to format. + * @param args positional arguments to format. */ - template - void info(std::string_view fmt, const Arg1& arg1, const Args&... args) { - logger_->info(fmt::runtime(fmt), arg1, args...); + template + void info(fmt::format_string fmt, Args&&... args) { + // Check that this level is enabled to avoid needlessly formatting the + // string. + if (!should_log(Level::INFO)) + return; + info(fmt::format(fmt, std::forward(args)...)); } /** @@ -225,12 +223,15 @@ class Logger { * * @param fmt A fmtlib format string, see http://fmtlib.net/latest/ for * details. - * @param arg positional argument to format. - * @param args optional additional positional arguments to format. + * @param args positional arguments to format. */ - template - void warn(std::string_view fmt, const Arg1& arg1, const Args&... args) { - logger_->warn(fmt::runtime(fmt), arg1, args...); + template + void warn(fmt::format_string fmt, Args&&... args) { + // Check that this level is enabled to avoid needlessly formatting the + // string. + if (!should_log(Level::WARN)) + return; + warn(fmt::format(fmt, std::forward(args)...)); } /** @@ -258,12 +259,15 @@ class Logger { * * @param fmt A fmtlib format string, see http://fmtlib.net/latest/ for * details. - * @param arg1 positional argument to format. - * @param args optional additional positional arguments to format. + * @param args positional arguments to format. */ - template - void error(std::string_view fmt, const Arg1& arg1, const Args&... args) { - logger_->error(fmt::runtime(fmt), arg1, args...); + template + void error(fmt::format_string fmt, Args&&... args) { + // Check that this level is enabled to avoid needlessly formatting the + // string. + if (!should_log(Level::ERR)) + return; + error(fmt::format(fmt, std::forward(args)...)); } /** @@ -287,6 +291,22 @@ class Logger { */ void critical(const std::stringstream& msg); + /** + * A formatted critical statment. + * + * @param fmt A fmtlib format string, see http://fmtlib.net/latest/ for + * details. + * @param args positional arguments to format. + */ + template + void critical(fmt::format_string fmt, Args&&... args) { + // Check that this level is enabled to avoid needlessly formatting the + // string. + if (!should_log(Level::FATAL)) + return; + critical(fmt::format(fmt, std::forward(args)...)); + } + /** * Log a message from a Status object and return * the same Status object @@ -324,17 +344,11 @@ class Logger { void fatal(const std::stringstream& msg); /** - * A formatted critical statment. + * Check whether events of a given level are logged. * - * @param fmt A fmtlib format string, see http://fmtlib.net/latest/ for - * details. - * @param arg positional argument to format. - * @param args optional additional positional arguments to format. + * @param lvl The level of events to check. */ - template - void critical(std::string_view fmt, const Arg1& arg1, const Args&... args) { - logger_->critical(fmt::runtime(fmt), arg1, args...); - } + bool should_log(Logger::Level lvl); /** * Set the logger level. diff --git a/tiledb/common/logger_public.h b/tiledb/common/logger_public.h index 58e489feb87..fa64ca3828f 100644 --- a/tiledb/common/logger_public.h +++ b/tiledb/common/logger_public.h @@ -28,8 +28,9 @@ * @section DESCRIPTION * * This file defines simple logging functions that can be exposed (by expedient) - * to the public API. Their implementations are in `logger.cc`. See the - * documentation in `logger.h` for the full story. + * to the public API. Their implementations are in `logger.cc`. They are + * declared in a separate header from `logger.h` for historical reasons, when + * `logger.h` was leaking spdlog's headers. */ #pragma once diff --git a/tiledb/sm/array/consistency.h b/tiledb/sm/array/consistency.h index 67a265e8dad..53eb34138d0 100644 --- a/tiledb/sm/array/consistency.h +++ b/tiledb/sm/array/consistency.h @@ -37,7 +37,6 @@ #include #include "tiledb/common/common.h" -#include "tiledb/common/logger.h" #include "tiledb/sm/enums/query_type.h" #include "tiledb/sm/filesystem/uri.h" diff --git a/tiledb/sm/array/test/unit_consistency.cc b/tiledb/sm/array/test/unit_consistency.cc index af24db27845..c2e2b7cf228 100644 --- a/tiledb/sm/array/test/unit_consistency.cc +++ b/tiledb/sm/array/test/unit_consistency.cc @@ -33,6 +33,7 @@ #include #include +#include "tiledb/common/logger.h" #include "unit_consistency.h" using namespace tiledb; diff --git a/tiledb/sm/array_schema/dimension.h b/tiledb/sm/array_schema/dimension.h index 45665f6e3ea..a62f78a8ae7 100644 --- a/tiledb/sm/array_schema/dimension.h +++ b/tiledb/sm/array_schema/dimension.h @@ -43,7 +43,6 @@ #include "tiledb/common/blank.h" #include "tiledb/common/common.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/macros.h" #include "tiledb/common/memory_tracker.h" #include "tiledb/common/status.h" diff --git a/tiledb/sm/compressors/dict_compressor.h b/tiledb/sm/compressors/dict_compressor.h index 1aff103ffb5..2dc3c7e6a0d 100644 --- a/tiledb/sm/compressors/dict_compressor.h +++ b/tiledb/sm/compressors/dict_compressor.h @@ -34,7 +34,6 @@ #define TILEDB_DICT_COMPRESSOR_H #include "tiledb/common/common.h" -#include "tiledb/common/logger.h" #include "tiledb/sm/misc/endian.h" #include diff --git a/tiledb/sm/consolidator/array_meta_consolidator.h b/tiledb/sm/consolidator/array_meta_consolidator.h index 6da4cc99748..0393db98e70 100644 --- a/tiledb/sm/consolidator/array_meta_consolidator.h +++ b/tiledb/sm/consolidator/array_meta_consolidator.h @@ -35,7 +35,6 @@ #include "tiledb/common/common.h" #include "tiledb/common/heap_memory.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array/array.h" #include "tiledb/sm/consolidator/consolidator.h" diff --git a/tiledb/sm/consolidator/commits_consolidator.h b/tiledb/sm/consolidator/commits_consolidator.h index 27ec1c4375b..b5c1100b4ef 100644 --- a/tiledb/sm/consolidator/commits_consolidator.h +++ b/tiledb/sm/consolidator/commits_consolidator.h @@ -35,7 +35,6 @@ #include "tiledb/common/common.h" #include "tiledb/common/heap_memory.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array/array.h" #include "tiledb/sm/consolidator/consolidator.h" diff --git a/tiledb/sm/consolidator/consolidator.h b/tiledb/sm/consolidator/consolidator.h index ddc1b8aea64..8f324995a66 100644 --- a/tiledb/sm/consolidator/consolidator.h +++ b/tiledb/sm/consolidator/consolidator.h @@ -35,7 +35,6 @@ #include "tiledb/common/common.h" #include "tiledb/common/heap_memory.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array/array.h" #include "tiledb/sm/storage_manager/storage_manager_declaration.h" diff --git a/tiledb/sm/consolidator/fragment_consolidator.h b/tiledb/sm/consolidator/fragment_consolidator.h index 4c3afb03e54..f1a5141e45e 100644 --- a/tiledb/sm/consolidator/fragment_consolidator.h +++ b/tiledb/sm/consolidator/fragment_consolidator.h @@ -35,7 +35,6 @@ #include "tiledb/common/common.h" #include "tiledb/common/heap_memory.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/pmr.h" #include "tiledb/common/status.h" #include "tiledb/sm/array/array.h" diff --git a/tiledb/sm/consolidator/fragment_meta_consolidator.h b/tiledb/sm/consolidator/fragment_meta_consolidator.h index 230d3490f5b..eb5027b0c17 100644 --- a/tiledb/sm/consolidator/fragment_meta_consolidator.h +++ b/tiledb/sm/consolidator/fragment_meta_consolidator.h @@ -35,7 +35,6 @@ #include "tiledb/common/common.h" #include "tiledb/common/heap_memory.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array/array.h" #include "tiledb/sm/consolidator/consolidator.h" diff --git a/tiledb/sm/filesystem/posix.h b/tiledb/sm/filesystem/posix.h index 1c397e8e21c..a97e1a0f678 100644 --- a/tiledb/sm/filesystem/posix.h +++ b/tiledb/sm/filesystem/posix.h @@ -45,7 +45,6 @@ #include #include -#include "tiledb/common/logger.h" #include "tiledb/common/status.h" #include "tiledb/sm/config/config.h" #include "tiledb/sm/filesystem/filesystem_base.h" diff --git a/tiledb/sm/misc/utils.h b/tiledb/sm/misc/utils.h index 2dbae1d5f36..804610a968e 100644 --- a/tiledb/sm/misc/utils.h +++ b/tiledb/sm/misc/utils.h @@ -41,7 +41,6 @@ #include #include "constants.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" namespace tiledb { diff --git a/tiledb/sm/query/legacy/reader.h b/tiledb/sm/query/legacy/reader.h index d544edee033..6cec8f488a7 100644 --- a/tiledb/sm/query/legacy/reader.h +++ b/tiledb/sm/query/legacy/reader.h @@ -37,7 +37,6 @@ #include "tiledb/common/common.h" #include "tiledb/common/indexed_list.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array_schema/dimension.h" #include "tiledb/sm/query/iquery_strategy.h" diff --git a/tiledb/sm/query/query.h b/tiledb/sm/query/query.h index b8d09331025..a2dff571217 100644 --- a/tiledb/sm/query/query.h +++ b/tiledb/sm/query/query.h @@ -39,7 +39,6 @@ #include #include -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array_schema/array_schema.h" #include "tiledb/sm/array_schema/dimension.h" diff --git a/tiledb/sm/query/readers/dense_reader.h b/tiledb/sm/query/readers/dense_reader.h index dcbf60f3fa6..e44c7903fdc 100644 --- a/tiledb/sm/query/readers/dense_reader.h +++ b/tiledb/sm/query/readers/dense_reader.h @@ -36,7 +36,6 @@ #include #include "tiledb/common/common.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array_schema/dimension.h" #include "tiledb/sm/array_schema/dynamic_array.h" diff --git a/tiledb/sm/query/readers/ordered_dim_label_reader.h b/tiledb/sm/query/readers/ordered_dim_label_reader.h index 3c5dd9ff8ef..afb8ad1fc3d 100644 --- a/tiledb/sm/query/readers/ordered_dim_label_reader.h +++ b/tiledb/sm/query/readers/ordered_dim_label_reader.h @@ -36,7 +36,6 @@ #include #include "tiledb/common/common.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array_schema/dimension.h" #include "tiledb/sm/misc/types.h" diff --git a/tiledb/sm/query/readers/sparse_global_order_reader.h b/tiledb/sm/query/readers/sparse_global_order_reader.h index d213af6a253..c113dc6c612 100644 --- a/tiledb/sm/query/readers/sparse_global_order_reader.h +++ b/tiledb/sm/query/readers/sparse_global_order_reader.h @@ -36,7 +36,6 @@ #include #include "tiledb/common/common.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array_schema/dimension.h" #include "tiledb/sm/query/iquery_strategy.h" diff --git a/tiledb/sm/query/readers/sparse_unordered_with_dups_reader.h b/tiledb/sm/query/readers/sparse_unordered_with_dups_reader.h index 7f271682c5f..1ec82e2c70c 100644 --- a/tiledb/sm/query/readers/sparse_unordered_with_dups_reader.h +++ b/tiledb/sm/query/readers/sparse_unordered_with_dups_reader.h @@ -36,7 +36,6 @@ #include #include "tiledb/common/common.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array_schema/dimension.h" #include "tiledb/sm/query/iquery_strategy.h" diff --git a/tiledb/sm/query/strategy_base.h b/tiledb/sm/query/strategy_base.h index d1d157c10c5..8aa91b6301a 100644 --- a/tiledb/sm/query/strategy_base.h +++ b/tiledb/sm/query/strategy_base.h @@ -34,7 +34,6 @@ #define TILEDB_STRATEGY_BASE_H #include "tiledb/common/common.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array_schema/dimension.h" #include "tiledb/sm/misc/types.h" diff --git a/tiledb/sm/query/writers/dense_tiler.h b/tiledb/sm/query/writers/dense_tiler.h index fdcfa6d5789..9404e0c5de7 100644 --- a/tiledb/sm/query/writers/dense_tiler.h +++ b/tiledb/sm/query/writers/dense_tiler.h @@ -38,7 +38,6 @@ #include #include "tiledb/common/common.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/sm/array_schema/array_schema.h" #include "tiledb/sm/query/query_buffer.h" diff --git a/tiledb/sm/query_plan/test/unit_query_plan.cc b/tiledb/sm/query_plan/test/unit_query_plan.cc index 02776c226f1..ba33ddb15df 100644 --- a/tiledb/sm/query_plan/test/unit_query_plan.cc +++ b/tiledb/sm/query_plan/test/unit_query_plan.cc @@ -36,6 +36,7 @@ #include "../query_plan.h" #include "external/include/nlohmann/json.hpp" #include "test/support/src/mem_helpers.h" +#include "tiledb/common/logger.h" #include "tiledb/sm/array/array.h" #include "tiledb/sm/array_schema/dimension.h" #include "tiledb/sm/enums/array_type.h" diff --git a/tiledb/sm/rest/curl.h b/tiledb/sm/rest/curl.h index dc7bf103646..3270f3ce5b6 100644 --- a/tiledb/sm/rest/curl.h +++ b/tiledb/sm/rest/curl.h @@ -48,7 +48,6 @@ #include #include "tiledb/common/dynamic_memory/dynamic_memory.h" -#include "tiledb/common/logger_public.h" #include "tiledb/sm/buffer/buffer.h" #include "tiledb/sm/buffer/buffer_list.h" #include "tiledb/sm/config/config.h" @@ -59,6 +58,11 @@ using namespace tiledb::common; namespace tiledb { + +namespace common { +class Logger; +} + namespace sm { /** diff --git a/tiledb/sm/rest/rest_client.h b/tiledb/sm/rest/rest_client.h index 25eb29507dd..3c03328127f 100644 --- a/tiledb/sm/rest/rest_client.h +++ b/tiledb/sm/rest/rest_client.h @@ -36,7 +36,6 @@ #include #include -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/common/thread_pool.h" #include "tiledb/sm/group/group.h" diff --git a/tiledb/sm/storage_manager/context.h b/tiledb/sm/storage_manager/context.h index cfed00dc1bb..a4811694fb0 100644 --- a/tiledb/sm/storage_manager/context.h +++ b/tiledb/sm/storage_manager/context.h @@ -34,7 +34,6 @@ #define TILEDB_CONTEXT_H #include "tiledb/common/exception/exception.h" -#include "tiledb/common/logger.h" #include "tiledb/common/thread_pool/thread_pool.h" #include "tiledb/sm/config/config.h" #include "tiledb/sm/stats/global_stats.h" @@ -43,6 +42,10 @@ #include +namespace tiledb::common { +class Logger; +} + using namespace tiledb::common; namespace tiledb::sm { diff --git a/tiledb/sm/storage_manager/context_resources.h b/tiledb/sm/storage_manager/context_resources.h index dea51cfde41..515dcdd450b 100644 --- a/tiledb/sm/storage_manager/context_resources.h +++ b/tiledb/sm/storage_manager/context_resources.h @@ -34,7 +34,6 @@ #define TILEDB_CONTEXT_RESOURCES_H #include "tiledb/common/exception/exception.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/thread_pool/thread_pool.h" #include "tiledb/sm/config/config.h" #include "tiledb/sm/filesystem/vfs.h" diff --git a/tiledb/sm/storage_manager/storage_manager_canonical.h b/tiledb/sm/storage_manager/storage_manager_canonical.h index 9f5017d7a2d..2a255ebd239 100644 --- a/tiledb/sm/storage_manager/storage_manager_canonical.h +++ b/tiledb/sm/storage_manager/storage_manager_canonical.h @@ -47,7 +47,6 @@ #include "tiledb/common/common.h" #include "tiledb/common/heap_memory.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/status.h" #include "tiledb/common/thread_pool.h" #include "tiledb/sm/array/array_directory.h" diff --git a/tiledb/sm/subarray/subarray.h b/tiledb/sm/subarray/subarray.h index 30251f07469..c07854bfef2 100644 --- a/tiledb/sm/subarray/subarray.h +++ b/tiledb/sm/subarray/subarray.h @@ -36,7 +36,6 @@ #include #include "tiledb/common/common.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/thread_pool.h" #include "tiledb/sm/buffer/buffer.h" #include "tiledb/sm/config/config.h" diff --git a/tiledb/sm/subarray/subarray_partitioner.h b/tiledb/sm/subarray/subarray_partitioner.h index 2915fd7aefe..e3df1540eb1 100644 --- a/tiledb/sm/subarray/subarray_partitioner.h +++ b/tiledb/sm/subarray/subarray_partitioner.h @@ -37,7 +37,6 @@ #include #include "tiledb/common/common.h" -#include "tiledb/common/logger_public.h" #include "tiledb/common/thread_pool.h" #include "tiledb/sm/misc/constants.h" #include "tiledb/sm/stats/stats.h"