Skip to content

Commit b046e09

Browse files
kwvgUdjinM6
kwvg
and
UdjinM6
committed
merge bitcoin#25202: Use severity-based logging for leveldb/libevent messages, reverse LogPrintLevel order
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
1 parent 7697b73 commit b046e09

17 files changed

+95
-66
lines changed

‎src/batchedlogger.cpp‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@
44

55
#include <batchedlogger.h>
66

7-
CBatchedLogger::CBatchedLogger(BCLog::LogFlags _category, const std::string& logging_function, const std::string& source_file, int source_line) :
8-
accept(LogAcceptCategory(_category)),
9-
m_logging_function(logging_function),
10-
m_source_file(source_file),
11-
m_source_line(source_line)
7+
CBatchedLogger::CBatchedLogger(BCLog::LogFlags category, BCLog::Level level, const std::string& logging_function,
8+
const std::string& source_file, int source_line) :
9+
m_accept{LogAcceptCategory(category, level)},
10+
m_category{category},
11+
m_level{level},
12+
m_logging_function{logging_function},
13+
m_source_file{source_file},
14+
m_source_line{source_line}
1215
{
1316
}
1417

@@ -19,9 +22,9 @@ CBatchedLogger::~CBatchedLogger()
1922

2023
void CBatchedLogger::Flush()
2124
{
22-
if (!accept || msg.empty()) {
25+
if (!m_accept || m_msg.empty()) {
2326
return;
2427
}
25-
LogInstance().LogPrintStr(msg, m_logging_function, m_source_file, m_source_line);
26-
msg.clear();
28+
LogInstance().LogPrintStr(m_msg, m_logging_function, m_source_file, m_source_line, m_category, m_level);
29+
m_msg.clear();
2730
}

‎src/batchedlogger.h‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,26 @@
1010
class CBatchedLogger
1111
{
1212
private:
13-
bool accept;
14-
std::string m_logging_function;;
13+
bool m_accept;
14+
BCLog::LogFlags m_category;
15+
BCLog::Level m_level;
16+
std::string m_logging_function;
1517
std::string m_source_file;
1618
const int m_source_line;
17-
std::string msg;
19+
std::string m_msg;
20+
1821
public:
19-
CBatchedLogger(BCLog::LogFlags _category, const std::string& logging_function, const std::string& m_source_file, int m_source_line);
22+
CBatchedLogger(BCLog::LogFlags category, BCLog::Level level, const std::string& logging_function,
23+
const std::string& m_source_file, int m_source_line);
2024
virtual ~CBatchedLogger();
2125

2226
template<typename... Args>
2327
void Batch(const std::string& fmt, const Args&... args)
2428
{
25-
if (!accept) {
29+
if (!m_accept) {
2630
return;
2731
}
28-
msg += " " + strprintf(fmt, args...) + "\n";
32+
m_msg += " " + strprintf(fmt, args...) + "\n";
2933
}
3034

3135
void Flush();

‎src/dbwrapper.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class CBitcoinLevelDBLogger : public leveldb::Logger {
2020
// This code is adapted from posix_logger.h, which is why it is using vsprintf.
2121
// Please do not do this in normal code
2222
void Logv(const char * format, va_list ap) override {
23-
if (!LogAcceptCategory(BCLog::LEVELDB)) {
23+
if (!LogAcceptCategory(BCLog::LEVELDB, BCLog::Level::Debug)) {
2424
return;
2525
}
2626
char buffer[500];
@@ -64,7 +64,7 @@ class CBitcoinLevelDBLogger : public leveldb::Logger {
6464

6565
assert(p <= limit);
6666
base[std::min(bufsize - 1, (int)(p - base))] = '\0';
67-
LogPrintf("leveldb: %s", base); /* Continued */
67+
LogPrintLevel(BCLog::LEVELDB, BCLog::Level::Debug, "%s", base); /* Continued */
6868
if (base != buffer) {
6969
delete[] base;
7070
}
@@ -187,7 +187,7 @@ CDBWrapper::~CDBWrapper()
187187

188188
bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync)
189189
{
190-
const bool log_memory = LogAcceptCategory(BCLog::LEVELDB);
190+
const bool log_memory = LogAcceptCategory(BCLog::LEVELDB, BCLog::Level::Debug);
191191
double mem_before = 0;
192192
if (log_memory) {
193193
mem_before = DynamicMemoryUsage() / 1024.0 / 1024;

‎src/httpserver.cpp‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,22 @@ static void HTTPWorkQueueRun(WorkQueue<HTTPClosure>* queue, int worker_num)
375375
/** libevent event log callback */
376376
static void libevent_log_cb(int severity, const char *msg)
377377
{
378-
if (severity >= EVENT_LOG_WARN) // Log warn messages and higher without debug category
379-
LogPrintf("libevent: %s\n", msg);
380-
else
381-
LogPrint(BCLog::LIBEVENT, "libevent: %s\n", msg);
378+
BCLog::Level level;
379+
switch (severity) {
380+
case EVENT_LOG_DEBUG:
381+
level = BCLog::Level::Debug;
382+
break;
383+
case EVENT_LOG_MSG:
384+
level = BCLog::Level::Info;
385+
break;
386+
case EVENT_LOG_WARN:
387+
level = BCLog::Level::Warning;
388+
break;
389+
default: // EVENT_LOG_ERR and others are mapped to error
390+
level = BCLog::Level::Error;
391+
break;
392+
}
393+
LogPrintLevel(BCLog::LIBEVENT, level, "%s\n", msg);
382394
}
383395

384396
bool InitHTTPServer()

‎src/llmq/commitment.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ bool CFinalCommitment::Verify(CDeterministicMNManager& dmnman, gsl::not_null<con
8282
return false;
8383
}
8484
auto members = utils::GetAllQuorumMembers(llmqType, dmnman, pQuorumBaseBlockIndex);
85-
if (LogAcceptCategory(BCLog::LLMQ)) {
85+
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
8686
std::stringstream ss;
8787
std::stringstream ss2;
8888
for (const auto i: irange::range(llmq_params.size)) {
@@ -106,7 +106,7 @@ bool CFinalCommitment::Verify(CDeterministicMNManager& dmnman, gsl::not_null<con
106106
// sigs are only checked when the block is processed
107107
if (checkSigs) {
108108
uint256 commitmentHash = BuildCommitmentHash(llmq_params.type, quorumHash, validMembers, quorumPublicKey, quorumVvecHash);
109-
if (LogAcceptCategory(BCLog::LLMQ)) {
109+
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
110110
std::stringstream ss3;
111111
for (const auto &mn: members) {
112112
ss3 << mn->proTxHash.ToString().substr(0, 4) << " | ";
@@ -181,7 +181,7 @@ bool CheckLLMQCommitment(CDeterministicMNManager& dmnman, const ChainstateManage
181181
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-qc-commitment-type");
182182
}
183183

184-
if (LogAcceptCategory(BCLog::LLMQ)) {
184+
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
185185
std::stringstream ss;
186186
for (const auto i: irange::range(llmq_params_opt->size)) {
187187
ss << "v[" << i << "]=" << qcTx.commitment.validMembers[i];

‎src/llmq/dkgsession.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace llmq
3333
{
3434

3535
CDKGLogger::CDKGLogger(const CDKGSession& _quorumDkg, std::string_view _func, int source_line) :
36-
CBatchedLogger(BCLog::LLMQ_DKG,
36+
CBatchedLogger(BCLog::LLMQ_DKG, BCLog::Level::Debug,
3737
strprintf("QuorumDKG(type=%s, qIndex=%d, h=%d, member=%d)", _quorumDkg.params.name, _quorumDkg.quorumIndex,
3838
_quorumDkg.m_quorum_base_block_index->nHeight, _quorumDkg.AreWeMember()),
3939
__FILE__, source_line)
@@ -120,7 +120,7 @@ bool CDKGSession::Init(const uint256& _myProTxHash, int _quorumIndex)
120120

121121
CDKGLogger logger(*this, __func__, __LINE__);
122122

123-
if (LogAcceptCategory(BCLog::LLMQ) && IsQuorumRotationEnabled(params, m_quorum_base_block_index)) {
123+
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug) && IsQuorumRotationEnabled(params, m_quorum_base_block_index)) {
124124
int cycleQuorumBaseHeight = m_quorum_base_block_index->nHeight - quorumIndex;
125125
const CBlockIndex* pCycleQuorumBaseBlockIndex = m_quorum_base_block_index->GetAncestor(cycleQuorumBaseHeight);
126126
std::stringstream ss;
@@ -138,7 +138,7 @@ bool CDKGSession::Init(const uint256& _myProTxHash, int _quorumIndex)
138138
if (!myProTxHash.IsNull()) {
139139
dkgDebugManager.InitLocalSessionStatus(params, quorumIndex, m_quorum_base_block_index->GetBlockHash(), m_quorum_base_block_index->nHeight);
140140
relayMembers = utils::GetQuorumRelayMembers(params, m_dmnman, m_quorum_base_block_index, myProTxHash, true);
141-
if (LogAcceptCategory(BCLog::LLMQ)) {
141+
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
142142
std::stringstream ss;
143143
for (const auto& r : relayMembers) {
144144
ss << r.ToString().substr(0, 4) << " | ";

‎src/llmq/instantsend.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ void CInstantSendManager::HandleNewInputLockRecoveredSig(const CRecoveredSig& re
639639
return;
640640
}
641641

642-
if (LogAcceptCategory(BCLog::INSTANTSEND)) {
642+
if (LogAcceptCategory(BCLog::INSTANTSEND, BCLog::Level::Debug)) {
643643
for (const auto& in : tx->vin) {
644644
auto id = ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, in.prevout));
645645
if (id == recoveredSig.getId()) {
@@ -1509,7 +1509,7 @@ void CInstantSendManager::ProcessPendingRetryLockTxs()
15091509

15101510
// CheckCanLock is already called by ProcessTx, so we should avoid calling it twice. But we also shouldn't spam
15111511
// the logs when retrying TXs that are not ready yet.
1512-
if (LogAcceptCategory(BCLog::INSTANTSEND)) {
1512+
if (LogAcceptCategory(BCLog::INSTANTSEND, BCLog::Level::Debug)) {
15131513
if (!CheckCanLock(*tx, false, Params().GetConsensus())) {
15141514
continue;
15151515
}

‎src/llmq/signing_shares.cpp‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,7 +1334,7 @@ void CSigSharesManager::Cleanup()
13341334
const auto& oneSigShare = m->begin()->second;
13351335

13361336
std::string strMissingMembers;
1337-
if (LogAcceptCategory(BCLog::LLMQ_SIGS)) {
1337+
if (LogAcceptCategory(BCLog::LLMQ_SIGS, BCLog::Level::Debug)) {
13381338
if (const auto quorumIt = quorums.find(std::make_pair(oneSigShare.getLlmqType(), oneSigShare.getQuorumHash())); quorumIt != quorums.end()) {
13391339
const auto& quorum = quorumIt->second;
13401340
for (const auto i : irange::range(quorum->members.size())) {
@@ -1346,11 +1346,15 @@ void CSigSharesManager::Cleanup()
13461346
}
13471347
}
13481348

1349-
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signing session timed out. signHash=%s, id=%s, msgHash=%s, sigShareCount=%d, missingMembers=%s\n", __func__,
1350-
signHash.ToString(), oneSigShare.getId().ToString(), oneSigShare.getMsgHash().ToString(), count, strMissingMembers);
1349+
LogPrintLevel(BCLog::LLMQ_SIGS, BCLog::Level::Info, /* Continued */
1350+
"CSigSharesManager::%s -- signing session timed out. signHash=%s, id=%s, msgHash=%s, "
1351+
"sigShareCount=%d, missingMembers=%s\n",
1352+
__func__, signHash.ToString(), oneSigShare.getId().ToString(),
1353+
oneSigShare.getMsgHash().ToString(), count, strMissingMembers);
13511354
} else {
1352-
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signing session timed out. signHash=%s, sigShareCount=%d\n", __func__,
1353-
signHash.ToString(), count);
1355+
LogPrintLevel(BCLog::LLMQ_SIGS, BCLog::Level::Info, /* Continued */
1356+
"CSigSharesManager::%s -- signing session timed out. signHash=%s, sigShareCount=%d\n",
1357+
__func__, signHash.ToString(), count);
13541358
}
13551359
RemoveSigSharesForSession(signHash);
13561360
}

‎src/llmq/utils.cpp‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> ComputeQuorumMembersByQuarterRota
216216
//TODO Check if it is triggered from outside (P2P, block validation). Throwing an exception is probably a wiser choice
217217
//assert (!newQuarterMembers.empty());
218218

219-
if (LogAcceptCategory(BCLog::LLMQ)) {
219+
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
220220
for (const size_t i : irange::range(nQuorums)) {
221221
std::stringstream ss;
222222

@@ -249,7 +249,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> ComputeQuorumMembersByQuarterRota
249249
std::move(previousQuarters.quarterHMinusC[i].begin(), previousQuarters.quarterHMinusC[i].end(), std::back_inserter(quorumMembers[i]));
250250
std::move(newQuarterMembers[i].begin(), newQuarterMembers[i].end(), std::back_inserter(quorumMembers[i]));
251251

252-
if (LogAcceptCategory(BCLog::LLMQ)) {
252+
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
253253
std::stringstream ss;
254254
ss << " [";
255255
for (const auto &m: quorumMembers[i]) {
@@ -397,7 +397,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> BuildNewQuorumQuarterMembers(cons
397397
sortedCombinedMnsList.push_back(std::move(m));
398398
}
399399

400-
if (LogAcceptCategory(BCLog::LLMQ)) {
400+
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
401401
std::stringstream ss;
402402
ss << " [";
403403
for (const auto &m: sortedCombinedMnsList) {
@@ -517,7 +517,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> GetQuorumQuarterMembersBySnapshot
517517
std::move(sortedMnsUsedAtH.begin(), sortedMnsUsedAtH.end(), std::back_inserter(sortedCombinedMns));
518518
}
519519

520-
if (LogAcceptCategory(BCLog::LLMQ)) {
520+
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
521521
std::stringstream ss;
522522
ss << " [";
523523
for (const auto &m: sortedCombinedMns) {
@@ -788,7 +788,8 @@ bool EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, CConnman&
788788
relayMembers = connections;
789789
}
790790
if (!connections.empty()) {
791-
if (!connman.HasMasternodeQuorumNodes(llmqParams.type, pQuorumBaseBlockIndex->GetBlockHash()) && LogAcceptCategory(BCLog::LLMQ)) {
791+
if (!connman.HasMasternodeQuorumNodes(llmqParams.type, pQuorumBaseBlockIndex->GetBlockHash()) &&
792+
LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
792793
std::string debugMsg = strprintf("%s -- adding masternodes quorum connections for quorum %s:\n", __func__, pQuorumBaseBlockIndex->GetBlockHash().ToString());
793794
for (const auto& c : connections) {
794795
auto dmn = tip_mn_list.GetValidMN(c);
@@ -835,7 +836,7 @@ void AddQuorumProbeConnections(const Consensus::LLMQParams& llmqParams, CConnman
835836
}
836837

837838
if (!probeConnections.empty()) {
838-
if (LogAcceptCategory(BCLog::LLMQ)) {
839+
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
839840
std::string debugMsg = strprintf("%s -- adding masternodes probes for quorum %s:\n", __func__, pQuorumBaseBlockIndex->GetBlockHash().ToString());
840841
for (const auto& c : probeConnections) {
841842
auto dmn = tip_mn_list.GetValidMN(c);

‎src/logging.h‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,14 @@ namespace BCLog {
191191

192192
BCLog::Logger& LogInstance();
193193

194-
/** Return true if log accepts specified category */
195-
static inline bool LogAcceptCategory(BCLog::LogFlags category)
194+
/** Return true if log accepts specified category, at the specified level. */
195+
static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level level)
196196
{
197+
// Log messages at Warning and Error level unconditionally, so that
198+
// important troubleshooting information doesn't get lost.
199+
if (level >= BCLog::Level::Warning) {
200+
return true;
201+
}
197202
return LogInstance().WillLogCategory(category);
198203
}
199204

@@ -241,14 +246,14 @@ static inline void LogPrintf_(const std::string& logging_function, const std::st
241246
// evaluating arguments when logging for the category is not enabled.
242247
#define LogPrint(category, ...) \
243248
do { \
244-
if (LogAcceptCategory((category))) { \
249+
if (LogAcceptCategory((category), BCLog::Level::Debug)) { \
245250
LogPrintLevel_(category, BCLog::Level::None, __VA_ARGS__); \
246251
} \
247252
} while (0)
248253

249-
#define LogPrintLevel(level, category, ...) \
254+
#define LogPrintLevel(category, level, ...) \
250255
do { \
251-
if (LogAcceptCategory((category))) { \
256+
if (LogAcceptCategory((category), (level))) { \
252257
LogPrintLevel_(category, level, __VA_ARGS__); \
253258
} \
254259
} while (0)

0 commit comments

Comments
 (0)