Skip to content

Commit 276c021

Browse files
a1q123456a1q123456
committed
Fix PR comments
Signed-off-by: JCW <[email protected]>
1 parent fb22886 commit 276c021

File tree

8 files changed

+17
-65
lines changed

8 files changed

+17
-65
lines changed

‎include/xrpl/beast/utility/Journal.h‎

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class Journal
7676

7777
std::unique_ptr<StructuredLogAttributes> m_attributes;
7878

79-
static StructuredJournalImpl* m_structuredJournalImpl;
79+
static std::unique_ptr<StructuredJournalImpl> m_structuredJournalImpl;
8080

8181
// Invariant: m_sink always points to a valid Sink
8282
Sink* m_sink = nullptr;
@@ -85,9 +85,9 @@ class Journal
8585
//--------------------------------------------------------------------------
8686

8787
static void
88-
enableStructuredJournal(StructuredJournalImpl* impl)
88+
enableStructuredJournal(std::unique_ptr<StructuredJournalImpl> impl)
8989
{
90-
m_structuredJournalImpl = impl;
90+
m_structuredJournalImpl = std::move(impl);
9191
}
9292

9393
static void
@@ -99,7 +99,7 @@ class Journal
9999
static bool
100100
isStructuredJournalEnabled()
101101
{
102-
return m_structuredJournalImpl;
102+
return m_structuredJournalImpl != nullptr;
103103
}
104104

105105
class StructuredJournalImpl
@@ -382,19 +382,13 @@ class Journal
382382
: m_sink(other.m_sink)
383383
{
384384
if (attributes)
385-
{
386385
m_attributes = std::move(attributes);
387-
}
388386
if (other.m_attributes)
389387
{
390388
if (m_attributes)
391-
{
392389
m_attributes->combine(other.m_attributes);
393-
}
394390
else
395-
{
396391
m_attributes = other.m_attributes->clone();
397-
}
398392
}
399393
}
400394

@@ -404,19 +398,13 @@ class Journal
404398
: m_sink(other.m_sink)
405399
{
406400
if (attributes)
407-
{
408401
m_attributes = std::move(attributes);
409-
}
410402
if (other.m_attributes)
411403
{
412404
if (m_attributes)
413-
{
414405
m_attributes->combine(std::move(other.m_attributes));
415-
}
416406
else
417-
{
418407
m_attributes = std::move(other.m_attributes);
419-
}
420408
}
421409
}
422410

@@ -439,9 +427,7 @@ class Journal
439427
{
440428
m_sink = other.m_sink;
441429
if (other.m_attributes)
442-
{
443430
m_attributes = other.m_attributes->clone();
444-
}
445431
return *this;
446432
}
447433

@@ -450,9 +436,7 @@ class Journal
450436
{
451437
m_sink = other.m_sink;
452438
if (other.m_attributes)
453-
{
454439
m_attributes = std::move(other.m_attributes);
455-
}
456440
return *this;
457441
}
458442

