docs(telemetry): rewrite the bucket-ladder and collector docs without change history

Each of these described how the code reached its current shape. A squash merge
does not publish the revision they compare against.

- HistogramBuckets: give the reason one header owns every ladder as a present
  statement about the alternative, not as 'before this existed'; describe what
  the 2/3/4 s edges resolve rather than what they previously forced; and state
  why check_bucket_parity.py machine-checks containment instead of narrating
  the drift that motivated it.
- Unit: 'That path is retired here' anchors on this branch as a moment in time.
  The StatsD path is simply out of service.
- OTelCollector: it is an adapter over the global Meter, not a 'legacy shim'
  that 'no longer owns' a pipeline, and endpoint is used only in the startup
  log line rather than 'retained for back-compat'.
- Telemetry: a byte count gets the byte ladder, rather than 'no longer
  inheriting' a latency one.

Comments only, no behaviour change.
This commit is contained in:
Pratik Mankawde
2026-09-02 19:46:42 +01:00
parent ecbf8350fe
commit a0ba7f6e1c
4 changed files with 17 additions and 16 deletions

View File

@@ -21,7 +21,7 @@ namespace beast::insight {
* 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
* every Event. That path is out of service -- 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.

View File

@@ -12,9 +12,9 @@ namespace xrpl::telemetry::buckets {
* @brief Explicit histogram bucket edges for xrpld's OTel instruments.
*
* One header owns every ladder so a reviewer sees all of them at once and a
* test can assert their invariants. Before this existed the edges lived as
* file-local `namespace {}` constants, unreachable from any test, and they
* drifted apart.
* test can assert their invariants. The alternative -- file-local
* `namespace {}` constants at each registration site -- is unreachable from
* any test and lets the ladders drift apart.
*
* Why a ladder is worth this much care: when a quantile falls in the `+Inf`
* bucket, Prometheus returns the *second-highest* edge, not `+Inf`. A
@@ -68,11 +68,11 @@ namespace xrpl::telemetry::buckets {
* **This list must contain every representable edge of the collector's
* spanmetrics ladder, and may extend above it.** Agreement over the shared
* range is deliberate: it lets a span-derived latency panel and a native
* histogram panel be read on the same scale. It was specified that way
* originally, then silently broken when the collector ladder alone was
* extended, which left this side capped at 5 s while spans reached 30 s and
* censored every quantile above 5 s. `check_bucket_parity.py` now enforces
* the containment -- add a collector edge, add it here too.
* histogram panel be read on the same scale. `check_bucket_parity.py`
* machine-checks the containment, because a ladder that agrees only by
* convention drifts the first time one side is extended alone, and a top edge
* below the collector's censors every quantile above it. Add a collector edge,
* add it here too.
*
* The sub-millisecond edges the collector carries (0.01 to 0.5 ms) are
* deliberately absent. `beast::insight::Event` rounds every duration up to
@@ -86,8 +86,9 @@ namespace xrpl::telemetry::buckets {
* as 5 s censors them today. All these Events share one ladder, so its
* ceiling has to cover the slowest member rather than the typical one.
*
* The 2, 3 and 4 s edges resolve second-scale work that previously had to
* interpolate across a single four-second-wide bucket.
* The 2, 3 and 4 s edges subdivide the 1 s to 5 s span, so second-scale work
* resolves to about a second rather than being interpolated across a single
* four-second-wide bucket.
*/
inline constexpr std::array kMillisecondBuckets{
1.0,

View File

@@ -5,7 +5,7 @@
* Compiled only when XRPL_ENABLE_TELEMETRY is defined (via CMake
* telemetry=ON). Maps beast::insight instruments to OTel SDK instruments
* created on the GLOBAL Meter published by the telemetry module. This class
* is a legacy shim: it no longer owns an export pipeline. The MeterProvider,
* is an adapter only: it owns no export pipeline. The MeterProvider,
* PeriodicExportingMetricReader, OTLP exporter and histogram view all live in
* xrpl::telemetry::Telemetry.
*
@@ -380,7 +380,7 @@ private:
//------------------------------------------------------------------------------
/**
* @brief Main OTel Collector implementation (legacy shim).
* @brief Main OTel Collector implementation (adapter over the global Meter).
*
* Obtains its Meter from the GLOBAL MeterProvider owned and published by the
* telemetry module (xrpl::telemetry::Telemetry), rather than building its own
@@ -445,8 +445,8 @@ public:
*
* @param endpoint OTLP/HTTP metrics endpoint URL. Informational only:
* the global telemetry pipeline is authoritative for
* the actual export endpoint. Retained for logging and
* back-compat with the New() signature.
* the actual export endpoint. Used only in the startup
* log line.
* @param prefix Legacy metric-name prefix. Not applied to metric
* names; used only in the startup log line.
* @param instanceId Value for the service.instance.id resource attribute.

View File

@@ -409,7 +409,7 @@ class TelemetryImpl : public Telemetry
// One histogram view per unit. The unit is the selector, so an
// instrument gets the ladder that fits what it measures -- a byte
// count no longer inherits a latency ladder. Edges come from
// count gets the byte ladder instead of a latency one. Edges come from
// HistogramBuckets.h, which owns every ladder.
//
// Both views keep the "*" name pattern and an EMPTY view name: a