Skip to content

Commit 7791941

Browse files
committed
kernel: make logging struct-based
Replace the string-based logging callback with btck_LogEntry containing full metadata (message, source location, timestamp, level, category). This lets consumers format and filter logs as needed. Use the global log dispatcher instead of BCLog::Logger's buffered callback system. Messages are discarded when no callbacks are registered.
1 parent d8e50cc commit 7791941

File tree

5 files changed

+69
-48
lines changed

5 files changed

+69
-48
lines changed

src/bitcoin-chainstate.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,12 @@ std::vector<std::byte> hex_string_to_byte_vec(std::string_view hex)
4444
class KernelLog
4545
{
4646
public:
47-
void LogMessage(std::string_view message)
47+
void LogMessage(const LogEntry& entry)
4848
{
49-
std::cout << "kernel: " << message;
49+
std::cout << "kernel: " << entry.Timestamp().time_since_epoch().count()
50+
<< " [" << log_category_get_name(entry.Category()) << ":"
51+
<< log_level_get_name(entry.Level()) << "] "
52+
<< entry.Message() << "\n";
5053
}
5154
};
5255

src/kernel/bitcoinkernel.cpp

Lines changed: 44 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@
3232
#include <util/fs.h>
3333
#include <util/result.h>
3434
#include <util/signalinterrupt.h>
35+
#include <util/string.h>
3536
#include <util/task_runner.h>
37+
#include <util/time.h>
3638
#include <util/translation.h>
3739
#include <validation.h>
3840
#include <validationinterface.h>
@@ -44,7 +46,6 @@
4446
#include <exception>
4547
#include <functional>
4648
#include <iterator>
47-
#include <list>
4849
#include <memory>
4950
#include <span>
5051
#include <stdexcept>
@@ -171,6 +172,14 @@ constexpr util::log::Level get_bclog_level(btck_LogLevel level)
171172
return LOG_LEVELS[level].bclog;
172173
}
173174

175+
btck_LogLevel get_btck_level(util::log::Level level)
176+
{
177+
for (size_t i = 0; i < LOG_LEVELS.size(); ++i) {
178+
if (LOG_LEVELS[i].bclog == level) return static_cast<btck_LogLevel>(i);
179+
}
180+
assert(false);
181+
}
182+
174183
struct LogCategoryMapping {
175184
BCLog::LogFlags bclog;
176185
std::string_view name;
@@ -200,6 +209,14 @@ constexpr BCLog::LogFlags get_bclog_flag(btck_LogCategory category)
200209
return LOG_CATEGORIES[category].bclog;
201210
}
202211

