From 623e3d8ad3c850fb240e347d7b1910e8d8b56ed1 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Mon, 7 Sep 2026 13:51:44 +0100 Subject: [PATCH] feat(telemetry): add batch-size and discovered-path histograms Review feedback asked for a Histogram rather than a span attribute at these two places. Both, not either: the attribute answers how big one sampled request was, which an aggregate cannot, and the histogram answers the distribution across all requests, which an unsampled trace never reveals. Both attributes stay. The metrics land here rather than with the attributes because neither HistogramBuckets.h nor the metric macro exists on the branch that added them. Both use kObjectCountBuckets. The argument is the floor, not the ceiling: the SDK default edges start 0,5,10,25, so an ordinary batch of one to five falls in a single bucket and every quantile becomes an interpolation on one edge. The object ladder puts five edges over the mass of both distributions. Path count is bounded at 352 by kMaxPaths times kMaxAutoSrcCur and cannot saturate. Batch size can, at roughly 333k, but no measured traffic goes near it, so the ladder is not widened for a range nothing occupies; the runbook records the overflow query and a test asserts it stays readable. --- .../scripts/levelization/results/ordering.txt | 2 + .../09-data-collection-reference.md | 65 ++++- docs/telemetry-runbook.md | 39 +++ include/xrpl/telemetry/RpcMetricNames.h | 101 ++++++++ .../libxrpl/telemetry/RpcMetricNames.cpp | 232 ++++++++++++++++++ src/xrpld/rpc/detail/PathRequest.cpp | 19 ++ src/xrpld/rpc/detail/ServerHandler.cpp | 20 ++ src/xrpld/telemetry/MetricsRegistry.cpp | 17 ++ 8 files changed, 490 insertions(+), 5 deletions(-) create mode 100644 include/xrpl/telemetry/RpcMetricNames.h create mode 100644 src/tests/libxrpl/telemetry/RpcMetricNames.cpp diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index d9c4f24ecf..be1115a063 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -193,6 +193,7 @@ tests.libxrpl > xrpl.config tests.libxrpl > xrpl.consensus tests.libxrpl > xrpl.core tests.libxrpl > xrpld.app +tests.libxrpl > xrpld.rpc tests.libxrpl > xrpld.telemetry tests.libxrpl > xrpl.json tests.libxrpl > xrpl.ledger @@ -313,6 +314,7 @@ xrpld.rpc > xrpl.basics xrpld.rpc > xrpl.config xrpld.rpc > xrpl.core xrpld.rpc > xrpld.core +xrpld.rpc > xrpld.telemetry xrpld.rpc > xrpl.json xrpld.rpc > xrpl.ledger xrpld.rpc > xrpl.net diff --git a/OpenTelemetryPlan/09-data-collection-reference.md b/OpenTelemetryPlan/09-data-collection-reference.md index 0e2a69139d..cee514ce78 100644 --- a/OpenTelemetryPlan/09-data-collection-reference.md +++ b/OpenTelemetryPlan/09-data-collection-reference.md @@ -1148,6 +1148,10 @@ repeated here: [Per-Job-Type Metrics](#per-job-type-metrics-synchronous-countershistogram). - Five `getobject_*` instruments covering the `TMGetObjectByHash` request path — see [GetObject Request Path](#getobject-request-path-synchronous-countershistograms). +- Two request-count histograms, `rpc_batch_size` and `pathfind_discovered_paths`, + which give the aggregate distribution of two values that until now existed only + as span attributes on a sampled trace — see + [RPC Request-Count Histograms](#rpc-request-count-histograms). - Three per-job-type queue gauge families (`jobq__waiting` / `_running` / `_deferred`). These travel the `beast::insight` pipeline, not the OTel SDK one, so they are documented in @@ -1518,6 +1522,55 @@ macro (see `src/xrpld/telemetry/MetricMacros.h` and `PerfLogImp.cpp`), not throu `MetricsRegistry` member. As an UpDownCounter it carries no `_total` suffix (that is reserved for monotonic counters). +#### RPC Request-Count Histograms + +Two histograms describing how much work one request asks for. Names and +descriptions are the `constexpr` constants in +`include/xrpl/telemetry/RpcMetricNames.h`; both are recorded at their call sites +via `XRPL_METRIC_*`, and both have an explicit-bucket view registered in +`src/xrpld/telemetry/MetricsRegistry.cpp`. + +| Prometheus Metric | Type | Labels | Description | +| --------------------------- | --------- | ------ | ------------------------------------------------------- | +| `rpc_batch_size` | Histogram | (none) | Sub-requests per batch JSON-RPC call | +| `pathfind_discovered_paths` | Histogram | (none) | Payment paths produced per pathfinding pass, all assets | + +| Metric | Recorded at | Beside the span attribute | +| --------------------------- | ----------------------------------------------- | ------------------------- | +| `rpc_batch_size` | `ServerHandler::processRequest`, `method=batch` | `batch_size` | +| `pathfind_discovered_paths` | `PathRequest::findPaths`, after the asset loop | `pathfind_num_paths` | + +**Why both a span attribute and a histogram for the same value.** The attribute +answers "how big was this one request" on a trace someone is already looking at. +It cannot give a distribution, because an unsampled trace is never read. The +histogram answers "how big are these requests" across every call. Neither +replaces the other, so both stay. + +`rpc_batch_size` is recorded only when `method == "batch"`, matching the +attribute. Recording a plain single request would add the value 1 on every RPC +and bury the batch distribution. + +`pathfind_discovered_paths` records inside the existing +`#ifdef XRPL_ENABLE_TELEMETRY` block, because the running total it reports is +only maintained in a telemetry build. Zero is a normal and interesting value: a +pass that found no path at all records it. + +**Both use `buckets::kObjectCountBuckets`, and the reason is the floor.** The SDK +default boundaries begin `0, 5, 10, 25`, so every batch of one to five +sub-requests — the ordinary case — lands in a single bucket and +`histogram_quantile` returns that edge scaled by the quantile rather than a +count. The object-count ladder's `1, 2, 4, 8, 16` edges sit where both +distributions have their mass. + +- **Path counts cannot saturate.** `PathRequest::kMaxPaths` (4) per source asset + times `tuning::kMaxAutoSrcCur` (88) bounds a pass at 352 paths, well under the + ladder's 12288 top edge. +- **Batch sizes can.** Nothing caps the sub-request count; the only bound is + `tuning::kMaxRequestSize` (1 MB) over the smallest sub-request an array can + hold, about 333,000. The ladder is not extended into a range no measured + workload occupies, so an over-ceiling batch lands in `+Inf` and is read as + `rpc_batch_size_count - rpc_batch_size_bucket{le="12288"}` instead. + #### Per-Job-Type Metrics (Synchronous Counters/Histogram) | Prometheus Metric | Type | Labels | Description | @@ -1595,8 +1648,8 @@ information the batch totals do not already carry. **All three histograms need an explicit bucket view.** The SDK's default histogram boundaries top out at 10000. Every one of these three exceeds that, so -without a view their top quantiles would all read as a flat 10000. Six views are -registered in `src/xrpld/telemetry/MetricsRegistry.cpp`, and three of the six are +without a view their top quantiles would all read as a flat 10000. Eight views are +registered in `src/xrpld/telemetry/MetricsRegistry.cpp`, and three of the eight are for this family: | Instrument | View helper | Boundaries | @@ -1605,9 +1658,11 @@ for this family: | `getobject_request_objects` | `addHistogramView()`, own set | `1, 2, 4, 8, 16, 64, 256, 1024, 4096, 12288` | | `getobject_charge` | `addHistogramView()`, own set | `0, 100, 500, 1000, 5000, 10000, 25000, 50000, 100000` | -The other three views are `addMicrosecondHistogramView()` on `job_queued_us`, -`job_running_us`, and `rpc_method_us` — four µs-ladder views plus these two -custom sets. +The other five views are `addMicrosecondHistogramView()` on `job_queued_us`, +`job_running_us` and `rpc_method_us`, plus `rpc_batch_size` and +`pathfind_discovered_paths` on the object-count ladder — four µs-ladder views and +four custom-boundary ones. See +[RPC Request-Count Histograms](#rpc-request-count-histograms) for the latter two. **Why the latter two do not use the µs ladder.** They are not durations. The µs ladder's buckets are chosen for time (sub-millisecond jobs through multi-second diff --git a/docs/telemetry-runbook.md b/docs/telemetry-runbook.md index 4705c26ced..f58c7b777c 100644 --- a/docs/telemetry-runbook.md +++ b/docs/telemetry-runbook.md @@ -1930,6 +1930,45 @@ Aggregation choices worth knowing when reading these: > peer sends an oversized request. Verify its panel with a synthetic oversized > request; do not assume it works because the query parses. +#### RPC Request-Count Metrics + +Two histograms describing how much work one RPC asks for. Names and descriptions +are the `constexpr` constants in `include/xrpl/telemetry/RpcMetricNames.h`; both +are recorded at their call sites and both get an explicit-bucket view in +`MetricsRegistry.cpp`. + +| Prometheus Metric | Kind | Labels | Recorded at | Description | +| --------------------------- | --------- | ------ | ----------------------------------------------- | --------------------------------------------------------- | +| `rpc_batch_size` | Histogram | none | `ServerHandler::processRequest`, `method=batch` | Sub-requests per batch JSON-RPC call | +| `pathfind_discovered_paths` | Histogram | none | `PathRequest::findPaths`, after the asset loop | Payment paths produced per pass, across all source assets | + +Reading them: + +- Each one sits beside a span attribute carrying the same value — + `batch_size` and `pathfind_num_paths`. Use the attribute to ask what one slow + request did; use the histogram to ask what requests do in general. An attribute + cannot answer the second question, because an unsampled trace is never read. +- `rpc_batch_size` records **only for `method == "batch"`**. A non-batch request + is not a batch of one, and recording it would put the value 1 on every RPC and + bury the distribution. +- `pathfind_discovered_paths` counts zero when a pass found no path, and that is + the most interesting reading on this instrument. It shares the lowest bucket + with "found exactly one path"; everything above is separated. +- Both use the object-count bucket ladder, not the µs one. They are counts, and + the SDK's default boundaries start `0, 5, 10, 25` — which puts every ordinary + batch in one bucket and makes each quantile an interpolation on the edge. +- **`rpc_batch_size` can saturate.** Nothing caps the sub-request count except + the 1 MB request-size limit, so a batch above the ladder's 12288 top edge lands + in `+Inf`. Read the overflow directly rather than trusting p99 there: + +```promql +rpc_batch_size_count - rpc_batch_size_bucket{le="12288"} +``` + +> On a healthy local network `rpc_batch_size` has **no series at all** — nothing +> issues batch RPCs. An empty panel is the expected reading, not a wiring fault. +> Verify it by sending one batch request, not by looking for a series. + #### Adding a New Metric diff --git a/include/xrpl/telemetry/RpcMetricNames.h b/include/xrpl/telemetry/RpcMetricNames.h new file mode 100644 index 0000000000..1869fd44d0 --- /dev/null +++ b/include/xrpl/telemetry/RpcMetricNames.h @@ -0,0 +1,101 @@ +#pragma once + +// cspell:ignore ISTOGRAM +// The all-caps macro name XRPL_METRIC_HISTOGRAM_RECORD trips cspell's +// compound-word splitter, which emits the subword "ISTOGRAM"; ignore it here. + +/** + * Metric names and descriptions for the RPC request-count histograms. + * + * These two instruments measure how much work one request asks for. A span + * attribute cannot answer that in aggregate: it is readable only on a trace + * that was sampled, so it describes one call and never the distribution over + * all of them. The span attributes stay in place for single-request + * debugging; these histograms carry the shape. + * + * Each name is used at two sites, which is why they are shared constants + * rather than literals: + * + * RpcMetricNames.h + * | + * +--> ServerHandler.cpp (records kRpcBatchSize) + * | + * +--> PathRequest.cpp (records kPathfindDiscoveredPaths) + * | + * +--> MetricsRegistry.cpp (addHistogramView: registers the + * explicit bucket edges for both) + * + * A drifted name silently drops the bucket override. The SDK default edges + * start at 0, 5, 10, 25, so every batch of one to five sub-requests would + * then land in one bucket and every quantile would be an interpolation + * inside it. + * + * Placed under `include/xrpl/telemetry/` for the same reason + * GetObjectMetricNames.h is: the record sites are `xrpld.rpc` and the view + * registration is `xrpld.telemetry`, and `include/xrpl/` is the one level + * both modules are allowed to reach. + * + * Example usage -- recording the batch size: + * @code + * if (batch) + * { + * span.setAttribute(rpc_span::attr::batchSize, static_cast(size)); + * XRPL_METRIC_HISTOGRAM_RECORD(app_, kRpcBatchSize, kRpcBatchSizeDesc, size); + * } + * @endcode + * + * Example usage -- edge case: a pathfinding pass that produced no paths still + * records, because "found nothing" is the observation that matters most on + * this instrument: + * @code + * std::int64_t totalPaths = 0; // no source asset yielded a path + * XRPL_METRIC_HISTOGRAM_RECORD( + * app_, kPathfindDiscoveredPaths, kPathfindDiscoveredPathsDesc, totalPaths); + * @endcode + * + * @note These are `constexpr char[]`, not `std::string_view`. The OTel C++ + * API takes `nostd::string_view`, which has no converting constructor from + * `std::string_view` on this build, so a `string_view` constant would not + * compile at the call sites. Same convention as GetObjectMetricNames.h. + * + * @note Both share `buckets::kObjectCountBuckets`, whose top edge is 12288. + * A pathfinding pass cannot reach it. A batch can: nothing caps the + * sub-request count except the one-megabyte request-size limit. An + * over-ceiling batch lands in `+Inf` and stays countable as + * `rpc_batch_size_count - rpc_batch_size_bucket{le="12288"}`, so the overflow + * is visible rather than lost. No measured workload occupies that range, so + * the ladder is not extended into it. + * + * @note Header-only constants with no runtime state, so there is nothing to + * synchronize -- safe to include from any thread context. + */ + +namespace xrpl::telemetry { + +// ===== Metric names ========================================================== + +/** + * Distribution of the sub-request count in a batch JSON-RPC call. + * + * Recorded only for `method == "batch"`. A plain single request would + * otherwise flood the histogram with the value 1 and bury the batch + * distribution this instrument exists to show. + */ +inline constexpr char kRpcBatchSize[] = "rpc_batch_size"; + +/** + * Distribution of the payment paths one pathfinding pass produced, summed + * across every candidate source asset. + */ +inline constexpr char kPathfindDiscoveredPaths[] = "pathfind_discovered_paths"; + +// ===== Instrument descriptions =============================================== + +/** @{ */ +inline constexpr char kRpcBatchSizeDesc[] = "Sub-requests per batch JSON-RPC call"; + +inline constexpr char kPathfindDiscoveredPathsDesc[] = + "Payment paths produced per pathfinding pass, across all source assets"; +/** @} */ + +} // namespace xrpl::telemetry diff --git a/src/tests/libxrpl/telemetry/RpcMetricNames.cpp b/src/tests/libxrpl/telemetry/RpcMetricNames.cpp new file mode 100644 index 0000000000..ea27f80f28 --- /dev/null +++ b/src/tests/libxrpl/telemetry/RpcMetricNames.cpp @@ -0,0 +1,232 @@ +/** + * GTest unit tests for the RPC request-count metric names and their bucket fit. + * + * Two facts here have no other guard in CI. First, a metric NAME is the + * Prometheus series every panel and alert selects on, and + * `check_otel_naming.py` checks span attribute keys, not metric names -- so a + * rename or a stray unit suffix would pass every gate and blank the panels. + * Second, the reason these two histograms need an explicit-bucket view is the + * ladder FLOOR, which is invisible in a dashboard: a quantile that falls inside + * bucket 0 is interpolated from the bucket edge and reads back as a plausible + * number. The bucket-index tests below pin that with the SDK's own placement + * rule rather than leaving it to review. + */ + +#include + +#include + +#include + +#include + +#include +#include +#include +#include +#include + +namespace xrpl::telemetry { + +namespace { + +/** + * Placement rule the OTel SDK uses: the bucket index of a sample is the count + * of edges strictly below it, found with `std::lower_bound` over the ascending + * edge list. Recomputed here rather than asserted from memory, so a ladder edit + * moves the expected indices with it. + * + * @param ladder Bucket upper bounds, ascending. + * @param sample Value to place. + * @return Zero-based bucket index; `ladder.size()` means the `+Inf` bucket. + */ +[[nodiscard]] std::size_t +bucketIndex(std::span ladder, double sample) +{ + return static_cast(std::ranges::lower_bound(ladder, sample) - ladder.begin()); +} + +/** + * Last underscore-separated segment of a metric name. + * + * @param name Metric name. + * @return The text after the final underscore, or the whole name when there is + * no underscore. + */ +[[nodiscard]] std::string_view +lastSegment(std::string_view name) +{ + auto const pos = name.rfind('_'); + return pos == std::string_view::npos ? name : name.substr(pos + 1); +} + +/** + * The opentelemetry-cpp default explicit-bucket boundaries, quoted from the + * SDK because they are the ladder these two instruments would fall back to if + * their view were dropped. Not a repo constant, so there is no symbol to read + * them from. + */ +inline constexpr std::array kSdkDefaultBuckets{ + 0.0, + 5.0, + 10.0, + 25.0, + 50.0, + 75.0, + 100.0, + 250.0, + 500.0, + 750.0, + 1'000.0, + 2'500.0, + 5'000.0, + 7'500.0, + 10'000.0}; + +/** + * Smallest number of bytes one batch sub-request can occupy inside the + * `params` array: the two braces of an empty object plus its separating comma. + * An empty object still passes `isObject()`, so the handler iterates over it. + */ +inline constexpr int kMinBatchItemBytes = 3; + +} // namespace + +TEST(RpcMetricNames, namesAreTheExactExportedSeriesNames) +{ + // These strings ARE the Prometheus series. Changing one is a + // dashboard-breaking change, so it has to be a deliberate edit here too. + EXPECT_EQ(std::string_view{kRpcBatchSize}, "rpc_batch_size"); + EXPECT_EQ(std::string_view{kPathfindDiscoveredPaths}, "pathfind_discovered_paths"); +} + +TEST(RpcMetricNames, descriptionsAreTheExactExportedHelpText) +{ + // The description becomes the Prometheus `# HELP` line, so it is part of + // the exported surface, not a code comment. + EXPECT_EQ(std::string_view{kRpcBatchSizeDesc}, "Sub-requests per batch JSON-RPC call"); + EXPECT_EQ( + std::string_view{kPathfindDiscoveredPathsDesc}, + "Payment paths produced per pathfinding pass, across all source assets"); +} + +TEST(RpcMetricNames, namesAreLowerSnakeCase) +{ + for (std::string_view const name : + {std::string_view{kRpcBatchSize}, std::string_view{kPathfindDiscoveredPaths}}) + { + ASSERT_FALSE(name.empty()); + EXPECT_TRUE(name.front() >= 'a' && name.front() <= 'z') + << name << " must start with a lowercase letter"; + EXPECT_NE(name.back(), '_') << name << " must not end with an underscore"; + for (char const c : name) + { + EXPECT_TRUE((c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '_') + << name << " contains '" << c << "', which is not lower_snake_case"; + } + EXPECT_EQ(name.find("__"), std::string_view::npos) + << name << " must not contain a double underscore"; + } +} + +TEST(RpcMetricNames, namesEndInTheCountedNounAndNotAUnitOrCounterSuffix) +{ + // Both instruments count things. `_total` is the Prometheus counter suffix + // and `_count`/`_sum`/`_bucket` are the ones the exporter appends to a + // histogram, so a base name ending in any of them collides with a series + // the exporter generates. `_us`/`_ms`/`_bytes` would claim a unit these + // values do not have. + EXPECT_EQ(lastSegment(kRpcBatchSize), "size"); + EXPECT_EQ(lastSegment(kPathfindDiscoveredPaths), "paths"); + + for (std::string_view const name : + {std::string_view{kRpcBatchSize}, std::string_view{kPathfindDiscoveredPaths}}) + { + for (std::string_view const reserved : + {"total", "count", "sum", "bucket", "us", "ms", "s", "seconds", "bytes"}) + { + EXPECT_NE(lastSegment(name), reserved) + << name << " ends in the reserved suffix _" << reserved; + } + } +} + +TEST(RpcMetricBucketFit, objectCountLadderSeparatesTheSmallestCounts) +{ + using buckets::kObjectCountBuckets; + + // The five edges that carry both distributions. Stated exactly: raising the + // floor is the defect this test exists to catch. + ASSERT_GE(kObjectCountBuckets.size(), 5u); + EXPECT_EQ(kObjectCountBuckets[0], 1.0); + EXPECT_EQ(kObjectCountBuckets[1], 2.0); + EXPECT_EQ(kObjectCountBuckets[2], 4.0); + EXPECT_EQ(kObjectCountBuckets[3], 8.0); + EXPECT_EQ(kObjectCountBuckets[4], 16.0); + + std::span const ladder{kObjectCountBuckets}; + + // A pathfinding pass that found nothing shares bucket 0 with a pass that + // found exactly one path; every larger count is separated. + EXPECT_EQ(bucketIndex(ladder, 0.0), 0u); + EXPECT_EQ(bucketIndex(ladder, 1.0), 0u); + EXPECT_EQ(bucketIndex(ladder, 2.0), 1u); + EXPECT_EQ(bucketIndex(ladder, 3.0), 2u); + EXPECT_EQ(bucketIndex(ladder, 4.0), 2u); + EXPECT_EQ(bucketIndex(ladder, 8.0), 3u); + EXPECT_EQ(bucketIndex(ladder, 16.0), 4u); +} + +TEST(RpcMetricBucketFit, theSdkDefaultLadderWouldCollapseEverySmallBatch) +{ + // This is why both instruments get an explicit-bucket view. On the SDK + // default ladder every batch from 1 to 5 sub-requests lands in one bucket, + // so histogram_quantile interpolates inside it and returns the edge scaled + // by the quantile -- a number that looks like a batch size and is not one. + std::span const sdk{kSdkDefaultBuckets}; + EXPECT_EQ(bucketIndex(sdk, 1.0), 1u); + EXPECT_EQ(bucketIndex(sdk, 2.0), 1u); + EXPECT_EQ(bucketIndex(sdk, 4.0), 1u); + EXPECT_EQ(bucketIndex(sdk, 5.0), 1u); + + // The chosen ladder spreads those same four values over three buckets. + std::span const chosen{buckets::kObjectCountBuckets}; + EXPECT_EQ(bucketIndex(chosen, 1.0), 0u); + EXPECT_EQ(bucketIndex(chosen, 2.0), 1u); + EXPECT_EQ(bucketIndex(chosen, 4.0), 2u); + EXPECT_EQ(bucketIndex(chosen, 5.0), 3u); +} + +TEST(RpcMetricBucketFit, aMaximumSizedBatchStillOverflowsTheLadderCeiling) +{ + // The documented limitation of rpc_batch_size, as an executable fact. + // Nothing caps the sub-request count except the request-size limit, so the + // largest possible batch is far above the ladder's top edge and lands in + // `+Inf`. RpcMetricNames.h documents the query that counts the overflow. + // If the ladder is ever raised past this bound, this test goes red and that + // note has to change with it. + constexpr int kLargestPossibleBatch = rpc::tuning::kMaxRequestSize / kMinBatchItemBytes; + EXPECT_EQ(kLargestPossibleBatch, 333'333); + EXPECT_EQ(buckets::kObjectCountBuckets.back(), 12'288.0); + EXPECT_GT(static_cast(kLargestPossibleBatch), buckets::kObjectCountBuckets.back()); + + // A pass-through check on the placement helper: the overflow really does + // land in the +Inf bucket, whose index is one past the last edge. + std::span const ladder{buckets::kObjectCountBuckets}; + EXPECT_EQ(bucketIndex(ladder, static_cast(kLargestPossibleBatch)), ladder.size()); +} + +// The placement helper must also disagree with the ladder when it should. A +// helper that always returned 0 would let every index assertion above pass. +TEST(RpcMetricBucketFit, bucketIndexPlacesAboveAndBelowEveryEdge) +{ + constexpr std::array probe{10.0, 20.0}; + std::span const ladder{probe}; + EXPECT_EQ(bucketIndex(ladder, 9.9), 0u); + EXPECT_EQ(bucketIndex(ladder, 10.0), 0u); + EXPECT_EQ(bucketIndex(ladder, 10.1), 1u); + EXPECT_EQ(bucketIndex(ladder, 20.0), 1u); + EXPECT_EQ(bucketIndex(ladder, 20.1), 2u); +} + +} // namespace xrpl::telemetry diff --git a/src/xrpld/rpc/detail/PathRequest.cpp b/src/xrpld/rpc/detail/PathRequest.cpp index 1154109fc9..b61bf1edee 100644 --- a/src/xrpld/rpc/detail/PathRequest.cpp +++ b/src/xrpld/rpc/detail/PathRequest.cpp @@ -1,3 +1,7 @@ +// cspell:ignore ISTOGRAM +// The all-caps macro name XRPL_METRIC_HISTOGRAM_RECORD trips cspell's +// compound-word splitter, which emits the subword "ISTOGRAM"; ignore it here. + #include #include @@ -8,6 +12,7 @@ #include #include #include +#include #include #include @@ -50,6 +55,14 @@ #include #include +// The path-count metric name and description. Both are only ever used inside an +// XRPL_METRIC_* argument list, and those macros expand to nothing when +// telemetry is off, so the include is guarded like its uses -- clang-tidy's +// misc-include-cleaner rejects an include nothing references. +#ifdef XRPL_ENABLE_TELEMETRY +#include +#endif // XRPL_ENABLE_TELEMETRY + namespace xrpl { PathRequest::PathRequest( @@ -745,6 +758,12 @@ PathRequest::findPaths( #ifdef XRPL_ENABLE_TELEMETRY span.setAttribute(pathfind_span::attr::numPaths, totalPaths); + // The attribute answers "how many paths did THIS pass find" on one sampled + // trace. The histogram answers "how many paths do passes find" across all + // of them, which no attribute can, since an unsampled trace is never read. + // Inside the guard because totalPaths only exists when telemetry is built. + XRPL_METRIC_HISTOGRAM_RECORD( + app_, kPathfindDiscoveredPaths, kPathfindDiscoveredPathsDesc, totalPaths); #endif /* The resource fee is based on the number of source currencies used. diff --git a/src/xrpld/rpc/detail/ServerHandler.cpp b/src/xrpld/rpc/detail/ServerHandler.cpp index 3c797d3fcb..38dddb1850 100644 --- a/src/xrpld/rpc/detail/ServerHandler.cpp +++ b/src/xrpld/rpc/detail/ServerHandler.cpp @@ -1,3 +1,7 @@ +// cspell:ignore ISTOGRAM +// The all-caps macro name XRPL_METRIC_HISTOGRAM_RECORD trips cspell's +// compound-word splitter, which emits the subword "ISTOGRAM"; ignore it here. + #include #include @@ -10,6 +14,7 @@ #include #include #include // IWYU pragma: keep +#include #include #include @@ -77,6 +82,14 @@ #include #include +// The batch-size metric name and description. Both are only ever used inside an +// XRPL_METRIC_* argument list, and those macros expand to nothing when +// telemetry is off, so the include is guarded like its uses -- clang-tidy's +// misc-include-cleaner rejects an include nothing references. +#ifdef XRPL_ENABLE_TELEMETRY +#include +#endif // XRPL_ENABLE_TELEMETRY + namespace xrpl { using namespace telemetry; @@ -760,7 +773,14 @@ ServerHandler::processRequest( } span.setAttribute(rpc_span::attr::isBatch, batch); if (batch) + { span.setAttribute(rpc_span::attr::batchSize, static_cast(size)); + // The attribute answers "how big was THIS batch" on one sampled trace. + // The histogram answers "how big are batches" across all of them, which + // no attribute can, since an unsampled trace is never read. + XRPL_METRIC_HISTOGRAM_RECORD( + app_, telemetry::kRpcBatchSize, telemetry::kRpcBatchSizeDesc, size); + } json::Value reply(batch ? json::ValueType::Array : json::ValueType::Object); diff --git a/src/xrpld/telemetry/MetricsRegistry.cpp b/src/xrpld/telemetry/MetricsRegistry.cpp index a3e706a73c..f335e26925 100644 --- a/src/xrpld/telemetry/MetricsRegistry.cpp +++ b/src/xrpld/telemetry/MetricsRegistry.cpp @@ -68,6 +68,7 @@ #include #include #include +#include #include // For networkTypeFromId(), the one xrpl.network.type mapping both export // paths use. Adds no levelization edge: xrpld.telemetry > xrpl.telemetry @@ -335,6 +336,22 @@ MetricsRegistry::initExporterAndProvider(StartOptions const& options) // so a dashboard can show how close charges run to each. addHistogramView(*views, kGetObjectCharge, buckets::toVector(buckets::kChargeBuckets)); + // The two RPC request-count histograms are recorded at their ServerHandler + // and PathRequest call sites, so the names come from the shared constants + // all three sites use. Both are small counts, and the reason they need a + // view is the FLOOR rather than the ceiling: the SDK default edges start + // 0, 5, 10, 25, so a batch of one to five sub-requests -- the normal case -- + // would land in a single bucket and every quantile over it would be an + // interpolation inside that bucket rather than a measurement. + // + // The object-count ladder is the fit: its 1, 2, 4, 8, 16 edges sit exactly + // where both distributions have their mass. Path counts are hard-bounded at + // kMaxPaths * kMaxAutoSrcCur = 352, well under its 12288 top. Batch sizes + // have no such cap; see the ceiling note in RpcMetricNames.h. + addHistogramView(*views, kRpcBatchSize, buckets::toVector(buckets::kObjectCountBuckets)); + addHistogramView( + *views, kPathfindDiscoveredPaths, buckets::toVector(buckets::kObjectCountBuckets)); + // Create MeterProvider with resource, then attach the metric reader. provider_ = metric_sdk::MeterProviderFactory::Create(std::move(views), resourceAttrs); provider_->AddMetricReader(std::move(reader));