metric bug fixes

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

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 ""

src/libxrpl/beast/insight/OTelCollector.cpp

@@ -55,6 +55,7 @@
 
 #include <algorithm>
 #include <atomic>
+#include <cctype>
 #include <chrono>
 #include <cstdint>
 #include <memory>
@@ -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<metrics_api::Meter> 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<metrics_api::Meter> const&
-OTelCollectorImp::otelMeter() const
-{
-    return otelMeter_;
-}
-
 std::string
 OTelCollectorImp::formatName(std::string const& name) const
 {

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<trace_api::TracerProvider>(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<int64_t>(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<metrics_sdk::ViewRegistry>(), 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<metrics_sdk::HistogramAggregationConfig>();
         histogramConfig->boundaries_ =
             std::vector<double>{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<metrics_api::MeterProvider>(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

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

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<double>(queuedDurUs),
             {{"job_type", std::string(jobType)}},