fix(telemetry): address phase-7 review comments

- cmake: keep the opentelemetry-cpp umbrella target for the beast metrics
  link and document why. The reviewer suggested linking individual
  component targets to avoid over-linking, but the OTel Conan package
  under-declares inter-component dependencies (the OTLP client references
  sdk::common symbols without a declared edge), so naming components
  directly reorders the static link into an unresolvable state. Verified
  by building xrpl_tests both ways.
- OTelCollector.h: add usage examples, thread-safety and limitations
  @note blocks to the class doc.
- OTelCollector.cpp: correct the @param name docs on the instrument
  Impl constructors to describe the already-formatName()'d value.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Pratik Mankawde
2026-07-07 14:27:01 +01:00
parent 134b419895
commit cc4457b5a2
3 changed files with 53 additions and 5 deletions

View File

@@ -78,7 +78,13 @@ include(target_link_modules)
# Level 01
add_module(xrpl beast)
target_link_libraries(xrpl.libxrpl.beast PUBLIC xrpl.imports.main)
# OTelCollector in beast/insight uses OTel Metrics SDK when telemetry is enabled.
# OTelCollector in beast/insight uses the OTel Metrics SDK when telemetry is
# enabled. Link the Conan-provided umbrella target rather than individual
# component targets: the OTel package's per-component dependency graph is
# under-declared (e.g. the OTLP client references sdk::common symbols without
# declaring the edge), so naming components directly reorders the static-link
# line into an unresolvable state. The umbrella carries the full, internally
# consistent graph the package authors validated.
if(telemetry)
target_link_libraries(
xrpl.libxrpl.beast

View File

@@ -54,6 +54,40 @@ namespace beast::insight {
* - Meter -> OTel Counter<uint64_t> (monotonic, unsigned)
* - Hook -> Called by PeriodicMetricReader at collection time
*
* Example — primary use (create the collector and record a metric):
* @code
* auto collector = beast::insight::OTelCollector::New(
* "http://localhost:4318/v1/metrics", // OTLP/HTTP endpoint
* "xrpld", // metric name prefix
* "node-1", // service.instance.id
* "xrpld", // service.name
* "mainnet", // xrpl.network.type
* journal);
*
* auto counter = collector->makeCounter("ledgers", "closed");
* ++counter; // exported on the next PeriodicMetricReader tick
* @endcode
*
* Example — edge case (telemetry disabled at compile time): New()
* returns a NullCollector, so callers need no #ifdef guard. The same
* instruments compile and run, but recording is a no-op.
* @code
* auto collector = beast::insight::OTelCollector::New(
* endpoint, prefix, instanceId, serviceName, networkType, journal);
* auto gauge = collector->makeGauge("peers", "count");
* gauge = 42; // silently discarded when XRPL_ENABLE_TELEMETRY is off
* @endcode
*
* @note Thread safety: instrument recording (Counter::Add,
* Histogram::Record, and atomic gauge writes) is thread-safe and
* may be called concurrently. Instrument *creation* (make_counter,
* make_gauge, etc.) is serialized by an internal mutex. Hook and
* observable-gauge callbacks run on the SDK's collection thread, so
* any state they read must itself be thread-safe.
* @note Limitations: metrics export over OTLP/HTTP only (no gRPC); the
* PeriodicMetricReader interval is fixed at 1s; and gauge values
* are stored as int64_t, so fractional gauges are truncated.
*
* @see StatsDCollector for the StatsD-based alternative.
* @see NullCollector for the no-op fallback.
*/

View File

@@ -142,7 +142,9 @@ class OTelCounterImpl : public CounterImpl
{
public:
/**
* @param name Fully-qualified metric name (prefix.group.name).
* @param name Export-ready metric name, already run through
* formatName() by the collector: prefix prepended and
* dots replaced with underscores (e.g. "xrpld_rpc_size").
* @param meter OTel Meter used to create the counter instrument.
*/
OTelCounterImpl(
@@ -182,7 +184,9 @@ class OTelEventImpl : public EventImpl
{
public:
/**
* @param name Fully-qualified metric name (prefix.group.name).
* @param name Export-ready metric name, already run through
* formatName() by the collector: prefix prepended and
* dots replaced with underscores (e.g. "xrpld_rpc_size").
* @param meter OTel Meter used to create the histogram instrument.
*/
OTelEventImpl(
@@ -227,7 +231,9 @@ class OTelGaugeImpl : public GaugeImpl
{
public:
/**
* @param name Fully-qualified metric name (prefix.group.name).
* @param name Export-ready metric name, already run through
* formatName() by the collector: prefix prepended
* and dots replaced with underscores.
* @param meter OTel Meter used to create the observable gauge.
* @param collector Owning collector, used to invoke hooks before reads.
*/
@@ -300,7 +306,9 @@ class OTelMeterImpl : public MeterImpl
{
public:
/**
* @param name Fully-qualified metric name (prefix.group.name).
* @param name Export-ready metric name, already run through
* formatName() by the collector: prefix prepended and
* dots replaced with underscores (e.g. "xrpld_rpc_size").
* @param meter OTel Meter used to create the counter instrument.
*/
OTelMeterImpl(