From 3e483cfa4c9674d3255e4a245d805d7b2918bd5d Mon Sep 17 00:00:00 2001 From: Brad Chase Date: Tue, 14 Nov 2017 13:27:02 -0500 Subject: [PATCH] Improve JSON logging --- src/ripple/consensus/Consensus.h | 7 +- src/ripple/consensus/DisputedTx.h | 5 +- src/ripple/json/impl/json_writer.cpp | 83 ---------------- src/ripple/json/json_value.h | 8 -- src/ripple/json/json_writer.h | 132 ++++++++++++++++++++++++- src/ripple/rpc/impl/ServerHandlerImp.h | 1 + src/ripple/rpc/impl/WSInfoSub.h | 5 +- src/test/json/json_value_test.cpp | 28 ++++++ 8 files changed, 169 insertions(+), 100 deletions(-) diff --git a/src/ripple/consensus/Consensus.h b/src/ripple/consensus/Consensus.h index 7b72e074d5..891dd18b67 100644 --- a/src/ripple/consensus/Consensus.h +++ b/src/ripple/consensus/Consensus.h @@ -1020,8 +1020,9 @@ Consensus::checkLedger() << ", " << " mode=" << to_string(mode_.get()); JLOG(j_.warn()) << prevLedgerID_ << " to " << netLgr; - JLOG(j_.warn()) << previousLedger_.getJson(); - JLOG(j_.debug()) << "State on consensus change " << getJson(true); + JLOG(j_.warn()) << Json::Compact{previousLedger_.getJson()}; + JLOG(j_.debug()) << "State on consensus change " + << Json::Compact{getJson(true)}; handleWrongLedger(netLgr); } else if (previousLedger_.id() != prevLedgerID_) @@ -1434,7 +1435,7 @@ Consensus::haveConsensus() if (result_->state == ConsensusState::MovedOn) { JLOG(j_.error()) << "Unable to reach consensus"; - JLOG(j_.error()) << getJson(true); + JLOG(j_.error()) << Json::Compact{getJson(true)}; } return true; diff --git a/src/ripple/consensus/DisputedTx.h b/src/ripple/consensus/DisputedTx.h index ed0360ebd0..93612e5818 100644 --- a/src/ripple/consensus/DisputedTx.h +++ b/src/ripple/consensus/DisputedTx.h @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -239,14 +240,14 @@ DisputedTx::updateVote( JLOG(j_.info()) << "No change (" << (ourVote_ ? "YES" : "NO") << ") : weight " << weight << ", percent " << percentTime; - JLOG(j_.debug()) << getJson(); + JLOG(j_.debug()) << Json::Compact{getJson()}; return false; } ourVote_ = newPosition; JLOG(j_.debug()) << "We now vote " << (ourVote_ ? "YES" : "NO") << " on " << tx_.id(); - JLOG(j_.debug()) << getJson(); + JLOG(j_.debug()) << Json::Compact{getJson()}; return true; } diff --git a/src/ripple/json/impl/json_writer.cpp b/src/ripple/json/impl/json_writer.cpp index 7cc3c5d100..8a05e5556d 100644 --- a/src/ripple/json/impl/json_writer.cpp +++ b/src/ripple/json/impl/json_writer.cpp @@ -743,87 +743,4 @@ std::ostream& operator<< ( std::ostream& sout, const Value& root ) return sout; } -//------------------------------------------------------------------------------ - -namespace detail { - -inline -void -write_string (write_t write, std::string const& s) -{ - write(s.data(), s.size()); -} - -void -write_value (write_t write, Value const& value) -{ - switch (value.type()) - { - case nullValue: - write("null", 4); - break; - - case intValue: - write_string(write, valueToString(value.asInt())); - break; - - case uintValue: - write_string(write, valueToString(value.asUInt())); - break; - - case realValue: - write_string(write, valueToString(value.asDouble())); - break; - - case stringValue: - write_string(write, valueToQuotedString(value.asCString())); - break; - - case booleanValue: - write_string(write, valueToString(value.asBool())); - break; - - case arrayValue: - { - write("[", 1); - int const size = value.size(); - for (int index = 0; index < size; ++index) - { - if (index > 0) - write(",", 1); - write_value(write, value[index]); - } - write("]", 1); - break; - } - - case objectValue: - { - Value::Members const members = value.getMemberNames(); - write("{", 1); - for (auto it = members.begin(); it != members.end(); ++it) - { - std::string const& name = *it; - if (it != members.begin()) - write(",", 1); - - write_string(write, valueToQuotedString(name.c_str())); - write(":", 1); - write_value(write, value[name]); - } - write("}", 1); - break; - } - } -} - -} // detail - -void -stream (Json::Value const& jv, write_t write) -{ - detail::write_value(write, jv); - write("\n", 1); -} - } // namespace Json diff --git a/src/ripple/json/json_value.h b/src/ripple/json/json_value.h index 22ccc6cf25..2644223ce9 100644 --- a/src/ripple/json/json_value.h +++ b/src/ripple/json/json_value.h @@ -606,14 +606,6 @@ public: } }; -//------------------------------------------------------------------------------ - -using write_t = std::function; - -/** Stream compact JSON to the specified function. */ -void -stream (Json::Value const& jv, write_t write); - } // namespace Json diff --git a/src/ripple/json/json_writer.h b/src/ripple/json/json_writer.h index e4392c4673..48a5374d57 100644 --- a/src/ripple/json/json_writer.h +++ b/src/ripple/json/json_writer.h @@ -22,6 +22,7 @@ #include #include +#include #include namespace Json @@ -175,8 +176,137 @@ std::string valueToQuotedString ( const char* value ); /// \see Json::operator>>() std::ostream& operator<< ( std::ostream&, const Value& root ); -} // namespace Json +//------------------------------------------------------------------------------ +// Helpers for stream +namespace detail { +template +void +write_string(Write const& write, std::string const& s) +{ + write(s.data(), s.size()); +} + +template +void +write_value(Write const& write, Value const& value) +{ + switch (value.type()) + { + case nullValue: + write("null", 4); + break; + + case intValue: + write_string(write, valueToString(value.asInt())); + break; + + case uintValue: + write_string(write, valueToString(value.asUInt())); + break; + + case realValue: + write_string(write, valueToString(value.asDouble())); + break; + + case stringValue: + write_string(write, valueToQuotedString(value.asCString())); + break; + + case booleanValue: + write_string(write, valueToString(value.asBool())); + break; + + case arrayValue: + { + write("[", 1); + int const size = value.size(); + for (int index = 0; index < size; ++index) + { + if (index > 0) + write(",", 1); + write_value(write, value[index]); + } + write("]", 1); + break; + } + + case objectValue: + { + Value::Members const members = value.getMemberNames(); + write("{", 1); + for (auto it = members.begin(); it != members.end(); ++it) + { + std::string const& name = *it; + if (it != members.begin()) + write(",", 1); + + write_string(write, valueToQuotedString(name.c_str())); + write(":", 1); + write_value(write, value[name]); + } + write("}", 1); + break; + } + } +} + +} // namespace detail + +/** Stream compact JSON to the specified function. + + @param jv The Json::Value to write + @param write Invocable with signature void(void const*, std::size_t) that + is called when output should be written to the stream. +*/ +template +void +stream(Json::Value const& jv, Write const& write) +{ + detail::write_value(write, jv); + write("\n", 1); +} + +/** Decorator for streaming out compact json + + Use + + Json::Value jv; + out << Json::Compact{jv} + + to write a single-line, compact version of `jv` to the stream, rather + than the styled format that comes from undecorated streaming. +*/ +class Compact +{ + Json::Value jv_; + +public: + /** Wrap a Json::Value for compact streaming + + @param jv The Json::Value to stream + + @note For now, we do not support wrapping lvalues to avoid + potentially costly copies. If we find a need, we can consider + adding support for compact lvalue streaming in the future. + */ + Compact(Json::Value&& jv) : jv_{std::move(jv)} + { + } + + friend std::ostream& + operator<<(std::ostream& o, Compact const& cJv) + { + detail::write_value( + [&o](void const* data, std::size_t n) { + o.write(static_cast(data), n); + }, + cJv.jv_); + return o; + } +}; + +} // namespace Json #endif // JSON_WRITER_H_INCLUDED diff --git a/src/ripple/rpc/impl/ServerHandlerImp.h b/src/ripple/rpc/impl/ServerHandlerImp.h index a94ccda883..cf83ce0073 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.h +++ b/src/ripple/rpc/impl/ServerHandlerImp.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include diff --git a/src/ripple/rpc/impl/WSInfoSub.h b/src/ripple/rpc/impl/WSInfoSub.h index c53a334716..0eaffd3baa 100644 --- a/src/ripple/rpc/impl/WSInfoSub.h +++ b/src/ripple/rpc/impl/WSInfoSub.h @@ -23,8 +23,7 @@ #include #include #include -#include -#include +#include #include #include #include @@ -75,7 +74,7 @@ public: if(! sp) return; beast::multi_buffer sb; - stream(jv, + Json::stream(jv, [&](void const* data, std::size_t n) { sb.commit(boost::asio::buffer_copy( diff --git a/src/test/json/json_value_test.cpp b/src/test/json/json_value_test.cpp index 081757c72b..cbe43b67ac 100644 --- a/src/test/json/json_value_test.cpp +++ b/src/test/json/json_value_test.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -222,6 +223,32 @@ struct json_value_test : beast::unit_test::suite testGreaterThan ("big"); } + void test_compact () + { + Json::Value j; + Json::Reader r; + char const* s ("{\"array\":[{\"12\":23},{},null,false,0.5]}"); + + auto countLines = [](std::string const & s) + { + return 1 + std::count_if(s.begin(), s.end(), [](char c){ + return c == '\n'; + }); + }; + + BEAST_EXPECT(r.parse(s,j)); + { + std::stringstream ss; + ss << j; + BEAST_EXPECT(countLines(ss.str()) > 1); + } + { + std::stringstream ss; + ss << Json::Compact(std::move(j)); + BEAST_EXPECT(countLines(ss.str()) == 1); + } + } + void run () { test_bool (); @@ -230,6 +257,7 @@ struct json_value_test : beast::unit_test::suite test_copy (); test_move (); test_comparisons (); + test_compact (); } };