@@ -487,9 +471,7 @@ class Journal
487471
trace(std::source_location location = std::source_location::current()) const
488472
{
489473
if (m_structuredJournalImpl)
490-
{
491474
m_structuredJournalImpl->initMessageContext(location);
492-
}
493475
return {
494476
m_attributes ? m_attributes->clone() : nullptr,
495477
*m_sink,
@@ -500,9 +482,7 @@ class Journal
500482
debug(std::source_location location = std::source_location::current()) const
501483
{
502484
if (m_structuredJournalImpl)
503-
{
504485
m_structuredJournalImpl->initMessageContext(location);
505-
}
506486
return {
507487
m_attributes ? m_attributes->clone() : nullptr,
508488
*m_sink,
@@ -513,9 +493,7 @@ class Journal
513493
info(std::source_location location = std::source_location::current()) const
514494
{
515495
if (m_structuredJournalImpl)
516-
{
517496
m_structuredJournalImpl->initMessageContext(location);
518-
}
519497
return {
520498
m_attributes ? m_attributes->clone() : nullptr,
521499
*m_sink,
@@ -526,9 +504,7 @@ class Journal
526504
warn(std::source_location location = std::source_location::current()) const
527505
{
528506
if (m_structuredJournalImpl)
529-
{
530507
m_structuredJournalImpl->initMessageContext(location);
531-
}
532508
return {
533509
m_attributes ? m_attributes->clone() : nullptr,
534510
*m_sink,
@@ -539,9 +515,7 @@ class Journal
539515
error(std::source_location location = std::source_location::current()) const
540516
{
541517
if (m_structuredJournalImpl)
542-
{
543518
m_structuredJournalImpl->initMessageContext(location);
544-
}
545519
return {
546520
m_attributes ? m_attributes->clone() : nullptr,
547521
*m_sink,
@@ -552,9 +526,7 @@ class Journal
552526
fatal(std::source_location location = std::source_location::current()) const
553527
{
554528
if (m_structuredJournalImpl)
555-
{
556529
m_structuredJournalImpl->initMessageContext(location);
557-
}
558530
return {
559531
m_attributes ? m_attributes->clone() : nullptr,
560532
*m_sink,

‎src/libxrpl/basics/Log.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,13 +167,9 @@ Logs::journal(
167167
if (globalLogAttributes_)
168168
{
169169
if (attributes)
170-
{
171170
attributes->combine(globalLogAttributes_);
172-
}
173171
else
174-
{
175172
attributes = globalLogAttributes_->clone();
176-
}
177173
}
178174
return beast::Journal(get(name), name, std::move(attributes));
179175
}

‎src/libxrpl/beast/utility/beast_Journal.cpp‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
namespace beast {
2727

28-
Journal::StructuredJournalImpl* Journal::m_structuredJournalImpl = nullptr;
28+
std::unique_ptr<Journal::StructuredJournalImpl> Journal::m_structuredJournalImpl;
2929

3030
//------------------------------------------------------------------------------
3131

@@ -179,26 +179,18 @@ Journal::ScopedStream::~ScopedStream()
179179
if (s == "\n")
180180
{
181181
if (m_structuredJournalImpl)
182-
{
183182
m_structuredJournalImpl->flush(
184183
&m_sink, m_level, "", m_attributes.get());
185-
}
186184
else
187-
{
188185
m_sink.write(m_level, "");
189-
}
190186
}
191187
else
192188
{
193189
if (m_structuredJournalImpl)
194-
{
195190
m_structuredJournalImpl->flush(
196191
&m_sink, m_level, s, m_attributes.get());
197-
}
198192
else
199-
{
200193
m_sink.write(m_level, s);
201-
}
202194
}
203195
}
204196
}

‎src/libxrpl/telemetry/JsonLogs.cpp‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,9 @@ JsonLogAttributes::combine(std::unique_ptr<StructuredLogAttributes>&& context)
6161
auto structuredContext = static_cast<JsonLogAttributes*>(context.get());
6262

6363
if (contextValues_.empty())
64-
{
6564
contextValues_ = std::move(structuredContext->contextValues_);
66-
}
6765
else
68-
{
6966
contextValues_.merge(structuredContext->contextValues_);
70-
}
7167
}
7268

7369
JsonStructuredJournal::Logger::Logger(
@@ -89,9 +85,7 @@ JsonStructuredJournal::Logger::write(
8985
{
9086
auto jsonContext = static_cast<JsonLogAttributes*>(context);
9187
for (auto const& [key, value] : jsonContext->contextValues())
92-
{
9388
globalContext[key] = value;
94-
}
9589
}
9690
globalContext["Function"] = location.function_name();
9791
globalContext["File"] = location.file_name();

‎src/tests/libxrpl/basics/log.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ TEST_CASE("Test format output")
108108

109109
TEST_CASE("Test format output when structured logs are enabled")
110110
{
111-
static log::JsonStructuredJournal structuredJournal;
112-
beast::Journal::enableStructuredJournal(&structuredJournal);
111+
auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
112+
beast::Journal::enableStructuredJournal(std::move(structuredJournal));
113113

114114
std::string output;
115115
Logs::format(output, "Message", beast::severities::kDebug, "Test");
@@ -121,7 +121,7 @@ TEST_CASE("Test format output when structured logs are enabled")
121121

122122
TEST_CASE("Enable json logs")
123123
{
124-
static log::JsonStructuredJournal structuredJournal;
124+
auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
125125

126126
std::stringstream logStream;
127127

@@ -133,7 +133,7 @@ TEST_CASE("Enable json logs")
133133

134134
logStream.str("");
135135

136-
beast::Journal::enableStructuredJournal(&structuredJournal);
136+
beast::Journal::enableStructuredJournal(std::move(structuredJournal));
137137

138138
logs.journal("Test").debug() << "\n";
139139

@@ -152,13 +152,13 @@ TEST_CASE("Enable json logs")
152152

153153
TEST_CASE("Global attributes")
154154
{
155-
static log::JsonStructuredJournal structuredJournal;
155+
auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
156156

157157
std::stringstream logStream;
158158

159159
MockLogs logs{logStream, beast::severities::kAll};
160160

161-
beast::Journal::enableStructuredJournal(&structuredJournal);
161+
beast::Journal::enableStructuredJournal(std::move(structuredJournal));
162162
MockLogs::setGlobalAttributes(log::attributes({{"Field1", "Value1"}}));
163163

164164
logs.journal("Test").debug() << "Test";
@@ -178,13 +178,13 @@ TEST_CASE("Global attributes")
178178

179179
TEST_CASE("Global attributes inheritable")
180180
{
181-
static log::JsonStructuredJournal structuredJournal;
181+
auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
182182

183183
std::stringstream logStream;
184184

185185
MockLogs logs{logStream, beast::severities::kAll};
186186

187-
beast::Journal::enableStructuredJournal(&structuredJournal);
187+
beast::Journal::enableStructuredJournal(std::move(structuredJournal));
188188
MockLogs::setGlobalAttributes(log::attributes({{"Field1", "Value1"}}));
189189

190190
logs.journal(

‎src/tests/libxrpl/telemetry/json_logs.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ class JsonLogStreamFixture
5858
JsonLogStreamFixture()
5959
: sink_(beast::severities::kAll, logStream_), j_(sink_)
6060
{
61-
static log::JsonStructuredJournal structuredJournal;
62-
beast::Journal::enableStructuredJournal(&structuredJournal);
61+
auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
62+
beast::Journal::enableStructuredJournal(std::move(structuredJournal));
6363
}
6464

6565
~JsonLogStreamFixture()

‎src/xrpld/app/main/Main.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -791,8 +791,8 @@ run(int argc, char** argv)
791791

792792
if (config->LOG_STYLE == LogStyle::Json)
793793
{
794-
static log::JsonStructuredJournal structuredJournal;
795-
beast::Journal::enableStructuredJournal(&structuredJournal);
794+
auto structuredJournal = std::make_unique<log::JsonStructuredJournal>();
795+
beast::Journal::enableStructuredJournal(std::move(structuredJournal));
796796
Logs::setGlobalAttributes(log::attributes(
797797
{{"Application", "rippled"}, {"NetworkID", config->NETWORK_ID}}));
798798
}

‎src/xrpld/core/detail/Config.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,9 +1085,7 @@ LogStyle::LogStyle
10851085
LogStyle::fromString(std::string const& str)
10861086
{
10871087
if (str == "json")
1088-
{
10891088
return Json;
1090-
}
10911089
return LogFmt;
10921090
}
10931091

0 commit comments

Comments
 (0)