From b52b3c7b8e635b4b91a77cfce06f11386280445f Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Mon, 14 Sep 2026 20:33:42 +0100 Subject: [PATCH] feat(telemetry): name the rotation-stall metrics and give phase durations a seconds ladder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the metric and label constants the rotation-stall proof chain uses: - `rotation_phase_duration_seconds` — per-phase wall-clock, labelled by `stage`, with 8 values in `lval::rotation_phase` matching the child span suffixes. - `jobq_stall_total` — a counter for jobs whose run time reached LoadMonitor's 1 s warn threshold. - `lval::cache_metrics::treenode_lock_hold_peak_us` / `fullbelow_lock_hold_peak_us` as `metric` values on the existing `cache_metrics` gauge, for the TaggedCache lock-hold peaks the next commits expose. `kRotationPhaseSecondsBuckets` in HistogramBuckets.h covers 1 s to 1 h, wired through `addRotationPhaseHistogramView` at instrument-registration time so `histogram_quantile` never has to interpolate inside the +Inf bucket. Ladder test and a labelled histogram macro test pin the values. --- include/xrpl/telemetry/HistogramBuckets.h | 21 ++++++++ .../libxrpl/telemetry/HistogramBuckets.cpp | 13 ++++- src/tests/libxrpl/telemetry/MetricMacros.cpp | 50 +++++++++++++++++++ src/xrpld/telemetry/MetricNames.h | 34 +++++++++++++ src/xrpld/telemetry/MetricsRegistry.cpp | 13 +++++ 5 files changed, 130 insertions(+), 1 deletion(-) diff --git a/include/xrpl/telemetry/HistogramBuckets.h b/include/xrpl/telemetry/HistogramBuckets.h index 73dac9c2c7..c0ed2540a2 100644 --- a/include/xrpl/telemetry/HistogramBuckets.h +++ b/include/xrpl/telemetry/HistogramBuckets.h @@ -105,6 +105,27 @@ inline constexpr std::array kMillisecondBuckets{ 60'000.0, 120'000.0}; +/** + * Bucket edges, in seconds, for `rotation_phase_duration_seconds`. + * + * Measured 2026-09-13 on aws-dev-xrpl-1: freshen.keys 5-6 s, freshen.fetch + * ~3 min, copy ~10 min, whole rotation 13-17 min. The 1 s floor sits under + * the lock hold; 3600 s leaves headroom above a slow rotation. + */ +inline constexpr std::array kRotationPhaseSecondsBuckets{ + 1.0, + 5.0, + 10.0, + 30.0, + 60.0, + 120.0, + 300.0, + 600.0, + 900.0, + 1'200.0, + 1'800.0, + 3'600.0}; + /** * Bucket edges, in bytes, for `beast::insight` Events whose samples are * sizes rather than durations. Currently only the RPC response size. diff --git a/src/tests/libxrpl/telemetry/HistogramBuckets.cpp b/src/tests/libxrpl/telemetry/HistogramBuckets.cpp index f54a7ecde1..4ec1fcc9c8 100644 --- a/src/tests/libxrpl/telemetry/HistogramBuckets.cpp +++ b/src/tests/libxrpl/telemetry/HistogramBuckets.cpp @@ -63,7 +63,18 @@ INSTANTIATE_TEST_SUITE_P( std::span{kByteBuckets}, std::span{kMicrosecondBuckets}, std::span{kObjectCountBuckets}, - std::span{kChargeBuckets})); + std::span{kChargeBuckets}, + std::span{kRotationPhaseSecondsBuckets})); + +TEST(HistogramBucketsRange, rotationPhaseLadderSpansSecondsToAnHour) +{ + // Measured 2026-09-13 on aws-dev-xrpl-1: freshen.keys 5-6 s, freshen.fetch + // ~3 min, copy ~10 min, whole rotation 13-17 min. The floor must sit under + // the lock hold; the ceiling above a whole rotation with headroom. + EXPECT_EQ(kRotationPhaseSecondsBuckets.front(), 1.0); + EXPECT_LE(kRotationPhaseSecondsBuckets.front(), 5.0); + EXPECT_EQ(kRotationPhaseSecondsBuckets.back(), 3600.0); +} TEST(HistogramBucketsRange, microsecondFloorLandsBelowTheMeasuredMass) { diff --git a/src/tests/libxrpl/telemetry/MetricMacros.cpp b/src/tests/libxrpl/telemetry/MetricMacros.cpp index 35422d73ba..90802bf3f0 100644 --- a/src/tests/libxrpl/telemetry/MetricMacros.cpp +++ b/src/tests/libxrpl/telemetry/MetricMacros.cpp @@ -462,6 +462,20 @@ histogramCountAndSum(CollectedMetrics const& data, std::string const& metric) return {hist.count_, opentelemetry::nostd::get(hist.sum_)}; } +/** + * Labelled overload: reads count and sum for the series carrying `labels`. + */ +[[nodiscard]] std::pair +histogramCountAndSum( + CollectedMetrics const& data, + std::string const& metric, + otel_sdk::PointAttributes const& labels) +{ + auto const& point = data.at(metric).at(labels); + auto const& hist = opentelemetry::nostd::get(point); + return {hist.count_, opentelemetry::nostd::get(hist.sum_)}; +} + /** * Fetches a real (non-noop) meter from whatever provider is globally * installed -- inside a test this is the ScopedBareProvider's SDK provider. @@ -3316,4 +3330,40 @@ TEST(MetricMacros, sweep_and_rotation_metrics_emit_nothing_when_registry_disable EXPECT_EQ(app.registry().meterCalls(), 0); } +TEST(MetricMacros, rotation_phase_duration_seconds_keys_series_on_stage) +{ + CollectingProvider const provider; + FakeApp app; + wire(app, /*enabled=*/true, provider.meter()); + + // Mirrors SHAMapStoreImp::RotationPhase::~RotationPhase: one Record per + // phase end, labelled by the stage name. Values in seconds. + auto const record = [&app](char const* stage, double seconds) { + XRPL_METRIC_HISTOGRAM_RECORD_LABELED( + app, + telemetry::metric::rotationPhaseDurationSeconds, + "Wall-clock seconds spent in one online-delete rotation phase", + seconds, + {{telemetry::label::stage, std::string(stage)}}); + }; + record(telemetry::lval::rotation_phase::copy, 611.0); + record(telemetry::lval::rotation_phase::freshenKeys, 5.3); + record(telemetry::lval::rotation_phase::freshenKeys, 6.0); + + auto const data = provider.collect(); + ASSERT_EQ(data.at("rotation_phase_duration_seconds").size(), 2u); + { + auto const [count, sum] = + histogramCountAndSum(data, "rotation_phase_duration_seconds", attrs("stage", "copy")); + EXPECT_EQ(count, 1u); + EXPECT_DOUBLE_EQ(sum, 611.0); + } + { + auto const [count, sum] = histogramCountAndSum( + data, "rotation_phase_duration_seconds", attrs("stage", "freshen.keys")); + EXPECT_EQ(count, 2u); + EXPECT_DOUBLE_EQ(sum, 11.3); + } +} + #endif // XRPL_ENABLE_TELEMETRY diff --git a/src/xrpld/telemetry/MetricNames.h b/src/xrpld/telemetry/MetricNames.h index 0eab560ff1..392b52ca41 100644 --- a/src/xrpld/telemetry/MetricNames.h +++ b/src/xrpld/telemetry/MetricNames.h @@ -207,6 +207,11 @@ inline constexpr char syncAddnodeTotal[] = "sync_addnode_total"; * Worker-pool saturation: tasks in flight, threads, and jobs queued. */ inline constexpr char jobqSaturation[] = "jobq_saturation"; +/** + * Jobs whose run time reached LoadMonitor's 1 s warn threshold, by job type. + * An exact counter for a process-wide freeze; the spans say what caused it. + */ +inline constexpr char jobqStallTotal[] = "jobq_stall_total"; // ===== Quorum and publish: can this node accept and publish a ledger? ======= @@ -320,6 +325,12 @@ inline constexpr char rotationCopyNodeRestoreTotal[] = "rotation_copy_node_resto */ inline constexpr char rotationState[] = "rotation_state"; +/** + * Wall-clock seconds spent in one online-delete rotation phase. Labelled by + * `stage`; see lval::rotation_phase. Recorded once per phase end. + */ +inline constexpr char rotationPhaseDurationSeconds[] = "rotation_phase_duration_seconds"; + // ===== Pre-existing instruments pulled in by the family ratchet ============== // // These predate the sync-diagnostics work. They are declared here because the @@ -658,6 +669,29 @@ inline constexpr char inFlight[] = "in_flight"; inline constexpr char copyForward[] = "copy_forward"; } // namespace rotation_state +/** + * `stage` values for rotation_phase_duration_seconds. Identical to the + * child span suffixes in SHAMapStoreSpanNames.h so a panel can join the two. + */ +namespace rotation_phase { +inline constexpr char clearPrior[] = "clear_prior"; +inline constexpr char copy[] = "copy"; +inline constexpr char freshenKeys[] = "freshen.keys"; +inline constexpr char freshenFetch[] = "freshen.fetch"; +inline constexpr char newBackend[] = "new_backend"; +inline constexpr char clearCaches[] = "clear_caches"; +inline constexpr char swap[] = "swap"; +inline constexpr char healthWait[] = "health_wait"; +} // namespace rotation_phase + +/** + * `metric` values added to the cache_metrics gauge for lock-hold peaks. + */ +namespace cache_metrics { +inline constexpr char treenodeLockHoldPeakUs[] = "treenode_lock_hold_peak_us"; +inline constexpr char fullbelowLockHoldPeakUs[] = "fullbelow_lock_hold_peak_us"; +} // namespace cache_metrics + /** * `ledger_quorum_publish` sub-metrics: the gate, and how late publish is. */ diff --git a/src/xrpld/telemetry/MetricsRegistry.cpp b/src/xrpld/telemetry/MetricsRegistry.cpp index 226fd6643d..ddac82b235 100644 --- a/src/xrpld/telemetry/MetricsRegistry.cpp +++ b/src/xrpld/telemetry/MetricsRegistry.cpp @@ -233,6 +233,15 @@ addRoundDurationHistogramView(metric_sdk::ViewRegistry& views, std::string const 120'000.0}); } +void +addRotationPhaseHistogramView(metric_sdk::ViewRegistry& views, std::string const& name) +{ + addHistogramView( + views, + name, + xrpl::telemetry::buckets::toVector(xrpl::telemetry::buckets::kRotationPhaseSecondsBuckets)); +} + } // namespace #endif // XRPL_ENABLE_TELEMETRY @@ -375,6 +384,10 @@ MetricsRegistry::initExporterAndProvider(StartOptions const& options) // view is declared here (see the constant's comment). addRoundDurationHistogramView(*views, kConsensusRoundDurationMs); + // Recorded at its SHAMapStoreImp RotationPhase destructor, only the view + // lives here. Seconds ladder from HistogramBuckets.h. + addRotationPhaseHistogramView(*views, metric::rotationPhaseDurationSeconds); + // Recorded at its PeerImp.cpp call site, not created here, so the name // comes from the shared constant both sites use. addMicrosecondHistogramView(*views, kGetObjectLookupUs);