Skip to content

Commit 2d6f82c

Browse files
kuznetsssclaude
andauthored
feat: Metrics for requested ledger age (#2947)
Adding metrics to be able to analyse requested ledger age distribution. --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 6ba58f4 commit 2d6f82c

File tree

9 files changed

+224
-14
lines changed

9 files changed

+224
-14
lines changed

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,5 @@
1010
.sanitizer-report
1111
CMakeUserPresets.json
1212
config.json
13+
CLAUDE.md
14+
.claude/**

src/rpc/Counters.cpp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,24 @@
2121

2222
#include "rpc/JS.hpp"
2323
#include "rpc/WorkQueue.hpp"
24+
#include "util/JsonUtils.hpp"
2425
#include "util/prometheus/Label.hpp"
2526
#include "util/prometheus/Prometheus.hpp"
2627

2728
#include <boost/json/object.hpp>
29+
#include <boost/json/value.hpp>
30+
#include <boost/json/value_to.hpp>
2831
#include <fmt/format.h>
2932
#include <xrpl/protocol/jss.h>
3033

3134
#include <chrono>
35+
#include <cstdint>
3236
#include <functional>
3337
#include <mutex>
38+
#include <optional>
3439
#include <string>
3540
#include <utility>
41+
#include <vector>
3642

3743
namespace rpc {
3844

@@ -138,6 +144,21 @@ Counters::Counters(Reportable const& wq)
138144
"Total number of internal errors"
139145
)
140146
)
147+
, ledgerAgeLedgersHistogram_(
148+
PrometheusService::histogramInt(
149+
"rpc_requested_ledger_age_histogram",
150+
Labels{},
151+
{0, 10, 100, 1'000, 10'000, 100'000, 1'000'000, 10'000'000, 100'000'000},
152+
"Age of requested ledgers in ledger count"
153+
)
154+
)
155+
, ledgerHashRequestsCounter_(
156+
PrometheusService::counterInt(
157+
"rpc_ledger_hash_requests_total_number",
158+
Labels{},
159+
"Total number of successful requests containing ledger_hash field"
160+
)
161+
)
141162
, workQueue_(std::cref(wq))
142163
, startupTime_{std::chrono::system_clock::now()}
143164
{
@@ -217,6 +238,35 @@ Counters::onInternalError()
217238
++internalErrorCounter_.get();
218239
}
219240

241+
void
242+
Counters::recordLedgerRequest(
243+
boost::json::object const& params,
244+
std::uint32_t currentLedgerSequence
245+
)
246+
{
247+
if (params.contains(JS(ledger_hash))) {
248+
++ledgerHashRequestsCounter_.get();
249+
return;
250+
}
251+
252+
if (not params.contains(JS(ledger_index))) {
253+
ledgerAgeLedgersHistogram_.get().observe(0);
254+
return;
255+
}
256+
auto const& indexValue = params.at("ledger_index");
257+
if (auto const parsed = util::getLedgerIndex(indexValue); parsed.has_value()) {
258+
if (*parsed <= currentLedgerSequence) {
259+
auto const ageLedgers = static_cast<std::int64_t>(currentLedgerSequence - *parsed);
260+
ledgerAgeLedgersHistogram_.get().observe(ageLedgers);
261+
}
262+
} else if (indexValue.is_string()) {
263+
auto const indexStr = boost::json::value_to<std::string>(indexValue);
264+
if (indexStr == "validated") {
265+
ledgerAgeLedgersHistogram_.get().observe(0);
266+
}
267+
}
268+
}
269+
220270
std::chrono::seconds
221271
Counters::uptime() const
222272
{

src/rpc/Counters.hpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@
2121

2222
#include "rpc/WorkQueue.hpp"
2323
#include "util/prometheus/Counter.hpp"
24+
#include "util/prometheus/Histogram.hpp"
2425

2526
#include <boost/json.hpp>
2627
#include <boost/json/object.hpp>
2728

2829
#include <chrono>
30+
#include <cstdint>
2931
#include <functional>
3032
#include <mutex>
3133
#include <string>
@@ -66,6 +68,9 @@ class Counters {
6668
CounterType unknownCommandCounter_;
6769
CounterType internalErrorCounter_;
6870

71+
std::reference_wrapper<util::prometheus::HistogramInt> ledgerAgeLedgersHistogram_;
72+
CounterType ledgerHashRequestsCounter_;
73+
6974
std::reference_wrapper<Reportable const> workQueue_;
7075
std::chrono::time_point<std::chrono::system_clock> startupTime_;
7176

@@ -150,6 +155,15 @@ class Counters {
150155
void
151156
onInternalError();
152157

158+
/**
159+
* @brief Records ledger request metrics based on the ledger parameter in the request.
160+
*
161+
* @param params The request parameters containing ledger information
162+
* @param currentLedgerSequence The current ledger sequence number
163+
*/
164+
void
165+
recordLedgerRequest(boost::json::object const& params, std::uint32_t currentLedgerSequence);
166+
153167
/** @return Uptime of this instance in seconds. */
154168
std::chrono::seconds
155169
uptime() const;

src/rpc/RPCEngine.hpp

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
#include "rpc/common/HandlerProvider.hpp"
2828
#include "rpc/common/Types.hpp"
2929
#include "rpc/common/impl/ForwardingProxy.hpp"
30-
#include "util/OverloadSet.hpp"
3130
#include "util/ResponseExpirationCache.hpp"
3231
#include "util/log/Logger.hpp"
3332
#include "web/Context.hpp"
@@ -41,6 +40,7 @@
4140
#include <xrpl/protocol/ErrorCodes.h>
4241

4342
#include <chrono>
43+
#include <cstdint>
4444
#include <exception>
4545
#include <functional>
4646
#include <memory>
@@ -228,16 +228,37 @@ class RPCEngine {
228228
}
229229

230230
/**
231-
* @brief Notify the system that specified method was executed.
231+
* @brief Notify the system that specified method was executed and record ledger metrics.
232232
*
233-
* @param method
233+
* @param context The web context containing method, params, and ledger information
234234
* @param duration The time it took to execute the method specified in microseconds
235+
* @param isForwarded Whether the request was forwarded to rippled or not
235236
*/
236237
void
237-
notifyComplete(std::string const& method, std::chrono::microseconds const& duration)
238+
notifyComplete(
239+
web::Context const& context,
240+
std::chrono::microseconds const& duration,
241+
bool isForwarded
242+
)
238243
{
239-
if (validHandler(method))
240-
counters_.get().rpcComplete(method, duration);
244+
if (validHandler(context.method)) {
245+
counters_.get().rpcComplete(context.method, duration);
246+
if (not isForwarded) {
247+
counters_.get().recordLedgerRequest(context.params, context.range.maxSequence);
248+
}
249+
}
250+
}
251+
252+
/**
253+
* @brief Record ledger request metrics.
254+
*
255+
* @param params The request parameters containing ledger information
256+
* @param currentLedgerSequence The current ledger sequence
257+
*/
258+
void
259+
recordLedgerMetrics(boost::json::object const& params, std::uint32_t currentLedgerSequence)
260+
{
261+
counters_.get().recordLedgerRequest(params, currentLedgerSequence);
241262
}
242263

243264
/**

src/web/RPCServerHandler.hpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,14 +229,14 @@ class RPCServerHandler {
229229
LOG(perfLog_.debug()) << context->tag() << "Encountered error: " << responseStr;
230230
LOG(log_.debug()) << context->tag() << "Encountered error: " << responseStr;
231231
} else {
232-
// This can still technically be an error. Clio counts forwarded requests as
233-
// successful.
234-
rpcEngine_->notifyComplete(context->method, us);
235-
236232
auto& json = result.response.value();
237233
auto const isForwarded = json.contains("forwarded") &&
238234
json.at("forwarded").is_bool() && json.at("forwarded").as_bool();
239235

236+
// This can still technically be an error. Clio counts forwarded requests
237+
// as successful.
238+
rpcEngine_->notifyComplete(*context, us, isForwarded);
239+
240240
if (isForwarded)
241241
json.erase("forwarded");
242242

src/web/ng/RPCServerHandler.hpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,14 +287,14 @@ class RPCServerHandler {
287287
LOG(perfLog_.debug()) << context->tag() << "Encountered error: " << responseStr;
288288
LOG(log_.debug()) << context->tag() << "Encountered error: " << responseStr;
289289
} else {
290-
// This can still technically be an error. Clio counts forwarded requests as
291-
// successful.
292-
rpcEngine_->notifyComplete(context->method, us);
293-
294290
auto& json = result.response.value();
295291
auto const isForwarded = json.contains("forwarded") &&
296292
json.at("forwarded").is_bool() && json.at("forwarded").as_bool();
297293

294+
// This can still technically be an error. Clio counts forwarded requests
295+
// as successful.
296+
rpcEngine_->notifyComplete(*context, us, isForwarded);
297+
298298
if (isForwarded)
299299
json.erase("forwarded");
300300

tests/common/util/MockRPCEngine.hpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,11 @@
2525
#include <boost/asio.hpp>
2626
#include <boost/asio/io_context.hpp>
2727
#include <boost/asio/spawn.hpp>
28+
#include <boost/json/object.hpp>
2829
#include <gtest/gtest.h>
2930

3031
#include <chrono>
32+
#include <cstdint>
3133
#include <functional>
3234
#include <string>
3335

@@ -49,6 +51,17 @@ struct MockAsyncRPCEngine {
4951
}
5052

5153
MOCK_METHOD(void, notifyComplete, (std::string const&, std::chrono::microseconds const&), ());
54+
void
55+
notifyComplete(
56+
web::Context const& ctx,
57+
std::chrono::microseconds const& duration,
58+
bool isForwarded
59+
)
60+
{
61+
notifyComplete(ctx.method, duration);
62+
if (not isForwarded)
63+
recordLedgerMetrics(ctx.params, ctx.range.maxSequence);
64+
}
5265
MOCK_METHOD(void, notifyFailed, (std::string const&), ());
5366
MOCK_METHOD(void, notifyErrored, (std::string const&), ());
5467
MOCK_METHOD(void, notifyForwarded, (std::string const&), ());
@@ -58,6 +71,7 @@ struct MockAsyncRPCEngine {
5871
MOCK_METHOD(void, notifyTooBusy, (), ());
5972
MOCK_METHOD(void, notifyUnknownCommand, (), ());
6073
MOCK_METHOD(void, notifyInternalError, (), ());
74+
MOCK_METHOD(void, recordLedgerMetrics, (boost::json::object const&, std::uint32_t), ());
6175
MOCK_METHOD(rpc::Result, buildResponse, (web::Context const&), ());
6276
};
6377

@@ -69,6 +83,17 @@ struct MockRPCEngine {
6983
()
7084
);
7185
MOCK_METHOD(void, notifyComplete, (std::string const&, std::chrono::microseconds const&), ());
86+
void
87+
notifyComplete(
88+
web::Context const& ctx,
89+
std::chrono::microseconds const& duration,
90+
bool isForwarded
91+
)
92+
{
93+
notifyComplete(ctx.method, duration);
94+
if (not isForwarded)
95+
recordLedgerMetrics(ctx.params, ctx.range.maxSequence);
96+
}
7297
MOCK_METHOD(void, notifyErrored, (std::string const&), ());
7398
MOCK_METHOD(void, notifyForwarded, (std::string const&), ());
7499
MOCK_METHOD(void, notifyFailedToForward, (std::string const&), ());
@@ -77,5 +102,6 @@ struct MockRPCEngine {
77102
MOCK_METHOD(void, notifyTooBusy, (), ());
78103
MOCK_METHOD(void, notifyUnknownCommand, (), ());
79104
MOCK_METHOD(void, notifyInternalError, (), ());
105+
MOCK_METHOD(void, recordLedgerMetrics, (boost::json::object const&, std::uint32_t), ());
80106
MOCK_METHOD(rpc::Result, buildResponse, (web::Context const&), ());
81107
};

tests/unit/rpc/CountersTests.cpp

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "rpc/WorkQueue.hpp"
2323
#include "util/MockPrometheus.hpp"
2424
#include "util/prometheus/Counter.hpp"
25+
#include "util/prometheus/Histogram.hpp"
2526

2627
#include <boost/json/object.hpp>
2728
#include <boost/json/value_to.hpp>
@@ -30,6 +31,7 @@
3031
#include <xrpl/protocol/jss.h>
3132

3233
#include <chrono>
34+
#include <cstdint>
3335
#include <string>
3436

3537
using namespace rpc;
@@ -272,3 +274,93 @@ TEST_F(RPCCountersMockPrometheusTests, onInternalError)
272274
EXPECT_CALL(internalErrorMock, add(1));
273275
counters.onInternalError();
274276
}
277+
278+
struct RPCCountersMockPrometheusRecotdLedgerRequestTest : RPCCountersMockPrometheusTests {
279+
testing::StrictMock<util::prometheus::MockHistogramImpl<int64_t>>& ageLedgersHistogramMock =
280+
makeMock<util::prometheus::HistogramInt>("rpc_requested_ledger_age_histogram", "");
281+
};
282+
283+
TEST_F(RPCCountersMockPrometheusRecotdLedgerRequestTest, currentLedger)
284+
{
285+
// "current" is not tracked in the histogram (it's not a historical ledger lookup)
286+
// No mock expectations needed
287+
boost::json::object params;
288+
params["ledger_index"] = "current";
289+
counters.recordLedgerRequest(params, 1000);
290+
}
291+
292+
TEST_F(RPCCountersMockPrometheusRecotdLedgerRequestTest, validateLedger)
293+
{
294+
EXPECT_CALL(ageLedgersHistogramMock, observe(0));
295+
296+
boost::json::object params;
297+
params["ledger_index"] = "validated";
298+
counters.recordLedgerRequest(params, 1000);
299+
}
300+
301+
TEST_F(RPCCountersMockPrometheusRecotdLedgerRequestTest, validatedDefaultLedger)
302+
{
303+
EXPECT_CALL(ageLedgersHistogramMock, observe(0));
304+
305+
boost::json::object params;
306+
counters.recordLedgerRequest(params, 1000);
307+
}
308+
309+
TEST_F(RPCCountersMockPrometheusRecotdLedgerRequestTest, specificLedger)
310+
{
311+
auto& ageLedgersHistogramMock =
312+
makeMock<util::prometheus::HistogramInt>("rpc_requested_ledger_age_histogram", "");
313+
314+
EXPECT_CALL(ageLedgersHistogramMock, observe(100)); // age is 1000 - 900 = 100
315+
316+
boost::json::object params;
317+
params["ledger_index"] = 900;
318+
counters.recordLedgerRequest(params, 1000);
319+
}
320+
321+
TEST_F(RPCCountersMockPrometheusRecotdLedgerRequestTest, stringNumberLedger)
322+
{
323+
EXPECT_CALL(ageLedgersHistogramMock, observe(50)); // 1000 - 950 = 50 ledgers
324+
325+
boost::json::object params;
326+
params["ledger_index"] = "950";
327+
counters.recordLedgerRequest(params, 1000);
328+
}
329+
330+
TEST_F(RPCCountersMockPrometheusRecotdLedgerRequestTest, zeroAgeLedger)
331+
{
332+
auto& ageLedgersHistogramMock =
333+
makeMock<util::prometheus::HistogramInt>("rpc_requested_ledger_age_histogram", "");
334+
335+
EXPECT_CALL(ageLedgersHistogramMock, observe(0)); // 1000 - 1000 = 0 ledgers
336+
337+
boost::json::object params;
338+
params["ledger_index"] = 1000; // Same as current
339+
counters.recordLedgerRequest(params, 1000);
340+
}
341+
342+
TEST_F(RPCCountersMockPrometheusRecotdLedgerRequestTest, ledgerHashRequest)
343+
{
344+
auto& ledgerHashCounterMock = makeMock<CounterInt>("rpc_ledger_hash_requests_total_number", "");
345+
346+
EXPECT_CALL(ledgerHashCounterMock, add(1));
347+
348+
boost::json::object params;
349+
params["ledger_hash"] = "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789";
350+
counters.recordLedgerRequest(params, 1000);
351+
}
352+
353+
TEST_F(RPCCountersMockPrometheusRecotdLedgerRequestTest, ledgerHashWithIndexIgnoresIndex)
354+
{
355+
auto& ledgerHashCounterMock = makeMock<CounterInt>("rpc_ledger_hash_requests_total_number", "");
356+
357+
// When both ledger_hash and ledger_index are present, only ledger_hash counter should be
358+
// incremented
359+
EXPECT_CALL(ledgerHashCounterMock, add(1));
360+
// No histogram call should be made
361+
362+
boost::json::object params;
363+
params["ledger_hash"] = "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789";
364+
params["ledger_index"] = 900; // This should be ignored
365+
counters.recordLedgerRequest(params, 1000);
366+
}

0 commit comments

Comments
 (0)