212+
btck_LogCategory get_btck_category(BCLog::LogFlags flag)
213+
{
214+
for (size_t i = 0; i < LOG_CATEGORIES.size(); ++i) {
215+
if (LOG_CATEGORIES[i].bclog == flag) return static_cast<btck_LogCategory>(i);
216+
}
217+
assert(false);
218+
}
219+
203220
btck_SynchronizationState cast_state(SynchronizationState state)
204221
{
205222
switch (state) {
@@ -225,47 +242,45 @@ btck_Warning cast_btck_warning(kernel::Warning warning)
225242
}
226243

227244
struct LoggingConnection {
228-
std::unique_ptr<std::list<std::function<void(const std::string&)>>::iterator> m_connection;
245+
util::log::Dispatcher::CallbackHandle m_callback_handle;
229246
void* m_user_data;
230247
std::function<void(void* user_data)> m_deleter;
231248

232249
LoggingConnection(btck_LogCallback callback, void* user_data, btck_DestroyCallback user_data_destroy_callback)
250+
: m_user_data{user_data}, m_deleter{user_data_destroy_callback}
233251
{
234-
LOCK(cs_main);
235-
236-
auto connection{LogInstance().PushBackCallback([callback, user_data](const std::string& str) { callback(user_data, str.c_str(), str.length()); })};
237-
238-
// Only start logging if we just added the connection.
239-
if (LogInstance().NumConnections() == 1 && !LogInstance().StartLogging()) {
240-
LogError("Logger start failed.");
241-
LogInstance().DeleteCallback(connection);
242-
if (user_data && user_data_destroy_callback) {
243-
user_data_destroy_callback(user_data);
252+
m_callback_handle = util::log::g_dispatcher().RegisterCallback([callback, user_data](const util::log::Entry& entry) {
253+
const auto timestamp_ns{Ticks<std::chrono::nanoseconds>(entry.timestamp.time_since_epoch())};
254+
std::string_view file_name{entry.source_loc.file_name()};
255+
std::string_view function_name{entry.source_loc.function_name_short()};
256+
// Some log statements are manually suffixed with a newline.
257+
std::string_view message{util::RemoveSuffixView(entry.message, "\n")};
258+
259+
btck_LogEntry btck_entry{
260+
.message = {message.data(), message.size()},
261+
.file_name = {file_name.data(), file_name.size()},
262+
.function_name = {function_name.data(), function_name.size()},
263+
.thread_name = {entry.thread_name.data(), entry.thread_name.size()},
264+
.timestamp_ns = timestamp_ns,
265+
.line = entry.source_loc.line(),
266+
.level = get_btck_level(entry.level),
267+
.category = get_btck_category(static_cast<BCLog::LogFlags>(entry.category)),
268+
};
269+
try {
270+
callback(user_data, &btck_entry);
271+
} catch (const std::exception& e) {
272+
LogError("Logging callback threw unexpected exception: %s", e.what());
244273
}
245-
throw std::runtime_error("Failed to start logging");
246-
}
247-
248-
m_connection = std::make_unique<std::list<std::function<void(const std::string&)>>::iterator>(connection);
249-
m_user_data = user_data;
250-
m_deleter = user_data_destroy_callback;
274+
});
251275

252276
LogDebug(BCLog::KERNEL, "Logger connected.");
253277
}
254278

255279
~LoggingConnection()
256280
{
257-
LOCK(cs_main);
258281
LogDebug(BCLog::KERNEL, "Logger disconnecting.");
282+
util::log::g_dispatcher().UnregisterCallback(m_callback_handle);
259283

260-
// Switch back to buffering by calling DisconnectTestLogger if the
261-
// connection that we are about to remove is the last one.
262-
if (LogInstance().NumConnections() == 1) {
263-
LogInstance().DisconnectTestLogger();
264-
} else {
265-
LogInstance().DeleteCallback(*m_connection);
266-
}
267-
268-
m_connection.reset();
269284
if (m_user_data && m_deleter) {
270285
m_deleter(m_user_data);
271286
}
@@ -773,6 +788,7 @@ void btck_logging_disable()
773788

774789
btck_LoggingConnection* btck_logging_connection_create(btck_LogCallback callback, void* user_data, btck_DestroyCallback user_data_destroy_callback)
775790
{
791+
assert(callback);
776792
try {
777793
return btck_LoggingConnection::create(callback, user_data, user_data_destroy_callback);
778794
} catch (const std::exception&) {

src/kernel/bitcoinkernel.h

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,6 @@ typedef struct btck_TransactionOutput btck_TransactionOutput;
133133
* Opaque data structure for holding a logging connection.
134134
*
135135
* The logging connection can be used to manually stop logging.
136-
*
137-
* Messages that were logged before a connection is created are buffered in a
138-
* 1MB buffer. Logging can alternatively be permanently disabled by calling
139-
* @ref btck_logging_disable. Functions changing the logging settings are
140-
* global and change the settings for all existing btck_LoggingConnection
141-
* instances.
142136
*/
143137
typedef struct btck_LoggingConnection btck_LoggingConnection;
144138

@@ -370,8 +364,11 @@ typedef struct {
370364
/**
371365
* Function signature for the global logging callback. All bitcoin kernel
372366
* internal logs will pass through this callback.
367+
*
368+
* @param[in] user_data User-defined data passed to btck_logging_connection_create.
369+
* @param[in] entry Structured log entry. Valid only for the duration of the callback.
373370
*/
374-
typedef void (*btck_LogCallback)(void* user_data, const char* message, size_t message_len);
371+
typedef void (*btck_LogCallback)(void* user_data, const btck_LogEntry* entry);
375372

376373
/**
377374
* Function signature for freeing user data.
@@ -834,11 +831,12 @@ BITCOINKERNEL_API void btck_logging_enable_category(btck_LogCategory category);
834831
BITCOINKERNEL_API void btck_logging_disable_category(btck_LogCategory category);
835832

836833
/**
837-
* @brief Start logging messages through the provided callback. Log messages
838-
* produced before this function is first called are buffered and on calling this
839-
* function are logged immediately.
834+
* @brief Start logging messages through the provided callback.
835+
*
836+
* Messages are delivered as structured btck_LogEntry data, allowing the
837+
* consumer to format and filter as needed.
840838
*
841-
* @param[in] log_callback Non-null, function through which messages will be logged.
839+
* @param[in] log_callback Non-null, function through which log entries will be delivered.
842840
* @param[in] user_data Nullable, holds a user-defined opaque structure. Is passed back
843841
* to the user through the callback. If the user_data_destroy_callback
844842
* is also defined it is assumed that ownership of the user_data is passed

src/kernel/bitcoinkernel_wrapper.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -866,8 +866,8 @@ inline std::string_view log_category_get_name(LogCategory category)
866866
}
867867

868868
template <typename T>
869-
concept Log = requires(T a, std::string_view message) {
870-
{ a.LogMessage(message) } -> std::same_as<void>;
869+
concept Log = requires(T a, const LogEntry& entry) {
870+
{ a.LogMessage(entry) } -> std::same_as<void>;
871871
};
872872

873873
template <Log T>
@@ -876,7 +876,7 @@ class Logger : UniqueHandle<btck_LoggingConnection, btck_logging_connection_dest
876876
public:
877877
Logger(std::unique_ptr<T> log)
878878
: UniqueHandle{btck_logging_connection_create(
879-
+[](void* user_data, const char* message, size_t message_len) { static_cast<T*>(user_data)->LogMessage({message, message_len}); },
879+
+[](void* user_data, const btck_LogEntry* entry) { static_cast<T*>(user_data)->LogMessage(LogEntry{*entry}); },
880880
log.release(),
881881
+[](void* user_data) { delete static_cast<T*>(user_data); })}
882882
{

src/test/kernel/test_kernel.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
#include <kernel/bitcoinkernel_wrapper.h>
77

88
#define BOOST_TEST_MODULE Bitcoin Kernel Test Suite
9-
#include <boost/test/included/unit_test.hpp>
10-
119
#include <test/kernel/block_data.h>
1210

11+
#include <boost/test/included/unit_test.hpp>
12+
1313
#include <charconv>
1414
#include <cstdint>
1515
#include <cstdlib>
@@ -91,9 +91,13 @@ void check_equal(std::span<const std::byte> _actual, std::span<const std::byte>
9191
class TestLog
9292
{
9393
public:
94-
void LogMessage(std::string_view message)
94+
void LogMessage(const LogEntry& entry)
9595
{
96-
std::cout << "kernel: " << message;
96+
// TODO: stream Timestamp() directly once minimum gcc version is bumped to 13.1 (required for std::format support)
97+
std::cout << "kernel: " << entry.Timestamp().time_since_epoch().count()
98+
<< " [" << log_category_get_name(entry.Category()) << ":"
99+
<< log_level_get_name(entry.Level()) << "] "
100+
<< entry.Message() << "\n";
97101
}
98102
};
99103

0 commit comments

Comments
 (0)