feat(insight): let an Event declare what it measures

beast::insight::Event documents itself as carrying "a millisecond time, or
other integral value", but both backends assumed the first case: the OTel
bridge declared every instrument with unit `ms` and StatsD tagged every
sample `|ms`. One Event does not measure time -- ServerHandler's "size"
records the serialized RPC response length -- so it exported as
rpc_size_milliseconds and inherited the millisecond bucket ladder. A quarter
of its samples landed above that ladder's top edge, and since Prometheus
returns the second-highest edge for a quantile in the `+Inf` bucket, its p95
panel showed a flat 5.00 kB rather than a measurement.

Adds beast::insight::Unit (Millis, Bytes) plus otelUnitCode(), carried on
EventImpl and selectable at makeEvent(). Naming the unit at creation is what
lets a backend pick the export unit and, through it, the bucket ladder.

- Collector gains a virtual makeEvent(name, Unit) whose default delegates to
  the millisecond overload, so a collector that cannot act on a unit keeps
  working unchanged. NullCollector and the Groups wrapper override it.
- The Groups override matters most: call sites reach a collector through a
  Group, so forwarding only the prefixed name would silently drop the unit.
  A test covers that hop specifically.
- Event gains notify(std::uint64_t) for non-duration samples, replacing
  ServerHandler's `Event::value_type{response.size()}` -- wrapping a byte
  count in a std::chrono::milliseconds compiles but reads as a duration to
  everything downstream.
- EventImpl::value_type stays std::chrono::milliseconds. Widening it would
  change the wire value of every existing StatsD timer, and metrics needing
  finer resolution use the OTel-native microsecond instruments.

The StatsD collector deliberately keeps emitting `|ms`: that path is retired
here (its UDP port is commented out of the compose file and the integration
test fails if anything listens on 8125), so changing its wire format would
alter a legacy contract with no consumer and no way to verify it.

The exported name does not change yet -- OTelEventImpl still hardcodes its
unit. That follows with the unit-keyed histogram views.

include/xrpl/beast/insight/Collector.h

@@ -6,6 +6,7 @@
 #include <xrpl/beast/insight/Hook.h>
 #include <xrpl/beast/insight/HookImpl.h>
 #include <xrpl/beast/insight/Meter.h>
+#include <xrpl/beast/insight/Unit.h>
 
 #include <memory>
 #include <string>
@@ -103,6 +104,24 @@ public:
     virtual Event
     makeEvent(std::string const& name) = 0;
 
+    /**
+     * Create an event whose samples measure `unit` rather than milliseconds.
+     *
+     * The default delegates to the millisecond overload, so a collector that
+     * cannot act on a unit keeps working unchanged -- the StatsD collector
+     * relies on this. Collectors that map a unit onto an export format, such
+     * as the OTel collector, override it.
+     *
+     * @param name Metric name, already prefixed if it came through a Group.
+     * @param unit What the samples measure.
+     */
+    virtual Event
+    makeEvent(std::string const& name, Unit unit)
+    {
+        (void)unit;
+        return makeEvent(name);
+    }
+
     Event
     makeEvent(std::string const& prefix, std::string const& name)
     {
@@ -110,6 +129,14 @@ public:
             return makeEvent(name);
         return makeEvent(prefix + "." + name);
     }
