Fix PR comments

Signed-off-by: JCW <a1q123456@users.noreply.github.com>
This commit is contained in:
JCW
2025-08-27 11:58:37 +01:00
parent df07f92ed7
commit c4464f5122
8 changed files with 17 additions and 65 deletions

View File

@@ -76,7 +76,7 @@ private:
std::unique_ptr<StructuredLogAttributes> m_attributes; std::unique_ptr<StructuredLogAttributes> m_attributes;
static StructuredJournalImpl* m_structuredJournalImpl; static std::unique_ptr<StructuredJournalImpl> m_structuredJournalImpl;
// Invariant: m_sink always points to a valid Sink // Invariant: m_sink always points to a valid Sink
Sink* m_sink = nullptr; Sink* m_sink = nullptr;
@@ -85,9 +85,9 @@ public:
//-------------------------------------------------------------------------- //--------------------------------------------------------------------------
static void static void
enableStructuredJournal(StructuredJournalImpl* impl) enableStructuredJournal(std::unique_ptr<StructuredJournalImpl> impl)
{ {
m_structuredJournalImpl = impl; m_structuredJournalImpl = std::move(impl);
} }
static void static void
@@ -99,7 +99,7 @@ public:
static bool static bool
isStructuredJournalEnabled() isStructuredJournalEnabled()
{ {
return m_structuredJournalImpl; return m_structuredJournalImpl != nullptr;
} }
class StructuredJournalImpl class StructuredJournalImpl
@@ -382,19 +382,13 @@ public:
: m_sink(other.m_sink) : m_sink(other.m_sink)
{ {
if (attributes) if (attributes)
{
m_attributes = std::move(attributes); m_attributes = std::move(attributes);
}
if (other.m_attributes) if (other.m_attributes)
{ {
if (m_attributes) if (m_attributes)
{
m_attributes->combine(other.m_attributes); m_attributes->combine(other.m_attributes);
}
else else
{
m_attributes = other.m_attributes->clone(); m_attributes = other.m_attributes->clone();
}
} }
} }
@@ -404,19 +398,13 @@ public:
: m_sink(other.m_sink) : m_sink(other.m_sink)
{ {
if (attributes) if (attributes)
{
m_attributes = std::move(attributes); m_attributes = std::move(attributes);
}
if (other.m_attributes) if (other.m_attributes)
{ {
if (m_attributes) if (m_attributes)
{
m_attributes->combine(std::move(other.m_attributes)); m_attributes->combine(std::move(other.m_attributes));
}
else else
{
m_attributes = std::move(other.m_attributes); m_attributes = std::move(other.m_attributes);
}
} }
} }
@@ -439,9 +427,7 @@ public:
{ {
m_sink = other.m_sink; m_sink = other.m_sink;
if (other.m_attributes) if (other.m_attributes)
{
m_attributes = other.m_attributes->clone(); m_attributes = other.m_attributes->clone();
}
return *this; return *this;
} }
@@ -450,9 +436,7 @@ public:
{ {
m_sink = other.m_sink; m_sink = other.m_sink;
if (other.m_attributes) if (other.m_attributes)
{
m_attributes = std::move(other.m_attributes); m_attributes = std::move(other.m_attributes);
}
return *this; return *this;
} }
@@ -487,9 +471,7 @@ public:
trace(std::source_location location = std::source_location::current()) const trace(std::source_location location = std::source_location::current()) const
{ {
if (m_structuredJournalImpl) if (m_structuredJournalImpl)
{
m_structuredJournalImpl->initMessageContext(location); m_structuredJournalImpl->initMessageContext(location);
}
return { return {
m_attributes ? m_attributes->clone() : nullptr, m_attributes ? m_attributes->clone() : nullptr,
*m_sink, *m_sink,
@@ -500,9 +482,7 @@ public:
debug(std::source_location location = std::source_location::current()) const debug(std::source_location location = std::source_location::current()) const
{ {
if (m_structuredJournalImpl) if (m_structuredJournalImpl)
{
m_structuredJournalImpl->initMessageContext(location); m_structuredJournalImpl->initMessageContext(location);
}
return { return {
m_attributes ? m_attributes->clone() : nullptr, m_attributes ? m_attributes->clone() : nullptr,
*m_sink, *m_sink,
@@ -513,9 +493,7 @@ public:
info(std::source_location location = std::source_location::current()) const info(std::source_location location = std::source_location::current()) const
{ {
if (m_structuredJournalImpl) if (m_structuredJournalImpl)
{
m_structuredJournalImpl->initMessageContext(location); m_structuredJournalImpl->initMessageContext(location);
}
return { return {
m_attributes ? m_attributes->clone() : nullptr, m_attributes ? m_attributes->clone() : nullptr,
*m_sink, *m_sink,
@@ -526,9 +504,7 @@ public:
warn(std::source_location location = std::source_location::current()) const warn(std::source_location location = std::source_location::current()) const
{ {
if (m_structuredJournalImpl) if (m_structuredJournalImpl)
{
m_structuredJournalImpl->initMessageContext(location); m_structuredJournalImpl->initMessageContext(location);
}
return { return {
m_attributes ? m_attributes->clone() : nullptr, m_attributes ? m_attributes->clone() : nullptr,
*m_sink, *m_sink,
@@ -539,9 +515,7 @@ public:
error(std::source_location location = std::source_location::current()) const error(std::source_location location = std::source_location::current()) const
{ {
if (m_structuredJournalImpl) if (m_structuredJournalImpl)
{
m_structuredJournalImpl->initMessageContext(location); m_structuredJournalImpl->initMessageContext(location);
}
return { return {
m_attributes ? m_attributes->clone() : nullptr, m_attributes ? m_attributes->clone() : nullptr,
*m_sink, *m_sink,
@@ -552,9 +526,7 @@ public:
fatal(std::source_location location = std::source_location::current()) const fatal(std::source_location location = std::source_location::current()) const
{ {
if (m_structuredJournalImpl) if (m_structuredJournalImpl)
{
m_structuredJournalImpl->initMessageContext(location); m_structuredJournalImpl->initMessageContext(location);
}
return { return {
m_attributes ? m_attributes->clone() : nullptr, m_attributes ? m_attributes->clone() : nullptr,
*m_sink, *m_sink,

View File

@@ -167,13 +167,9 @@ Logs::journal(
if (globalLogAttributes_) if (globalLogAttributes_)
{ {
if (attributes) if (attributes)
{
attributes->combine(globalLogAttributes_); attributes->combine(globalLogAttributes_);
}
else else
{
attributes = globalLogAttributes_->clone(); attributes = globalLogAttributes_->clone();
}
} }
return beast::Journal(get(name), name, std::move(attributes)); return beast::Journal(get(name), name, std::move(attributes));
} }

View File

@@ -25,7 +25,7 @@
namespace beast { namespace beast {
Journal::StructuredJournalImpl* Journal::m_structuredJournalImpl = nullptr; std::unique_ptr<Journal::StructuredJournalImpl> Journal::m_structuredJournalImpl;
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
@@ -179,26 +179,18 @@ Journal::ScopedStream::~ScopedStream()
if (s == "\n") if (s == "\n")
{ {
if (m_structuredJournalImpl) if (m_structuredJournalImpl)
{
m_structuredJournalImpl->flush( m_structuredJournalImpl->flush(
&m_sink, m_level, "", m_attributes.get()); &m_sink, m_level, "", m_attributes.get());
}
else else
{
m_sink.write(m_level, ""); m_sink.write(m_level, "");
}
} }
else else
{ {
if (m_structuredJournalImpl) if (m_structuredJournalImpl)
{
m_structuredJournalImpl->flush( m_structuredJournalImpl->flush(
&m_sink, m_level, s, m_attributes.get()); &m_sink, m_level, s, m_attributes.get());
}
else else
{
m_sink.write(m_level, s); m_sink.write(m_level, s);
}
} }
} }
} }

View File

@@ -61,13 +61,9 @@ JsonLogAttributes::combine(std::unique_ptr<StructuredLogAttributes>&& context)
auto structuredContext = static_cast<JsonLogAttributes*>(context.get()); auto structuredContext = static_cast<JsonLogAttributes*>(context.get());
if (contextValues_.empty()) if (contextValues_.empty())
{
contextValues_ = std::move(structuredContext->contextValues_); contextValues_ = std::move(structuredContext->contextValues_);
}
else else
{
contextValues_.merge(structuredContext->contextValues_); contextValues_.merge(structuredContext->contextValues_);
}
} }
JsonStructuredJournal::Logger::Logger( JsonStructuredJournal::Logger::Logger(
@@ -89,9 +85,7 @@ JsonStructuredJournal::Logger::write(
{ {
auto jsonContext = static_cast<JsonLogAttributes*>(context); auto jsonContext = static_cast<JsonLogAttributes*>(context);
for (auto const& [key, value] : jsonContext->contextValues()) for (auto const& [key, value] : jsonContext->contextValues())
{
globalContext[key] = value; globalContext[key] = value;
}
} }
globalContext["Function"] = location.function_name(); globalContext["Function"] = location.function_name();
globalContext["File"] = location.file_name(); globalContext["File"] = location.file_name();

View File

@@ -108,8 +108,8 @@ TEST_CASE("Test format output")
TEST_CASE("Test format output when structured logs are enabled") TEST_CASE("Test format output when structured logs are enabled")
{ {
static log::JsonStructuredJournal structuredJournal; auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
beast::Journal::enableStructuredJournal(&structuredJournal); beast::Journal::enableStructuredJournal(std::move(structuredJournal));
std::string output; std::string output;
Logs::format(output, "Message", beast::severities::kDebug, "Test"); Logs::format(output, "Message", beast::severities::kDebug, "Test");
@@ -121,7 +121,7 @@ TEST_CASE("Test format output when structured logs are enabled")
TEST_CASE("Enable json logs") TEST_CASE("Enable json logs")
{ {
static log::JsonStructuredJournal structuredJournal; auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
std::stringstream logStream; std::stringstream logStream;
@@ -133,7 +133,7 @@ TEST_CASE("Enable json logs")
logStream.str(""); logStream.str("");
beast::Journal::enableStructuredJournal(&structuredJournal); beast::Journal::enableStructuredJournal(std::move(structuredJournal));
logs.journal("Test").debug() << "\n"; logs.journal("Test").debug() << "\n";
@@ -152,13 +152,13 @@ TEST_CASE("Enable json logs")
TEST_CASE("Global attributes") TEST_CASE("Global attributes")
{ {
static log::JsonStructuredJournal structuredJournal; auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
std::stringstream logStream; std::stringstream logStream;
MockLogs logs{logStream, beast::severities::kAll}; MockLogs logs{logStream, beast::severities::kAll};
beast::Journal::enableStructuredJournal(&structuredJournal); beast::Journal::enableStructuredJournal(std::move(structuredJournal));
MockLogs::setGlobalAttributes(log::attributes({{"Field1", "Value1"}})); MockLogs::setGlobalAttributes(log::attributes({{"Field1", "Value1"}}));
logs.journal("Test").debug() << "Test"; logs.journal("Test").debug() << "Test";
@@ -178,13 +178,13 @@ TEST_CASE("Global attributes")
TEST_CASE("Global attributes inheritable") TEST_CASE("Global attributes inheritable")
{ {
static log::JsonStructuredJournal structuredJournal; auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
std::stringstream logStream; std::stringstream logStream;
MockLogs logs{logStream, beast::severities::kAll}; MockLogs logs{logStream, beast::severities::kAll};
beast::Journal::enableStructuredJournal(&structuredJournal); beast::Journal::enableStructuredJournal(std::move(structuredJournal));
MockLogs::setGlobalAttributes(log::attributes({{"Field1", "Value1"}})); MockLogs::setGlobalAttributes(log::attributes({{"Field1", "Value1"}}));
logs.journal( logs.journal(

View File

@@ -58,8 +58,8 @@ public:
JsonLogStreamFixture() JsonLogStreamFixture()
: sink_(beast::severities::kAll, logStream_), j_(sink_) : sink_(beast::severities::kAll, logStream_), j_(sink_)
{ {
static log::JsonStructuredJournal structuredJournal; auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
beast::Journal::enableStructuredJournal(&structuredJournal); beast::Journal::enableStructuredJournal(std::move(structuredJournal));
} }
~JsonLogStreamFixture() ~JsonLogStreamFixture()

View File

@@ -791,8 +791,8 @@ run(int argc, char** argv)
if (config->LOG_STYLE == LogStyle::Json) if (config->LOG_STYLE == LogStyle::Json)
{ {
static log::JsonStructuredJournal structuredJournal; auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
beast::Journal::enableStructuredJournal(&structuredJournal); beast::Journal::enableStructuredJournal(std::move(structuredJournal));
Logs::setGlobalAttributes(log::attributes( Logs::setGlobalAttributes(log::attributes(
{{"Application", "rippled"}, {"NetworkID", config->NETWORK_ID}})); {{"Application", "rippled"}, {"NetworkID", config->NETWORK_ID}}));
} }

View File

@@ -1085,9 +1085,7 @@ LogStyle::LogStyle
LogStyle::fromString(std::string const& str) LogStyle::fromString(std::string const& str)
{ {
if (str == "json") if (str == "json")
{
return Json; return Json;
}
return LogFmt; return LogFmt;
} }