diff --git a/docker/telemetry/integration-test.sh b/docker/telemetry/integration-test.sh index 92fdc374bd..b4401d5906 100755 --- a/docker/telemetry/integration-test.sh +++ b/docker/telemetry/integration-test.sh @@ -628,22 +628,22 @@ check_otel_metric() { } # Node health gauges (ObservableGauge — no _total suffix) -check_otel_metric "rippled_LedgerMaster_Validated_Ledger_Age" -check_otel_metric "rippled_LedgerMaster_Published_Ledger_Age" -check_otel_metric "rippled_job_count" +check_otel_metric "ledgermaster_validated_ledger_age" +check_otel_metric "ledgermaster_published_ledger_age" +check_otel_metric "job_count" # State accounting -check_otel_metric "rippled_State_Accounting_Full_duration" +check_otel_metric "state_accounting_full_duration" # Peer finder -check_otel_metric "rippled_Peer_Finder_Active_Inbound_Peers" -check_otel_metric "rippled_Peer_Finder_Active_Outbound_Peers" +check_otel_metric "peer_finder_active_inbound_peers" +check_otel_metric "peer_finder_active_outbound_peers" # RPC counters (Counter — Prometheus adds _total suffix automatically) -check_otel_metric "rippled_rpc_requests_total" +check_otel_metric "rpc_requests_total" # Overlay traffic -check_otel_metric "rippled_total_Bytes_In" +check_otel_metric "total_bytes_in" # Verify StatsD receiver is NOT required (no statsd receiver in pipeline) log "" diff --git a/src/libxrpl/beast/insight/OTelCollector.cpp b/src/libxrpl/beast/insight/OTelCollector.cpp index 5e6509ff52..0abb7191e8 100644 --- a/src/libxrpl/beast/insight/OTelCollector.cpp +++ b/src/libxrpl/beast/insight/OTelCollector.cpp @@ -55,6 +55,7 @@ #include #include +#include #include #include #include @@ -472,13 +473,6 @@ public: removeGauge(OTelGaugeImpl* gauge); /** @} */ - /** - * @brief Get the OTel Meter instance for creating instruments. - * @return Shared pointer to the OTel Meter. - */ - opentelemetry::nostd::shared_ptr const& - otelMeter() const; - /** * @brief Format a metric name with the configured prefix. * @@ -813,12 +807,6 @@ OTelCollectorImp::removeGauge(OTelGaugeImpl* gauge) std::erase(gauges_, gauge); } -opentelemetry::nostd::shared_ptr const& -OTelCollectorImp::otelMeter() const -{ - return otelMeter_; -} - std::string OTelCollectorImp::formatName(std::string const& name) const { diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp index 72447c0a33..bdabaa2699 100644 --- a/src/libxrpl/telemetry/Telemetry.cpp +++ b/src/libxrpl/telemetry/Telemetry.cpp @@ -299,6 +299,21 @@ class TelemetryImpl : public Telemetry public: TelemetryImpl(Setup setup, beast::Journal journal) : setup_(std::move(setup)), journal_(journal) { + // Build the metrics pipeline NOW, in the constructor, so the global + // MeterProvider is published before any subsystem is constructed. + // beast::insight instruments are created eagerly in subsystem + // constructors (e.g. LedgerMaster, NetworkOPs, ServerHandler), which + // run during ApplicationImp's member-init list — long before start(). + // opentelemetry-cpp has no proxy MeterProvider, so an instrument + // created before SetMeterProvider() binds to the noop provider forever. + // Tracing does not have this problem because getTracer() is called + // fresh at each span creation (runtime, after start()). + // + // The metrics resource uses setup_.serviceInstanceId as provided by + // config. A later setServiceInstanceId() (node-key fallback) cannot + // change this immutable resource, so operators relying on the node-key + // identity should set [telemetry] service_instance_id explicitly. + initMetrics(); } void @@ -368,10 +383,29 @@ 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. + // The metrics pipeline (meterProvider_) was already built and published + // in the constructor via initMetrics(), before any subsystem could + // create a beast::insight instrument. See initMetrics(). + // Register as the global Telemetry instance so SpanGuard factory + // methods can access it without callers passing a reference. + Telemetry::setInstance(this); + + JLOG(journal_.info()) << "Telemetry started successfully"; + } + + /** Build and publish the metrics pipeline (MeterProvider + periodic + reader + OTLP exporter + histogram view). + + Called from the constructor, NOT start(), so the global MeterProvider + exists before subsystems construct their beast::insight instruments + during ApplicationImp's member-init list. The metrics resource uses + setup_.serviceInstanceId from config; it is immutable once the provider + is built, so a later node-key setServiceInstanceId() does not affect it. + */ + void + initMetrics() + { // 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. @@ -405,24 +439,44 @@ public: auto reader = metrics_sdk::PeriodicExportingMetricReaderFactory::Create( std::move(metricExporter), readerOpts); + // Metrics resource: same attributes as the tracer resource so metrics + // and traces share one identity. Built here (not shared with start()) + // because start() runs later; serviceInstanceId comes from config. + auto resourceAttrs = resource::Resource::Create({ + {opentelemetry::semconv::service::kServiceName, setup_.serviceName}, + {opentelemetry::semconv::service::kServiceVersion, setup_.serviceVersion}, + {opentelemetry::semconv::service::kServiceInstanceId, setup_.serviceInstanceId}, + {std::string(attr::networkId), static_cast(setup_.networkId)}, + {std::string(attr::networkType), setup_.networkType}, + }); + // 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. + // histogram instruments align with the collector's SpanMetrics. The + // view is created with an EMPTY name so it applies the buckets WITHOUT + // renaming instruments — a non-empty view name would collapse every + // matching histogram (ios_latency, rpc_size, rpc_time, pathfind_*) + // into a single series under that one name. auto histogramSelector = metrics_sdk::InstrumentSelectorFactory::Create( metrics_sdk::InstrumentType::kHistogram, "*", "ms"); - auto meterSelector = metrics_sdk::MeterSelectorFactory::Create("metrics", "", ""); + // Meter selector MUST match the meter name used by getMeter() and the + // beast OTelCollector (kMeterName = "xrpld"); otherwise this histogram + // view never applies and duration histograms fall back to the SDK + // default boundaries instead of these SpanMetrics-aligned buckets. + auto meterSelector = + metrics_sdk::MeterSelectorFactory::Create(std::string(kMeterName), "", ""); 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", + "", // empty name: keep each instrument's own name, only set buckets + "SpanMetrics-compatible histogram buckets", metrics_sdk::AggregationType::kHistogram, - std::move(histogramConfig)); + histogramConfig); meterProvider_->AddView( std::move(histogramSelector), std::move(meterSelector), std::move(histogramView)); @@ -431,12 +485,6 @@ public: // 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); - - JLOG(journal_.info()) << "Telemetry started successfully"; } void diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index 8dda41efcc..f7c3dddfaa 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -1549,22 +1549,16 @@ ApplicationImp::start(bool withTimers) setEntropyTimer(); } - io_latency_sampler_.start(); - resolver_->start(); - loadManager_->start(); - shaMapStore_->start(); - if (overlay_) - overlay_->start(); - - if (grpcServer_->start()) - fixConfigPorts(*config_, {{Sections::kPortGrpc, grpcServer_->getEndpoint()}}); - - ledgerCleaner_->start(); - perfLog_->start(); + // Start telemetry before the other services so the tracer is live and + // their startup/early activity can be traced. (The metrics MeterProvider + // is already published in the Telemetry constructor — before subsystems + // create their beast::insight instruments during ApplicationImp init — so + // start() ordering does not affect metrics; only tracing benefits here.) telemetry_->start(); - // Start the metrics pipeline after telemetry; the endpoint uses the - // same base URL but the /v1/metrics path. + // Start the metrics pipeline right after telemetry, before subsystems + // begin recording; the endpoint uses the same base URL with the + // /v1/metrics path. if (metricsRegistry_) { auto const& section = config_->section("telemetry"); @@ -1580,6 +1574,19 @@ ApplicationImp::start(bool withTimers) metricsRegistry_->start(endpoint, instanceId); } + + io_latency_sampler_.start(); + resolver_->start(); + loadManager_->start(); + shaMapStore_->start(); + if (overlay_) + overlay_->start(); + + if (grpcServer_->start()) + fixConfigPorts(*config_, {{Sections::kPortGrpc, grpcServer_->getEndpoint()}}); + + ledgerCleaner_->start(); + perfLog_->start(); } void diff --git a/src/xrpld/telemetry/MetricsRegistry.cpp b/src/xrpld/telemetry/MetricsRegistry.cpp index 73c40ffbef..f970ea109d 100644 --- a/src/xrpld/telemetry/MetricsRegistry.cpp +++ b/src/xrpld/telemetry/MetricsRegistry.cpp @@ -388,8 +388,12 @@ MetricsRegistry::recordJobStarted(std::string_view jobType, std::int64_t queuedD if (!enabled_ || !jobStartedCounter_) return; jobStartedCounter_->Add(1, {{"job_type", std::string(jobType)}}); - if (jobQueuedDurationHistogram_) + if (jobQueuedDurationHistogram_ && queuedDurUs >= 0) { + // Guard against negative queued durations: the caller derives this + // from a steady-clock delta that can go slightly negative under clock + // skew or reordering. The OTel SDK rejects negative histogram values + // (logging a warning per call), so skip them rather than spam. jobQueuedDurationHistogram_->Record( static_cast(queuedDurUs), {{"job_type", std::string(jobType)}},