+
+    Event
+    makeEvent(std::string const& prefix, std::string const& name, Unit unit)
+    {
+        if (prefix.empty())
+            return makeEvent(name, unit);
+        return makeEvent(prefix + "." + name, unit);
+    }
     /** @} */
 
     /**

include/xrpl/beast/insight/Event.h

@@ -51,6 +51,24 @@ public:
             impl_->notify(ceil<value_type>(value));
     }
 
+    /**
+     * Push a raw integral sample.
+     *
+     * For Events whose unit is not a duration, such as a byte count. The
+     * value is stored in the same integral field the duration overload uses
+     * and is interpreted per the Event's unit by the backend.
+     *
+     * Prefer this over constructing an `Event::value_type` at the call site:
+     * wrapping a byte count in a `std::chrono::milliseconds` compiles, but
+     * reads as a duration to everything downstream.
+     */
+    void
+    notify(std::uint64_t value) const
+    {
+        if (impl_)
+            impl_->notify(value_type{value});
+    }
+
     [[nodiscard]] std::shared_ptr<EventImpl> const&
     impl() const
     {

include/xrpl/beast/insight/EventImpl.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <xrpl/beast/insight/Unit.h>
+
 #include <chrono>
 #include <memory>
 
@@ -10,11 +12,48 @@ class Event;
 class EventImpl : public std::enable_shared_from_this<EventImpl>
 {
 public:
+    /**
+     * The integral type every sample is stored as.
+     *
+     * Named for the common case -- durations -- and deliberately left as a
+     * duration type. Widening it would change the wire value of every
+     * existing StatsD timer, and metrics that need finer resolution than a
+     * whole millisecond use the OTel-native microsecond instruments instead.
+     * A sample whose unit() is not a duration is carried in the same integral
+     * field and interpreted per unit() by the backend.
+     */
     using value_type = std::chrono::milliseconds;
 
     virtual ~EventImpl() = 0;
     virtual void
     notify(value_type const& value) = 0;
+
+    /**
+     * @brief What this Event's samples measure. Fixed at construction.
+     *
+     * The OTel backend reads this to choose the instrument's declared unit
+     * and, through that, its bucket ladder. The StatsD backend ignores it.
+     */
+    [[nodiscard]] Unit
+    unit() const noexcept
+    {
+        return unit_;
+    }
+
+protected:
+    /**
+     * @param unit What the samples measure. Defaults to milliseconds so
+     *             existing implementations keep their behaviour unchanged.
+     */
+    explicit EventImpl(Unit unit = Unit::Millis) : unit_(unit)
+    {
+    }
+
+private:
+    /**
+     * What the samples measure; selects the export unit and bucket ladder.
+     */
+    Unit unit_;
 };
 
 }  // namespace beast::insight

include/xrpl/beast/insight/Unit.h

@@ -0,0 +1,69 @@
+#pragma once
+
+#include <cstdint>
+
+namespace beast::insight {
+
+/**
+ * @brief What an Event's samples measure.
+ *
+ * `Event` documents itself as carrying "a millisecond time, or other integral
+ * value", but both backends used to assume the first case: the OTel bridge
+ * declared every instrument with unit `ms`, and StatsD tagged every sample
+ * `|ms`. A size metric therefore exported under a `_milliseconds` name and
+ * inherited a latency bucket ladder, which censored a quarter of its samples
+ * and pinned its p95 to a constant.
+ *
+ * Naming the unit at creation time is what lets the OTel bridge pick both the
+ * instrument unit and the matching bucket ladder:
+ *
+ *     makeEvent("time", Unit::Millis) --> OTel unit "ms" --> millisecond ladder
+ *     makeEvent("size", Unit::Bytes)  --> OTel unit "By" --> byte ladder
+ *
+ * The StatsD backend deliberately ignores this and keeps emitting `|ms` for
+ * every Event. That path is retired here -- its UDP port is commented out of
+ * the compose file and the integration test fails if anything is listening on
+ * 8125 -- so changing its wire format would alter a legacy contract for no
+ * local benefit and with no way to verify it.
+ *
+ * @note Adding a member requires extending otelUnitCode(), which switches
+ *       exhaustively so a new member is a compile error rather than a silent
+ *       fallthrough to milliseconds.
+ */
+enum class Unit : std::uint8_t {
+    /**
+     * Whole milliseconds. The default, and what every duration Event uses.
+     */
+    Millis,
+
+    /**
+     * A byte count, such as a serialized response size.
+     */
+    Bytes
+};
+
+/**
+ * @brief The OTel (UCUM) unit code for a Unit.
+ *
+ * The collector's Prometheus exporter derives the exported metric-name suffix
+ * from this code, so `ms` yields `_milliseconds` and `By` yields `_bytes`. It
+ * is also the key the histogram views match on, which is how each unit gets
+ * its own bucket ladder.
+ *
+ * @param unit The unit to translate.
+ * @return A static, null-terminated UCUM code.
+ */
+constexpr char const*
+otelUnitCode(Unit unit) noexcept
+{
+    switch (unit)
+    {
+        case Unit::Bytes:
+            return "By";
+        case Unit::Millis:
+            break;
+    }
+    return "ms";
+}
+
+}  // namespace beast::insight

