diff --git a/cmake/XrplCore.cmake b/cmake/XrplCore.cmake index fa54ca7255..40e7c0f854 100644 --- a/cmake/XrplCore.cmake +++ b/cmake/XrplCore.cmake @@ -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 @@ -219,6 +225,11 @@ target_link_libraries( PRIVATE xrpl.libxrpl.protocol ) if(telemetry) + # Telemetry owns both the trace and (as of the direct-metrics API) the + # metrics pipeline. Link the umbrella target: it supplies the trace and + # metrics SDK components with the correct static-link ordering, which + # naming components individually does not (the package under-declares + # inter-component dependencies). target_link_libraries( xrpl.libxrpl.telemetry PUBLIC opentelemetry-cpp::opentelemetry-cpp diff --git a/docker/telemetry/integration-test.sh b/docker/telemetry/integration-test.sh index f0e63f1590..38d76e65a6 100755 --- a/docker/telemetry/integration-test.sh +++ b/docker/telemetry/integration-test.sh @@ -645,11 +645,15 @@ check_otel_metric "rippled_total_Bytes_In" # Verify StatsD receiver is NOT required (no statsd receiver in pipeline) log "" log "--- Verify StatsD receiver is not required ---" -statsd_port_check=$(curl -sf "http://localhost:8125" 2>&1 || echo "refused") -if echo "$statsd_port_check" | grep -qi "refused\|error\|connection"; then - ok "StatsD port 8125 is not listening (not required)" +# StatsD listens on UDP 8125, so probe with a UDP-aware tool, not curl (TCP). +if command -v ss >/dev/null 2>&1; then + if ss -ulnp 2>/dev/null | grep -q ":8125"; then + fail "StatsD port 8125 appears to be listening (should not be needed)" + else + ok "StatsD port 8125 is not listening (not required)" + fi else - fail "StatsD port 8125 appears to be listening (should not be needed)" + log "ss not found -- skipping StatsD UDP port check" fi # --------------------------------------------------------------------------- diff --git a/include/xrpl/beast/insight/OTelCollector.h b/include/xrpl/beast/insight/OTelCollector.h index f3387614a8..e043531f99 100644 --- a/include/xrpl/beast/insight/OTelCollector.h +++ b/include/xrpl/beast/insight/OTelCollector.h @@ -54,6 +54,40 @@ namespace beast::insight { * - Meter -> OTel Counter (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. */ diff --git a/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h index d998450066..643371b629 100644 --- a/include/xrpl/telemetry/Telemetry.h +++ b/include/xrpl/telemetry/Telemetry.h @@ -97,6 +97,7 @@ #ifdef XRPL_ENABLE_TELEMETRY #include +#include #include #include #include @@ -109,6 +110,13 @@ namespace xrpl::telemetry { source of spans; distinct from the `service.name` resource attribute (Setup::serviceName), which is config-overridable. */ inline constexpr std::string_view kTracerName{"xrpld"}; + +/** OTel instrumentation scope (meter) name. Identifies this library as the + source of metrics; symmetric with kTracerName for the tracing side. */ +inline constexpr std::string_view kMeterName{"xrpld"}; + +/** OTel instrumentation scope version reported for the meter. */ +inline constexpr std::string_view kMeterVersion{"1.0.0"}; #endif class Telemetry @@ -300,6 +308,20 @@ public: virtual opentelemetry::nostd::shared_ptr getTracer(std::string_view name = kTracerName) = 0; + /** Get or create a named meter instance. + + Returns the raw OTel Meter, giving developers direct access to the + full metrics API. From the returned Meter any instrument type can be + created: Counter, UpDownCounter, Gauge, and Histogram (synchronous), + plus their observable/async variants (ObservableCounter, + ObservableUpDownCounter, ObservableGauge). + + @param name Meter name used to identify the instrumentation scope. + @return A shared pointer to the Meter. + */ + virtual opentelemetry::nostd::shared_ptr + getMeter(std::string_view name = kMeterName) = 0; + /** Start a new span on the current thread's context. The span becomes a child of the current active span (if any) via diff --git a/src/libxrpl/beast/insight/OTelCollector.cpp b/src/libxrpl/beast/insight/OTelCollector.cpp index 2f9bb7ac64..1bd6a018af 100644 --- a/src/libxrpl/beast/insight/OTelCollector.cpp +++ b/src/libxrpl/beast/insight/OTelCollector.cpp @@ -4,7 +4,10 @@ * * Compiled only when XRPL_ENABLE_TELEMETRY is defined (via CMake * telemetry=ON). Maps beast::insight instruments to OTel SDK instruments - * and exports them via OTLP/HTTP using a PeriodicMetricReader. + * 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, + * PeriodicExportingMetricReader, OTLP exporter and histogram view all live in + * xrpl::telemetry::Telemetry. * * When XRPL_ENABLE_TELEMETRY is not defined, OTelCollector::New() returns * a NullCollector so the build succeeds without OTel dependencies. @@ -22,10 +25,10 @@ * +--------------------+----------------+--------------+ * | * v - * PeriodicMetricReader (1s interval) + * GLOBAL Meter (from metrics::Provider::GetMeterProvider()) * | * v - * OtlpHttpMetricExporter -> OTel Collector -> Prometheus + * telemetry-owned PeriodicMetricReader (1s) -> OTLP exporter -> Prometheus */ #ifdef XRPL_ENABLE_TELEMETRY @@ -40,28 +43,15 @@ #include #include -#include -#include #include #include #include #include +#include #include #include #include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include #include #include @@ -78,9 +68,6 @@ namespace beast::insight { namespace detail { namespace metrics_api = opentelemetry::metrics; -namespace metrics_sdk = opentelemetry::sdk::metrics; -namespace otlp_http = opentelemetry::exporter::otlp; -namespace resource = opentelemetry::sdk::resource; class OTelCollectorImp; @@ -141,7 +128,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( @@ -181,7 +170,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( @@ -226,7 +217,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. */ @@ -299,7 +292,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( @@ -326,12 +321,16 @@ private: //------------------------------------------------------------------------------ /** - * @brief Main OTel Collector implementation. + * @brief Main OTel Collector implementation (legacy shim). * - * Creates an OTel MeterProvider with a PeriodicMetricReader that - * exports metrics via OTLP/HTTP at 1-second intervals. Implements - * all Collector::make_*() factory methods to create OTel-backed - * instrument wrappers. + * Obtains its Meter from the GLOBAL MeterProvider owned and published by the + * telemetry module (xrpl::telemetry::Telemetry), rather than building its own + * export pipeline. Implements all Collector::make_*() factory methods to + * create OTel-backed instrument wrappers on that shared Meter. + * + * The metrics pipeline (MeterProvider + PeriodicExportingMetricReader + OTLP + * HTTP exporter + histogram view) lives in the telemetry module. This class is + * a thin adapter kept for beast::insight callers during deprecation. * * Class diagram: * @@ -344,19 +343,20 @@ private: * | OTelCollectorImp |-------------+ * +------------------+ * | - journal_ | - * | - prefix_ | - * | - provider_ | +---------------------+ - * | - otelMeter_ |---->| OTel MeterProvider | - * | - hooks_[] | | + PeriodicReader | - * | - gauges_[] | | + OtlpHttpExporter | - * +------------------+ +---------------------+ + * | - prefix_ | +--------------------------+ + * | - otelMeter_ |---->| GLOBAL MeterProvider | + * | - hooks_[] | | (owned by Telemetry) | + * | - gauges_[] | | + PeriodicReader | + * +------------------+ | + OtlpHttpExporter | + * +--------------------------+ * * Lifecycle: - * 1. Constructor creates MeterProvider + exporter pipeline. - * 2. make_*() methods create instruments registered with the provider. - * 3. PeriodicMetricReader collects every 1s, calling observable callbacks. + * 1. Constructor fetches the Meter from the global MeterProvider. + * 2. make_*() methods create instruments on that shared Meter. + * 3. The telemetry-owned PeriodicMetricReader collects every 1s, calling + * observable callbacks. * 4. Observable callbacks invoke hooks, read gauge atomics. - * 5. Destructor shuts down MeterProvider (flushes pending exports). + * 5. Destructor only logs; the telemetry module owns pipeline teardown. * * Caveats: * - Observable gauge callbacks run on the SDK's internal thread. Hook @@ -381,9 +381,12 @@ class OTelCollectorImp : public OTelCollector, public std::enable_shared_from_th { public: /** - * @brief Construct the OTel collector and initialize the export pipeline. + * @brief Construct the OTel collector over the global MeterProvider. * - * @param endpoint OTLP/HTTP metrics endpoint URL. + * @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. * @param prefix Prefix for all metric names. * @param instanceId Value for the service.instance.id resource attribute. * When empty, the attribute is omitted. @@ -496,9 +499,6 @@ private: /** Prefix for all metric names (e.g., "xrpld"). */ std::string prefix_; - /** OTel SDK MeterProvider owning the export pipeline. RAII lifecycle. */ - std::shared_ptr provider_; - /** OTel Meter used to create all instruments. */ opentelemetry::nostd::shared_ptr otelMeter_; @@ -673,69 +673,33 @@ OTelCollectorImp::OTelCollectorImp( Journal journal) : journal_(journal), prefix_(std::move(prefix)) { + // instanceId/serviceName/networkType are retained on the New() signature + // for back-compat but no longer used here: the telemetry module owns the + // resource attributes for the shared metrics pipeline. + (void)instanceId; + (void)serviceName; + (void)networkType; + if (journal_.info()) { + // endpoint is informational: the global telemetry pipeline owns the + // real exporter. It is logged here for back-compat and diagnostics. journal_.info() << "OTelCollector starting: endpoint=" << endpoint << " prefix=" << prefix_; } - // Configure OTLP HTTP metric exporter. - otlp_http::OtlpHttpMetricExporterOptions exporterOpts; - exporterOpts.url = endpoint; - - auto exporter = otlp_http::OtlpHttpMetricExporterFactory::Create(exporterOpts); - - // Configure periodic metric reader (1-second export interval). - metrics_sdk::PeriodicExportingMetricReaderOptions readerOpts; - readerOpts.export_interval_millis = std::chrono::milliseconds(1000); - readerOpts.export_timeout_millis = std::chrono::milliseconds(500); - - auto reader = - metrics_sdk::PeriodicExportingMetricReaderFactory::Create(std::move(exporter), readerOpts); - - // Configure resource attributes matching the trace exporter so metrics - // and traces share one identity. service.name defaults to "xrpld" when - // unset. service.instance.id and xrpl.network.type are added when - // provided so Prometheus labels distinguish node and network. - // "xrpl.network.type" is a string literal (not the telemetry SpanNames - // const) because beast/insight sits below the telemetry module and - // cannot include it; the value must match the trace exporter's key. - resource::ResourceAttributes attrs; - attrs[opentelemetry::semconv::service::kServiceName] = - serviceName.empty() ? "xrpld" : serviceName; - if (!instanceId.empty()) - { - attrs[opentelemetry::semconv::service::kServiceInstanceId] = instanceId; - } - if (!networkType.empty()) - { - attrs["xrpl.network.type"] = networkType; - } - auto resourceAttrs = resource::Resource::Create(attrs); - - // Create MeterProvider with resource, then attach the metric reader. - provider_ = metrics_sdk::MeterProviderFactory::Create( - std::make_unique(), resourceAttrs); - provider_->AddMetricReader(std::move(reader)); - - // Configure histogram bucket boundaries for Event instruments. - // These match the SpanMetrics connector buckets for consistency. - auto histogramSelector = metrics_sdk::InstrumentSelectorFactory::Create( - metrics_sdk::InstrumentType::kHistogram, "*", "ms"); - auto meterSelector = metrics_sdk::MeterSelectorFactory::Create("xrpld_metrics", "", ""); - auto histogramConfig = std::make_shared(); - histogramConfig->boundaries_ = - std::vector{1.0, 5.0, 10.0, 25.0, 50.0, 100.0, 250.0, 500.0, 1000.0, 5000.0}; - auto histogramView = metrics_sdk::ViewFactory::Create( - "default_histogram", - "Default histogram view with SpanMetrics-compatible buckets", - metrics_sdk::AggregationType::kHistogram, - std::move(histogramConfig)); - - provider_->AddView( - std::move(histogramSelector), std::move(meterSelector), std::move(histogramView)); - - // Create the OTel Meter for creating instruments. - otelMeter_ = provider_->GetMeter("xrpld_metrics", "1.0.0"); + // Fetch the Meter from the GLOBAL MeterProvider. The telemetry module + // (xrpl::telemetry::Telemetry) builds the metrics pipeline (exporter, + // periodic reader, histogram view, resource attributes) and registers it + // via metrics::Provider::SetMeterProvider() during start(). beast metrics + // ride that shared pipeline, so both direct-API and beast-sourced metrics + // export under one resource identity. + // + // The name/version literals MUST match the telemetry module's kMeterName + // ("xrpld") and kMeterVersion ("1.0.0"). They are written as literals (not + // referenced from Telemetry.h) because beast/insight sits below the + // telemetry module in the layering and cannot include its header. + otelMeter_ = metrics_api::Provider::GetMeterProvider()->GetMeter( + std::string{"xrpld"}, std::string{"1.0.0"}); if (journal_.info()) { @@ -749,11 +713,8 @@ OTelCollectorImp::~OTelCollectorImp() { journal_.info() << "OTelCollector shutting down"; } - if (provider_) - { - provider_->ForceFlush(std::chrono::milliseconds(2000)); - provider_->Shutdown(); - } + // No pipeline teardown here: the telemetry module owns the global + // MeterProvider lifecycle (ForceFlush/Shutdown happen in Telemetry::stop()). if (journal_.info()) { journal_.info() << "OTelCollector stopped"; diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp index 63fe0a726a..6a594b3123 100644 --- a/src/libxrpl/telemetry/Telemetry.cpp +++ b/src/libxrpl/telemetry/Telemetry.cpp @@ -25,7 +25,23 @@ #include #include #include +#include +#include +#include +#include +#include +#include #include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include #include #include @@ -50,6 +66,7 @@ #include #include #include +#include namespace xrpl::telemetry { @@ -57,6 +74,8 @@ namespace { namespace trace_api = opentelemetry::trace; namespace trace_sdk = opentelemetry::sdk::trace; +namespace metrics_api = opentelemetry::metrics; +namespace metrics_sdk = opentelemetry::sdk::metrics; namespace otlp_http = opentelemetry::exporter::otlp; namespace resource = opentelemetry::sdk::resource; @@ -222,6 +241,16 @@ public: return noopTracer; } + opentelemetry::nostd::shared_ptr + getMeter(std::string_view name) override + { + // Serve a meter from a process-wide noop provider, mirroring the + // noop tracer above. Instruments created from it are inert. + static auto noopProvider = opentelemetry::nostd::shared_ptr( + new metrics_api::NoopMeterProvider()); + return noopProvider->GetMeter(std::string(name), std::string(kMeterVersion)); + } + opentelemetry::nostd::shared_ptr startSpan(std::string_view, trace_api::SpanKind) override { @@ -259,6 +288,14 @@ class TelemetryImpl : public Telemetry */ std::shared_ptr sdkProvider_; + /** The SDK MeterProvider that owns the metric export pipeline. + + Symmetric with sdkProvider_ on the tracing side. Held as + std::shared_ptr so we can ForceFlush() on shutdown; wrapped in a + nostd::shared_ptr when registered as the global meter provider. + */ + std::shared_ptr meterProvider_; + public: TelemetryImpl(Setup setup, beast::Journal journal) : setup_(std::move(setup)), journal_(journal) { @@ -331,6 +368,70 @@ public: trace_api::Provider::SetTracerProvider( opentelemetry::nostd::shared_ptr(sdkProvider_)); + // Build the metrics pipeline, parallel to the tracer above and + // reusing the same resourceAttrs so metrics and traces share one + // resource identity. + + // Derive the metrics endpoint from the trace endpoint by swapping + // the trailing "/v1/traces" path for "/v1/metrics". Any other URL + // shape is used as-is. + std::string metricsEndpoint = setup_.exporterEndpoint; + constexpr std::string_view tracesPath{"/v1/traces"}; + if (metricsEndpoint.ends_with(tracesPath)) + { + metricsEndpoint.replace( + metricsEndpoint.size() - tracesPath.size(), tracesPath.size(), "/v1/metrics"); + } + + // Configure OTLP HTTP metric exporter, honoring the same TLS + // options as the trace exporter. + otlp_http::OtlpHttpMetricExporterOptions metricExporterOpts; + metricExporterOpts.url = metricsEndpoint; + if (setup_.useTls) + { + metricExporterOpts.ssl_ca_cert_path = setup_.tlsCertPath; + metricExporterOpts.ssl_client_cert_path = setup_.tlsClientCertPath; + metricExporterOpts.ssl_client_key_path = setup_.tlsClientKeyPath; + } + + auto metricExporter = otlp_http::OtlpHttpMetricExporterFactory::Create(metricExporterOpts); + + // Configure periodic metric reader (1-second export interval, + // matching the beast OTelCollector path). + metrics_sdk::PeriodicExportingMetricReaderOptions readerOpts; + readerOpts.export_interval_millis = std::chrono::milliseconds(1000); + readerOpts.export_timeout_millis = std::chrono::milliseconds(500); + + auto reader = metrics_sdk::PeriodicExportingMetricReaderFactory::Create( + std::move(metricExporter), readerOpts); + + // Create MeterProvider with the shared resource, then attach reader. + meterProvider_ = metrics_sdk::MeterProviderFactory::Create( + std::make_unique(), resourceAttrs); + meterProvider_->AddMetricReader(std::move(reader)); + + // Histogram view: SpanMetrics-compatible bucket boundaries (ms) so + // histogram instruments align with the collector's SpanMetrics. + auto histogramSelector = metrics_sdk::InstrumentSelectorFactory::Create( + metrics_sdk::InstrumentType::kHistogram, "*", "ms"); + auto meterSelector = metrics_sdk::MeterSelectorFactory::Create("xrpld_metrics", "", ""); + auto histogramConfig = std::make_shared(); + histogramConfig->boundaries_ = + std::vector{1.0, 5.0, 10.0, 25.0, 50.0, 100.0, 250.0, 500.0, 1000.0, 5000.0}; + auto histogramView = metrics_sdk::ViewFactory::Create( + "default_histogram", + "Default histogram view with SpanMetrics-compatible buckets", + metrics_sdk::AggregationType::kHistogram, + std::move(histogramConfig)); + + meterProvider_->AddView( + std::move(histogramSelector), std::move(meterSelector), std::move(histogramView)); + + // Publish as the global meter provider so developers (and the beast + // OTelCollector shim) reach the same pipeline. + metrics_api::Provider::SetMeterProvider( + opentelemetry::nostd::shared_ptr(meterProvider_)); + // Register as the global Telemetry instance so SpanGuard factory // methods can access it without callers passing a reference. Telemetry::setInstance(this); @@ -363,6 +464,18 @@ public: opentelemetry::nostd::shared_ptr( new trace_api::NoopTracerProvider())); } + + if (meterProvider_) + { + // Mirror the tracer teardown: bounded flush, drop our provider, + // then restore a noop global provider. Same shutdown-order + // caveat as sdkProvider_ above applies to getMeter() callers. + meterProvider_->ForceFlush(std::chrono::milliseconds(5000)); + meterProvider_.reset(); + metrics_api::Provider::SetMeterProvider( + opentelemetry::nostd::shared_ptr( + new metrics_api::NoopMeterProvider())); + } JLOG(journal_.info()) << "Telemetry stopped"; } @@ -416,6 +529,15 @@ public: return sdkProvider_->GetTracer(std::string(name)); } + opentelemetry::nostd::shared_ptr + getMeter(std::string_view name = kMeterName) override + { + if (!meterProvider_) + return metrics_api::Provider::GetMeterProvider()->GetMeter( + std::string(name), std::string(kMeterVersion)); + return meterProvider_->GetMeter(std::string(name), std::string(kMeterVersion)); + } + opentelemetry::nostd::shared_ptr startSpan(std::string_view name, trace_api::SpanKind kind) override { diff --git a/src/tests/libxrpl/telemetry/GetMeter.cpp b/src/tests/libxrpl/telemetry/GetMeter.cpp new file mode 100644 index 0000000000..b69920fdeb --- /dev/null +++ b/src/tests/libxrpl/telemetry/GetMeter.cpp @@ -0,0 +1,111 @@ +/** @file GetMeter.cpp + Unit tests for the direct OTel metrics API surface: + - xrpl::telemetry::Telemetry::getMeter() on the disabled (noop) path. + - The global MeterProvider contract that beast's OTelCollector shim + relies on (GetMeterProvider()->GetMeter(kMeterName, kMeterVersion)). + + These tests deliberately exercise the global-provider level rather than a + fully started TelemetryImpl: TelemetryImpl::start() spins up an OTLP HTTP + exporter with background export threads and network connect attempts, which + is unsuitable for a hermetic unit test. The design spec permits testing the + metrics unlock at the provider level, which is the exact path the beast shim + and getMeter() delegate to. + + Compiled only when XRPL_ENABLE_TELEMETRY is defined; the metrics API and SDK + headers exist only in that build. +*/ + +#ifdef XRPL_ENABLE_TELEMETRY + +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +using namespace xrpl; +using namespace xrpl::telemetry; + +namespace metrics_api = opentelemetry::metrics; +namespace metrics_sdk = opentelemetry::sdk::metrics; + +// --------------------------------------------------------------- +// 1. Disabled path: Telemetry::getMeter() returns a usable noop meter. +// makeTelemetry() with enabled=false yields NullTelemetryOtel, whose +// getMeter() serves a meter from a process-wide NoopMeterProvider. The +// meter and any instrument built from it must be non-null, and Add() +// (positive and negative) must be inert but safe. +// --------------------------------------------------------------- +TEST(GetMeter, disabled_path_returns_usable_noop_meter) +{ + Telemetry::Setup setup; + setup.enabled = false; + + beast::Journal::Sink& sink = beast::Journal::getNullSink(); + beast::Journal const journal(sink); + auto telemetry = makeTelemetry(setup, journal); + ASSERT_NE(telemetry, nullptr); + + // getMeter() must return a concrete (non-null) meter, never a raw nullptr. + auto meter = telemetry->getMeter(); + ASSERT_TRUE(static_cast(meter)); + + // A synchronous UpDownCounter — one of the three instrument kinds the + // direct API unlocks — must be creatable and non-null. + auto upDown = meter->CreateInt64UpDownCounter("test_open_ledgers"); + ASSERT_TRUE(static_cast(upDown)); + + // Both increment and decrement paths must execute without error (inert on + // the noop instrument, but exercised here to prove both directions work). + upDown->Add(static_cast(1)); + upDown->Add(static_cast(-1)); +} + +// --------------------------------------------------------------- +// 2. Global-provider contract (the beast OTelCollector shim path). +// After a real SDK MeterProvider is registered via SetMeterProvider(), +// GetMeterProvider()->GetMeter(kMeterName, kMeterVersion) — the exact call +// the shim makes — must return a non-null meter, and an UpDownCounter built +// from it must be non-null and accept positive and negative Add(). +// --------------------------------------------------------------- +TEST(GetMeter, global_provider_meter_accepts_updown_counter) +{ + // Preserve and later restore the process-wide provider so this test does + // not leak state into other telemetry tests in the same binary. + auto const previous = metrics_api::Provider::GetMeterProvider(); + + // A views-less SDK MeterProvider with no reader is sufficient to prove the + // API contract: it hands out a real (non-noop) Meter that creates working + // instruments. No exporter/reader means no background threads or network. + std::shared_ptr sdkProvider = + metrics_sdk::MeterProviderFactory::Create(); + metrics_api::Provider::SetMeterProvider( + opentelemetry::nostd::shared_ptr(sdkProvider)); + + // Fetch the meter exactly as the beast shim does: kMeterName + kMeterVersion + // from the global provider. + auto meter = metrics_api::Provider::GetMeterProvider()->GetMeter( + std::string(kMeterName), std::string(kMeterVersion)); + ASSERT_TRUE(static_cast(meter)); + + auto upDown = meter->CreateInt64UpDownCounter("test_in_flight_requests"); + ASSERT_TRUE(static_cast(upDown)); + + // Positive path: value goes up. + upDown->Add(static_cast(1)); + // Negative path: UpDownCounter permits decrement (unlike a plain Counter). + upDown->Add(static_cast(-1)); + + // Restore the previous global provider. + metrics_api::Provider::SetMeterProvider(previous); +} + +#endif // XRPL_ENABLE_TELEMETRY diff --git a/src/xrpld/telemetry/detail/ValidationTracker.cpp b/src/xrpld/telemetry/detail/ValidationTracker.cpp index a3124100d0..ffe8951a3b 100644 --- a/src/xrpld/telemetry/detail/ValidationTracker.cpp +++ b/src/xrpld/telemetry/detail/ValidationTracker.cpp @@ -141,25 +141,11 @@ ValidationTracker::evictOldPending(TimePoint now) } } - // Hard trim if still over limit -- remove reconciled entries that are - // past the late-repair window first, then any reconciled entry as a - // last resort. + // Hard trim if still over limit. The loop above already removed every + // reconciled entry older than the late-repair window, so here we drop + // any remaining reconciled entry as a last resort. if (pending_.size() > kMaxPendingEvents) { - // Pass 1: only entries past late-repair window. - for (auto it = pending_.begin(); - it != pending_.end() && pending_.size() > kMaxPendingEvents;) - { - if (it->second.reconciled && it->second.recordTime < cutoff) - { - it = pending_.erase(it); - } - else - { - ++it; - } - } - // Pass 2: any reconciled entry if still over limit. for (auto it = pending_.begin(); it != pending_.end() && pending_.size() > kMaxPendingEvents;) {