From d8c502f3f2595bbea8048d00fae42a62d3e33c2d Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Tue, 29 Sep 2026 04:01:29 -0700 Subject: [PATCH 1/8] [C API] Initial logging API definition with Logger handle. --- bindings/c/include/svs/c/svs_c.h | 154 +++++++++++++++++++++++++++++++ bindings/c/src/svs_c.cpp | 152 ++++++++++++++++++++++++++++++ 2 files changed, 306 insertions(+) diff --git a/bindings/c/include/svs/c/svs_c.h b/bindings/c/include/svs/c/svs_c.h index 5034ecdf..77e03b7c 100644 --- a/bindings/c/include/svs/c/svs_c.h +++ b/bindings/c/include/svs/c/svs_c.h @@ -58,6 +58,25 @@ enum svs_error_code { SVS_ERROR_UNKNOWN = 1000 }; +enum svs_log_level { + SVS_LOG_LEVEL_TRACE = 0, + SVS_LOG_LEVEL_DEBUG = 1, + SVS_LOG_LEVEL_INFO = 2, + SVS_LOG_LEVEL_WARN = 3, + SVS_LOG_LEVEL_ERROR = 4, + SVS_LOG_LEVEL_CRITICAL = 5, + SVS_LOG_LEVEL_OFF = 6 +}; + +enum svs_logging_kind { + SVS_LOGGING_KIND_NONE = 0, + SVS_LOGGING_KIND_STDOUT = 1, + SVS_LOGGING_KIND_STDERR = 2, + SVS_LOGGING_KIND_FILE_APPEND = 3, + SVS_LOGGING_KIND_FILE_TRUNCATE = 4, + SVS_LOGGING_KIND_CUSTOM = 5 +}; + typedef struct svs_error_desc* svs_error_h; /// @brief Distance metric used to compare vectors. @@ -124,6 +143,48 @@ enum svs_threadpool_kind { SVS_THREADPOOL_KIND_CUSTOM = 3 }; +/// @brief Operations table for a custom logging interface. +/// @remarks The user must ensure that the logging implementation is thread-safe and +/// that the provided function pointers remain valid for the lifetime of the logging +/// interface. +/// +/// @var svs_logging_interface_ops::version +/// Version of the logging interface. +/// @var svs_logging_interface_ops::struct_size +/// Size of the structure, used for versioning and compatibility checks. +/// @var svs_logging_interface_ops::log +/// Function pointer to log a message. +/// @param self Pointer to the logging interface instance. +/// @param level Logging level of the message. +/// @param message Null-terminated string containing the message to log. +/// @var svs_logging_interface_ops::flush +/// Function pointer to flush the logging output. +/// @param self Pointer to the logging interface instance. +/// @remarks This function should ensure that all pending log messages are written out. +struct svs_logging_interface_ops { + uint32_t version; + size_t struct_size; + void (*log)(void* self, enum svs_log_level level, const char* message); +}; + +/// @brief Macro to create a user-defined logging interface operations structure. +#define SVS_INIT_LOGGING_OPS(log_func) \ + { \ + .version = SVS_C_API_VERSION, \ + .struct_size = sizeof(struct svs_logging_interface_ops), .log = &log_func \ + } + +/// @brief Represents a user-defined logging interface instance. +/// @var svs_logging_interface::ops +/// Pointer to the operations table defining the behavior of the logging interface. +/// @var svs_logging_interface::self +/// Pointer to user-defined data associated with the logging interface instance. This +/// pointer is passed to the logging functions as the @p self parameter. +struct svs_logging_interface { + const struct svs_logging_interface_ops* ops; + void* self; +}; + /// @brief Operations table for a custom thread pool interface /// @remarks The user must ensure that the thread pool implementation is thread-safe and /// that the provided function pointers remain valid for the lifetime of the thread pool @@ -477,9 +538,12 @@ typedef struct svs_algorithm* svs_algorithm_h; typedef struct svs_storage* svs_storage_h; typedef struct svs_search_params* svs_search_params_h; typedef struct svs_leanvec_training_data* svs_leanvec_training_data_h; +typedef struct svs_logger* svs_logger_h; // Fully defined types; "_t" suffix indicates a fully defined struct typedef enum svs_error_code svs_error_code_t; +typedef enum svs_logging_kind svs_logging_kind_t; +typedef enum svs_log_level svs_log_level_t; typedef enum svs_distance_metric svs_distance_metric_t; typedef enum svs_algorithm_type svs_algorithm_type_t; typedef enum svs_data_type svs_data_type_t; @@ -487,6 +551,9 @@ typedef enum svs_storage_kind svs_storage_kind_t; typedef enum svs_threadpool_kind svs_threadpool_kind_t; typedef enum svs_allocator_kind svs_allocator_kind_t; +typedef struct svs_logging_interface_ops svs_logging_ops_t; +typedef struct svs_logging_interface svs_logging_t; +typedef struct svs_logging_interface* svs_logging_i; typedef struct svs_threadpool_interface_ops svs_threadpool_ops_t; typedef struct svs_threadpool_interface svs_threadpool_t; typedef struct svs_threadpool_interface* svs_threadpool_i; @@ -546,6 +613,83 @@ SVS_API const char* svs_error_get_message(svs_error_h err); /// @param err The error handle to free SVS_API void svs_error_free(svs_error_h err); +/// @brief Create a logger with default settings +/// @param out_err An optional error handle to capture errors +/// @return A handle to the created logger or NULL if creation failed +/// @remarks See SVS logger environment configuration for details on how the default logger +/// is set up. +SVS_API svs_logger_h svs_logger_create(svs_error_h out_err /*=NULL*/); + +/// @brief Set the logging kind for an existing logger +/// @param logger The logger handle to set the kind for +/// @param kind The logging kind to set +/// @param path The file path to use for file-based logging kinds (e.g., +/// SVS_LOGGING_KIND_FILE_APPEND or SVS_LOGGING_KIND_FILE_TRUNCATE) +/// @param out_err An optional error handle to capture errors +/// @return true on success, false on failure +SVS_API bool svs_logger_set_kind( + svs_logger_h logger, + svs_logging_kind_t kind, + const char* path, + svs_error_h out_err /*=NULL*/ +); + +/// @brief Create a custom logger for the SVS library +/// @param logger The logger handle to set as custom +/// @param user_logger The custom logger to create for the SVS library +/// @param level The logging level to set for the custom logger +/// @param out_err An optional error handle to capture errors +/// @return true on success, false on failure +SVS_API bool svs_logger_set_custom( + svs_logger_h logger, svs_logging_i user_logger, svs_error_h out_err /*=NULL*/ +); + +/// @brief Set the logging level for a logger +/// @param logger The logger handle +/// @param level The logging level to set +/// @param out_err An optional error handle to capture errors +/// @return true on success, false on failure +SVS_API bool svs_logger_set_level( + svs_logger_h logger, svs_log_level_t level, svs_error_h out_err /*=NULL*/ +); + +/// @brief Get the logging level for a logger +/// @param logger The logger handle +/// @param out_level Pointer to store the retrieved logging level +/// @param out_err An optional error handle to capture errors +/// @return true on success, false on failure +SVS_API bool svs_logger_get_level( + svs_logger_h logger, svs_log_level_t* out_level, svs_error_h out_err /*=NULL*/ +); + +/// @brief Set format pattern for a logger +/// @param logger The logger handle +/// @param pattern The format pattern to set +/// @param out_err An optional error handle to capture errors +/// @return true on success, false on failure +SVS_API bool svs_logger_set_pattern( + svs_logger_h logger, const char* pattern, svs_error_h out_err /*=NULL*/ +); + +/// @brief Get the format pattern for a logger +/// @param logger The logger handle +/// @param out_pattern Pointer to store the retrieved format pattern +/// @param out_err An optional error handle to capture errors +/// @return true on success, false on failure +SVS_API bool svs_logger_get_pattern( + svs_logger_h logger, const char** out_pattern, svs_error_h out_err /*=NULL*/ +); + +/// @brief Set default logger for SVS library +/// @param logger The logger handle to set as default +/// @param out_err An optional error handle to capture errors +/// @return true on success, false on failure +/// @remarks The default logger will be used for all subsequent logging operations unless +/// explicitly overridden by index builder. +SVS_API bool svs_set_default_logger( + svs_logger_h logger, svs_error_h out_err /*=NULL*/ +); + /// @brief Create a Vamana algorithm configuration /// @param graph_degree The graph degree parameter /// @param build_window_size The build window size parameter @@ -778,6 +922,16 @@ SVS_API svs_index_builder_h svs_index_builder_create( /// @param builder The index builder handle to free SVS_API void svs_index_builder_free(svs_index_builder_h builder); +/// @brief Set the logger for the index builder +/// @param builder The index builder handle +/// @param logger The logger handle to set for the index builder +/// @param path The file path to use for file-based logging kinds (if applicable) +/// @param out_err An optional error handle to capture errors +/// @return true on success, false on failure +SVS_API bool svs_index_builder_set_logger( + svs_index_builder_h builder, svs_logger_h logger, svs_error_h out_err /*=NULL*/ +); + /// @brief Set the storage configuration for the index builder /// @param builder The index builder handle /// @param storage The storage configuration handle diff --git a/bindings/c/src/svs_c.cpp b/bindings/c/src/svs_c.cpp index c8c4f953..9cbac5c2 100644 --- a/bindings/c/src/svs_c.cpp +++ b/bindings/c/src/svs_c.cpp @@ -35,10 +35,14 @@ #include #include +#include #include #include #include +#include "spdlog/pattern_formatter.h" +#include "spdlog/sinks/callback_sink.h" + // C API implementation struct svs_index { std::shared_ptr impl; @@ -64,10 +68,158 @@ struct svs_leanvec_training_data { std::shared_ptr impl; }; +struct svs_logger { + svs::logging::logger_ptr impl; +}; + extern "C" uint32_t svs_get_version() { return SVS_C_API_VERSION; } extern "C" const char* svs_get_version_string() { return SVS_C_API_VERSION_STRING; } +inline svs::logging::Level to_logging_level(svs_log_level_t level) { + switch (level) { + case SVS_LOG_LEVEL_TRACE: + return svs::logging::Level::Trace; + case SVS_LOG_LEVEL_DEBUG: + return svs::logging::Level::Debug; + case SVS_LOG_LEVEL_INFO: + return svs::logging::Level::Info; + case SVS_LOG_LEVEL_WARN: + return svs::logging::Level::Warn; + case SVS_LOG_LEVEL_ERROR: + return svs::logging::Level::Error; + case SVS_LOG_LEVEL_CRITICAL: + return svs::logging::Level::Critical; + case SVS_LOG_LEVEL_OFF: + return svs::logging::Level::Off; + default: + return svs::logging::Level::Info; + } +} + +extern "C" svs_logger_h svs_logger_create(svs_error_h out_err /*=NULL*/) { + using namespace svs::c_runtime; + return wrap_exceptions( + [&]() { + auto logger = svs::logging::detail::default_logger(); + return new svs_logger{std::move(logger)}; + }, + out_err + ); +} + +extern "C" bool svs_logger_set_kind( + svs_logger_h logger, + svs_logging_kind_t kind, + const char* path, + svs_error_h out_err /*=NULL*/ +) { + using namespace svs::c_runtime; + using logger_type = svs::logging::logger_ptr::element_type; + return wrap_exceptions( + [&]() { + INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); + svs::logging::sink_ptr sink{}; + switch (kind) { + case SVS_LOGGING_KIND_NONE: + sink = svs::logging::null_sink(); + break; + case SVS_LOGGING_KIND_STDOUT: + sink = svs::logging::stdout_sink(); + break; + case SVS_LOGGING_KIND_STDERR: + sink = svs::logging::stderr_sink(); + break; + case SVS_LOGGING_KIND_FILE_APPEND: + case SVS_LOGGING_KIND_FILE_TRUNCATE: + INVALID_ARGUMENT_IF( + path == nullptr || std::string(path).empty(), + "File path must be provided for file logging kind" + ); + sink = svs::logging::file_sink( + path, kind == SVS_LOGGING_KIND_FILE_TRUNCATE + ); + break; + case SVS_LOGGING_KIND_CUSTOM: + INVALID_ARGUMENT_IF( + true, + "Custom logging kind to be set using svs_default_logger_set_custom" + ); + default: + INVALID_ARGUMENT_IF(true, "Invalid logging kind"); + } + auto log_ptr = std::make_shared(std::move(sink)); + logger->impl = std::move(log_ptr); + return true; + }, + out_err + ); +} + +extern "C" bool svs_logger_set_custom( + svs_logger_h logger, svs_logging_i user_logger, svs_error_h out_err /*=NULL*/ +) { + using namespace svs::c_runtime; + using logger_type = svs::logging::logger_ptr::element_type; + return wrap_exceptions( + [&]() { + INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); + INVALID_ARGUMENT_IF(user_logger == nullptr, "Custom logger must not be null"); + INVALID_ARGUMENT_IF( + user_logger->ops == nullptr, "Custom logger ops must not be null" + ); + INVALID_ARGUMENT_IF( + user_logger->ops->log == nullptr, + "Custom logger ops must have a valid log function" + ); + // Kind of the custom logger implementation: + auto self = user_logger->self; + auto log = user_logger->ops->log; + auto callback_closure = [self, log](const auto& log_msg) { + static spdlog::pattern_formatter formatter; + /*static?*/ spdlog::memory_buf_t formatted; + formatter.format(log_msg, formatted); + log(self, + static_cast(log_msg.level), + fmt::to_string(formatted).c_str()); + }; + + auto callback_sink = spdlog::sinks::callback_sink_mt(callback_closure); + auto log_ptr = std::make_shared(std::move(callback_sink)); + logger->impl = std::move(log_ptr); + return true; + }, + out_err + ); +} + +extern "C" bool svs_logger_set_level( + svs_logger_h logger, svs_log_level_t level, svs_error_h out_err /*=NULL*/ +) { + using namespace svs::c_runtime; + return wrap_exceptions( + [&]() { + INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); + // Set the logging level for the logger here + svs::logging::set_level(logger->impl, to_logging_level(level)); + return true; + }, + out_err + ); +} + +extern "C" bool svs_set_default_logger(svs_logger_h logger, svs_error_h out_err /*=NULL*/) { + using namespace svs::c_runtime; + return wrap_exceptions( + [&]() { + INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); + svs::logging::set(logger->impl); + return true; + }, + out_err + ); +} + extern "C" svs_algorithm_h svs_algorithm_create_vamana( size_t graph_degree, size_t build_window_size, From 959176227265329bac5363f2e059e135da1819ef Mon Sep 17 00:00:00 2001 From: yuejiaointel Date: Thu, 1 Oct 2026 14:04:06 -0700 Subject: [PATCH 2/8] [C API] Complete logger handle implementation and tests Builds on the initial logger handle API definition: - fix compile errors; add CallbackSink (base_sink + own formatter) so svs_logger_set_pattern applies to custom loggers and is thread-safe - validate custom logger ops version/struct_size - add svs_logger_free; implement get_level, set_pattern, get_pattern - svs_set_default_logger(NULL) restores the SVS default logger - new logger starts with no output (no env read / file reopen) - output changes apply in place to all users of a logger (dist_sink) - default pattern %v: callback receives the bare message - tests: c_api_logging.cpp; C99 consumer uses SVS_INIT_LOGGING_OPS --- bindings/c/CMakeLists.txt | 1 + bindings/c/include/svs/c/svs_c.h | 68 ++- bindings/c/src/logger.hpp | 108 +++++ bindings/c/src/svs_c.cpp | 132 ++++-- bindings/c/tests/CMakeLists.txt | 1 + bindings/c/tests/README.md | 12 + bindings/c/tests/c_api_logging.cpp | 656 +++++++++++++++++++++++++++++ bindings/c/tests/consumer/main.c | 37 ++ 8 files changed, 957 insertions(+), 58 deletions(-) create mode 100644 bindings/c/src/logger.hpp create mode 100644 bindings/c/tests/c_api_logging.cpp diff --git a/bindings/c/CMakeLists.txt b/bindings/c/CMakeLists.txt index 3a526590..359b4c69 100644 --- a/bindings/c/CMakeLists.txt +++ b/bindings/c/CMakeLists.txt @@ -38,6 +38,7 @@ set(SVS_C_API_SOURCES src/index.hpp src/index_builder.hpp src/leanvec_training_data.hpp + src/logger.hpp src/storage.hpp src/threadpool.hpp src/types_support.hpp diff --git a/bindings/c/include/svs/c/svs_c.h b/bindings/c/include/svs/c/svs_c.h index 77e03b7c..51ed25c9 100644 --- a/bindings/c/include/svs/c/svs_c.h +++ b/bindings/c/include/svs/c/svs_c.h @@ -144,9 +144,17 @@ enum svs_threadpool_kind { }; /// @brief Operations table for a custom logging interface. -/// @remarks The user must ensure that the logging implementation is thread-safe and -/// that the provided function pointers remain valid for the lifetime of the logging -/// interface. +/// @remarks Lifetime: the operations table, the function it points to and the @p self +/// pointer of the owning svs_logging_interface must remain valid for as long as any +/// logger configured with them is in use, i.e. while the svs_logger_h handle, any index +/// built with it, or the SVS global default logger (see svs_set_default_logger) still +/// refers to it. Once svs_logger_set_kind or svs_logger_set_custom has replaced the +/// output of the handle and returned, the previous callback is no longer called. +/// @remarks Thread safety: the @p log function may be called from SVS worker threads. +/// Calls through one logger handle are serialized by an internal mutex, and the +/// callback runs while that mutex is held. The callback must not call back into SVS +/// logging functions (it would deadlock), and must not throw C++ exceptions or +/// longjmp out of the call. /// /// @var svs_logging_interface_ops::version /// Version of the logging interface. @@ -156,11 +164,9 @@ enum svs_threadpool_kind { /// Function pointer to log a message. /// @param self Pointer to the logging interface instance. /// @param level Logging level of the message. -/// @param message Null-terminated string containing the message to log. -/// @var svs_logging_interface_ops::flush -/// Function pointer to flush the logging output. -/// @param self Pointer to the logging interface instance. -/// @remarks This function should ensure that all pending log messages are written out. +/// @param message Null-terminated string containing the message formatted with the +/// logger pattern (see svs_logger_set_pattern), without a trailing newline. The +/// pointer is only valid for the duration of the call. struct svs_logging_interface_ops { uint32_t version; size_t struct_size; @@ -616,10 +622,26 @@ SVS_API void svs_error_free(svs_error_h err); /// @brief Create a logger with default settings /// @param out_err An optional error handle to capture errors /// @return A handle to the created logger or NULL if creation failed -/// @remarks See SVS logger environment configuration for details on how the default logger -/// is set up. +/// @remarks A new logger has no output until svs_logger_set_kind() or +/// svs_logger_set_custom() is called. Its level is SVS_LOG_LEVEL_WARN and its pattern is +/// "%v" (the bare message). The SVS_LOG_SINK / SVS_LOG_LEVEL environment variables are +/// not read here; they only configure the SVS global default logger. +/// @remarks Output, level and pattern changes made through a handle apply immediately to +/// everything already using its logger (the SVS global default logger and indexes). +/// @remarks Thread safety: the functions that modify a logger handle +/// (svs_logger_set_kind, svs_logger_set_custom, svs_logger_set_level, +/// svs_logger_set_pattern, svs_logger_free) are not thread-safe. Do not call them +/// concurrently with each other or with other functions using the same handle. SVS_API svs_logger_h svs_logger_create(svs_error_h out_err /*=NULL*/); +/// @brief Free a logger handle +/// @param logger The logger handle to free. Passing NULL is a no-op. +/// @remarks The SVS global default logger (see svs_set_default_logger) keeps its own +/// reference to the underlying logger, so freeing the handle does not stop it from +/// logging. User data referenced by a custom logger must stay valid as long as such a +/// reference exists. +SVS_API void svs_logger_free(svs_logger_h logger); + /// @brief Set the logging kind for an existing logger /// @param logger The logger handle to set the kind for /// @param kind The logging kind to set @@ -627,6 +649,9 @@ SVS_API svs_logger_h svs_logger_create(svs_error_h out_err /*=NULL*/); /// SVS_LOGGING_KIND_FILE_APPEND or SVS_LOGGING_KIND_FILE_TRUNCATE) /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure +/// @remarks SVS_LOGGING_KIND_CUSTOM is rejected; use svs_logger_set_custom instead. +/// @remarks The current level and pattern of the handle are kept. The new output applies +/// immediately to everything already using this logger. SVS_API bool svs_logger_set_kind( svs_logger_h logger, svs_logging_kind_t kind, @@ -637,9 +662,13 @@ SVS_API bool svs_logger_set_kind( /// @brief Create a custom logger for the SVS library /// @param logger The logger handle to set as custom /// @param user_logger The custom logger to create for the SVS library -/// @param level The logging level to set for the custom logger /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure +/// @remarks @p user_logger->ops must be initialized with SVS_INIT_LOGGING_OPS; the +/// version and struct_size fields are validated. See svs_logging_interface_ops for the +/// lifetime and thread-safety requirements of the callback. +/// @remarks The current level and pattern of the handle are kept. The new output applies +/// immediately to everything already using this logger. SVS_API bool svs_logger_set_custom( svs_logger_h logger, svs_logging_i user_logger, svs_error_h out_err /*=NULL*/ ); @@ -649,6 +678,7 @@ SVS_API bool svs_logger_set_custom( /// @param level The logging level to set /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure +/// @remarks Applies immediately to everything already using this logger. SVS_API bool svs_logger_set_level( svs_logger_h logger, svs_log_level_t level, svs_error_h out_err /*=NULL*/ ); @@ -664,16 +694,23 @@ SVS_API bool svs_logger_get_level( /// @brief Set format pattern for a logger /// @param logger The logger handle -/// @param pattern The format pattern to set +/// @param pattern The format pattern to set, in spdlog pattern syntax (e.g. "%v" for +/// the bare message, or "[index A] %v" to prefix every message). Must not be NULL. /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure +/// @remarks The pattern applies to all output kinds and is kept across +/// svs_logger_set_kind / svs_logger_set_custom. Applies immediately to everything +/// already using this logger. SVS_API bool svs_logger_set_pattern( svs_logger_h logger, const char* pattern, svs_error_h out_err /*=NULL*/ ); /// @brief Get the format pattern for a logger /// @param logger The logger handle -/// @param out_pattern Pointer to store the retrieved format pattern +/// @param out_pattern Pointer to store the retrieved format pattern. The string is +/// owned by the logger handle and stays valid until the next svs_logger_set_pattern +/// call on the handle or until svs_logger_free. If no pattern was set, returns the +/// default pattern "%v". /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure SVS_API bool svs_logger_get_pattern( @@ -681,7 +718,9 @@ SVS_API bool svs_logger_get_pattern( ); /// @brief Set default logger for SVS library -/// @param logger The logger handle to set as default +/// @param logger The logger handle to set as default. NULL restores the SVS built-in +/// default logger (configured from the SVS_LOG_LEVEL / SVS_LOG_SINK environment +/// variables). /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure /// @remarks The default logger will be used for all subsequent logging operations unless @@ -925,7 +964,6 @@ SVS_API void svs_index_builder_free(svs_index_builder_h builder); /// @brief Set the logger for the index builder /// @param builder The index builder handle /// @param logger The logger handle to set for the index builder -/// @param path The file path to use for file-based logging kinds (if applicable) /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure SVS_API bool svs_index_builder_set_logger( diff --git a/bindings/c/src/logger.hpp b/bindings/c/src/logger.hpp new file mode 100644 index 00000000..42acc53b --- /dev/null +++ b/bindings/c/src/logger.hpp @@ -0,0 +1,108 @@ +/* + * Copyright 2026 Intel Corporation + * + * Licensed 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 "svs/c/svs_c.h" + +#include "error.hpp" + +#include + +#include "spdlog/sinks/base_sink.h" +#include "spdlog/sinks/dist_sink.h" + +#include +#include +#include +#include + +namespace svs { +namespace c_runtime { + +// spdlog level values are passed to the user callback by a plain cast. +static_assert(static_cast(SVS_LOG_LEVEL_TRACE) == SPDLOG_LEVEL_TRACE); +static_assert(static_cast(SVS_LOG_LEVEL_DEBUG) == SPDLOG_LEVEL_DEBUG); +static_assert(static_cast(SVS_LOG_LEVEL_INFO) == SPDLOG_LEVEL_INFO); +static_assert(static_cast(SVS_LOG_LEVEL_WARN) == SPDLOG_LEVEL_WARN); +static_assert(static_cast(SVS_LOG_LEVEL_ERROR) == SPDLOG_LEVEL_ERROR); +static_assert(static_cast(SVS_LOG_LEVEL_CRITICAL) == SPDLOG_LEVEL_CRITICAL); +static_assert(static_cast(SVS_LOG_LEVEL_OFF) == SPDLOG_LEVEL_OFF); + +/// Name given to every spdlog logger created by the C API. +inline constexpr const char* logger_name = "svs_c"; + +/// Default pattern of every logger handle, for all output kinds: the bare message. +/// Hosts that forward messages to their own logger (custom kind) add their own timestamp +/// and level, so SVS does not add them by default. +inline constexpr const char* default_log_pattern = "%v"; + +/// A spdlog sink forwarding formatted messages to a user-provided C callback. +/// +/// Messages are formatted with the sink's own formatter, so the logger pattern +/// (svs_logger_set_pattern) applies. The trailing end-of-line added by the formatter is +/// stripped before calling the user callback. +/// The base_sink mutex serializes calls to the user callback. +class CallbackSink final : public spdlog::sinks::base_sink { + public: + using log_func_t = void (*)(void*, enum svs_log_level, const char*); + + static void validate(const svs_logging_i user_logger) { + if (user_logger == nullptr) { + throw std::invalid_argument("Custom logger pointer cannot be null."); + } + if (user_logger->ops == nullptr) { + throw std::invalid_argument("Custom logger interface is not initialized."); + } + if (user_logger->ops->version > svs_get_version()) { + throw std::invalid_argument("Custom logger interface version is not supported." + ); + } + if (user_logger->ops->struct_size < sizeof(svs_logging_ops_t)) { + throw std::invalid_argument("Incompatible custom logger interface struct size." + ); + } + if (user_logger->ops->log == nullptr) { + throw std::invalid_argument("Custom logger interface has null log function."); + } + } + + /// Copies the log function pointer and the user's self pointer. + /// @p user_logger must have been checked with validate(). + explicit CallbackSink(const svs_logging_i user_logger) + : log_{user_logger->ops->log} + , self_{user_logger->self} {} + + protected: + void sink_it_(const spdlog::details::log_msg& msg) override { + spdlog::memory_buf_t formatted; + this->formatter_->format(msg, formatted); + auto text = std::string(formatted.data(), formatted.size()); + // Strip the end-of-line appended by the formatter. + while (!text.empty() && (text.back() == '\n' || text.back() == '\r')) { + text.pop_back(); + } + log_(self_, static_cast(msg.level), text.c_str()); + } + + void flush_() override {} + + private: + log_func_t log_; + void* self_; +}; + +} // namespace c_runtime +} // namespace svs diff --git a/bindings/c/src/svs_c.cpp b/bindings/c/src/svs_c.cpp index 9cbac5c2..79d49a67 100644 --- a/bindings/c/src/svs_c.cpp +++ b/bindings/c/src/svs_c.cpp @@ -22,6 +22,7 @@ #include "index.hpp" #include "index_builder.hpp" #include "leanvec_training_data.hpp" +#include "logger.hpp" #include "storage.hpp" #include "threadpool.hpp" #include "types_support.hpp" @@ -31,6 +32,7 @@ #include #include #include +#include #include #include @@ -40,9 +42,6 @@ #include #include -#include "spdlog/pattern_formatter.h" -#include "spdlog/sinks/callback_sink.h" - // C API implementation struct svs_index { std::shared_ptr impl; @@ -69,7 +68,32 @@ struct svs_leanvec_training_data { }; struct svs_logger { + // The handle keeps this one spdlog logger for its whole life. Indexes and the SVS + // global default logger hold references to it, so output, level and pattern changes + // made through the handle apply to them immediately. svs::logging::logger_ptr impl; + // Single sink of `impl`; forwards to the current output sink. Replacing the output + // swaps the sink held here (dist_sink::set_sinks locks the sink mutex). + std::shared_ptr output; + // Current pattern; returned by svs_logger_get_pattern(). + std::string pattern = svs::c_runtime::default_log_pattern; + + svs_logger() + : output{std::make_shared()} { + // A new logger has no output until svs_logger_set_kind/set_custom is called. + output->set_sinks({svs::logging::null_sink()}); + impl = std::make_shared(svs::c_runtime::logger_name, output); + impl->set_level(spdlog::level::warn); + impl->set_pattern(pattern); + } + + // Replace the output sink in place, keeping the current level and pattern. + void set_output(svs::logging::sink_ptr sink) { + // dist_sink only passes its formatter to sub-sinks present at set_pattern time, + // so give the new sink the current pattern before installing it. + sink->set_pattern(pattern); + output->set_sinks({std::move(sink)}); + } }; extern "C" uint32_t svs_get_version() { return SVS_C_API_VERSION; } @@ -99,15 +123,11 @@ inline svs::logging::Level to_logging_level(svs_log_level_t level) { extern "C" svs_logger_h svs_logger_create(svs_error_h out_err /*=NULL*/) { using namespace svs::c_runtime; - return wrap_exceptions( - [&]() { - auto logger = svs::logging::detail::default_logger(); - return new svs_logger{std::move(logger)}; - }, - out_err - ); + return wrap_exceptions([&]() { return new svs_logger{}; }, out_err); } +extern "C" void svs_logger_free(svs_logger_h logger) { delete logger; } + extern "C" bool svs_logger_set_kind( svs_logger_h logger, svs_logging_kind_t kind, @@ -115,7 +135,6 @@ extern "C" bool svs_logger_set_kind( svs_error_h out_err /*=NULL*/ ) { using namespace svs::c_runtime; - using logger_type = svs::logging::logger_ptr::element_type; return wrap_exceptions( [&]() { INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); @@ -141,15 +160,13 @@ extern "C" bool svs_logger_set_kind( ); break; case SVS_LOGGING_KIND_CUSTOM: - INVALID_ARGUMENT_IF( - true, - "Custom logging kind to be set using svs_default_logger_set_custom" + throw std::invalid_argument( + "Custom logging kind must be set using svs_logger_set_custom" ); default: - INVALID_ARGUMENT_IF(true, "Invalid logging kind"); + throw std::invalid_argument("Invalid logging kind"); } - auto log_ptr = std::make_shared(std::move(sink)); - logger->impl = std::move(log_ptr); + logger->set_output(std::move(sink)); return true; }, out_err @@ -160,33 +177,11 @@ extern "C" bool svs_logger_set_custom( svs_logger_h logger, svs_logging_i user_logger, svs_error_h out_err /*=NULL*/ ) { using namespace svs::c_runtime; - using logger_type = svs::logging::logger_ptr::element_type; return wrap_exceptions( [&]() { INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); - INVALID_ARGUMENT_IF(user_logger == nullptr, "Custom logger must not be null"); - INVALID_ARGUMENT_IF( - user_logger->ops == nullptr, "Custom logger ops must not be null" - ); - INVALID_ARGUMENT_IF( - user_logger->ops->log == nullptr, - "Custom logger ops must have a valid log function" - ); - // Kind of the custom logger implementation: - auto self = user_logger->self; - auto log = user_logger->ops->log; - auto callback_closure = [self, log](const auto& log_msg) { - static spdlog::pattern_formatter formatter; - /*static?*/ spdlog::memory_buf_t formatted; - formatter.format(log_msg, formatted); - log(self, - static_cast(log_msg.level), - fmt::to_string(formatted).c_str()); - }; - - auto callback_sink = spdlog::sinks::callback_sink_mt(callback_closure); - auto log_ptr = std::make_shared(std::move(callback_sink)); - logger->impl = std::move(log_ptr); + CallbackSink::validate(user_logger); + logger->set_output(std::make_shared(user_logger)); return true; }, out_err @@ -208,12 +203,63 @@ extern "C" bool svs_logger_set_level( ); } -extern "C" bool svs_set_default_logger(svs_logger_h logger, svs_error_h out_err /*=NULL*/) { +extern "C" bool svs_logger_get_level( + svs_logger_h logger, svs_log_level_t* out_level, svs_error_h out_err /*=NULL*/ +) { + using namespace svs::c_runtime; + return wrap_exceptions( + [&]() { + INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); + EXPECT_ARG_NOT_NULL(out_level); + *out_level = static_cast(logger->impl->level()); + return true; + }, + out_err + ); +} + +extern "C" bool svs_logger_set_pattern( + svs_logger_h logger, const char* pattern, svs_error_h out_err /*=NULL*/ +) { using namespace svs::c_runtime; return wrap_exceptions( [&]() { INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); - svs::logging::set(logger->impl); + EXPECT_ARG_NOT_NULL(pattern); + auto new_pattern = std::string(pattern); + logger->impl->set_pattern(new_pattern); + logger->pattern = std::move(new_pattern); + return true; + }, + out_err + ); +} + +extern "C" bool svs_logger_get_pattern( + svs_logger_h logger, const char** out_pattern, svs_error_h out_err /*=NULL*/ +) { + using namespace svs::c_runtime; + return wrap_exceptions( + [&]() { + INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); + EXPECT_ARG_NOT_NULL(out_pattern); + *out_pattern = logger->pattern.c_str(); + return true; + }, + out_err + ); +} + +extern "C" bool svs_set_default_logger(svs_logger_h logger, svs_error_h out_err /*=NULL*/) { + using namespace svs::c_runtime; + return wrap_exceptions( + [&]() { + if (logger == nullptr) { + // NULL restores the SVS built-in default logger. + svs::logging::reset_to_default(); + } else { + svs::logging::set(logger->impl); + } return true; }, out_err diff --git a/bindings/c/tests/CMakeLists.txt b/bindings/c/tests/CMakeLists.txt index 3b199579..4cf54bf2 100644 --- a/bindings/c/tests/CMakeLists.txt +++ b/bindings/c/tests/CMakeLists.txt @@ -50,6 +50,7 @@ set(C_API_TEST_SOURCES c_api_index_builder.cpp c_api_index.cpp c_api_dynamic_index.cpp + c_api_logging.cpp ) # Create test executable diff --git a/bindings/c/tests/README.md b/bindings/c/tests/README.md index 4a6ba567..d181f58e 100644 --- a/bindings/c/tests/README.md +++ b/bindings/c/tests/README.md @@ -29,6 +29,7 @@ The tests are organized into separate files by functionality: - **c_api_index_builder.cpp**: Tests for index builder creation and configuration - **c_api_index.cpp**: Tests for index building, searching, and basic operations - **c_api_dynamic_index.cpp**: Tests for dynamic index operations (add, delete, consolidate, compact) +- **c_api_logging.cpp**: Tests for logger handles (output kinds, custom callback, level, pattern, default logger) Note: The main() function is provided by Catch2::Catch2WithMain automatically. @@ -70,6 +71,9 @@ cmake -DSVS_BUILD_C_API_TESTS=OFF .. # Run dynamic index tests ./svs_c_api_test "[c_api][dynamic]" + +# Run logging tests +./svs_c_api_test "[c_api][logging]" ``` ### Run with verbose output @@ -134,6 +138,14 @@ The tests cover the following aspects of the C API: - Vector reconstruction - Thread count management +### Logging + +- Logger handle creation and cleanup +- Output kinds (none, stdout, stderr, file truncate/append) and invalid kinds/paths +- Custom logging callback, including ops version/struct_size validation +- Level and pattern getters/setters, filtering, preservation across output changes +- Default logger receives SVS internal messages + ### Dynamic Index Operations - Dynamic index building with/without explicit IDs diff --git a/bindings/c/tests/c_api_logging.cpp b/bindings/c/tests/c_api_logging.cpp new file mode 100644 index 00000000..12866f48 --- /dev/null +++ b/bindings/c/tests/c_api_logging.cpp @@ -0,0 +1,656 @@ +/* + * Copyright 2026 Intel Corporation + * + * Licensed 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. + */ + +// C API +#include "svs/c/svs_c.h" + +// catch2 +#include "catch2/catch_test_macros.hpp" + +// Test utilities +#include "c_api_test_utils.h" + +// Standard library +#include +#include +#include +#include +#include +#include +#include + +namespace { + +// Records every message received by the custom logger callback. +struct LogRecorder { + std::mutex mutex; + std::vector> messages; + + bool contains(svs_log_level_t level, const std::string& text) { + std::lock_guard lock{mutex}; + return std::any_of(messages.begin(), messages.end(), [&](const auto& m) { + return m.first == level && m.second.find(text) != std::string::npos; + }); + } + + bool any_below(svs_log_level_t level) { + std::lock_guard lock{mutex}; + return std::any_of(messages.begin(), messages.end(), [&](const auto& m) { + return m.first < level; + }); + } +}; + +void record_log(void* self, enum svs_log_level level, const char* message) { + auto* recorder = static_cast(self); + std::lock_guard lock{recorder->mutex}; + recorder->messages.emplace_back(level, message); +} + +void noop_log(void* /*self*/, enum svs_log_level /*level*/, const char* /*message*/) {} + +// Restores the SVS built-in default logger when a test that calls +// svs_set_default_logger ends, so later tests never log through a callback whose `self` +// has been destroyed. +struct DefaultLoggerGuard { + DefaultLoggerGuard() = default; + DefaultLoggerGuard(const DefaultLoggerGuard&) = delete; + DefaultLoggerGuard& operator=(const DefaultLoggerGuard&) = delete; + ~DefaultLoggerGuard() { svs_set_default_logger(nullptr, nullptr); } +}; + +// Builds (and frees) a small static index. Vamana build logs at TRACE level through the +// global default logger, e.g. "Number of syncs: ..." and "Completed pass ...". +void build_small_index() { + const size_t num_vectors = 100; + const size_t dimension = 16; + std::vector data; + generate_test_data(data, num_vectors, dimension); + + svs_error_h error = svs_error_create(); + svs_algorithm_h algorithm = svs_algorithm_create_vamana(16, 32, 50, error); + CATCH_REQUIRE(algorithm != nullptr); + svs_index_builder_h builder = svs_index_builder_create( + SVS_DISTANCE_METRIC_EUCLIDEAN, dimension, algorithm, error + ); + CATCH_REQUIRE(builder != nullptr); + CATCH_REQUIRE( + svs_index_builder_set_threadpool(builder, SVS_THREADPOOL_KIND_NATIVE, 2, error) + ); + svs_index_h index = svs_index_build(builder, data.data(), num_vectors, error); + CATCH_REQUIRE(index != nullptr); + CATCH_REQUIRE(svs_error_ok(error)); + + svs_index_free(index); + svs_index_builder_free(builder); + svs_algorithm_free(algorithm); + svs_error_free(error); +} + +std::string read_file(const std::string& path) { + std::ifstream in(path); + return std::string( + std::istreambuf_iterator(in), std::istreambuf_iterator() + ); +} + +void write_file(const std::string& path, const std::string& content) { + std::ofstream out(path, std::ios::trunc); + out << content; +} + +} // namespace + +CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { + CATCH_SECTION("Create and Free") { + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + CATCH_REQUIRE(svs_error_ok(error)); + svs_logger_free(logger); + + // NULL error handle and freeing NULL are allowed. + logger = svs_logger_create(nullptr); + CATCH_REQUIRE(logger != nullptr); + svs_logger_free(logger); + svs_logger_free(nullptr); + svs_error_free(error); + } + + CATCH_SECTION("NULL Arguments") { + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + auto expect_invalid = [&](bool result) { + CATCH_REQUIRE(result == false); + CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); + }; + + svs_log_level_t level; + const char* pattern = nullptr; + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(noop_log); + svs_logging_t user_logger = {&ops, nullptr}; + + expect_invalid(svs_logger_set_kind(nullptr, SVS_LOGGING_KIND_STDOUT, nullptr, error) + ); + expect_invalid(svs_logger_set_custom(nullptr, &user_logger, error)); + expect_invalid(svs_logger_set_level(nullptr, SVS_LOG_LEVEL_INFO, error)); + expect_invalid(svs_logger_get_level(nullptr, &level, error)); + expect_invalid(svs_logger_get_level(logger, nullptr, error)); + expect_invalid(svs_logger_set_pattern(nullptr, "%v", error)); + expect_invalid(svs_logger_set_pattern(logger, nullptr, error)); + expect_invalid(svs_logger_get_pattern(nullptr, &pattern, error)); + expect_invalid(svs_logger_get_pattern(logger, nullptr, error)); + + // NULL error handle: failure is still reported by the return value. + CATCH_REQUIRE(svs_logger_set_level(nullptr, SVS_LOG_LEVEL_INFO, nullptr) == false); + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Set Kind Console and None") { + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + for (auto kind : + {SVS_LOGGING_KIND_NONE, SVS_LOGGING_KIND_STDOUT, SVS_LOGGING_KIND_STDERR}) { + CATCH_REQUIRE(svs_logger_set_kind(logger, kind, nullptr, error)); + CATCH_REQUIRE(svs_error_ok(error)); + } + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Set Kind Invalid") { + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + // Custom output must go through svs_logger_set_custom. + CATCH_REQUIRE(!svs_logger_set_kind(logger, SVS_LOGGING_KIND_CUSTOM, nullptr, error) + ); + CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); + CATCH_REQUIRE( + std::string(svs_error_get_message(error)).find("svs_logger_set_custom") != + std::string::npos + ); + + CATCH_REQUIRE( + !svs_logger_set_kind(logger, static_cast(6), nullptr, error) + ); + CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); + + // File kinds need a non-empty path. + for (auto kind : {SVS_LOGGING_KIND_FILE_APPEND, SVS_LOGGING_KIND_FILE_TRUNCATE}) { + CATCH_REQUIRE(!svs_logger_set_kind(logger, kind, nullptr, error)); + CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); + CATCH_REQUIRE(!svs_logger_set_kind(logger, kind, "", error)); + CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); + } + + // A path that cannot be opened as a file (an existing directory). + TempDir tmp; + CATCH_REQUIRE(!svs_logger_set_kind( + logger, SVS_LOGGING_KIND_FILE_TRUNCATE, tmp.string().c_str(), error + )); + CATCH_REQUIRE(svs_error_get_code(error) != SVS_OK); + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Set Custom Invalid") { + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + auto expect_invalid = [&](svs_logging_i user_logger) { + CATCH_REQUIRE(!svs_logger_set_custom(logger, user_logger, error)); + CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); + }; + + expect_invalid(nullptr); + + svs_logging_t no_ops = {nullptr, nullptr}; + expect_invalid(&no_ops); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(noop_log); + ops.log = nullptr; + svs_logging_t no_log = {&ops, nullptr}; + expect_invalid(&no_log); + + svs_logging_ops_t bad_version = SVS_INIT_LOGGING_OPS(noop_log); + bad_version.version = svs_get_version() + 1; + svs_logging_t bad_version_logger = {&bad_version, nullptr}; + expect_invalid(&bad_version_logger); + + svs_logging_ops_t bad_size = SVS_INIT_LOGGING_OPS(noop_log); + bad_size.struct_size = sizeof(svs_logging_ops_t) - 1; + svs_logging_t bad_size_logger = {&bad_size, nullptr}; + expect_invalid(&bad_size_logger); + + svs_logging_ops_t good = SVS_INIT_LOGGING_OPS(noop_log); + svs_logging_t good_logger = {&good, nullptr}; + CATCH_REQUIRE(svs_logger_set_custom(logger, &good_logger, error)); + CATCH_REQUIRE(svs_error_ok(error)); + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Level Round Trip") { + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + for (auto level : + {SVS_LOG_LEVEL_TRACE, + SVS_LOG_LEVEL_DEBUG, + SVS_LOG_LEVEL_INFO, + SVS_LOG_LEVEL_WARN, + SVS_LOG_LEVEL_ERROR, + SVS_LOG_LEVEL_CRITICAL, + SVS_LOG_LEVEL_OFF}) { + CATCH_REQUIRE(svs_logger_set_level(logger, level, error)); + svs_log_level_t out_level = SVS_LOG_LEVEL_OFF; + CATCH_REQUIRE(svs_logger_get_level(logger, &out_level, error)); + CATCH_REQUIRE(out_level == level); + } + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Pattern Round Trip") { + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + const char* pattern = nullptr; + CATCH_REQUIRE(svs_logger_get_pattern(logger, &pattern, error)); + CATCH_REQUIRE(pattern != nullptr); + CATCH_REQUIRE(std::string(pattern) == "%v"); + + CATCH_REQUIRE(svs_logger_set_pattern(logger, "[%l] %v", error)); + CATCH_REQUIRE(svs_logger_get_pattern(logger, &pattern, error)); + CATCH_REQUIRE(std::string(pattern) == "[%l] %v"); + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("New Logger Defaults") { + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + svs_log_level_t level = SVS_LOG_LEVEL_OFF; + const char* pattern = nullptr; + CATCH_REQUIRE(svs_logger_get_level(logger, &level, error)); + CATCH_REQUIRE(level == SVS_LOG_LEVEL_WARN); + CATCH_REQUIRE(svs_logger_get_pattern(logger, &pattern, error)); + CATCH_REQUIRE(std::string(pattern) == "%v"); + + // No output configured: using it as default (even at TRACE) writes nothing and + // must not crash. + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + build_small_index(); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + build_small_index(); + CATCH_REQUIRE(svs_error_ok(error)); + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Level and Pattern Preserved Across Output Changes") { + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_DEBUG, error)); + CATCH_REQUIRE(svs_logger_set_pattern(logger, "%v", error)); + + auto check = [&]() { + svs_log_level_t level = SVS_LOG_LEVEL_OFF; + const char* pattern = nullptr; + CATCH_REQUIRE(svs_logger_get_level(logger, &level, error)); + CATCH_REQUIRE(level == SVS_LOG_LEVEL_DEBUG); + CATCH_REQUIRE(svs_logger_get_pattern(logger, &pattern, error)); + CATCH_REQUIRE(std::string(pattern) == "%v"); + }; + + CATCH_REQUIRE(svs_logger_set_kind(logger, SVS_LOGGING_KIND_STDERR, nullptr, error)); + check(); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(noop_log); + svs_logging_t user_logger = {&ops, nullptr}; + CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + check(); + + CATCH_REQUIRE(svs_logger_set_kind(logger, SVS_LOGGING_KIND_NONE, nullptr, error)); + check(); + + svs_logger_free(logger); + svs_error_free(error); + } +} + +CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { + CATCH_SECTION("Custom Callback Receives Messages") { + LogRecorder recorder; // Declared first: must outlive the guard. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t user_logger = {&ops, &recorder}; + CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + CATCH_REQUIRE(svs_error_ok(error)); + + build_small_index(); + + CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + // Default pattern "%v": bare message, no timestamp/level prefix, no newline. + { + std::lock_guard lock{recorder.mutex}; + for (const auto& [level, text] : recorder.messages) { + CATCH_REQUIRE(!text.empty()); + CATCH_REQUIRE(text.front() != '['); + CATCH_REQUIRE(text.back() != '\n'); + } + } + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Output Change Reaches Existing Users") { + LogRecorder first; // Declared first: must outlive the guard. + LogRecorder second; // Declared first: must outlive the guard. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t first_logger = {&ops, &first}; + svs_logging_t second_logger = {&ops, &second}; + CATCH_REQUIRE(svs_logger_set_custom(logger, &first_logger, error)); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + + build_small_index(); + CATCH_REQUIRE(first.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + { + std::lock_guard lock{first.mutex}; + first.messages.clear(); + } + + // Change the output of the SAME handle without calling svs_set_default_logger + // again: the global default logger must follow. + CATCH_REQUIRE(svs_logger_set_custom(logger, &second_logger, error)); + build_small_index(); + CATCH_REQUIRE(second.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + { + std::lock_guard lock{first.mutex}; + CATCH_REQUIRE(first.messages.empty()); + } + + // Level changes also apply immediately. + { + std::lock_guard lock{second.mutex}; + second.messages.clear(); + } + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_OFF, error)); + build_small_index(); + { + std::lock_guard lock{second.mutex}; + CATCH_REQUIRE(second.messages.empty()); + } + + // Switching to no output stops the callback. + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_logger_set_kind(logger, SVS_LOGGING_KIND_NONE, nullptr, error)); + build_small_index(); + { + std::lock_guard lock{second.mutex}; + CATCH_REQUIRE(second.messages.empty()); + } + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Pattern Prefix Per Logger") { + LogRecorder recorder; // Declared first: must outlive the guard. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t user_logger = {&ops, &recorder}; + CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + + // Pattern set after the output (and after svs_set_default_logger). + CATCH_REQUIRE(svs_logger_set_pattern(logger, "[index A] %v", error)); + build_small_index(); + + { + std::lock_guard lock{recorder.mutex}; + CATCH_REQUIRE(!recorder.messages.empty()); + for (const auto& [level, text] : recorder.messages) { + CATCH_REQUIRE(text.rfind("[index A] ", 0) == 0); + } + bool found = std::any_of( + recorder.messages.begin(), + recorder.messages.end(), + [](const auto& m) { + return m.second.rfind("[index A] Number of syncs: ", 0) == 0; + } + ); + CATCH_REQUIRE(found); + } + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Level Filtering") { + LogRecorder recorder; // Declared first: must outlive the guard. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t user_logger = {&ops, &recorder}; + CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_WARN, error)); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + + build_small_index(); + + CATCH_REQUIRE(!recorder.any_below(SVS_LOG_LEVEL_WARN)); + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Bare Message Pattern") { + LogRecorder recorder; // Declared first: must outlive the guard. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t user_logger = {&ops, &recorder}; + CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_logger_set_pattern(logger, "%v", error)); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + + build_small_index(); + + std::lock_guard lock{recorder.mutex}; + bool found = std::any_of( + recorder.messages.begin(), + recorder.messages.end(), + [](const auto& m) { return m.second.rfind("Number of syncs: ", 0) == 0; } + ); + CATCH_REQUIRE(found); + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Pattern Set Before Custom Output Applies") { + LogRecorder recorder; // Declared first: must outlive the guard. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + // Level and pattern are set first, then carried over to the custom output. + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_logger_set_pattern(logger, "<%l> %v", error)); + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t user_logger = {&ops, &recorder}; + CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + + build_small_index(); + + CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, " Number of syncs")); + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Default Logger Outlives Handle") { + LogRecorder recorder; // Declared first: must outlive the guard. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t user_logger = {&ops, &recorder}; + CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + svs_logger_free(logger); + + build_small_index(); + + CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + svs_error_free(error); + } + + CATCH_SECTION("Reset Default Logger With NULL") { + LogRecorder recorder; // Declared first: must outlive the guard. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t user_logger = {&ops, &recorder}; + CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + + build_small_index(); + CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + + // NULL restores the built-in default; the callback must no longer be called. + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); + CATCH_REQUIRE(svs_error_ok(error)); + { + std::lock_guard lock{recorder.mutex}; + recorder.messages.clear(); + } + build_small_index(); + { + std::lock_guard lock{recorder.mutex}; + CATCH_REQUIRE(recorder.messages.empty()); + } + + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("File Truncate") { + TempDir tmp; + const std::string path = (tmp.path() / "svs.log").string(); + write_file(path, "OLD CONTENT\n"); + { + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_logger_set_kind( + logger, SVS_LOGGING_KIND_FILE_TRUNCATE, path.c_str(), error + )); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + svs_logger_free(logger); + + build_small_index(); + svs_error_free(error); + } + // The guard released the last reference, which closes (and flushes) the file. + auto content = read_file(path); + CATCH_REQUIRE(content.find("OLD CONTENT") == std::string::npos); + CATCH_REQUIRE(content.find("Number of syncs") != std::string::npos); + } + + CATCH_SECTION("File Append") { + TempDir tmp; + const std::string path = (tmp.path() / "svs.log").string(); + write_file(path, "OLD CONTENT\n"); + { + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_logger_set_kind( + logger, SVS_LOGGING_KIND_FILE_APPEND, path.c_str(), error + )); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + svs_logger_free(logger); + + build_small_index(); + svs_error_free(error); + } + auto content = read_file(path); + CATCH_REQUIRE(content.rfind("OLD CONTENT\n", 0) == 0); + CATCH_REQUIRE(content.find("Number of syncs") != std::string::npos); + } +} diff --git a/bindings/c/tests/consumer/main.c b/bindings/c/tests/consumer/main.c index 07b41ebe..90cc3955 100644 --- a/bindings/c/tests/consumer/main.c +++ b/bindings/c/tests/consumer/main.c @@ -48,6 +48,38 @@ static void report(const char* name, svs_storage_h storage, svs_error_h error) { printf("%-24s %s (%s)\n", name, reason, svs_error_get_message(error)); } +/* Custom logging callback; counts the messages it receives. */ +static void count_log(void* self, enum svs_log_level level, const char* message) { + (void)level; + (void)message; + ++*(int*)self; +} + +/* Compile SVS_INIT_LOGGING_OPS as C and route a logger handle to a C callback. */ +static int check_logging(svs_error_h error) { + int count = 0; + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(count_log); + svs_logging_t user_logger; + user_logger.ops = &ops; + user_logger.self = &count; + + svs_logger_h logger = svs_logger_create(error); + if (logger == NULL) { + fprintf(stderr, "failed to create a logger: %s\n", svs_error_get_message(error)); + return 0; + } + if (!svs_logger_set_custom(logger, &user_logger, error)) { + fprintf( + stderr, "failed to set a custom logger: %s\n", svs_error_get_message(error) + ); + svs_logger_free(logger); + return 0; + } + svs_logger_free(logger); + printf("%-24s available\n", "logging/custom"); + return 1; +} + int main(void) { svs_error_h error = svs_error_create(); if (error == NULL) { @@ -68,6 +100,11 @@ int main(void) { printf("%-24s available\n", "simple/float32"); svs_storage_free(simple); + if (!check_logging(error)) { + svs_error_free(error); + return EXIT_FAILURE; + } + report("sq/int8", svs_storage_create_sq(SVS_DATA_TYPE_INT8, error), error); report( From 0b1753bb0f39938826a07da5f0d35976616916c2 Mon Sep 17 00:00:00 2001 From: yuejiaointel Date: Mon, 5 Oct 2026 23:43:23 -0700 Subject: [PATCH 3/8] [C API] Use spdlog callback_sink for custom loggers Custom loggers now use spdlog::sinks::callback_sink_mt with a lambda that forwards the bare message payload, level and self to the user callback, replacing the CallbackSink class. The logger pattern applies to stdout, stderr and file outputs only; custom callbacks format messages themselves. --- bindings/c/include/svs/c/svs_c.h | 22 +++++---- bindings/c/src/logger.hpp | 78 ++++++++---------------------- bindings/c/src/svs_c.cpp | 12 ++++- bindings/c/tests/c_api_logging.cpp | 70 +++++++++++++-------------- 4 files changed, 75 insertions(+), 107 deletions(-) diff --git a/bindings/c/include/svs/c/svs_c.h b/bindings/c/include/svs/c/svs_c.h index 51ed25c9..048a644b 100644 --- a/bindings/c/include/svs/c/svs_c.h +++ b/bindings/c/include/svs/c/svs_c.h @@ -164,9 +164,10 @@ enum svs_threadpool_kind { /// Function pointer to log a message. /// @param self Pointer to the logging interface instance. /// @param level Logging level of the message. -/// @param message Null-terminated string containing the message formatted with the -/// logger pattern (see svs_logger_set_pattern), without a trailing newline. The -/// pointer is only valid for the duration of the call. +/// @param message Null-terminated string containing the bare message text, without a +/// trailing newline. The logger pattern (see svs_logger_set_pattern) is not applied; +/// use @p level and @p self to format it. The pointer is only valid for the duration +/// of the call. struct svs_logging_interface_ops { uint32_t version; size_t struct_size; @@ -667,8 +668,9 @@ SVS_API bool svs_logger_set_kind( /// @remarks @p user_logger->ops must be initialized with SVS_INIT_LOGGING_OPS; the /// version and struct_size fields are validated. See svs_logging_interface_ops for the /// lifetime and thread-safety requirements of the callback. -/// @remarks The current level and pattern of the handle are kept. The new output applies -/// immediately to everything already using this logger. +/// @remarks The callback always receives the bare message text (plus level and self); +/// the logger pattern does not apply to it. The current level of the handle is kept. +/// The new output applies immediately to everything already using this logger. SVS_API bool svs_logger_set_custom( svs_logger_h logger, svs_logging_i user_logger, svs_error_h out_err /*=NULL*/ ); @@ -698,16 +700,18 @@ SVS_API bool svs_logger_get_level( /// the bare message, or "[index A] %v" to prefix every message). Must not be NULL. /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure -/// @remarks The pattern applies to all output kinds and is kept across -/// svs_logger_set_kind / svs_logger_set_custom. Applies immediately to everything -/// already using this logger. +/// @remarks The pattern applies to the stdout, stderr and file output kinds and is kept +/// across svs_logger_set_kind / svs_logger_set_custom. It does not apply to custom +/// callbacks (svs_logger_set_custom), which always receive the bare message text. +/// Applies immediately to everything already using this logger. SVS_API bool svs_logger_set_pattern( svs_logger_h logger, const char* pattern, svs_error_h out_err /*=NULL*/ ); /// @brief Get the format pattern for a logger /// @param logger The logger handle -/// @param out_pattern Pointer to store the retrieved format pattern. The string is +/// @param out_pattern Pointer to store the retrieved format pattern (used by the +/// stdout, stderr and file outputs, not by custom callbacks). The string is /// owned by the logger handle and stays valid until the next svs_logger_set_pattern /// call on the handle or until svs_logger_free. If no pattern was set, returns the /// default pattern "%v". diff --git a/bindings/c/src/logger.hpp b/bindings/c/src/logger.hpp index 42acc53b..60369719 100644 --- a/bindings/c/src/logger.hpp +++ b/bindings/c/src/logger.hpp @@ -21,13 +21,10 @@ #include -#include "spdlog/sinks/base_sink.h" +#include "spdlog/sinks/callback_sink.h" #include "spdlog/sinks/dist_sink.h" -#include #include -#include -#include namespace svs { namespace c_runtime { @@ -44,65 +41,28 @@ static_assert(static_cast(SVS_LOG_LEVEL_OFF) == SPDLOG_LEVEL_OFF); /// Name given to every spdlog logger created by the C API. inline constexpr const char* logger_name = "svs_c"; -/// Default pattern of every logger handle, for all output kinds: the bare message. -/// Hosts that forward messages to their own logger (custom kind) add their own timestamp -/// and level, so SVS does not add them by default. +/// Default pattern of every logger handle: the bare message. Patterns apply to the +/// stdout, stderr and file outputs; custom callbacks always receive the bare message. inline constexpr const char* default_log_pattern = "%v"; -/// A spdlog sink forwarding formatted messages to a user-provided C callback. -/// -/// Messages are formatted with the sink's own formatter, so the logger pattern -/// (svs_logger_set_pattern) applies. The trailing end-of-line added by the formatter is -/// stripped before calling the user callback. -/// The base_sink mutex serializes calls to the user callback. -class CallbackSink final : public spdlog::sinks::base_sink { - public: - using log_func_t = void (*)(void*, enum svs_log_level, const char*); - - static void validate(const svs_logging_i user_logger) { - if (user_logger == nullptr) { - throw std::invalid_argument("Custom logger pointer cannot be null."); - } - if (user_logger->ops == nullptr) { - throw std::invalid_argument("Custom logger interface is not initialized."); - } - if (user_logger->ops->version > svs_get_version()) { - throw std::invalid_argument("Custom logger interface version is not supported." - ); - } - if (user_logger->ops->struct_size < sizeof(svs_logging_ops_t)) { - throw std::invalid_argument("Incompatible custom logger interface struct size." - ); - } - if (user_logger->ops->log == nullptr) { - throw std::invalid_argument("Custom logger interface has null log function."); - } +/// Checks the version, struct size and NULL pointers of a user-provided custom logger. +inline void validate_custom_logger(const svs_logging_i user_logger) { + if (user_logger == nullptr) { + throw std::invalid_argument("Custom logger pointer cannot be null."); } - - /// Copies the log function pointer and the user's self pointer. - /// @p user_logger must have been checked with validate(). - explicit CallbackSink(const svs_logging_i user_logger) - : log_{user_logger->ops->log} - , self_{user_logger->self} {} - - protected: - void sink_it_(const spdlog::details::log_msg& msg) override { - spdlog::memory_buf_t formatted; - this->formatter_->format(msg, formatted); - auto text = std::string(formatted.data(), formatted.size()); - // Strip the end-of-line appended by the formatter. - while (!text.empty() && (text.back() == '\n' || text.back() == '\r')) { - text.pop_back(); - } - log_(self_, static_cast(msg.level), text.c_str()); + if (user_logger->ops == nullptr) { + throw std::invalid_argument("Custom logger interface is not initialized."); } - - void flush_() override {} - - private: - log_func_t log_; - void* self_; -}; + if (user_logger->ops->version > svs_get_version()) { + throw std::invalid_argument("Custom logger interface version is not supported."); + } + if (user_logger->ops->struct_size < sizeof(svs_logging_ops_t)) { + throw std::invalid_argument("Incompatible custom logger interface struct size."); + } + if (user_logger->ops->log == nullptr) { + throw std::invalid_argument("Custom logger interface has null log function."); + } +} } // namespace c_runtime } // namespace svs diff --git a/bindings/c/src/svs_c.cpp b/bindings/c/src/svs_c.cpp index 79d49a67..903eddb4 100644 --- a/bindings/c/src/svs_c.cpp +++ b/bindings/c/src/svs_c.cpp @@ -180,8 +180,16 @@ extern "C" bool svs_logger_set_custom( return wrap_exceptions( [&]() { INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); - CallbackSink::validate(user_logger); - logger->set_output(std::make_shared(user_logger)); + validate_custom_logger(user_logger); + // Forward the bare message; the logger pattern does not apply here. + auto log = user_logger->ops->log; + auto self = user_logger->self; + logger->set_output(std::make_shared( + [log, self](const spdlog::details::log_msg& msg) { + std::string text(msg.payload.data(), msg.payload.size()); + log(self, static_cast(msg.level), text.c_str()); + } + )); return true; }, out_err diff --git a/bindings/c/tests/c_api_logging.cpp b/bindings/c/tests/c_api_logging.cpp index 12866f48..6b4e1e53 100644 --- a/bindings/c/tests/c_api_logging.cpp +++ b/bindings/c/tests/c_api_logging.cpp @@ -443,7 +443,7 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { svs_error_free(error); } - CATCH_SECTION("Pattern Prefix Per Logger") { + CATCH_SECTION("Pattern Not Applied To Custom Callback") { LogRecorder recorder; // Declared first: must outlive the guard. DefaultLoggerGuard guard; svs_error_h error = svs_error_create(); @@ -455,26 +455,21 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); CATCH_REQUIRE(svs_set_default_logger(logger, error)); + CATCH_REQUIRE(svs_logger_set_pattern(logger, "[x] %v", error)); - // Pattern set after the output (and after svs_set_default_logger). - CATCH_REQUIRE(svs_logger_set_pattern(logger, "[index A] %v", error)); build_small_index(); - { - std::lock_guard lock{recorder.mutex}; - CATCH_REQUIRE(!recorder.messages.empty()); - for (const auto& [level, text] : recorder.messages) { - CATCH_REQUIRE(text.rfind("[index A] ", 0) == 0); - } - bool found = std::any_of( - recorder.messages.begin(), - recorder.messages.end(), - [](const auto& m) { - return m.second.rfind("[index A] Number of syncs: ", 0) == 0; - } - ); - CATCH_REQUIRE(found); + std::lock_guard lock{recorder.mutex}; + CATCH_REQUIRE(!recorder.messages.empty()); + for (const auto& [level, text] : recorder.messages) { + CATCH_REQUIRE(text.rfind("[x] ", 0) == std::string::npos); } + bool found = std::any_of( + recorder.messages.begin(), + recorder.messages.end(), + [](const auto& m) { return m.second.rfind("Number of syncs: ", 0) == 0; } + ); + CATCH_REQUIRE(found); svs_logger_free(logger); svs_error_free(error); @@ -529,27 +524,28 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { svs_error_free(error); } - CATCH_SECTION("Pattern Set Before Custom Output Applies") { - LogRecorder recorder; // Declared first: must outlive the guard. - DefaultLoggerGuard guard; - svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); - - // Level and pattern are set first, then carried over to the custom output. - CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); - CATCH_REQUIRE(svs_logger_set_pattern(logger, "<%l> %v", error)); - svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); - svs_logging_t user_logger = {&ops, &recorder}; - CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); - CATCH_REQUIRE(svs_set_default_logger(logger, error)); - - build_small_index(); - - CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, " Number of syncs")); + CATCH_SECTION("Pattern Applied To File Output") { + TempDir tmp; + const std::string path = (tmp.path() / "svs.log").string(); + { + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(error); + CATCH_REQUIRE(logger != nullptr); + // Set before the output: carried over to the file output. + CATCH_REQUIRE(svs_logger_set_pattern(logger, "[x] %v", error)); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_logger_set_kind( + logger, SVS_LOGGING_KIND_FILE_TRUNCATE, path.c_str(), error + )); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + svs_logger_free(logger); - svs_logger_free(logger); - svs_error_free(error); + build_small_index(); + svs_error_free(error); + } + auto content = read_file(path); + CATCH_REQUIRE(content.find("[x] Number of syncs") != std::string::npos); } CATCH_SECTION("Default Logger Outlives Handle") { From 201760c7a7bc5183eb1413e7f58a17525d2ab0a5 Mon Sep 17 00:00:00 2001 From: yuejiaointel Date: Tue, 6 Oct 2026 10:56:46 -0700 Subject: [PATCH 4/8] [C API] Create loggers with their output; move logger code to logger.hpp A logger handle now gets its output at creation and keeps it for life: svs_logger_create(kind, path) for the built-in outputs (none, stdout, stderr, file append/truncate) and svs_logger_create_custom(user_logger) for a user callback. svs_logger_set_kind, svs_logger_set_custom and the dist_sink-based in-place output swap are removed; to log somewhere else, create another logger. Level and pattern setters are unchanged and still apply to everything already using the logger. struct svs_logger, to_logging_level() and the sink construction move from svs_c.cpp to logger.hpp. Docs, the C consumer program and the logging tests follow the new API. --- bindings/c/include/svs/c/svs_c.h | 99 +++--- bindings/c/src/logger.hpp | 86 +++++- bindings/c/src/svs_c.cpp | 123 +------- bindings/c/tests/README.md | 11 +- bindings/c/tests/c_api_logging.cpp | 464 +++++++++++++++-------------- bindings/c/tests/consumer/main.c | 9 +- 6 files changed, 382 insertions(+), 410 deletions(-) diff --git a/bindings/c/include/svs/c/svs_c.h b/bindings/c/include/svs/c/svs_c.h index 048a644b..8fc4aa56 100644 --- a/bindings/c/include/svs/c/svs_c.h +++ b/bindings/c/include/svs/c/svs_c.h @@ -146,10 +146,9 @@ enum svs_threadpool_kind { /// @brief Operations table for a custom logging interface. /// @remarks Lifetime: the operations table, the function it points to and the @p self /// pointer of the owning svs_logging_interface must remain valid for as long as any -/// logger configured with them is in use, i.e. while the svs_logger_h handle, any index -/// built with it, or the SVS global default logger (see svs_set_default_logger) still -/// refers to it. Once svs_logger_set_kind or svs_logger_set_custom has replaced the -/// output of the handle and returned, the previous callback is no longer called. +/// logger created with them (see svs_logger_create_custom) is in use, i.e. while the +/// svs_logger_h handle, any index built with it, or the SVS global default logger (see +/// svs_set_default_logger) still refers to it. /// @remarks Thread safety: the @p log function may be called from SVS worker threads. /// Calls through one logger handle are serialized by an internal mutex, and the /// callback runs while that mutex is held. The callback must not call back into SVS @@ -620,60 +619,53 @@ SVS_API const char* svs_error_get_message(svs_error_h err); /// @param err The error handle to free SVS_API void svs_error_free(svs_error_h err); -/// @brief Create a logger with default settings +/// @brief Create a logger writing to a built-in output +/// @param kind The output of the logger: SVS_LOGGING_KIND_NONE (discard everything), +/// SVS_LOGGING_KIND_STDOUT, SVS_LOGGING_KIND_STDERR, SVS_LOGGING_KIND_FILE_APPEND or +/// SVS_LOGGING_KIND_FILE_TRUNCATE. SVS_LOGGING_KIND_CUSTOM is rejected; use +/// svs_logger_create_custom() instead. +/// @param path The file to write to for SVS_LOGGING_KIND_FILE_APPEND and +/// SVS_LOGGING_KIND_FILE_TRUNCATE (must not be NULL or empty; missing directories are +/// created). Ignored by the other kinds; may be NULL. /// @param out_err An optional error handle to capture errors /// @return A handle to the created logger or NULL if creation failed -/// @remarks A new logger has no output until svs_logger_set_kind() or -/// svs_logger_set_custom() is called. Its level is SVS_LOG_LEVEL_WARN and its pattern is -/// "%v" (the bare message). The SVS_LOG_SINK / SVS_LOG_LEVEL environment variables are -/// not read here; they only configure the SVS global default logger. -/// @remarks Output, level and pattern changes made through a handle apply immediately to +/// @remarks The output of a logger is fixed at creation. To log somewhere else, create +/// another logger and pass it to svs_set_default_logger() or +/// svs_index_builder_set_logger(). +/// @remarks A new logger has level SVS_LOG_LEVEL_WARN and pattern "%v" (the bare +/// message). The SVS_LOG_SINK / SVS_LOG_LEVEL environment variables are not read here; +/// they only configure the SVS built-in default logger. +/// @remarks Level and pattern changes made through a handle apply immediately to /// everything already using its logger (the SVS global default logger and indexes). /// @remarks Thread safety: the functions that modify a logger handle -/// (svs_logger_set_kind, svs_logger_set_custom, svs_logger_set_level, -/// svs_logger_set_pattern, svs_logger_free) are not thread-safe. Do not call them -/// concurrently with each other or with other functions using the same handle. -SVS_API svs_logger_h svs_logger_create(svs_error_h out_err /*=NULL*/); - -/// @brief Free a logger handle -/// @param logger The logger handle to free. Passing NULL is a no-op. -/// @remarks The SVS global default logger (see svs_set_default_logger) keeps its own -/// reference to the underlying logger, so freeing the handle does not stop it from -/// logging. User data referenced by a custom logger must stay valid as long as such a -/// reference exists. -SVS_API void svs_logger_free(svs_logger_h logger); - -/// @brief Set the logging kind for an existing logger -/// @param logger The logger handle to set the kind for -/// @param kind The logging kind to set -/// @param path The file path to use for file-based logging kinds (e.g., -/// SVS_LOGGING_KIND_FILE_APPEND or SVS_LOGGING_KIND_FILE_TRUNCATE) -/// @param out_err An optional error handle to capture errors -/// @return true on success, false on failure -/// @remarks SVS_LOGGING_KIND_CUSTOM is rejected; use svs_logger_set_custom instead. -/// @remarks The current level and pattern of the handle are kept. The new output applies -/// immediately to everything already using this logger. -SVS_API bool svs_logger_set_kind( - svs_logger_h logger, - svs_logging_kind_t kind, - const char* path, - svs_error_h out_err /*=NULL*/ +/// (svs_logger_set_level, svs_logger_set_pattern, svs_logger_free) are not thread-safe. +/// Do not call them concurrently with each other or with other functions using the same +/// handle. +SVS_API svs_logger_h svs_logger_create( + svs_logging_kind_t kind, const char* path, svs_error_h out_err /*=NULL*/ ); -/// @brief Create a custom logger for the SVS library -/// @param logger The logger handle to set as custom -/// @param user_logger The custom logger to create for the SVS library +/// @brief Create a logger forwarding every message to a user callback +/// @param user_logger The custom logging interface; @p user_logger->ops must be +/// initialized with SVS_INIT_LOGGING_OPS. The version, struct_size and log fields are +/// validated. See svs_logging_interface_ops for the lifetime and thread-safety +/// requirements of the callback. /// @param out_err An optional error handle to capture errors -/// @return true on success, false on failure -/// @remarks @p user_logger->ops must be initialized with SVS_INIT_LOGGING_OPS; the -/// version and struct_size fields are validated. See svs_logging_interface_ops for the -/// lifetime and thread-safety requirements of the callback. +/// @return A handle to the created logger or NULL if creation failed /// @remarks The callback always receives the bare message text (plus level and self); -/// the logger pattern does not apply to it. The current level of the handle is kept. -/// The new output applies immediately to everything already using this logger. -SVS_API bool svs_logger_set_custom( - svs_logger_h logger, svs_logging_i user_logger, svs_error_h out_err /*=NULL*/ -); +/// the logger pattern does not apply to it. +/// @remarks Same defaults, lifetime and thread-safety rules as svs_logger_create(): the +/// output is fixed at creation, level SVS_LOG_LEVEL_WARN, pattern "%v". +SVS_API svs_logger_h +svs_logger_create_custom(svs_logging_i user_logger, svs_error_h out_err /*=NULL*/); + +/// @brief Free a logger handle +/// @param logger The logger handle to free. Passing NULL is a no-op. +/// @remarks The SVS global default logger (see svs_set_default_logger) and indexes built +/// with the logger keep their own reference to the underlying logger, so freeing the +/// handle does not stop it from logging. User data referenced by a custom logger must +/// stay valid as long as such a reference exists. +SVS_API void svs_logger_free(svs_logger_h logger); /// @brief Set the logging level for a logger /// @param logger The logger handle @@ -700,10 +692,9 @@ SVS_API bool svs_logger_get_level( /// the bare message, or "[index A] %v" to prefix every message). Must not be NULL. /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure -/// @remarks The pattern applies to the stdout, stderr and file output kinds and is kept -/// across svs_logger_set_kind / svs_logger_set_custom. It does not apply to custom -/// callbacks (svs_logger_set_custom), which always receive the bare message text. -/// Applies immediately to everything already using this logger. +/// @remarks The pattern applies to the stdout, stderr and file output kinds. It does not +/// apply to custom callbacks (svs_logger_create_custom), which always receive the bare +/// message text. Applies immediately to everything already using this logger. SVS_API bool svs_logger_set_pattern( svs_logger_h logger, const char* pattern, svs_error_h out_err /*=NULL*/ ); diff --git a/bindings/c/src/logger.hpp b/bindings/c/src/logger.hpp index 60369719..9a880644 100644 --- a/bindings/c/src/logger.hpp +++ b/bindings/c/src/logger.hpp @@ -22,9 +22,11 @@ #include #include "spdlog/sinks/callback_sink.h" -#include "spdlog/sinks/dist_sink.h" +#include #include +#include +#include namespace svs { namespace c_runtime { @@ -64,5 +66,87 @@ inline void validate_custom_logger(const svs_logging_i user_logger) { } } +/// Maps a C API log level to the SVS logging level. Unknown values map to Info. +inline svs::logging::Level to_logging_level(svs_log_level_t level) { + switch (level) { + case SVS_LOG_LEVEL_TRACE: + return svs::logging::Level::Trace; + case SVS_LOG_LEVEL_DEBUG: + return svs::logging::Level::Debug; + case SVS_LOG_LEVEL_INFO: + return svs::logging::Level::Info; + case SVS_LOG_LEVEL_WARN: + return svs::logging::Level::Warn; + case SVS_LOG_LEVEL_ERROR: + return svs::logging::Level::Error; + case SVS_LOG_LEVEL_CRITICAL: + return svs::logging::Level::Critical; + case SVS_LOG_LEVEL_OFF: + return svs::logging::Level::Off; + default: + return svs::logging::Level::Info; + } +} + +/// Creates the spdlog sink for a built-in output kind. `path` is only used by the file +/// kinds. SVS_LOGGING_KIND_CUSTOM is rejected: use svs_logger_create_custom(). +inline svs::logging::sink_ptr make_sink(svs_logging_kind_t kind, const char* path) { + switch (kind) { + case SVS_LOGGING_KIND_NONE: + return svs::logging::null_sink(); + case SVS_LOGGING_KIND_STDOUT: + return svs::logging::stdout_sink(); + case SVS_LOGGING_KIND_STDERR: + return svs::logging::stderr_sink(); + case SVS_LOGGING_KIND_FILE_APPEND: + case SVS_LOGGING_KIND_FILE_TRUNCATE: + if (path == nullptr || *path == '\0') { + throw std::invalid_argument( + "File path must be provided for file logging kind" + ); + } + return svs::logging::file_sink(path, kind == SVS_LOGGING_KIND_FILE_TRUNCATE); + case SVS_LOGGING_KIND_CUSTOM: + throw std::invalid_argument( + "Custom loggers must be created with svs_logger_create_custom" + ); + default: + throw std::invalid_argument("Invalid logging kind"); + } +} + +/// Creates the spdlog sink that forwards every message to a user callback. The callback +/// receives the bare message text; the logger pattern does not apply to it. +inline svs::logging::sink_ptr make_custom_sink(const svs_logging_i user_logger) { + validate_custom_logger(user_logger); + auto log = user_logger->ops->log; + auto self = user_logger->self; + return std::make_shared( + [log, self](const spdlog::details::log_msg& msg) { + std::string text(msg.payload.data(), msg.payload.size()); + log(self, static_cast(msg.level), text.c_str()); + } + ); +} + } // namespace c_runtime } // namespace svs + +/// The logger handle of the C API (svs_logger_h). +struct svs_logger { + // The handle keeps this one spdlog logger for its whole life. Indexes and the SVS + // global default logger hold references to it, so level and pattern changes made + // through the handle apply to them immediately. The output sink is fixed at + // creation: to log somewhere else, create another logger. + svs::logging::logger_ptr impl; + // Current pattern; returned by svs_logger_get_pattern(). + std::string pattern = svs::c_runtime::default_log_pattern; + + explicit svs_logger(svs::logging::sink_ptr sink) + : impl{std::make_shared( + svs::c_runtime::logger_name, std::move(sink) + )} { + impl->set_level(spdlog::level::warn); + impl->set_pattern(pattern); + } +}; diff --git a/bindings/c/src/svs_c.cpp b/bindings/c/src/svs_c.cpp index 903eddb4..6c21b556 100644 --- a/bindings/c/src/svs_c.cpp +++ b/bindings/c/src/svs_c.cpp @@ -67,135 +67,32 @@ struct svs_leanvec_training_data { std::shared_ptr impl; }; -struct svs_logger { - // The handle keeps this one spdlog logger for its whole life. Indexes and the SVS - // global default logger hold references to it, so output, level and pattern changes - // made through the handle apply to them immediately. - svs::logging::logger_ptr impl; - // Single sink of `impl`; forwards to the current output sink. Replacing the output - // swaps the sink held here (dist_sink::set_sinks locks the sink mutex). - std::shared_ptr output; - // Current pattern; returned by svs_logger_get_pattern(). - std::string pattern = svs::c_runtime::default_log_pattern; - - svs_logger() - : output{std::make_shared()} { - // A new logger has no output until svs_logger_set_kind/set_custom is called. - output->set_sinks({svs::logging::null_sink()}); - impl = std::make_shared(svs::c_runtime::logger_name, output); - impl->set_level(spdlog::level::warn); - impl->set_pattern(pattern); - } - - // Replace the output sink in place, keeping the current level and pattern. - void set_output(svs::logging::sink_ptr sink) { - // dist_sink only passes its formatter to sub-sinks present at set_pattern time, - // so give the new sink the current pattern before installing it. - sink->set_pattern(pattern); - output->set_sinks({std::move(sink)}); - } -}; +// Defined in logger.hpp +struct svs_logger; extern "C" uint32_t svs_get_version() { return SVS_C_API_VERSION; } extern "C" const char* svs_get_version_string() { return SVS_C_API_VERSION_STRING; } -inline svs::logging::Level to_logging_level(svs_log_level_t level) { - switch (level) { - case SVS_LOG_LEVEL_TRACE: - return svs::logging::Level::Trace; - case SVS_LOG_LEVEL_DEBUG: - return svs::logging::Level::Debug; - case SVS_LOG_LEVEL_INFO: - return svs::logging::Level::Info; - case SVS_LOG_LEVEL_WARN: - return svs::logging::Level::Warn; - case SVS_LOG_LEVEL_ERROR: - return svs::logging::Level::Error; - case SVS_LOG_LEVEL_CRITICAL: - return svs::logging::Level::Critical; - case SVS_LOG_LEVEL_OFF: - return svs::logging::Level::Off; - default: - return svs::logging::Level::Info; - } -} - -extern "C" svs_logger_h svs_logger_create(svs_error_h out_err /*=NULL*/) { - using namespace svs::c_runtime; - return wrap_exceptions([&]() { return new svs_logger{}; }, out_err); -} - -extern "C" void svs_logger_free(svs_logger_h logger) { delete logger; } - -extern "C" bool svs_logger_set_kind( - svs_logger_h logger, - svs_logging_kind_t kind, - const char* path, - svs_error_h out_err /*=NULL*/ +extern "C" svs_logger_h svs_logger_create( + svs_logging_kind_t kind, const char* path, svs_error_h out_err /*=NULL*/ ) { using namespace svs::c_runtime; return wrap_exceptions( - [&]() { - INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); - svs::logging::sink_ptr sink{}; - switch (kind) { - case SVS_LOGGING_KIND_NONE: - sink = svs::logging::null_sink(); - break; - case SVS_LOGGING_KIND_STDOUT: - sink = svs::logging::stdout_sink(); - break; - case SVS_LOGGING_KIND_STDERR: - sink = svs::logging::stderr_sink(); - break; - case SVS_LOGGING_KIND_FILE_APPEND: - case SVS_LOGGING_KIND_FILE_TRUNCATE: - INVALID_ARGUMENT_IF( - path == nullptr || std::string(path).empty(), - "File path must be provided for file logging kind" - ); - sink = svs::logging::file_sink( - path, kind == SVS_LOGGING_KIND_FILE_TRUNCATE - ); - break; - case SVS_LOGGING_KIND_CUSTOM: - throw std::invalid_argument( - "Custom logging kind must be set using svs_logger_set_custom" - ); - default: - throw std::invalid_argument("Invalid logging kind"); - } - logger->set_output(std::move(sink)); - return true; - }, - out_err + [&]() { return new svs_logger{make_sink(kind, path)}; }, out_err ); } -extern "C" bool svs_logger_set_custom( - svs_logger_h logger, svs_logging_i user_logger, svs_error_h out_err /*=NULL*/ -) { +extern "C" svs_logger_h +svs_logger_create_custom(svs_logging_i user_logger, svs_error_h out_err /*=NULL*/) { using namespace svs::c_runtime; return wrap_exceptions( - [&]() { - INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); - validate_custom_logger(user_logger); - // Forward the bare message; the logger pattern does not apply here. - auto log = user_logger->ops->log; - auto self = user_logger->self; - logger->set_output(std::make_shared( - [log, self](const spdlog::details::log_msg& msg) { - std::string text(msg.payload.data(), msg.payload.size()); - log(self, static_cast(msg.level), text.c_str()); - } - )); - return true; - }, - out_err + [&]() { return new svs_logger{make_custom_sink(user_logger)}; }, out_err ); } +extern "C" void svs_logger_free(svs_logger_h logger) { delete logger; } + extern "C" bool svs_logger_set_level( svs_logger_h logger, svs_log_level_t level, svs_error_h out_err /*=NULL*/ ) { diff --git a/bindings/c/tests/README.md b/bindings/c/tests/README.md index d181f58e..92f79440 100644 --- a/bindings/c/tests/README.md +++ b/bindings/c/tests/README.md @@ -140,11 +140,12 @@ The tests cover the following aspects of the C API: ### Logging -- Logger handle creation and cleanup -- Output kinds (none, stdout, stderr, file truncate/append) and invalid kinds/paths -- Custom logging callback, including ops version/struct_size validation -- Level and pattern getters/setters, filtering, preservation across output changes -- Default logger receives SVS internal messages +- Logger handle creation (one per output: none, stdout, stderr, file truncate/append, + custom callback) and cleanup; invalid kinds/paths and invalid custom ops are rejected +- Custom logging callback receives the bare message, including ops version/struct_size + validation +- Level and pattern getters/setters, defaults, filtering, pattern applied to file output +- Default logger receives SVS internal messages; NULL resets it; it outlives the handle ### Dynamic Index Operations diff --git a/bindings/c/tests/c_api_logging.cpp b/bindings/c/tests/c_api_logging.cpp index 6b4e1e53..e159b744 100644 --- a/bindings/c/tests/c_api_logging.cpp +++ b/bindings/c/tests/c_api_logging.cpp @@ -115,114 +115,104 @@ void write_file(const std::string& path, const std::string& content) { } // namespace CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { - CATCH_SECTION("Create and Free") { + CATCH_SECTION("Create With Each Kind") { svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); - CATCH_REQUIRE(svs_error_ok(error)); - svs_logger_free(logger); - - // NULL error handle and freeing NULL are allowed. - logger = svs_logger_create(nullptr); - CATCH_REQUIRE(logger != nullptr); - svs_logger_free(logger); - svs_logger_free(nullptr); - svs_error_free(error); - } - - CATCH_SECTION("NULL Arguments") { - svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); - - auto expect_invalid = [&](bool result) { - CATCH_REQUIRE(result == false); - CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); - }; - - svs_log_level_t level; - const char* pattern = nullptr; - svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(noop_log); - svs_logging_t user_logger = {&ops, nullptr}; - - expect_invalid(svs_logger_set_kind(nullptr, SVS_LOGGING_KIND_STDOUT, nullptr, error) - ); - expect_invalid(svs_logger_set_custom(nullptr, &user_logger, error)); - expect_invalid(svs_logger_set_level(nullptr, SVS_LOG_LEVEL_INFO, error)); - expect_invalid(svs_logger_get_level(nullptr, &level, error)); - expect_invalid(svs_logger_get_level(logger, nullptr, error)); - expect_invalid(svs_logger_set_pattern(nullptr, "%v", error)); - expect_invalid(svs_logger_set_pattern(logger, nullptr, error)); - expect_invalid(svs_logger_get_pattern(nullptr, &pattern, error)); - expect_invalid(svs_logger_get_pattern(logger, nullptr, error)); - - // NULL error handle: failure is still reported by the return value. - CATCH_REQUIRE(svs_logger_set_level(nullptr, SVS_LOG_LEVEL_INFO, nullptr) == false); - - svs_logger_free(logger); - svs_error_free(error); - } - - CATCH_SECTION("Set Kind Console and None") { - svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); + TempDir tmp; + const std::string path = (tmp.path() / "svs.log").string(); for (auto kind : {SVS_LOGGING_KIND_NONE, SVS_LOGGING_KIND_STDOUT, SVS_LOGGING_KIND_STDERR}) { - CATCH_REQUIRE(svs_logger_set_kind(logger, kind, nullptr, error)); + // The path is ignored by these kinds. + svs_logger_h logger = svs_logger_create(kind, nullptr, error); + CATCH_REQUIRE(logger != nullptr); CATCH_REQUIRE(svs_error_ok(error)); + svs_logger_free(logger); + + logger = svs_logger_create(kind, "ignored", error); + CATCH_REQUIRE(logger != nullptr); + CATCH_REQUIRE(svs_error_ok(error)); + svs_logger_free(logger); } + for (auto kind : {SVS_LOGGING_KIND_FILE_APPEND, SVS_LOGGING_KIND_FILE_TRUNCATE}) { + svs_logger_h logger = svs_logger_create(kind, path.c_str(), error); + CATCH_REQUIRE(logger != nullptr); + CATCH_REQUIRE(svs_error_ok(error)); + svs_logger_free(logger); + } + + // NULL error handle and freeing NULL are allowed. + svs_logger_h logger = svs_logger_create(SVS_LOGGING_KIND_NONE, nullptr, nullptr); + CATCH_REQUIRE(logger != nullptr); svs_logger_free(logger); + svs_logger_free(nullptr); svs_error_free(error); } - CATCH_SECTION("Set Kind Invalid") { + CATCH_SECTION("Create Invalid") { svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); - // Custom output must go through svs_logger_set_custom. - CATCH_REQUIRE(!svs_logger_set_kind(logger, SVS_LOGGING_KIND_CUSTOM, nullptr, error) + // Custom output must go through svs_logger_create_custom. + CATCH_REQUIRE( + svs_logger_create(SVS_LOGGING_KIND_CUSTOM, nullptr, error) == nullptr ); CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); CATCH_REQUIRE( - std::string(svs_error_get_message(error)).find("svs_logger_set_custom") != + std::string(svs_error_get_message(error)).find("svs_logger_create_custom") != std::string::npos ); CATCH_REQUIRE( - !svs_logger_set_kind(logger, static_cast(6), nullptr, error) + svs_logger_create(static_cast(6), nullptr, error) == nullptr ); CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); // File kinds need a non-empty path. for (auto kind : {SVS_LOGGING_KIND_FILE_APPEND, SVS_LOGGING_KIND_FILE_TRUNCATE}) { - CATCH_REQUIRE(!svs_logger_set_kind(logger, kind, nullptr, error)); + CATCH_REQUIRE(svs_logger_create(kind, nullptr, error) == nullptr); CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); - CATCH_REQUIRE(!svs_logger_set_kind(logger, kind, "", error)); + CATCH_REQUIRE(svs_logger_create(kind, "", error) == nullptr); CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); } // A path that cannot be opened as a file (an existing directory). TempDir tmp; - CATCH_REQUIRE(!svs_logger_set_kind( - logger, SVS_LOGGING_KIND_FILE_TRUNCATE, tmp.string().c_str(), error - )); + CATCH_REQUIRE( + svs_logger_create( + SVS_LOGGING_KIND_FILE_TRUNCATE, tmp.string().c_str(), error + ) == nullptr + ); CATCH_REQUIRE(svs_error_get_code(error) != SVS_OK); - svs_logger_free(logger); + // NULL error handle: failure is still reported by the return value. + CATCH_REQUIRE( + svs_logger_create(SVS_LOGGING_KIND_FILE_APPEND, nullptr, nullptr) == nullptr + ); + svs_error_free(error); } - CATCH_SECTION("Set Custom Invalid") { + CATCH_SECTION("Create Custom") { svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(noop_log); + svs_logging_t user_logger = {&ops, nullptr}; + + svs_logger_h logger = svs_logger_create_custom(&user_logger, error); CATCH_REQUIRE(logger != nullptr); + CATCH_REQUIRE(svs_error_ok(error)); + svs_logger_free(logger); + + logger = svs_logger_create_custom(&user_logger, nullptr); + CATCH_REQUIRE(logger != nullptr); + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Create Custom Invalid") { + svs_error_h error = svs_error_create(); auto expect_invalid = [&](svs_logging_i user_logger) { - CATCH_REQUIRE(!svs_logger_set_custom(logger, user_logger, error)); + CATCH_REQUIRE(svs_logger_create_custom(user_logger, error) == nullptr); CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); }; @@ -246,18 +236,73 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { svs_logging_t bad_size_logger = {&bad_size, nullptr}; expect_invalid(&bad_size_logger); - svs_logging_ops_t good = SVS_INIT_LOGGING_OPS(noop_log); - svs_logging_t good_logger = {&good, nullptr}; - CATCH_REQUIRE(svs_logger_set_custom(logger, &good_logger, error)); - CATCH_REQUIRE(svs_error_ok(error)); + // NULL error handle: failure is still reported by the return value. + CATCH_REQUIRE(svs_logger_create_custom(nullptr, nullptr) == nullptr); + + svs_error_free(error); + } + + CATCH_SECTION("NULL Arguments") { + svs_error_h error = svs_error_create(); + svs_logger_h logger = svs_logger_create(SVS_LOGGING_KIND_NONE, nullptr, error); + CATCH_REQUIRE(logger != nullptr); + + auto expect_invalid = [&](bool result) { + CATCH_REQUIRE(result == false); + CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); + }; + + svs_log_level_t level; + const char* pattern = nullptr; + + expect_invalid(svs_logger_set_level(nullptr, SVS_LOG_LEVEL_INFO, error)); + expect_invalid(svs_logger_get_level(nullptr, &level, error)); + expect_invalid(svs_logger_get_level(logger, nullptr, error)); + expect_invalid(svs_logger_set_pattern(nullptr, "%v", error)); + expect_invalid(svs_logger_set_pattern(logger, nullptr, error)); + expect_invalid(svs_logger_get_pattern(nullptr, &pattern, error)); + expect_invalid(svs_logger_get_pattern(logger, nullptr, error)); + + // NULL error handle: failure is still reported by the return value. + CATCH_REQUIRE(svs_logger_set_level(nullptr, SVS_LOG_LEVEL_INFO, nullptr) == false); svs_logger_free(logger); svs_error_free(error); } + CATCH_SECTION("Defaults After Create") { + svs_error_h error = svs_error_create(); + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(noop_log); + svs_logging_t user_logger = {&ops, nullptr}; + TempDir tmp; + const std::string path = (tmp.path() / "svs.log").string(); + + auto check_defaults = [&](svs_logger_h logger) { + CATCH_REQUIRE(logger != nullptr); + svs_log_level_t level = SVS_LOG_LEVEL_OFF; + const char* pattern = nullptr; + CATCH_REQUIRE(svs_logger_get_level(logger, &level, error)); + CATCH_REQUIRE(level == SVS_LOG_LEVEL_WARN); + CATCH_REQUIRE(svs_logger_get_pattern(logger, &pattern, error)); + CATCH_REQUIRE(pattern != nullptr); + CATCH_REQUIRE(std::string(pattern) == "%v"); + svs_logger_free(logger); + }; + + check_defaults(svs_logger_create(SVS_LOGGING_KIND_NONE, nullptr, error)); + check_defaults(svs_logger_create(SVS_LOGGING_KIND_STDOUT, nullptr, error)); + check_defaults(svs_logger_create(SVS_LOGGING_KIND_STDERR, nullptr, error)); + check_defaults( + svs_logger_create(SVS_LOGGING_KIND_FILE_TRUNCATE, path.c_str(), error) + ); + check_defaults(svs_logger_create_custom(&user_logger, error)); + + svs_error_free(error); + } + CATCH_SECTION("Level Round Trip") { svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); + svs_logger_h logger = svs_logger_create(SVS_LOGGING_KIND_NONE, nullptr, error); CATCH_REQUIRE(logger != nullptr); for (auto level : @@ -280,7 +325,7 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { CATCH_SECTION("Pattern Round Trip") { svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); + svs_logger_h logger = svs_logger_create(SVS_LOGGING_KIND_NONE, nullptr, error); CATCH_REQUIRE(logger != nullptr); const char* pattern = nullptr; @@ -295,63 +340,6 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { svs_logger_free(logger); svs_error_free(error); } - - CATCH_SECTION("New Logger Defaults") { - DefaultLoggerGuard guard; - svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); - - svs_log_level_t level = SVS_LOG_LEVEL_OFF; - const char* pattern = nullptr; - CATCH_REQUIRE(svs_logger_get_level(logger, &level, error)); - CATCH_REQUIRE(level == SVS_LOG_LEVEL_WARN); - CATCH_REQUIRE(svs_logger_get_pattern(logger, &pattern, error)); - CATCH_REQUIRE(std::string(pattern) == "%v"); - - // No output configured: using it as default (even at TRACE) writes nothing and - // must not crash. - CATCH_REQUIRE(svs_set_default_logger(logger, error)); - build_small_index(); - CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); - build_small_index(); - CATCH_REQUIRE(svs_error_ok(error)); - - svs_logger_free(logger); - svs_error_free(error); - } - - CATCH_SECTION("Level and Pattern Preserved Across Output Changes") { - svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); - - CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_DEBUG, error)); - CATCH_REQUIRE(svs_logger_set_pattern(logger, "%v", error)); - - auto check = [&]() { - svs_log_level_t level = SVS_LOG_LEVEL_OFF; - const char* pattern = nullptr; - CATCH_REQUIRE(svs_logger_get_level(logger, &level, error)); - CATCH_REQUIRE(level == SVS_LOG_LEVEL_DEBUG); - CATCH_REQUIRE(svs_logger_get_pattern(logger, &pattern, error)); - CATCH_REQUIRE(std::string(pattern) == "%v"); - }; - - CATCH_REQUIRE(svs_logger_set_kind(logger, SVS_LOGGING_KIND_STDERR, nullptr, error)); - check(); - - svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(noop_log); - svs_logging_t user_logger = {&ops, nullptr}; - CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); - check(); - - CATCH_REQUIRE(svs_logger_set_kind(logger, SVS_LOGGING_KIND_NONE, nullptr, error)); - check(); - - svs_logger_free(logger); - svs_error_free(error); - } } CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { @@ -359,12 +347,11 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { LogRecorder recorder; // Declared first: must outlive the guard. DefaultLoggerGuard guard; svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); svs_logging_t user_logger = {&ops, &recorder}; - CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + svs_logger_h logger = svs_logger_create_custom(&user_logger, error); + CATCH_REQUIRE(logger != nullptr); CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); CATCH_REQUIRE(svs_set_default_logger(logger, error)); CATCH_REQUIRE(svs_error_ok(error)); @@ -382,63 +369,61 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { } } + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); svs_logger_free(logger); svs_error_free(error); } - CATCH_SECTION("Output Change Reaches Existing Users") { - LogRecorder first; // Declared first: must outlive the guard. - LogRecorder second; // Declared first: must outlive the guard. + CATCH_SECTION("None Output Is Silent") { DefaultLoggerGuard guard; svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); + svs_logger_h logger = svs_logger_create(SVS_LOGGING_KIND_NONE, nullptr, error); CATCH_REQUIRE(logger != nullptr); - svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); - svs_logging_t first_logger = {&ops, &first}; - svs_logging_t second_logger = {&ops, &second}; - CATCH_REQUIRE(svs_logger_set_custom(logger, &first_logger, error)); + // No output: using it as default (even at TRACE) writes nothing and must not + // crash. CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); CATCH_REQUIRE(svs_set_default_logger(logger, error)); - build_small_index(); - CATCH_REQUIRE(first.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); - { - std::lock_guard lock{first.mutex}; - first.messages.clear(); - } + CATCH_REQUIRE(svs_error_ok(error)); + + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); + svs_logger_free(logger); + svs_error_free(error); + } + + CATCH_SECTION("Level Changes Reach Existing Users") { + LogRecorder recorder; // Declared first: must outlive the guard. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t user_logger = {&ops, &recorder}; + svs_logger_h logger = svs_logger_create_custom(&user_logger, error); + CATCH_REQUIRE(logger != nullptr); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); - // Change the output of the SAME handle without calling svs_set_default_logger - // again: the global default logger must follow. - CATCH_REQUIRE(svs_logger_set_custom(logger, &second_logger, error)); build_small_index(); - CATCH_REQUIRE(second.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); - { - std::lock_guard lock{first.mutex}; - CATCH_REQUIRE(first.messages.empty()); - } + CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); - // Level changes also apply immediately. + // Level changes apply without calling svs_set_default_logger again. { - std::lock_guard lock{second.mutex}; - second.messages.clear(); + std::lock_guard lock{recorder.mutex}; + recorder.messages.clear(); } CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_OFF, error)); build_small_index(); { - std::lock_guard lock{second.mutex}; - CATCH_REQUIRE(second.messages.empty()); + std::lock_guard lock{recorder.mutex}; + CATCH_REQUIRE(recorder.messages.empty()); } - // Switching to no output stops the callback. CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); - CATCH_REQUIRE(svs_logger_set_kind(logger, SVS_LOGGING_KIND_NONE, nullptr, error)); build_small_index(); - { - std::lock_guard lock{second.mutex}; - CATCH_REQUIRE(second.messages.empty()); - } + CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); svs_logger_free(logger); svs_error_free(error); } @@ -447,30 +432,32 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { LogRecorder recorder; // Declared first: must outlive the guard. DefaultLoggerGuard guard; svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); svs_logging_t user_logger = {&ops, &recorder}; - CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + svs_logger_h logger = svs_logger_create_custom(&user_logger, error); + CATCH_REQUIRE(logger != nullptr); CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); CATCH_REQUIRE(svs_set_default_logger(logger, error)); CATCH_REQUIRE(svs_logger_set_pattern(logger, "[x] %v", error)); build_small_index(); - std::lock_guard lock{recorder.mutex}; - CATCH_REQUIRE(!recorder.messages.empty()); - for (const auto& [level, text] : recorder.messages) { - CATCH_REQUIRE(text.rfind("[x] ", 0) == std::string::npos); + { + std::lock_guard lock{recorder.mutex}; + CATCH_REQUIRE(!recorder.messages.empty()); + for (const auto& [level, text] : recorder.messages) { + CATCH_REQUIRE(text.rfind("[x] ", 0) == std::string::npos); + } + bool found = std::any_of( + recorder.messages.begin(), + recorder.messages.end(), + [](const auto& m) { return m.second.rfind("Number of syncs: ", 0) == 0; } + ); + CATCH_REQUIRE(found); } - bool found = std::any_of( - recorder.messages.begin(), - recorder.messages.end(), - [](const auto& m) { return m.second.rfind("Number of syncs: ", 0) == 0; } - ); - CATCH_REQUIRE(found); + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); svs_logger_free(logger); svs_error_free(error); } @@ -479,47 +466,25 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { LogRecorder recorder; // Declared first: must outlive the guard. DefaultLoggerGuard guard; svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); svs_logging_t user_logger = {&ops, &recorder}; - CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); - CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_WARN, error)); + svs_logger_h logger = svs_logger_create_custom(&user_logger, error); + CATCH_REQUIRE(logger != nullptr); + // Default level is WARN: the TRACE/DEBUG build messages must be filtered. CATCH_REQUIRE(svs_set_default_logger(logger, error)); - build_small_index(); - CATCH_REQUIRE(!recorder.any_below(SVS_LOG_LEVEL_WARN)); - svs_logger_free(logger); - svs_error_free(error); - } - - CATCH_SECTION("Bare Message Pattern") { - LogRecorder recorder; // Declared first: must outlive the guard. - DefaultLoggerGuard guard; - svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_INFO, error)); + build_small_index(); + CATCH_REQUIRE(!recorder.any_below(SVS_LOG_LEVEL_INFO)); - svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); - svs_logging_t user_logger = {&ops, &recorder}; - CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); - CATCH_REQUIRE(svs_logger_set_pattern(logger, "%v", error)); - CATCH_REQUIRE(svs_set_default_logger(logger, error)); - build_small_index(); + CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); - std::lock_guard lock{recorder.mutex}; - bool found = std::any_of( - recorder.messages.begin(), - recorder.messages.end(), - [](const auto& m) { return m.second.rfind("Number of syncs: ", 0) == 0; } - ); - CATCH_REQUIRE(found); - + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); svs_logger_free(logger); svs_error_free(error); } @@ -530,20 +495,20 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { { DefaultLoggerGuard guard; svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); + svs_logger_h logger = + svs_logger_create(SVS_LOGGING_KIND_FILE_TRUNCATE, path.c_str(), error); CATCH_REQUIRE(logger != nullptr); - // Set before the output: carried over to the file output. CATCH_REQUIRE(svs_logger_set_pattern(logger, "[x] %v", error)); CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); - CATCH_REQUIRE(svs_logger_set_kind( - logger, SVS_LOGGING_KIND_FILE_TRUNCATE, path.c_str(), error - )); CATCH_REQUIRE(svs_set_default_logger(logger, error)); svs_logger_free(logger); build_small_index(); + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); svs_error_free(error); } + // Resetting the default logger released the last reference, which closes (and + // flushes) the file. auto content = read_file(path); CATCH_REQUIRE(content.find("[x] Number of syncs") != std::string::npos); } @@ -552,12 +517,11 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { LogRecorder recorder; // Declared first: must outlive the guard. DefaultLoggerGuard guard; svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); svs_logging_t user_logger = {&ops, &recorder}; - CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + svs_logger_h logger = svs_logger_create_custom(&user_logger, error); + CATCH_REQUIRE(logger != nullptr); CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); CATCH_REQUIRE(svs_set_default_logger(logger, error)); svs_logger_free(logger); @@ -565,6 +529,7 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { build_small_index(); CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); svs_error_free(error); } @@ -572,12 +537,11 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { LogRecorder recorder; // Declared first: must outlive the guard. DefaultLoggerGuard guard; svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); - CATCH_REQUIRE(logger != nullptr); svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); svs_logging_t user_logger = {&ops, &recorder}; - CATCH_REQUIRE(svs_logger_set_custom(logger, &user_logger, error)); + svs_logger_h logger = svs_logger_create_custom(&user_logger, error); + CATCH_REQUIRE(logger != nullptr); CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); CATCH_REQUIRE(svs_set_default_logger(logger, error)); @@ -601,6 +565,49 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { svs_error_free(error); } + CATCH_SECTION("Switch Default Logger To Another Logger") { + LogRecorder first; // Declared first: must outlive the guard. + LogRecorder second; // Declared first: must outlive the guard. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t first_logger = {&ops, &first}; + svs_logging_t second_logger = {&ops, &second}; + svs_logger_h logger_a = svs_logger_create_custom(&first_logger, error); + svs_logger_h logger_b = svs_logger_create_custom(&second_logger, error); + CATCH_REQUIRE(logger_a != nullptr); + CATCH_REQUIRE(logger_b != nullptr); + CATCH_REQUIRE(svs_logger_set_level(logger_a, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_logger_set_level(logger_b, SVS_LOG_LEVEL_TRACE, error)); + + CATCH_REQUIRE(svs_set_default_logger(logger_a, error)); + build_small_index(); + CATCH_REQUIRE(first.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + { + std::lock_guard lock{second.mutex}; + CATCH_REQUIRE(second.messages.empty()); + } + + // The output of a logger cannot change; to log elsewhere, install another logger. + { + std::lock_guard lock{first.mutex}; + first.messages.clear(); + } + CATCH_REQUIRE(svs_set_default_logger(logger_b, error)); + build_small_index(); + CATCH_REQUIRE(second.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + { + std::lock_guard lock{first.mutex}; + CATCH_REQUIRE(first.messages.empty()); + } + + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); + svs_logger_free(logger_a); + svs_logger_free(logger_b); + svs_error_free(error); + } + CATCH_SECTION("File Truncate") { TempDir tmp; const std::string path = (tmp.path() / "svs.log").string(); @@ -608,19 +615,17 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { { DefaultLoggerGuard guard; svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); + svs_logger_h logger = + svs_logger_create(SVS_LOGGING_KIND_FILE_TRUNCATE, path.c_str(), error); CATCH_REQUIRE(logger != nullptr); CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); - CATCH_REQUIRE(svs_logger_set_kind( - logger, SVS_LOGGING_KIND_FILE_TRUNCATE, path.c_str(), error - )); CATCH_REQUIRE(svs_set_default_logger(logger, error)); svs_logger_free(logger); build_small_index(); + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); svs_error_free(error); } - // The guard released the last reference, which closes (and flushes) the file. auto content = read_file(path); CATCH_REQUIRE(content.find("OLD CONTENT") == std::string::npos); CATCH_REQUIRE(content.find("Number of syncs") != std::string::npos); @@ -633,16 +638,15 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { { DefaultLoggerGuard guard; svs_error_h error = svs_error_create(); - svs_logger_h logger = svs_logger_create(error); + svs_logger_h logger = + svs_logger_create(SVS_LOGGING_KIND_FILE_APPEND, path.c_str(), error); CATCH_REQUIRE(logger != nullptr); CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); - CATCH_REQUIRE(svs_logger_set_kind( - logger, SVS_LOGGING_KIND_FILE_APPEND, path.c_str(), error - )); CATCH_REQUIRE(svs_set_default_logger(logger, error)); svs_logger_free(logger); build_small_index(); + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); svs_error_free(error); } auto content = read_file(path); diff --git a/bindings/c/tests/consumer/main.c b/bindings/c/tests/consumer/main.c index 90cc3955..9f61ac7c 100644 --- a/bindings/c/tests/consumer/main.c +++ b/bindings/c/tests/consumer/main.c @@ -63,16 +63,11 @@ static int check_logging(svs_error_h error) { user_logger.ops = &ops; user_logger.self = &count; - svs_logger_h logger = svs_logger_create(error); + svs_logger_h logger = svs_logger_create_custom(&user_logger, error); if (logger == NULL) { - fprintf(stderr, "failed to create a logger: %s\n", svs_error_get_message(error)); - return 0; - } - if (!svs_logger_set_custom(logger, &user_logger, error)) { fprintf( - stderr, "failed to set a custom logger: %s\n", svs_error_get_message(error) + stderr, "failed to create a custom logger: %s\n", svs_error_get_message(error) ); - svs_logger_free(logger); return 0; } svs_logger_free(logger); From 58052ab8c932954f69d659d7d330dfba1f75de90 Mon Sep 17 00:00:00 2001 From: yuejiaointel Date: Tue, 6 Oct 2026 12:04:35 -0700 Subject: [PATCH 5/8] [C API] Logger review fixes: drop unimplemented decl and dead kind, fix docs - Remove the svs_index_builder_set_logger declaration: it has no definition in this change, so any caller would fail to link. Per-index loggers come in a follow-up once the core orchestrators accept a logger. - Remove SVS_LOGGING_KIND_CUSTOM: custom loggers are created with svs_logger_create_custom, so the kind had no use and svs_logger_create only rejected it. The remaining values keep their numbering; out-of-range kinds still return SVS_ERROR_INVALID_ARGUMENT. - svs_set_default_logger docs: indexes capture the default logger current when they are built or loaded and keep using it; a later default change does not affect them, so a custom callback and its self must outlive such indexes. NULL rebuilds the built-in default from SVS_LOG_SINK/SVS_LOG_LEVEL. - svs_c.cpp: drop the redundant forward declaration of svs_logger and the duplicate include (both come from logger.hpp). - Tests: check the exact bare message "Number of syncs: 40" instead of scanning for a '[' prefix; add "Default Logger Change Does Not Affect Built Index" (dynamic index keeps logging to the captured logger after svs_set_default_logger(NULL)); kind value 5 is now rejected. --- bindings/c/include/svs/c/svs_c.h | 33 ++++------ bindings/c/src/logger.hpp | 6 +- bindings/c/src/svs_c.cpp | 4 -- bindings/c/tests/c_api_logging.cpp | 97 +++++++++++++++++++++++++----- 4 files changed, 95 insertions(+), 45 deletions(-) diff --git a/bindings/c/include/svs/c/svs_c.h b/bindings/c/include/svs/c/svs_c.h index 8fc4aa56..1a0ac51f 100644 --- a/bindings/c/include/svs/c/svs_c.h +++ b/bindings/c/include/svs/c/svs_c.h @@ -73,8 +73,7 @@ enum svs_logging_kind { SVS_LOGGING_KIND_STDOUT = 1, SVS_LOGGING_KIND_STDERR = 2, SVS_LOGGING_KIND_FILE_APPEND = 3, - SVS_LOGGING_KIND_FILE_TRUNCATE = 4, - SVS_LOGGING_KIND_CUSTOM = 5 + SVS_LOGGING_KIND_FILE_TRUNCATE = 4 }; typedef struct svs_error_desc* svs_error_h; @@ -622,16 +621,14 @@ SVS_API void svs_error_free(svs_error_h err); /// @brief Create a logger writing to a built-in output /// @param kind The output of the logger: SVS_LOGGING_KIND_NONE (discard everything), /// SVS_LOGGING_KIND_STDOUT, SVS_LOGGING_KIND_STDERR, SVS_LOGGING_KIND_FILE_APPEND or -/// SVS_LOGGING_KIND_FILE_TRUNCATE. SVS_LOGGING_KIND_CUSTOM is rejected; use -/// svs_logger_create_custom() instead. +/// SVS_LOGGING_KIND_FILE_TRUNCATE. For a user callback use svs_logger_create_custom(). /// @param path The file to write to for SVS_LOGGING_KIND_FILE_APPEND and /// SVS_LOGGING_KIND_FILE_TRUNCATE (must not be NULL or empty; missing directories are /// created). Ignored by the other kinds; may be NULL. /// @param out_err An optional error handle to capture errors /// @return A handle to the created logger or NULL if creation failed /// @remarks The output of a logger is fixed at creation. To log somewhere else, create -/// another logger and pass it to svs_set_default_logger() or -/// svs_index_builder_set_logger(). +/// another logger and pass it to svs_set_default_logger(). /// @remarks A new logger has level SVS_LOG_LEVEL_WARN and pattern "%v" (the bare /// message). The SVS_LOG_SINK / SVS_LOG_LEVEL environment variables are not read here; /// they only configure the SVS built-in default logger. @@ -662,9 +659,9 @@ svs_logger_create_custom(svs_logging_i user_logger, svs_error_h out_err /*=NULL* /// @brief Free a logger handle /// @param logger The logger handle to free. Passing NULL is a no-op. /// @remarks The SVS global default logger (see svs_set_default_logger) and indexes built -/// with the logger keep their own reference to the underlying logger, so freeing the -/// handle does not stop it from logging. User data referenced by a custom logger must -/// stay valid as long as such a reference exists. +/// or loaded while it was the default keep their own reference to the underlying logger, +/// so freeing the handle does not stop it from logging. User data referenced by a custom +/// logger must stay valid as long as such a reference exists. SVS_API void svs_logger_free(svs_logger_h logger); /// @brief Set the logging level for a logger @@ -714,12 +711,13 @@ SVS_API bool svs_logger_get_pattern( /// @brief Set default logger for SVS library /// @param logger The logger handle to set as default. NULL restores the SVS built-in -/// default logger (configured from the SVS_LOG_LEVEL / SVS_LOG_SINK environment -/// variables). +/// default logger, which is rebuilt from the SVS_LOG_SINK / SVS_LOG_LEVEL environment +/// variables (for a "file:" sink this reopens the file in truncate mode). /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure -/// @remarks The default logger will be used for all subsequent logging operations unless -/// explicitly overridden by index builder. +/// @remarks Indexes capture the default logger current when they are built or loaded and +/// keep using it. Setting a new default (or NULL) does not change already-built indexes, +/// so a custom logger's callback and @p self must stay valid while any such index exists. SVS_API bool svs_set_default_logger( svs_logger_h logger, svs_error_h out_err /*=NULL*/ ); @@ -956,15 +954,6 @@ SVS_API svs_index_builder_h svs_index_builder_create( /// @param builder The index builder handle to free SVS_API void svs_index_builder_free(svs_index_builder_h builder); -/// @brief Set the logger for the index builder -/// @param builder The index builder handle -/// @param logger The logger handle to set for the index builder -/// @param out_err An optional error handle to capture errors -/// @return true on success, false on failure -SVS_API bool svs_index_builder_set_logger( - svs_index_builder_h builder, svs_logger_h logger, svs_error_h out_err /*=NULL*/ -); - /// @brief Set the storage configuration for the index builder /// @param builder The index builder handle /// @param storage The storage configuration handle diff --git a/bindings/c/src/logger.hpp b/bindings/c/src/logger.hpp index 9a880644..017100c2 100644 --- a/bindings/c/src/logger.hpp +++ b/bindings/c/src/logger.hpp @@ -89,7 +89,7 @@ inline svs::logging::Level to_logging_level(svs_log_level_t level) { } /// Creates the spdlog sink for a built-in output kind. `path` is only used by the file -/// kinds. SVS_LOGGING_KIND_CUSTOM is rejected: use svs_logger_create_custom(). +/// kinds. inline svs::logging::sink_ptr make_sink(svs_logging_kind_t kind, const char* path) { switch (kind) { case SVS_LOGGING_KIND_NONE: @@ -106,10 +106,6 @@ inline svs::logging::sink_ptr make_sink(svs_logging_kind_t kind, const char* pat ); } return svs::logging::file_sink(path, kind == SVS_LOGGING_KIND_FILE_TRUNCATE); - case SVS_LOGGING_KIND_CUSTOM: - throw std::invalid_argument( - "Custom loggers must be created with svs_logger_create_custom" - ); default: throw std::invalid_argument("Invalid logging kind"); } diff --git a/bindings/c/src/svs_c.cpp b/bindings/c/src/svs_c.cpp index 6c21b556..f1fd097e 100644 --- a/bindings/c/src/svs_c.cpp +++ b/bindings/c/src/svs_c.cpp @@ -37,7 +37,6 @@ #include #include -#include #include #include #include @@ -67,9 +66,6 @@ struct svs_leanvec_training_data { std::shared_ptr impl; }; -// Defined in logger.hpp -struct svs_logger; - extern "C" uint32_t svs_get_version() { return SVS_C_API_VERSION; } extern "C" const char* svs_get_version_string() { return SVS_C_API_VERSION_STRING; } diff --git a/bindings/c/tests/c_api_logging.cpp b/bindings/c/tests/c_api_logging.cpp index e159b744..10732138 100644 --- a/bindings/c/tests/c_api_logging.cpp +++ b/bindings/c/tests/c_api_logging.cpp @@ -152,16 +152,11 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { CATCH_SECTION("Create Invalid") { svs_error_h error = svs_error_create(); - // Custom output must go through svs_logger_create_custom. + // Out-of-range kinds (the enum ends at SVS_LOGGING_KIND_FILE_TRUNCATE = 4). CATCH_REQUIRE( - svs_logger_create(SVS_LOGGING_KIND_CUSTOM, nullptr, error) == nullptr + svs_logger_create(static_cast(5), nullptr, error) == nullptr ); CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); - CATCH_REQUIRE( - std::string(svs_error_get_message(error)).find("svs_logger_create_custom") != - std::string::npos - ); - CATCH_REQUIRE( svs_logger_create(static_cast(6), nullptr, error) == nullptr ); @@ -358,15 +353,21 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { build_small_index(); - CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); - // Default pattern "%v": bare message, no timestamp/level prefix, no newline. + // Default pattern "%v": the callback gets the bare message, no timestamp/level + // prefix, no trailing newline. build_small_index() builds 100 vectors, which + // Vamana splits into max(40, ceil(100 / 4096)) = 40 batches, so the exact text + // is known. { std::lock_guard lock{recorder.mutex}; - for (const auto& [level, text] : recorder.messages) { - CATCH_REQUIRE(!text.empty()); - CATCH_REQUIRE(text.front() != '['); - CATCH_REQUIRE(text.back() != '\n'); - } + bool found = std::any_of( + recorder.messages.begin(), + recorder.messages.end(), + [](const auto& m) { + return m.first == SVS_LOG_LEVEL_TRACE && + m.second == "Number of syncs: 40"; + } + ); + CATCH_REQUIRE(found); } CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); @@ -653,4 +654,72 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { CATCH_REQUIRE(content.rfind("OLD CONTENT\n", 0) == 0); CATCH_REQUIRE(content.find("Number of syncs") != std::string::npos); } + + CATCH_SECTION("Default Logger Change Does Not Affect Built Index") { + LogRecorder recorder; // Declared first: must outlive the guard and the index. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t user_logger = {&ops, &recorder}; + svs_logger_h logger = svs_logger_create_custom(&user_logger, error); + CATCH_REQUIRE(logger != nullptr); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + CATCH_REQUIRE(svs_set_default_logger(logger, error)); + + // A dynamic index captures the default logger current at build time ... + const size_t num_vectors = 100; + const size_t dimension = 16; + std::vector data; + generate_test_data(data, num_vectors, dimension); + std::vector ids(num_vectors); + for (size_t i = 0; i < num_vectors; ++i) { + ids[i] = i; + } + svs_algorithm_h algorithm = svs_algorithm_create_vamana(16, 32, 50, error); + CATCH_REQUIRE(algorithm != nullptr); + svs_index_builder_h builder = svs_index_builder_create( + SVS_DISTANCE_METRIC_EUCLIDEAN, dimension, algorithm, error + ); + CATCH_REQUIRE(builder != nullptr); + CATCH_REQUIRE( + svs_index_builder_set_threadpool(builder, SVS_THREADPOOL_KIND_NATIVE, 2, error) + ); + svs_index_h index = svs_index_build_dynamic( + builder, data.data(), ids.data(), num_vectors, 0, error + ); + CATCH_REQUIRE(index != nullptr); + CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + + // ... and keeps using it after the default is restored: later operations on the + // index still reach the callback, so its `self` must outlive the index. + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); + { + std::lock_guard lock{recorder.mutex}; + recorder.messages.clear(); + } + std::vector new_data; + generate_test_data(new_data, num_vectors, dimension); + std::vector new_ids(num_vectors); + for (size_t i = 0; i < num_vectors; ++i) { + new_ids[i] = num_vectors + i; + } + CATCH_REQUIRE(svs_index_dynamic_add_points( + index, new_data.data(), new_ids.data(), num_vectors, nullptr, error + )); + // Deleting every original point deletes the entry point, so consolidate logs + // "Replacing entry point." at DEBUG level through the index's logger. + CATCH_REQUIRE( + svs_index_dynamic_delete_points(index, ids.data(), num_vectors, nullptr, error) + ); + CATCH_REQUIRE(svs_index_dynamic_consolidate(index, error)); + CATCH_REQUIRE(svs_error_ok(error)); + CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_DEBUG, "Replacing entry point")); + + svs_index_free(index); + svs_index_builder_free(builder); + svs_algorithm_free(algorithm); + svs_logger_free(logger); + svs_error_free(error); + } } From 3ee9e29d4ca8772e793c325601077167eefe3e97 Mon Sep 17 00:00:00 2001 From: yuejiaointel Date: Tue, 6 Oct 2026 15:43:38 -0700 Subject: [PATCH 6/8] [C API] Trim logger comments to header style --- bindings/c/include/svs/c/svs_c.h | 103 ++++++++++------------------- bindings/c/src/logger.hpp | 14 +--- bindings/c/src/svs_c.cpp | 2 - bindings/c/tests/README.md | 9 +-- bindings/c/tests/c_api_logging.cpp | 36 ++-------- 5 files changed, 47 insertions(+), 117 deletions(-) diff --git a/bindings/c/include/svs/c/svs_c.h b/bindings/c/include/svs/c/svs_c.h index 1a0ac51f..bb6475b6 100644 --- a/bindings/c/include/svs/c/svs_c.h +++ b/bindings/c/include/svs/c/svs_c.h @@ -58,6 +58,7 @@ enum svs_error_code { SVS_ERROR_UNKNOWN = 1000 }; +/// @brief Severity of a log message. enum svs_log_level { SVS_LOG_LEVEL_TRACE = 0, SVS_LOG_LEVEL_DEBUG = 1, @@ -68,6 +69,7 @@ enum svs_log_level { SVS_LOG_LEVEL_OFF = 6 }; +/// @brief Output of a logger created with svs_logger_create(). enum svs_logging_kind { SVS_LOGGING_KIND_NONE = 0, SVS_LOGGING_KIND_STDOUT = 1, @@ -142,18 +144,10 @@ enum svs_threadpool_kind { SVS_THREADPOOL_KIND_CUSTOM = 3 }; -/// @brief Operations table for a custom logging interface. -/// @remarks Lifetime: the operations table, the function it points to and the @p self -/// pointer of the owning svs_logging_interface must remain valid for as long as any -/// logger created with them (see svs_logger_create_custom) is in use, i.e. while the -/// svs_logger_h handle, any index built with it, or the SVS global default logger (see -/// svs_set_default_logger) still refers to it. -/// @remarks Thread safety: the @p log function may be called from SVS worker threads. -/// Calls through one logger handle are serialized by an internal mutex, and the -/// callback runs while that mutex is held. The callback must not call back into SVS -/// logging functions (it would deadlock), and must not throw C++ exceptions or -/// longjmp out of the call. -/// +/// @brief Operations table for a custom logging interface +/// @remarks The user must ensure that the log function is thread-safe (it may be called +/// from SVS worker threads and must not throw or longjmp) and that the operations table +/// and @p self remain valid while any logger, index or default logger using them exists. /// @var svs_logging_interface_ops::version /// Version of the logging interface. /// @var svs_logging_interface_ops::struct_size @@ -162,29 +156,28 @@ enum svs_threadpool_kind { /// Function pointer to log a message. /// @param self Pointer to the logging interface instance. /// @param level Logging level of the message. -/// @param message Null-terminated string containing the bare message text, without a -/// trailing newline. The logger pattern (see svs_logger_set_pattern) is not applied; -/// use @p level and @p self to format it. The pointer is only valid for the duration -/// of the call. +/// @param message Null-terminated bare message text, valid only for the duration of the +/// call. struct svs_logging_interface_ops { uint32_t version; size_t struct_size; void (*log)(void* self, enum svs_log_level level, const char* message); }; -/// @brief Macro to create a user-defined logging interface operations structure. +/// @brief Macro to create a user-defined logging interface operations structure +/// @param log_func Function pointer to log a message #define SVS_INIT_LOGGING_OPS(log_func) \ { \ .version = SVS_C_API_VERSION, \ .struct_size = sizeof(struct svs_logging_interface_ops), .log = &log_func \ } -/// @brief Represents a user-defined logging interface instance. +/// @brief Structure representing a custom logging interface /// @var svs_logging_interface::ops -/// Pointer to the operations table defining the behavior of the logging interface. +/// Function pointers for the logging operations. /// @var svs_logging_interface::self -/// Pointer to user-defined data associated with the logging interface instance. This -/// pointer is passed to the logging functions as the @p self parameter. +/// Pointer to the user-defined logger instance. This pointer is passed to the +/// function pointers in @p ops when they are called. struct svs_logging_interface { const struct svs_logging_interface_ops* ops; void* self; @@ -619,49 +612,31 @@ SVS_API const char* svs_error_get_message(svs_error_h err); SVS_API void svs_error_free(svs_error_h err); /// @brief Create a logger writing to a built-in output -/// @param kind The output of the logger: SVS_LOGGING_KIND_NONE (discard everything), -/// SVS_LOGGING_KIND_STDOUT, SVS_LOGGING_KIND_STDERR, SVS_LOGGING_KIND_FILE_APPEND or -/// SVS_LOGGING_KIND_FILE_TRUNCATE. For a user callback use svs_logger_create_custom(). -/// @param path The file to write to for SVS_LOGGING_KIND_FILE_APPEND and -/// SVS_LOGGING_KIND_FILE_TRUNCATE (must not be NULL or empty; missing directories are -/// created). Ignored by the other kinds; may be NULL. +/// @param kind The output kind of the logger +/// @param path The file to write to for the SVS_LOGGING_KIND_FILE_* kinds; ignored +/// otherwise /// @param out_err An optional error handle to capture errors /// @return A handle to the created logger or NULL if creation failed -/// @remarks The output of a logger is fixed at creation. To log somewhere else, create -/// another logger and pass it to svs_set_default_logger(). -/// @remarks A new logger has level SVS_LOG_LEVEL_WARN and pattern "%v" (the bare -/// message). The SVS_LOG_SINK / SVS_LOG_LEVEL environment variables are not read here; -/// they only configure the SVS built-in default logger. -/// @remarks Level and pattern changes made through a handle apply immediately to -/// everything already using its logger (the SVS global default logger and indexes). -/// @remarks Thread safety: the functions that modify a logger handle -/// (svs_logger_set_level, svs_logger_set_pattern, svs_logger_free) are not thread-safe. -/// Do not call them concurrently with each other or with other functions using the same -/// handle. +/// @remarks The output of a logger is fixed at creation; a new logger has level +/// SVS_LOG_LEVEL_WARN and pattern "%v". SVS_API svs_logger_h svs_logger_create( svs_logging_kind_t kind, const char* path, svs_error_h out_err /*=NULL*/ ); /// @brief Create a logger forwarding every message to a user callback -/// @param user_logger The custom logging interface; @p user_logger->ops must be -/// initialized with SVS_INIT_LOGGING_OPS. The version, struct_size and log fields are -/// validated. See svs_logging_interface_ops for the lifetime and thread-safety -/// requirements of the callback. +/// @param user_logger The custom logging interface, initialized with SVS_INIT_LOGGING_OPS /// @param out_err An optional error handle to capture errors /// @return A handle to the created logger or NULL if creation failed -/// @remarks The callback always receives the bare message text (plus level and self); -/// the logger pattern does not apply to it. -/// @remarks Same defaults, lifetime and thread-safety rules as svs_logger_create(): the -/// output is fixed at creation, level SVS_LOG_LEVEL_WARN, pattern "%v". +/// @remarks The callback may be called from SVS worker threads and must be thread-safe and +/// must not throw or longjmp. It receives the bare message; the logger pattern does not +/// apply to it. SVS_API svs_logger_h svs_logger_create_custom(svs_logging_i user_logger, svs_error_h out_err /*=NULL*/); /// @brief Free a logger handle -/// @param logger The logger handle to free. Passing NULL is a no-op. -/// @remarks The SVS global default logger (see svs_set_default_logger) and indexes built -/// or loaded while it was the default keep their own reference to the underlying logger, -/// so freeing the handle does not stop it from logging. User data referenced by a custom -/// logger must stay valid as long as such a reference exists. +/// @param logger The logger handle to free +/// @remarks Indexes and the SVS default logger using this logger keep it alive after the +/// handle is freed. SVS_API void svs_logger_free(svs_logger_h logger); /// @brief Set the logging level for a logger @@ -669,7 +644,6 @@ SVS_API void svs_logger_free(svs_logger_h logger); /// @param level The logging level to set /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure -/// @remarks Applies immediately to everything already using this logger. SVS_API bool svs_logger_set_level( svs_logger_h logger, svs_log_level_t level, svs_error_h out_err /*=NULL*/ ); @@ -685,39 +659,32 @@ SVS_API bool svs_logger_get_level( /// @brief Set format pattern for a logger /// @param logger The logger handle -/// @param pattern The format pattern to set, in spdlog pattern syntax (e.g. "%v" for -/// the bare message, or "[index A] %v" to prefix every message). Must not be NULL. +/// @param pattern The format pattern to set, in spdlog syntax (e.g. "[index A] %v") /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure -/// @remarks The pattern applies to the stdout, stderr and file output kinds. It does not -/// apply to custom callbacks (svs_logger_create_custom), which always receive the bare -/// message text. Applies immediately to everything already using this logger. +/// @remarks The pattern applies to the stdout, stderr and file outputs; custom callbacks +/// receive the bare message. SVS_API bool svs_logger_set_pattern( svs_logger_h logger, const char* pattern, svs_error_h out_err /*=NULL*/ ); /// @brief Get the format pattern for a logger /// @param logger The logger handle -/// @param out_pattern Pointer to store the retrieved format pattern (used by the -/// stdout, stderr and file outputs, not by custom callbacks). The string is -/// owned by the logger handle and stays valid until the next svs_logger_set_pattern -/// call on the handle or until svs_logger_free. If no pattern was set, returns the -/// default pattern "%v". +/// @param out_pattern Pointer to store the retrieved format pattern (default "%v") /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure +/// @remarks The returned pointer is valid until the next svs_logger_set_pattern() call on +/// the handle or until svs_logger_free(). SVS_API bool svs_logger_get_pattern( svs_logger_h logger, const char** out_pattern, svs_error_h out_err /*=NULL*/ ); /// @brief Set default logger for SVS library -/// @param logger The logger handle to set as default. NULL restores the SVS built-in -/// default logger, which is rebuilt from the SVS_LOG_SINK / SVS_LOG_LEVEL environment -/// variables (for a "file:" sink this reopens the file in truncate mode). +/// @param logger The logger handle to set as default; NULL restores the SVS built-in +/// default /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure -/// @remarks Indexes capture the default logger current when they are built or loaded and -/// keep using it. Setting a new default (or NULL) does not change already-built indexes, -/// so a custom logger's callback and @p self must stay valid while any such index exists. +/// @remarks Indexes keep the default logger current when they are built or loaded. SVS_API bool svs_set_default_logger( svs_logger_h logger, svs_error_h out_err /*=NULL*/ ); diff --git a/bindings/c/src/logger.hpp b/bindings/c/src/logger.hpp index 017100c2..b856dcc7 100644 --- a/bindings/c/src/logger.hpp +++ b/bindings/c/src/logger.hpp @@ -43,8 +43,7 @@ static_assert(static_cast(SVS_LOG_LEVEL_OFF) == SPDLOG_LEVEL_OFF); /// Name given to every spdlog logger created by the C API. inline constexpr const char* logger_name = "svs_c"; -/// Default pattern of every logger handle: the bare message. Patterns apply to the -/// stdout, stderr and file outputs; custom callbacks always receive the bare message. +/// Default pattern of every logger handle: the bare message. inline constexpr const char* default_log_pattern = "%v"; /// Checks the version, struct size and NULL pointers of a user-provided custom logger. @@ -88,8 +87,7 @@ inline svs::logging::Level to_logging_level(svs_log_level_t level) { } } -/// Creates the spdlog sink for a built-in output kind. `path` is only used by the file -/// kinds. +/// Creates the spdlog sink for a built-in output kind. inline svs::logging::sink_ptr make_sink(svs_logging_kind_t kind, const char* path) { switch (kind) { case SVS_LOGGING_KIND_NONE: @@ -111,8 +109,7 @@ inline svs::logging::sink_ptr make_sink(svs_logging_kind_t kind, const char* pat } } -/// Creates the spdlog sink that forwards every message to a user callback. The callback -/// receives the bare message text; the logger pattern does not apply to it. +/// Creates the spdlog sink that forwards every message to a user callback. inline svs::logging::sink_ptr make_custom_sink(const svs_logging_i user_logger) { validate_custom_logger(user_logger); auto log = user_logger->ops->log; @@ -130,12 +127,7 @@ inline svs::logging::sink_ptr make_custom_sink(const svs_logging_i user_logger) /// The logger handle of the C API (svs_logger_h). struct svs_logger { - // The handle keeps this one spdlog logger for its whole life. Indexes and the SVS - // global default logger hold references to it, so level and pattern changes made - // through the handle apply to them immediately. The output sink is fixed at - // creation: to log somewhere else, create another logger. svs::logging::logger_ptr impl; - // Current pattern; returned by svs_logger_get_pattern(). std::string pattern = svs::c_runtime::default_log_pattern; explicit svs_logger(svs::logging::sink_ptr sink) diff --git a/bindings/c/src/svs_c.cpp b/bindings/c/src/svs_c.cpp index f1fd097e..600d5196 100644 --- a/bindings/c/src/svs_c.cpp +++ b/bindings/c/src/svs_c.cpp @@ -96,7 +96,6 @@ extern "C" bool svs_logger_set_level( return wrap_exceptions( [&]() { INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); - // Set the logging level for the logger here svs::logging::set_level(logger->impl, to_logging_level(level)); return true; }, @@ -156,7 +155,6 @@ extern "C" bool svs_set_default_logger(svs_logger_h logger, svs_error_h out_err return wrap_exceptions( [&]() { if (logger == nullptr) { - // NULL restores the SVS built-in default logger. svs::logging::reset_to_default(); } else { svs::logging::set(logger->impl); diff --git a/bindings/c/tests/README.md b/bindings/c/tests/README.md index 92f79440..4d2cf04e 100644 --- a/bindings/c/tests/README.md +++ b/bindings/c/tests/README.md @@ -140,12 +140,9 @@ The tests cover the following aspects of the C API: ### Logging -- Logger handle creation (one per output: none, stdout, stderr, file truncate/append, - custom callback) and cleanup; invalid kinds/paths and invalid custom ops are rejected -- Custom logging callback receives the bare message, including ops version/struct_size - validation -- Level and pattern getters/setters, defaults, filtering, pattern applied to file output -- Default logger receives SVS internal messages; NULL resets it; it outlives the handle +- Logger creation for each output kind and custom callbacks; invalid arguments rejected +- Level and pattern getters/setters +- Default logger set/reset ### Dynamic Index Operations diff --git a/bindings/c/tests/c_api_logging.cpp b/bindings/c/tests/c_api_logging.cpp index 10732138..4fcaeb0d 100644 --- a/bindings/c/tests/c_api_logging.cpp +++ b/bindings/c/tests/c_api_logging.cpp @@ -62,9 +62,7 @@ void record_log(void* self, enum svs_log_level level, const char* message) { void noop_log(void* /*self*/, enum svs_log_level /*level*/, const char* /*message*/) {} -// Restores the SVS built-in default logger when a test that calls -// svs_set_default_logger ends, so later tests never log through a callback whose `self` -// has been destroyed. +// Restores the SVS default logger so later tests never log through a destroyed callback. struct DefaultLoggerGuard { DefaultLoggerGuard() = default; DefaultLoggerGuard(const DefaultLoggerGuard&) = delete; @@ -72,8 +70,7 @@ struct DefaultLoggerGuard { ~DefaultLoggerGuard() { svs_set_default_logger(nullptr, nullptr); } }; -// Builds (and frees) a small static index. Vamana build logs at TRACE level through the -// global default logger, e.g. "Number of syncs: ..." and "Completed pass ...". +// Builds a small index; Vamana logs at TRACE level through the global default logger. void build_small_index() { const size_t num_vectors = 100; const size_t dimension = 16; @@ -122,7 +119,6 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { for (auto kind : {SVS_LOGGING_KIND_NONE, SVS_LOGGING_KIND_STDOUT, SVS_LOGGING_KIND_STDERR}) { - // The path is ignored by these kinds. svs_logger_h logger = svs_logger_create(kind, nullptr, error); CATCH_REQUIRE(logger != nullptr); CATCH_REQUIRE(svs_error_ok(error)); @@ -141,7 +137,6 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { svs_logger_free(logger); } - // NULL error handle and freeing NULL are allowed. svs_logger_h logger = svs_logger_create(SVS_LOGGING_KIND_NONE, nullptr, nullptr); CATCH_REQUIRE(logger != nullptr); svs_logger_free(logger); @@ -152,7 +147,6 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { CATCH_SECTION("Create Invalid") { svs_error_h error = svs_error_create(); - // Out-of-range kinds (the enum ends at SVS_LOGGING_KIND_FILE_TRUNCATE = 4). CATCH_REQUIRE( svs_logger_create(static_cast(5), nullptr, error) == nullptr ); @@ -162,7 +156,6 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { ); CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); - // File kinds need a non-empty path. for (auto kind : {SVS_LOGGING_KIND_FILE_APPEND, SVS_LOGGING_KIND_FILE_TRUNCATE}) { CATCH_REQUIRE(svs_logger_create(kind, nullptr, error) == nullptr); CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); @@ -170,7 +163,6 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); } - // A path that cannot be opened as a file (an existing directory). TempDir tmp; CATCH_REQUIRE( svs_logger_create( @@ -179,7 +171,6 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { ); CATCH_REQUIRE(svs_error_get_code(error) != SVS_OK); - // NULL error handle: failure is still reported by the return value. CATCH_REQUIRE( svs_logger_create(SVS_LOGGING_KIND_FILE_APPEND, nullptr, nullptr) == nullptr ); @@ -231,7 +222,6 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { svs_logging_t bad_size_logger = {&bad_size, nullptr}; expect_invalid(&bad_size_logger); - // NULL error handle: failure is still reported by the return value. CATCH_REQUIRE(svs_logger_create_custom(nullptr, nullptr) == nullptr); svs_error_free(error); @@ -258,7 +248,6 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { expect_invalid(svs_logger_get_pattern(nullptr, &pattern, error)); expect_invalid(svs_logger_get_pattern(logger, nullptr, error)); - // NULL error handle: failure is still reported by the return value. CATCH_REQUIRE(svs_logger_set_level(nullptr, SVS_LOG_LEVEL_INFO, nullptr) == false); svs_logger_free(logger); @@ -353,10 +342,7 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { build_small_index(); - // Default pattern "%v": the callback gets the bare message, no timestamp/level - // prefix, no trailing newline. build_small_index() builds 100 vectors, which - // Vamana splits into max(40, ceil(100 / 4096)) = 40 batches, so the exact text - // is known. + // Vamana splits the 100 vectors into max(40, ceil(100 / 4096)) = 40 batches. { std::lock_guard lock{recorder.mutex}; bool found = std::any_of( @@ -381,8 +367,6 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { svs_logger_h logger = svs_logger_create(SVS_LOGGING_KIND_NONE, nullptr, error); CATCH_REQUIRE(logger != nullptr); - // No output: using it as default (even at TRACE) writes nothing and must not - // crash. CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); CATCH_REQUIRE(svs_set_default_logger(logger, error)); build_small_index(); @@ -408,7 +392,6 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { build_small_index(); CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); - // Level changes apply without calling svs_set_default_logger again. { std::lock_guard lock{recorder.mutex}; recorder.messages.clear(); @@ -472,7 +455,6 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { svs_logging_t user_logger = {&ops, &recorder}; svs_logger_h logger = svs_logger_create_custom(&user_logger, error); CATCH_REQUIRE(logger != nullptr); - // Default level is WARN: the TRACE/DEBUG build messages must be filtered. CATCH_REQUIRE(svs_set_default_logger(logger, error)); build_small_index(); CATCH_REQUIRE(!recorder.any_below(SVS_LOG_LEVEL_WARN)); @@ -508,8 +490,7 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); svs_error_free(error); } - // Resetting the default logger released the last reference, which closes (and - // flushes) the file. + // Resetting the default logger released the last reference, which flushes the file. auto content = read_file(path); CATCH_REQUIRE(content.find("[x] Number of syncs") != std::string::npos); } @@ -549,7 +530,6 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { build_small_index(); CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); - // NULL restores the built-in default; the callback must no longer be called. CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); CATCH_REQUIRE(svs_error_ok(error)); { @@ -590,7 +570,6 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { CATCH_REQUIRE(second.messages.empty()); } - // The output of a logger cannot change; to log elsewhere, install another logger. { std::lock_guard lock{first.mutex}; first.messages.clear(); @@ -667,7 +646,6 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); CATCH_REQUIRE(svs_set_default_logger(logger, error)); - // A dynamic index captures the default logger current at build time ... const size_t num_vectors = 100; const size_t dimension = 16; std::vector data; @@ -691,8 +669,7 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { CATCH_REQUIRE(index != nullptr); CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); - // ... and keeps using it after the default is restored: later operations on the - // index still reach the callback, so its `self` must outlive the index. + // The index keeps the logger captured at build time after the default is restored. CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); { std::lock_guard lock{recorder.mutex}; @@ -707,8 +684,7 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { CATCH_REQUIRE(svs_index_dynamic_add_points( index, new_data.data(), new_ids.data(), num_vectors, nullptr, error )); - // Deleting every original point deletes the entry point, so consolidate logs - // "Replacing entry point." at DEBUG level through the index's logger. + // Deleting every original point makes consolidate log "Replacing entry point.". CATCH_REQUIRE( svs_index_dynamic_delete_points(index, ids.data(), num_vectors, nullptr, error) ); From eef772c56718c1577d3a19c6911a860fb2b117fb Mon Sep 17 00:00:00 2001 From: yuejiaointel Date: Wed, 7 Oct 2026 15:23:07 -0700 Subject: [PATCH 7/8] [C API] Reject invalid log level and empty pattern; link pattern syntax --- bindings/c/include/svs/c/svs_c.h | 2 ++ bindings/c/src/logger.hpp | 4 ++-- bindings/c/src/svs_c.cpp | 1 + bindings/c/tests/c_api_logging.cpp | 13 +++++++++++++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/bindings/c/include/svs/c/svs_c.h b/bindings/c/include/svs/c/svs_c.h index bb6475b6..f3a190a3 100644 --- a/bindings/c/include/svs/c/svs_c.h +++ b/bindings/c/include/svs/c/svs_c.h @@ -644,6 +644,7 @@ SVS_API void svs_logger_free(svs_logger_h logger); /// @param level The logging level to set /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure +/// @remarks Unknown level values fail with SVS_ERROR_INVALID_ARGUMENT. SVS_API bool svs_logger_set_level( svs_logger_h logger, svs_log_level_t level, svs_error_h out_err /*=NULL*/ ); @@ -660,6 +661,7 @@ SVS_API bool svs_logger_get_level( /// @brief Set format pattern for a logger /// @param logger The logger handle /// @param pattern The format pattern to set, in spdlog syntax (e.g. "[index A] %v") +/// Pattern syntax: https://github.com/gabime/spdlog/wiki/Custom-formatting /// @param out_err An optional error handle to capture errors /// @return true on success, false on failure /// @remarks The pattern applies to the stdout, stderr and file outputs; custom callbacks diff --git a/bindings/c/src/logger.hpp b/bindings/c/src/logger.hpp index b856dcc7..3de55081 100644 --- a/bindings/c/src/logger.hpp +++ b/bindings/c/src/logger.hpp @@ -65,7 +65,7 @@ inline void validate_custom_logger(const svs_logging_i user_logger) { } } -/// Maps a C API log level to the SVS logging level. Unknown values map to Info. +/// Maps a C API log level to the SVS logging level. Throws on unknown values. inline svs::logging::Level to_logging_level(svs_log_level_t level) { switch (level) { case SVS_LOG_LEVEL_TRACE: @@ -83,7 +83,7 @@ inline svs::logging::Level to_logging_level(svs_log_level_t level) { case SVS_LOG_LEVEL_OFF: return svs::logging::Level::Off; default: - return svs::logging::Level::Info; + throw std::invalid_argument("Invalid log level"); } } diff --git a/bindings/c/src/svs_c.cpp b/bindings/c/src/svs_c.cpp index 600d5196..bbb52d97 100644 --- a/bindings/c/src/svs_c.cpp +++ b/bindings/c/src/svs_c.cpp @@ -127,6 +127,7 @@ extern "C" bool svs_logger_set_pattern( INVALID_ARGUMENT_IF(logger == nullptr, "Logger must not be null"); EXPECT_ARG_NOT_NULL(pattern); auto new_pattern = std::string(pattern); + INVALID_ARGUMENT_IF(new_pattern.empty(), "Pattern should not be empty"); logger->impl->set_pattern(new_pattern); logger->pattern = std::move(new_pattern); return true; diff --git a/bindings/c/tests/c_api_logging.cpp b/bindings/c/tests/c_api_logging.cpp index 4fcaeb0d..2b6157c9 100644 --- a/bindings/c/tests/c_api_logging.cpp +++ b/bindings/c/tests/c_api_logging.cpp @@ -303,6 +303,14 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { CATCH_REQUIRE(out_level == level); } + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_WARN, error)); + CATCH_REQUIRE(!svs_logger_set_level(logger, static_cast(7), error) + ); + CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); + svs_log_level_t out_level = SVS_LOG_LEVEL_OFF; + CATCH_REQUIRE(svs_logger_get_level(logger, &out_level, error)); + CATCH_REQUIRE(out_level == SVS_LOG_LEVEL_WARN); + svs_logger_free(logger); svs_error_free(error); } @@ -321,6 +329,11 @@ CATCH_TEST_CASE("C API Logger Handle", "[c_api][logging]") { CATCH_REQUIRE(svs_logger_get_pattern(logger, &pattern, error)); CATCH_REQUIRE(std::string(pattern) == "[%l] %v"); + CATCH_REQUIRE(!svs_logger_set_pattern(logger, "", error)); + CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); + CATCH_REQUIRE(svs_logger_get_pattern(logger, &pattern, error)); + CATCH_REQUIRE(std::string(pattern) == "[%l] %v"); + svs_logger_free(logger); svs_error_free(error); } From 1aad49f3092afe163bdbc8a3a0f42f564a896cf2 Mon Sep 17 00:00:00 2001 From: yuejiaointel Date: Thu, 8 Oct 2026 09:15:47 -0700 Subject: [PATCH 8/8] [C API] Add per-index logger via svs_index_builder_set_logger Indexes built, loaded from a directory, or converted with the builder use its logger instead of the global default. NULL clears it back to the global logger. --- bindings/c/include/svs/c/svs_c.h | 12 + bindings/c/src/dispatcher_dynamic_vamana.cpp | 51 ++- bindings/c/src/dispatcher_dynamic_vamana.hpp | 10 +- bindings/c/src/dispatcher_vamana.cpp | 53 ++- bindings/c/src/dispatcher_vamana.hpp | 10 +- bindings/c/src/index_builder.cpp | 18 +- bindings/c/src/index_builder.hpp | 15 +- bindings/c/src/svs_c.cpp | 17 + bindings/c/tests/c_api_logging.cpp | 369 +++++++++++++++++++ 9 files changed, 510 insertions(+), 45 deletions(-) diff --git a/bindings/c/include/svs/c/svs_c.h b/bindings/c/include/svs/c/svs_c.h index 41bf016d..c5b99f7f 100644 --- a/bindings/c/include/svs/c/svs_c.h +++ b/bindings/c/include/svs/c/svs_c.h @@ -986,6 +986,18 @@ SVS_API svs_index_builder_h svs_index_builder_create( /// @param builder The index builder handle to free SVS_API void svs_index_builder_free(svs_index_builder_h builder); +/// @brief Set the logger for the index builder +/// @param builder The index builder handle +/// @param logger The logger handle for indexes built, loaded or converted with this +/// builder; NULL uses the global default logger +/// @param out_err An optional error handle to capture errors +/// @return true on success, false on failure +/// @remarks Indexes keep the logger they were created with; the handle may be freed after +/// this call. Stream loads ignore it and use the global default logger. +SVS_API bool svs_index_builder_set_logger( + svs_index_builder_h builder, svs_logger_h logger, svs_error_h out_err /*=NULL*/ +); + /// @brief Set the storage configuration for the index builder /// @param builder The index builder handle /// @param storage The storage configuration handle diff --git a/bindings/c/src/dispatcher_dynamic_vamana.cpp b/bindings/c/src/dispatcher_dynamic_vamana.cpp index 993c751e..b2daec0e 100644 --- a/bindings/c/src/dispatcher_dynamic_vamana.cpp +++ b/bindings/c/src/dispatcher_dynamic_vamana.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -48,7 +49,8 @@ svs::DynamicVamana build_dynamic_vamana_index( Distance D, svs::threads::ThreadPoolHandle pool, const AllocatorBuilder& allocator_builder, - size_t blocksize_bytes + size_t blocksize_bytes, + svs::logging::logger_ptr logger ) { svs::data::BlockingParameters block_params; if (blocksize_bytes != 0) { @@ -69,7 +71,8 @@ svs::DynamicVamana build_dynamic_vamana_index( std::move(src_data.second), std::move(D), std::move(pool), - graph_allocator + graph_allocator, + std::move(logger) ); } @@ -81,7 +84,8 @@ svs::DynamicVamana load_dynamic_vamana_index( Distance D, svs::threads::ThreadPoolHandle pool, const AllocatorBuilder& allocator_builder, - size_t blocksize_bytes + size_t blocksize_bytes, + svs::logging::logger_ptr logger ) { svs::data::BlockingParameters block_params; if (blocksize_bytes != 0) { @@ -101,7 +105,9 @@ svs::DynamicVamana load_dynamic_vamana_index( svs::GraphLoader{directory / "graph", graph_allocator}, std::move(data), std::move(D), - std::move(pool) + std::move(pool), + /*debug_load_from_static=*/false, + std::move(logger) ); } @@ -113,7 +119,8 @@ svs::DynamicVamana load_stream_dynamic_vamana_index( Distance distance, svs::threads::ThreadPoolHandle pool, const AllocatorBuilder& allocator_builder, - size_t blocksize_bytes + size_t blocksize_bytes, + svs::logging::logger_ptr SVS_UNUSED(logger) ) { svs::data::BlockingParameters block_params; if (blocksize_bytes != 0) { @@ -176,7 +183,8 @@ using BuildDynamicIndexDispatcher = svs::lib::Dispatcher< svs::DistanceType, svs::threads::ThreadPoolHandle, const AllocatorBuilder&, - size_t>; + size_t, + svs::logging::logger_ptr>; const BuildDynamicIndexDispatcher& build_dynamic_vamana_index_dispatcher() { static BuildDynamicIndexDispatcher dispatcher = [] { @@ -196,7 +204,8 @@ using CopyDynamicIndexDispatcher = svs::lib::Dispatcher< svs::DistanceType, svs::threads::ThreadPoolHandle, const AllocatorBuilder&, - size_t>; + size_t, + svs::logging::logger_ptr>; template svs::DynamicVamana copy_dynamic_vamana_index( @@ -207,7 +216,8 @@ svs::DynamicVamana copy_dynamic_vamana_index( Distance distance, svs::threads::ThreadPoolHandle pool, const AllocatorBuilder& allocator_builder, - size_t blocksize_bytes + size_t blocksize_bytes, + svs::logging::logger_ptr logger ) { auto config = src_index.parameters(); @@ -273,7 +283,9 @@ svs::DynamicVamana copy_dynamic_vamana_index( std::move(graph), std::move(data), distance, - std::move(pool) + std::move(pool), + /*debug_load_from_static=*/false, + std::move(logger) ); } @@ -342,7 +354,8 @@ svs::DynamicVamana dispatch_dynamic_vamana_index_build( svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, const AllocatorBuilder& allocator_builder, - size_t blocksize_bytes + size_t blocksize_bytes, + svs::logging::logger_ptr logger ) { return build_dynamic_vamana_index_dispatcher().invoke( build_params, @@ -351,7 +364,8 @@ svs::DynamicVamana dispatch_dynamic_vamana_index_build( distance_type, std::move(pool), allocator_builder, - blocksize_bytes + blocksize_bytes, + std::move(logger) ); } @@ -362,7 +376,8 @@ svs::DynamicVamana dispatch_dynamic_vamana_index_load( svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, const AllocatorBuilder& allocator_builder, - size_t blocksize_bytes + size_t blocksize_bytes, + svs::logging::logger_ptr logger ) { return build_dynamic_vamana_index_dispatcher().invoke( build_params, @@ -371,7 +386,8 @@ svs::DynamicVamana dispatch_dynamic_vamana_index_load( distance_type, std::move(pool), allocator_builder, - blocksize_bytes + blocksize_bytes, + std::move(logger) ); } @@ -391,7 +407,8 @@ svs::DynamicVamana dispatch_dynamic_vamana_index_load_stream( distance_type, std::move(pool), allocator_builder, - blocksize_bytes + blocksize_bytes, + nullptr ); } @@ -403,7 +420,8 @@ svs::DynamicVamana dispatch_dynamic_vamana_index_copy( svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, const AllocatorBuilder& allocator_builder, - size_t blocksize_bytes + size_t blocksize_bytes, + svs::logging::logger_ptr logger ) { return copy_dynamic_index_dispatcher().invoke( build_params, @@ -413,7 +431,8 @@ svs::DynamicVamana dispatch_dynamic_vamana_index_copy( distance_type, std::move(pool), allocator_builder, - blocksize_bytes + blocksize_bytes, + std::move(logger) ); } diff --git a/bindings/c/src/dispatcher_dynamic_vamana.hpp b/bindings/c/src/dispatcher_dynamic_vamana.hpp index afaeb366..f83f1fd4 100644 --- a/bindings/c/src/dispatcher_dynamic_vamana.hpp +++ b/bindings/c/src/dispatcher_dynamic_vamana.hpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -40,7 +41,8 @@ svs::DynamicVamana dispatch_dynamic_vamana_index_build( svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, const AllocatorBuilder& allocator_builder, - size_t blocksize_bytes + size_t blocksize_bytes, + svs::logging::logger_ptr logger ); svs::DynamicVamana dispatch_dynamic_vamana_index_load( @@ -50,7 +52,8 @@ svs::DynamicVamana dispatch_dynamic_vamana_index_load( svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, const AllocatorBuilder& allocator_builder, - size_t blocksize_bytes + size_t blocksize_bytes, + svs::logging::logger_ptr logger ); svs::DynamicVamana dispatch_dynamic_vamana_index_load_stream( @@ -71,7 +74,8 @@ svs::DynamicVamana dispatch_dynamic_vamana_index_copy( svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, const AllocatorBuilder& allocator_builder, - size_t blocksize_bytes + size_t blocksize_bytes, + svs::logging::logger_ptr logger ); svs::index::vamana::MemoryBreakdown dispatch_dynamic_vamana_memory_estimate( diff --git a/bindings/c/src/dispatcher_vamana.cpp b/bindings/c/src/dispatcher_vamana.cpp index e2414d95..09c6b3b8 100644 --- a/bindings/c/src/dispatcher_vamana.cpp +++ b/bindings/c/src/dispatcher_vamana.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -46,7 +47,8 @@ svs::Vamana build_vamana_index( DataBuilder builder, Distance distance, svs::threads::ThreadPoolHandle pool, - const AllocatorBuilder& allocator_builder + const AllocatorBuilder& allocator_builder, + svs::logging::logger_ptr logger ) { using value_type = typename DataBuilder::allocator_type::value_type; auto data = @@ -56,7 +58,8 @@ svs::Vamana build_vamana_index( std::move(data), distance, std::move(pool), - allocator_builder.build_for_graph() + allocator_builder.build_for_graph(), + std::move(logger) ); } @@ -67,7 +70,8 @@ svs::Vamana load_vamana_index( DataLoader loader, Distance distance, svs::threads::ThreadPoolHandle pool, - const AllocatorBuilder& allocator_builder + const AllocatorBuilder& allocator_builder, + svs::logging::logger_ptr logger ) { using value_type = typename DataLoader::allocator_type::value_type; auto data = loader.load(directory / "data", allocator_builder.build()); @@ -77,7 +81,8 @@ svs::Vamana load_vamana_index( directory / "graph", allocator_builder.build_for_graph()}, std::move(data), distance, - std::move(pool) + std::move(pool), + std::move(logger) ); } @@ -88,7 +93,8 @@ svs::Vamana load_stream_vamana_index( DataLoader SVS_UNUSED(loader), Distance distance, svs::threads::ThreadPoolHandle pool, - const AllocatorBuilder& allocator_builder + const AllocatorBuilder& allocator_builder, + svs::logging::logger_ptr SVS_UNUSED(logger) ) { using value_type = typename DataLoader::allocator_type::value_type; using data_type = typename DataLoader::data_type; @@ -143,7 +149,8 @@ using BuildIndexDispatcher = svs::lib::Dispatcher< const Storage*, svs::DistanceType, svs::threads::ThreadPoolHandle, - const AllocatorBuilder&>; + const AllocatorBuilder&, + svs::logging::logger_ptr>; const BuildIndexDispatcher& build_vamana_index_dispatcher() { static BuildIndexDispatcher dispatcher = [] { @@ -162,7 +169,8 @@ using CopyIndexDispatcher = svs::lib::Dispatcher< const Storage*, // dst svs::DistanceType, svs::threads::ThreadPoolHandle, - const AllocatorBuilder&>; + const AllocatorBuilder&, + svs::logging::logger_ptr>; template svs::Vamana copy_vamana_index( @@ -172,7 +180,8 @@ svs::Vamana copy_vamana_index( DstDataBuilder dst_builder, Distance distance, svs::threads::ThreadPoolHandle pool, - const AllocatorBuilder& allocator_builder + const AllocatorBuilder& allocator_builder, + svs::logging::logger_ptr logger ) { auto config = src_index.parameters(); @@ -216,7 +225,12 @@ svs::Vamana copy_vamana_index( auto data = dst_builder.build(src_data, pool, allocator_builder.build()); return svs::Vamana::assemble( - config, std::move(graph), std::move(data), distance, std::move(pool) + config, + std::move(graph), + std::move(data), + distance, + std::move(pool), + std::move(logger) ); } @@ -284,7 +298,8 @@ svs::Vamana dispatch_vamana_index_build( const Storage* storage, svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, - const AllocatorBuilder& allocator_builder + const AllocatorBuilder& allocator_builder, + svs::logging::logger_ptr logger ) { return build_vamana_index_dispatcher().invoke( build_params, @@ -292,7 +307,8 @@ svs::Vamana dispatch_vamana_index_build( storage, distance_type, std::move(pool), - allocator_builder + allocator_builder, + std::move(logger) ); } @@ -302,7 +318,8 @@ svs::Vamana dispatch_vamana_index_load( const Storage* storage, svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, - const AllocatorBuilder& allocator_builder + const AllocatorBuilder& allocator_builder, + svs::logging::logger_ptr logger ) { return build_vamana_index_dispatcher().invoke( build_params, @@ -310,7 +327,8 @@ svs::Vamana dispatch_vamana_index_load( storage, distance_type, std::move(pool), - allocator_builder + allocator_builder, + std::move(logger) ); } @@ -328,7 +346,8 @@ svs::Vamana dispatch_vamana_index_load_stream( storage, distance_type, std::move(pool), - allocator_builder + allocator_builder, + nullptr ); } @@ -339,7 +358,8 @@ svs::Vamana dispatch_vamana_index_copy( const Storage* dst_storage, svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, - const AllocatorBuilder& allocator_builder + const AllocatorBuilder& allocator_builder, + svs::logging::logger_ptr logger ) { return copy_vamana_index_dispatcher().invoke( build_params, @@ -348,7 +368,8 @@ svs::Vamana dispatch_vamana_index_copy( dst_storage, distance_type, std::move(pool), - allocator_builder + allocator_builder, + std::move(logger) ); } diff --git a/bindings/c/src/dispatcher_vamana.hpp b/bindings/c/src/dispatcher_vamana.hpp index d3f497ec..8c6416f5 100644 --- a/bindings/c/src/dispatcher_vamana.hpp +++ b/bindings/c/src/dispatcher_vamana.hpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -37,7 +38,8 @@ svs::Vamana dispatch_vamana_index_build( const Storage* storage, svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, - const AllocatorBuilder& allocator_builder + const AllocatorBuilder& allocator_builder, + svs::logging::logger_ptr logger ); svs::Vamana dispatch_vamana_index_load( @@ -46,7 +48,8 @@ svs::Vamana dispatch_vamana_index_load( const Storage* storage, svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, - const AllocatorBuilder& allocator_builder + const AllocatorBuilder& allocator_builder, + svs::logging::logger_ptr logger ); svs::Vamana dispatch_vamana_index_load_stream( @@ -65,7 +68,8 @@ svs::Vamana dispatch_vamana_index_copy( const Storage* dst_storage, svs::DistanceType distance_type, svs::threads::ThreadPoolHandle pool, - const AllocatorBuilder& allocator_builder + const AllocatorBuilder& allocator_builder, + svs::logging::logger_ptr logger ); svs::index::vamana::MemoryBreakdown dispatch_vamana_memory_estimate( diff --git a/bindings/c/src/index_builder.cpp b/bindings/c/src/index_builder.cpp index d543f8d9..41dbbeff 100644 --- a/bindings/c/src/index_builder.cpp +++ b/bindings/c/src/index_builder.cpp @@ -58,7 +58,8 @@ std::shared_ptr IndexBuilder::build(const svs::data::ConstSimpleDataView< storage.get(), to_distance_type(distance_metric), pool_builder.build(), - allocator_builder + allocator_builder, + get_logger() ) ); @@ -79,7 +80,8 @@ std::shared_ptr IndexBuilder::load(const std::filesystem::path& directory storage.get(), to_distance_type(distance_metric), pool_builder.build(), - allocator_builder + allocator_builder, + get_logger() ) ); @@ -167,7 +169,8 @@ std::shared_ptr IndexBuilder::copy(const std::shared_ptr& src_inde storage.get(), to_distance_type(distance_metric), pool_builder.build(), - allocator_builder + allocator_builder, + get_logger() ) ); @@ -212,7 +215,8 @@ std::shared_ptr IndexBuilder::copy_dynamic( to_distance_type(distance_metric), pool_builder.build(), allocator_builder, - blocksize_bytes + blocksize_bytes, + get_logger() ) ); @@ -238,7 +242,8 @@ std::shared_ptr IndexBuilder::build_dynamic( to_distance_type(distance_metric), pool_builder.build(), allocator_builder, - blocksize_bytes + blocksize_bytes, + get_logger() ) ); @@ -262,7 +267,8 @@ IndexBuilder::load_dynamic(const std::filesystem::path& directory, size_t blocks to_distance_type(distance_metric), pool_builder.build(), allocator_builder, - blocksize_bytes + blocksize_bytes, + get_logger() ) ); diff --git a/bindings/c/src/index_builder.hpp b/bindings/c/src/index_builder.hpp index c98ecca8..333f0db8 100644 --- a/bindings/c/src/index_builder.hpp +++ b/bindings/c/src/index_builder.hpp @@ -26,6 +26,7 @@ #include #include +#include #include #include @@ -46,6 +47,9 @@ struct IndexBuilder { std::unique_ptr storage; ThreadPoolBuilder pool_builder; AllocatorBuilder allocator_builder; + // Logger for indexes built or loaded by this builder. Empty means: use the SVS global + // default logger current at build/load time. + svs::logging::logger_ptr logger; IndexBuilder( svs_distance_metric_t distance_metric, @@ -65,7 +69,8 @@ struct IndexBuilder { , algorithm(other.algorithm->clone()) , storage(other.storage->clone()) , pool_builder(other.pool_builder) - , allocator_builder(other.allocator_builder) {} + , allocator_builder(other.allocator_builder) + , logger(other.logger) {} IndexBuilder& operator=(const IndexBuilder& other) { if (this != &other) { @@ -75,6 +80,7 @@ struct IndexBuilder { storage = other.storage->clone(); pool_builder = other.pool_builder; allocator_builder = other.allocator_builder; + logger = other.logger; } return *this; } @@ -95,6 +101,13 @@ struct IndexBuilder { std::swap(this->allocator_builder, allocator_builder); } + void set_logger(svs::logging::logger_ptr logger) { this->logger = std::move(logger); } + + // The logger to pass to SVS: the builder's logger if set, else the current global. + svs::logging::logger_ptr get_logger() const { + return logger ? logger : svs::logging::get(); + } + std::shared_ptr build(const svs::data::ConstSimpleDataView& data); std::shared_ptr load(const std::filesystem::path& directory); diff --git a/bindings/c/src/svs_c.cpp b/bindings/c/src/svs_c.cpp index 41ea904f..b811015b 100644 --- a/bindings/c/src/svs_c.cpp +++ b/bindings/c/src/svs_c.cpp @@ -605,6 +605,23 @@ extern "C" bool svs_index_builder_set_storage( ); } +extern "C" bool svs_index_builder_set_logger( + svs_index_builder_h builder, svs_logger_h logger, svs_error_h out_err /*=NULL*/ +) { + using namespace svs::c_runtime; + return wrap_exceptions( + [&]() { + EXPECT_ARG_NOT_NULL(builder); + // NULL clears the builder logger: indexes then use the global default logger. + // Otherwise share the handle's spdlog logger, so the handle may be freed. + builder->impl->set_logger(logger == nullptr ? nullptr : logger->impl); + return true; + }, + out_err, + false + ); +} + extern "C" bool svs_index_builder_set_threadpool( svs_index_builder_h builder, svs_threadpool_kind_t kind, diff --git a/bindings/c/tests/c_api_logging.cpp b/bindings/c/tests/c_api_logging.cpp index 2b6157c9..be367eda 100644 --- a/bindings/c/tests/c_api_logging.cpp +++ b/bindings/c/tests/c_api_logging.cpp @@ -712,3 +712,372 @@ CATCH_TEST_CASE("C API Logger Output", "[c_api][logging]") { svs_error_free(error); } } + +namespace { + +constexpr size_t kBuilderLoggerNumVectors = 100; +constexpr size_t kBuilderLoggerDimension = 16; + +// Creates a logger handle that forwards every message (TRACE and up) to `recorder`. +svs_logger_h make_recording_logger(LogRecorder& recorder, svs_error_h error) { + // The callback sink copies the function pointer and `self`, so `ops` may be local. + svs_logging_ops_t ops = SVS_INIT_LOGGING_OPS(record_log); + svs_logging_t user_logger = {&ops, &recorder}; + svs_logger_h logger = svs_logger_create_custom(&user_logger, error); + CATCH_REQUIRE(logger != nullptr); + CATCH_REQUIRE(svs_logger_set_level(logger, SVS_LOG_LEVEL_TRACE, error)); + return logger; +} + +// Owns an algorithm and an index builder for small Vamana test indexes. +struct BuilderFixture { + svs_error_h error = svs_error_create(); + svs_algorithm_h algorithm = nullptr; + svs_index_builder_h builder = nullptr; + std::vector data; + + BuilderFixture() { + generate_test_data(data, kBuilderLoggerNumVectors, kBuilderLoggerDimension); + algorithm = svs_algorithm_create_vamana(16, 32, 50, error); + CATCH_REQUIRE(algorithm != nullptr); + builder = svs_index_builder_create( + SVS_DISTANCE_METRIC_EUCLIDEAN, kBuilderLoggerDimension, algorithm, error + ); + CATCH_REQUIRE(builder != nullptr); + CATCH_REQUIRE( + svs_index_builder_set_threadpool(builder, SVS_THREADPOOL_KIND_NATIVE, 2, error) + ); + } + BuilderFixture(const BuilderFixture&) = delete; + BuilderFixture& operator=(const BuilderFixture&) = delete; + ~BuilderFixture() { + svs_index_builder_free(builder); + svs_algorithm_free(algorithm); + svs_error_free(error); + } + + svs_index_h build() { + svs_index_h index = + svs_index_build(builder, data.data(), kBuilderLoggerNumVectors, error); + CATCH_REQUIRE(index != nullptr); + CATCH_REQUIRE(svs_error_ok(error)); + return index; + } + + svs_index_h build_dynamic() { + svs_index_h index = svs_index_build_dynamic( + builder, data.data(), nullptr, kBuilderLoggerNumVectors, 0, error + ); + CATCH_REQUIRE(index != nullptr); + CATCH_REQUIRE(svs_error_ok(error)); + return index; + } +}; + +size_t message_count(LogRecorder& recorder) { + std::lock_guard lock{recorder.mutex}; + return recorder.messages.size(); +} + +void clear_messages(LogRecorder& recorder) { + std::lock_guard lock{recorder.mutex}; + recorder.messages.clear(); +} + +} // namespace + +CATCH_TEST_CASE("C API Index Builder Logger", "[c_api][logging]") { + CATCH_SECTION("Two Builders Two Loggers") { + LogRecorder global_recorder; // Declared first: must outlive the guard. + LogRecorder recorder_a; + LogRecorder recorder_b; + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + + svs_logger_h global_logger = make_recording_logger(global_recorder, error); + CATCH_REQUIRE(svs_set_default_logger(global_logger, error)); + svs_logger_h logger_a = make_recording_logger(recorder_a, error); + svs_logger_h logger_b = make_recording_logger(recorder_b, error); + + { + BuilderFixture fixture_a; + BuilderFixture fixture_b; + CATCH_REQUIRE(svs_index_builder_set_logger(fixture_a.builder, logger_a, error)); + CATCH_REQUIRE(svs_index_builder_set_logger(fixture_b.builder, logger_b, error)); + CATCH_REQUIRE(svs_error_ok(error)); + + // Build A alone first: only logger A may receive anything. + svs_index_h index_a = fixture_a.build(); + CATCH_REQUIRE(recorder_a.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + CATCH_REQUIRE(message_count(recorder_b) == 0); + CATCH_REQUIRE(message_count(global_recorder) == 0); + + const size_t count_a = message_count(recorder_a); + svs_index_h index_b = fixture_b.build(); + CATCH_REQUIRE(recorder_b.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + CATCH_REQUIRE(message_count(recorder_a) == count_a); + CATCH_REQUIRE(message_count(global_recorder) == 0); + + svs_index_free(index_a); + svs_index_free(index_b); + } + + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); + svs_logger_free(logger_a); + svs_logger_free(logger_b); + svs_logger_free(global_logger); + svs_error_free(error); + } + + CATCH_SECTION("Builder Without Logger Uses Global") { + LogRecorder global_recorder; // Declared first: must outlive the guard. + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h global_logger = make_recording_logger(global_recorder, error); + CATCH_REQUIRE(svs_set_default_logger(global_logger, error)); + + { + BuilderFixture fixture; + svs_index_free(fixture.build()); + } + CATCH_REQUIRE(global_recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); + svs_logger_free(global_logger); + svs_error_free(error); + } + + CATCH_SECTION("Dynamic Index Build And Load") { + LogRecorder global_recorder; // Declared first: must outlive the guard. + LogRecorder build_recorder; + LogRecorder load_recorder; + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h global_logger = make_recording_logger(global_recorder, error); + CATCH_REQUIRE(svs_set_default_logger(global_logger, error)); + svs_logger_h build_logger = make_recording_logger(build_recorder, error); + svs_logger_h load_logger = make_recording_logger(load_recorder, error); + + TempDir tmp; + const std::string dir = tmp.string(); + + std::vector new_points; + generate_test_data(new_points, 50, kBuilderLoggerDimension); + std::vector new_ids(50); + for (size_t i = 0; i < new_ids.size(); ++i) { + new_ids[i] = kBuilderLoggerNumVectors + i; + } + + { + BuilderFixture fixture; + CATCH_REQUIRE(svs_index_builder_set_logger(fixture.builder, build_logger, error) + ); + svs_index_h index = fixture.build_dynamic(); + CATCH_REQUIRE(build_recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + + clear_messages(build_recorder); + CATCH_REQUIRE(svs_index_dynamic_add_points( + index, new_points.data(), new_ids.data(), 25, nullptr, error + )); + CATCH_REQUIRE(build_recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + + CATCH_REQUIRE(svs_index_save(index, dir.c_str(), error)); + svs_index_free(index); + } + + clear_messages(build_recorder); + { + BuilderFixture fixture; + CATCH_REQUIRE(svs_index_builder_set_logger(fixture.builder, load_logger, error) + ); + svs_index_h index = + svs_index_load_dynamic(fixture.builder, dir.c_str(), 0, error); + CATCH_REQUIRE(index != nullptr); + CATCH_REQUIRE(svs_error_ok(error)); + + CATCH_REQUIRE(svs_index_dynamic_add_points( + index, + new_points.data() + 25 * kBuilderLoggerDimension, + new_ids.data() + 25, + 25, + nullptr, + error + )); + CATCH_REQUIRE(load_recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + + // Delete every original point so the entry point is deleted and consolidate + // logs "Replacing entry point." at DEBUG level. + std::vector old_ids(kBuilderLoggerNumVectors); + for (size_t i = 0; i < old_ids.size(); ++i) { + old_ids[i] = i; + } + CATCH_REQUIRE(svs_index_dynamic_delete_points( + index, old_ids.data(), old_ids.size(), nullptr, error + )); + CATCH_REQUIRE(svs_index_dynamic_consolidate(index, error)); + CATCH_REQUIRE( + load_recorder.contains(SVS_LOG_LEVEL_DEBUG, "Replacing entry point") + ); + svs_index_free(index); + } + + CATCH_REQUIRE(message_count(build_recorder) == 0); + CATCH_REQUIRE(!global_recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + CATCH_REQUIRE( + !global_recorder.contains(SVS_LOG_LEVEL_DEBUG, "Replacing entry point") + ); + + svs_logger_free(build_logger); + svs_logger_free(load_logger); + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); + svs_logger_free(global_logger); + svs_error_free(error); + } + + CATCH_SECTION("Static Save And Load With Logger") { + LogRecorder global_recorder; // Declared first: must outlive the guard. + LogRecorder build_recorder; + LogRecorder load_recorder; + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h global_logger = make_recording_logger(global_recorder, error); + CATCH_REQUIRE(svs_set_default_logger(global_logger, error)); + svs_logger_h build_logger = make_recording_logger(build_recorder, error); + svs_logger_h load_logger = make_recording_logger(load_recorder, error); + + TempDir tmp; + const std::string dir = tmp.string(); + { + BuilderFixture fixture; + CATCH_REQUIRE(svs_index_builder_set_logger(fixture.builder, build_logger, error) + ); + svs_index_h index = fixture.build(); + CATCH_REQUIRE(svs_index_save(index, dir.c_str(), error)); + svs_index_free(index); + } + clear_messages(build_recorder); + clear_messages(global_recorder); + { + BuilderFixture fixture; + CATCH_REQUIRE(svs_index_builder_set_logger(fixture.builder, load_logger, error) + ); + svs_index_h index = svs_index_load(fixture.builder, dir.c_str(), error); + CATCH_REQUIRE(index != nullptr); + CATCH_REQUIRE(svs_error_ok(error)); + svs_index_free(index); + } + // Static assemble may log nothing; no message may reach the other loggers. + CATCH_REQUIRE(message_count(build_recorder) == 0); + CATCH_REQUIRE(message_count(global_recorder) == 0); + + svs_logger_free(build_logger); + svs_logger_free(load_logger); + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); + svs_logger_free(global_logger); + svs_error_free(error); + } + + CATCH_SECTION("Converted Dynamic Index Uses Builder Logger") { + LogRecorder global_recorder; // Declared first: must outlive the guard. + LogRecorder src_recorder; + LogRecorder dst_recorder; + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h global_logger = make_recording_logger(global_recorder, error); + CATCH_REQUIRE(svs_set_default_logger(global_logger, error)); + svs_logger_h src_logger = make_recording_logger(src_recorder, error); + svs_logger_h dst_logger = make_recording_logger(dst_recorder, error); + + std::vector new_points; + generate_test_data(new_points, 25, kBuilderLoggerDimension); + std::vector new_ids(25); + for (size_t i = 0; i < new_ids.size(); ++i) { + new_ids[i] = kBuilderLoggerNumVectors + i; + } + { + BuilderFixture src; + BuilderFixture dst; + CATCH_REQUIRE(svs_index_builder_set_logger(src.builder, src_logger, error)); + CATCH_REQUIRE(svs_index_builder_set_logger(dst.builder, dst_logger, error)); + svs_index_h src_index = src.build_dynamic(); + clear_messages(src_recorder); + + svs_index_h index = svs_index_convert_dynamic(dst.builder, src_index, 0, error); + CATCH_REQUIRE(index != nullptr); + CATCH_REQUIRE(svs_error_ok(error)); + CATCH_REQUIRE(svs_index_dynamic_add_points( + index, new_points.data(), new_ids.data(), new_ids.size(), nullptr, error + )); + CATCH_REQUIRE(dst_recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + svs_index_free(index); + svs_index_free(src_index); + } + CATCH_REQUIRE(message_count(src_recorder) == 0); + CATCH_REQUIRE(message_count(global_recorder) == 0); + + svs_logger_free(src_logger); + svs_logger_free(dst_logger); + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); + svs_logger_free(global_logger); + svs_error_free(error); + } + + CATCH_SECTION("NULL Logger Clears And NULL Builder Fails") { + LogRecorder global_recorder; // Declared first: must outlive the guard. + LogRecorder builder_recorder; + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h global_logger = make_recording_logger(global_recorder, error); + CATCH_REQUIRE(svs_set_default_logger(global_logger, error)); + svs_logger_h builder_logger = make_recording_logger(builder_recorder, error); + + { + BuilderFixture fixture; + CATCH_REQUIRE( + svs_index_builder_set_logger(fixture.builder, builder_logger, error) + ); + CATCH_REQUIRE(svs_index_builder_set_logger(fixture.builder, nullptr, error)); + CATCH_REQUIRE(svs_error_ok(error)); + svs_index_free(fixture.build()); + } + CATCH_REQUIRE(message_count(builder_recorder) == 0); + CATCH_REQUIRE(global_recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + + CATCH_REQUIRE( + svs_index_builder_set_logger(nullptr, builder_logger, error) == false + ); + CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); + CATCH_REQUIRE(svs_index_builder_set_logger(nullptr, nullptr, error) == false); + CATCH_REQUIRE(svs_error_get_code(error) == SVS_ERROR_INVALID_ARGUMENT); + // NULL error handle is allowed. + CATCH_REQUIRE(svs_index_builder_set_logger(nullptr, nullptr, nullptr) == false); + + svs_logger_free(builder_logger); + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); + svs_logger_free(global_logger); + svs_error_free(error); + } + + CATCH_SECTION("Logger Handle Freed After Set") { + LogRecorder global_recorder; // Declared first: must outlive the guard. + LogRecorder recorder; + DefaultLoggerGuard guard; + svs_error_h error = svs_error_create(); + svs_logger_h global_logger = make_recording_logger(global_recorder, error); + CATCH_REQUIRE(svs_set_default_logger(global_logger, error)); + + { + BuilderFixture fixture; + svs_logger_h logger = make_recording_logger(recorder, error); + CATCH_REQUIRE(svs_index_builder_set_logger(fixture.builder, logger, error)); + svs_logger_free(logger); + svs_index_free(fixture.build()); + } + CATCH_REQUIRE(recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + CATCH_REQUIRE(!global_recorder.contains(SVS_LOG_LEVEL_TRACE, "Number of syncs")); + + CATCH_REQUIRE(svs_set_default_logger(nullptr, error)); + svs_logger_free(global_logger); + svs_error_free(error); + } +}