src/libxrpl/beast/insight/Groups.cpp

@@ -9,6 +9,7 @@
 #include <xrpl/beast/insight/Hook.h>
 #include <xrpl/beast/insight/HookImpl.h>
 #include <xrpl/beast/insight/Meter.h>
+#include <xrpl/beast/insight/Unit.h>
 
 #include <memory>
 #include <string>
@@ -56,12 +57,25 @@ public:
         return collector_->makeCounter(makeName(name));
     }
 
+    using Collector::makeEvent;
+
     Event
     makeEvent(std::string const& name) override
     {
         return collector_->makeEvent(makeName(name));
     }
 
+    // Forwards the unit as well as the prefixed name. Without this override
+    // the base-class default would delegate to the single-argument overload
+    // above and silently drop the unit, which is how a byte-valued Event ends
+    // up declared as milliseconds -- call sites reach a collector through a
+    // Group, so this is the hop that actually matters.
+    Event
+    makeEvent(std::string const& name, Unit unit) override
+    {
+        return collector_->makeEvent(makeName(name), unit);
+    }
+
     Gauge
     makeGauge(std::string const& name) override
     {

src/libxrpl/beast/insight/NullCollector.cpp

@@ -11,6 +11,7 @@
 #include <xrpl/beast/insight/HookImpl.h>
 #include <xrpl/beast/insight/Meter.h>
 #include <xrpl/beast/insight/MeterImpl.h>
+#include <xrpl/beast/insight/Unit.h>
 
 #include <memory>
 #include <string>
@@ -49,7 +50,15 @@ public:
 class NullEventImpl : public EventImpl
 {
 public:
-    explicit NullEventImpl() = default;
+    /**
+     * @param unit What the samples would measure. Recorded even though
+     *             nothing is collected, so a caller can still read back the
+     *             unit it asked for -- which is what makes the null collector
+     *             usable for testing the unit plumbing.
+     */
+    explicit NullEventImpl(Unit unit = Unit::Millis) : EventImpl(unit)
+    {
+    }
 
     void
     notify(value_type const&) override
@@ -119,12 +128,20 @@ public:
         return Counter(std::make_shared<detail::NullCounterImpl>());
     }
 
+    using Collector::makeEvent;
+
     Event
     makeEvent(std::string const&) override
     {
         return Event(std::make_shared<detail::NullEventImpl>());
     }
 
+    Event
+    makeEvent(std::string const&, Unit unit) override
+    {
+        return Event(std::make_shared<detail::NullEventImpl>(unit));
+    }
+
     Gauge
     makeGauge(std::string const&) override
     {

src/tests/libxrpl/beast/insight/Unit.cpp

@@ -0,0 +1,166 @@
+/**
+ * GTest unit tests for beast::insight::Unit and its plumbing.
+ *
+ * A metric's unit decides two things that are invisible at the call site: the
+ * name suffix the exporter appends, and which bucket ladder the histogram
+ * view applies. Getting it wrong is silent -- a byte count declared as
+ * milliseconds still records, still exports, still draws a graph, and the
+ * graph is wrong. So each hop the unit has to survive is asserted here
+ * rather than left to inspection.
+ *
+ * The hop that matters most is the group wrapper. Call sites reach a
+ * collector through Groups, so a unit that reaches OTelCollector correctly
+ * but is dropped by the group prefixing layer would pass a naive test while
+ * failing in production.
+ */
+
+#include <xrpl/beast/insight/Unit.h>
+
+#include <xrpl/beast/insight/Event.h>
+#include <xrpl/beast/insight/EventImpl.h>
+#include <xrpl/beast/insight/Groups.h>
+#include <xrpl/beast/insight/NullCollector.h>
+
+#include <gtest/gtest.h>
+
+#include <chrono>
+#include <cstdint>
+#include <memory>
+#include <vector>
+
+namespace beast::insight {
+
+namespace {
+
+/**
+ * An EventImpl that records what it was notified with.
+ *
+ * Needed because every shipped implementation either discards the sample
+ * (NullCollector) or sends it somewhere external. Asserting the recorded
+ * value proves the raw-integral path preserves it, rather than only proving
+ * that notify() can be called without crashing.
+ */
+class RecordingEventImpl : public EventImpl
+{
+public:
+    explicit RecordingEventImpl(Unit unit) : EventImpl(unit)
+    {
+    }
+
+    void
+    notify(value_type const& value) override
+    {
+        samples.push_back(value);
+    }
+
+    /**
+     * Every value passed to notify(), in call order.
+     */
+    std::vector<value_type> samples;
+};
+
+}  // namespace
+
+// The unit code is a contract with the collector's Prometheus exporter: it
+// derives the exported name suffix from this string. Assert the exact codes,
+// not merely that they differ.
+TEST(InsightUnit, otelCodeIsTheUcumCodeForEachUnit)
+{
+    EXPECT_STREQ(otelUnitCode(Unit::Millis), "ms");
+    EXPECT_STREQ(otelUnitCode(Unit::Bytes), "By");
+}
+
+TEST(InsightUnit, defaultEventUnitIsMillisForBackwardCompatibility)
+{
+    // Every pre-existing makeEvent(name) call site records a duration, so the
+    // one-argument overload must keep meaning milliseconds.
+    auto const collector = NullCollector::make();
+    auto const event = collector->makeEvent("legacy");
+    ASSERT_NE(event.impl(), nullptr);
+    EXPECT_EQ(event.impl()->unit(), Unit::Millis);
+}
+
+TEST(InsightUnit, makeEventCarriesTheRequestedUnitToTheImpl)
+{
+    auto const collector = NullCollector::make();
+    auto const event = collector->makeEvent("size", Unit::Bytes);
+    ASSERT_NE(event.impl(), nullptr);
+    EXPECT_EQ(event.impl()->unit(), Unit::Bytes);
+}
+
+TEST(InsightUnit, prefixedMakeEventCarriesTheUnit)
+{
+    auto const collector = NullCollector::make();
+    auto const event = collector->makeEvent("rpc", "size", Unit::Bytes);
+    ASSERT_NE(event.impl(), nullptr);
+    EXPECT_EQ(event.impl()->unit(), Unit::Bytes);
+}
+
+TEST(InsightUnit, groupWrapperForwardsTheUnitAlongWithThePrefix)
+{
+    // ServerHandler creates its events through a Group, not through the
+    // collector directly. If the group's makeEvent override forwards only the
+    // name, the unit silently reverts to milliseconds and the byte histogram
+    // inherits the latency ladder again.
+    auto const collector = NullCollector::make();
+    auto const groups = makeGroups(collector);
+    auto const event = groups->get("rpc")->makeEvent("size", Unit::Bytes);
+    ASSERT_NE(event.impl(), nullptr);
+    EXPECT_EQ(event.impl()->unit(), Unit::Bytes);
+}
+
+TEST(InsightUnit, groupWrapperStillDefaultsToMillis)
+{
+    auto const collector = NullCollector::make();
+    auto const groups = makeGroups(collector);
+    auto const event = groups->get("rpc")->makeEvent("time");
+    ASSERT_NE(event.impl(), nullptr);
+    EXPECT_EQ(event.impl()->unit(), Unit::Millis);
+}
+
+TEST(InsightUnit, rawIntegralNotifyPreservesTheValueExactly)
+{
+    // The byte path must not be rounded or scaled on its way through the
+    // duration-typed storage field.
+    auto const impl = std::make_shared<RecordingEventImpl>(Unit::Bytes);
+    Event const event(impl);
+
+    event.notify(std::uint64_t{4096});
+    event.notify(std::uint64_t{0});
+    event.notify(std::uint64_t{1'048'577});
+
+    ASSERT_EQ(impl->samples.size(), 3U);
+    EXPECT_EQ(impl->samples[0].count(), 4096);
+    EXPECT_EQ(impl->samples[1].count(), 0);
+    EXPECT_EQ(impl->samples[2].count(), 1'048'577);
+}
+
+TEST(InsightUnit, durationNotifyStillRoundsUpToWholeMilliseconds)
+{
+    // Pre-existing behaviour, asserted so the new overload cannot quietly
+    // change it: Event applies ceil to whole milliseconds, which is why
+    // sub-millisecond resolution is impossible on this path.
+    auto const impl = std::make_shared<RecordingEventImpl>(Unit::Millis);
+    Event const event(impl);
+
+    event.notify(std::chrono::microseconds{40});
+    event.notify(std::chrono::microseconds{1'000});
+    event.notify(std::chrono::milliseconds{7});
+
+    ASSERT_EQ(impl->samples.size(), 3U);
+    EXPECT_EQ(impl->samples[0].count(), 1) << "40us must round up to 1ms, not down to 0";
+    EXPECT_EQ(impl->samples[1].count(), 1);
+    EXPECT_EQ(impl->samples[2].count(), 7);
+}
+
+TEST(InsightUnit, notifyOnANullEventIsSafeForBothOverloads)
+{
+    // A default-constructed Event has no impl. Both overloads must be no-ops
+    // rather than dereferencing null.
+    Event const none;
+    ASSERT_EQ(none.impl(), nullptr);
+    EXPECT_NO_THROW(none.notify(std::uint64_t{4096}));
+    EXPECT_NO_THROW(none.notify(std::chrono::milliseconds{5}));
+}
+
+}  // namespace beast::insight

src/xrpld/rpc/detail/ServerHandler.cpp

@@ -16,6 +16,7 @@
 #include <xrpl/basics/base64.h>
 #include <xrpl/basics/contract.h>
 #include <xrpl/basics/make_SSLContext.h>
+#include <xrpl/beast/insight/Unit.h>
 #include <xrpl/beast/net/IPAddress.h>
 #include <xrpl/beast/net/IPAddressConversion.h>
 #include <xrpl/beast/rfc2616.h>
@@ -183,7 +184,11 @@ ServerHandler::ServerHandler(
 {
     auto const& group(cm.group("rpc"));
     rpcRequests_ = group->makeCounter("requests");
-    rpcSize_ = group->makeEvent("size");
+    // "size" measures the serialized response in bytes, not a duration. It
+    // has to say so: the unit picks both the exported name suffix and the
+    // histogram bucket ladder, and borrowing the millisecond ladder censored
+    // a quarter of these samples.
+    rpcSize_ = group->makeEvent("size", beast::insight::Unit::Bytes);
     rpcTime_ = group->makeEvent("time");
 }
 
@@ -1125,7 +1130,7 @@ ServerHandler::processRequest(
         std::chrono::duration_cast<std::chrono::milliseconds>(
             std::chrono::high_resolution_clock::now() - start));
     ++rpcRequests_;
-    rpcSize_.notify(beast::insight::Event::value_type{response.size()});
+    rpcSize_.notify(static_cast<std::uint64_t>(response.size()));
 
     response += '\n';