mirror of
https://github.com/XRPLF/rippled.git
synced 2026-09-27 15:28:03 +00:00
fix(telemetry): gate record calls so stop() cannot hit a dead pipeline
stop() destroys the MeterProvider, and with it every View's AggregationConfig. The SDK's SyncMetricStorage keeps a raw pointer to that config, and the call-site statics keep the storage alive, so a histogram record with a first-seen attribute set during the shutdown drain would dereference freed memory. Application::run() stops the registry before the job queue and server handler, so that window is real. phase_ is now atomic and stop() stores Stopped before tearing down. Every XRPL_METRIC_* macro and every record*/increment* method checks recording() (enabled and not stopped) instead of isEnabled(). meter_ is never written after construction, so record threads read it without a lock. Also: an empty [telemetry] service_instance_id now falls back to the node key on both the trace and the metrics side, so one node reports one identity; disablePipeline() uses telemetry::noopMeter(); comments corrected. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
@@ -343,7 +343,13 @@ makeTelemetrySetup(
|
||||
setup.enabled = section.valueOr<int>(key::enabled, 0) != 0;
|
||||
setup.serviceName = section.valueOr<std::string>(key::serviceName, dflt::serviceName);
|
||||
setup.serviceVersion = version;
|
||||
// Match makeMetricsRegistryOptions() in Application.cpp: an empty
|
||||
// configured value is treated as absent and falls back to the node key,
|
||||
// so traces and metrics stamp the same identity. Otherwise one node
|
||||
// reports two identities and every $node filter shows half the series.
|
||||
setup.serviceInstanceId = section.valueOr<std::string>(key::serviceInstanceId, nodePublicKey);
|
||||
if (setup.serviceInstanceId.empty())
|
||||
setup.serviceInstanceId = nodePublicKey;
|
||||
|
||||
setup.tracesEndpoint = section.valueOr<std::string>(key::tracesEndpoint, dflt::tracesEndpoint);
|
||||
setup.metricsEndpoint =
|
||||
|
||||
@@ -60,9 +60,21 @@ public:
|
||||
configure(bool enabled, opentelemetry::nostd::shared_ptr<opentelemetry::metrics::Meter> meter)
|
||||
{
|
||||
enabled_ = enabled;
|
||||
stopped_ = false;
|
||||
meter_ = std::move(meter);
|
||||
}
|
||||
|
||||
/**
|
||||
* Simulate MetricsRegistry::stop(): the recording gate flips closed even
|
||||
* while enabled_ stays true, matching the real class where a call after
|
||||
* stop() must not touch the SDK instrument cache.
|
||||
*/
|
||||
void
|
||||
stop() noexcept
|
||||
{
|
||||
stopped_ = true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Number of times meter() has been consulted, so a test can assert the
|
||||
* create-once (function-local static) and disabled-gating behavior exactly.
|
||||
@@ -79,6 +91,16 @@ public:
|
||||
return enabled_;
|
||||
}
|
||||
|
||||
/**
|
||||
* Mirrors MetricsRegistry::recording(): the macros consult this instead of
|
||||
* isEnabled() so a stopped registry records nothing.
|
||||
*/
|
||||
[[nodiscard]] bool
|
||||
recording() const noexcept
|
||||
{
|
||||
return enabled_ && !stopped_;
|
||||
}
|
||||
|
||||
[[nodiscard]] opentelemetry::nostd::shared_ptr<opentelemetry::metrics::Meter>
|
||||
meter() const noexcept
|
||||
{
|
||||
@@ -92,6 +114,12 @@ private:
|
||||
*/
|
||||
bool enabled_ = true;
|
||||
|
||||
/**
|
||||
* Set by stop() to model the real registry's post-shutdown state:
|
||||
* enabled_ stays true but recording() flips to false.
|
||||
*/
|
||||
bool stopped_ = false;
|
||||
|
||||
/**
|
||||
* Meter handed to the macro; sourced from a bare SDK provider.
|
||||
*/
|
||||
@@ -320,6 +348,28 @@ TEST(MetricMacros, observable_counter_and_updown_register_do_not_crash)
|
||||
EXPECT_EQ(app.registry().meterCalls(), 2);
|
||||
}
|
||||
|
||||
TEST(MetricMacros, stopped_registry_records_nothing)
|
||||
{
|
||||
ScopedBareProvider const bareProvider;
|
||||
FakeApp app;
|
||||
wire(app, /*enabled=*/true);
|
||||
|
||||
// Simulate MetricsRegistry::stop(): recording() flips closed even while
|
||||
// isEnabled() stays true, because the OTel provider is torn down in
|
||||
// stop() and a Record on a stale SDK instrument would deref a dangling
|
||||
// AggregationConfig for a first-seen attribute set.
|
||||
app.registry().stop();
|
||||
ASSERT_TRUE(app.registry().isEnabled());
|
||||
ASSERT_FALSE(app.registry().recording());
|
||||
|
||||
XRPL_METRIC_COUNTER_INC(
|
||||
app, "test_macro_stopped_counter_total", "Counter after stop() must be inert");
|
||||
|
||||
// The recording() gate short-circuits before the create-once static path
|
||||
// runs, so meter() is never consulted.
|
||||
EXPECT_EQ(app.registry().meterCalls(), 0);
|
||||
}
|
||||
|
||||
TEST(MetricMacros, disabled_registry_is_noop)
|
||||
{
|
||||
ScopedBareProvider const bareProvider;
|
||||
|
||||
@@ -322,6 +322,18 @@ TEST(TelemetryConfig, parse_empty_section)
|
||||
EXPECT_TRUE(setup.traceLedger);
|
||||
}
|
||||
|
||||
TEST(TelemetryConfig, empty_service_instance_id_falls_back_to_node_key)
|
||||
{
|
||||
// An empty value is indistinguishable from an unset key on the metrics
|
||||
// side (makeMetricsRegistryOptions falls back to the node key), so the
|
||||
// trace side must do the same. Without this fallback traces stamp "" while
|
||||
// metrics stamp the node key and every $node filter shows half the series.
|
||||
Section section;
|
||||
section.set("service_instance_id", "");
|
||||
auto const setup = telemetry::makeTelemetrySetup(section, "nHUtest123", "2.0.0", 0);
|
||||
EXPECT_EQ(setup.serviceInstanceId, "nHUtest123");
|
||||
}
|
||||
|
||||
TEST(TelemetryConfig, parse_full_section)
|
||||
{
|
||||
// The CA path has to name a real file: with enabled=1 and use_tls=1 the
|
||||
|
||||
@@ -1453,8 +1453,11 @@ ApplicationImp::setup(boost::program_options::variables_map const& cmdline)
|
||||
|
||||
// The metrics resource was fixed at construction, but the tracer resource is
|
||||
// built by start() below, so the stored key still reaches spans if it
|
||||
// differs from the resolved one.
|
||||
if (!config_->section("telemetry").exists("service_instance_id"))
|
||||
// differs from the resolved one. Treat an empty configured value as absent,
|
||||
// matching makeMetricsRegistryOptions() and makeTelemetrySetup(), so the
|
||||
// trace side does not keep an empty instance id while the metrics side
|
||||
// holds the node key.
|
||||
if (config_->section("telemetry").valueOr<std::string>("service_instance_id", "").empty())
|
||||
telemetry_->setServiceInstanceId(toBase58(TokenType::NodePublic, nodeIdentity_.first));
|
||||
|
||||
// xrpl.node.id always carries the node public key. Unlike
|
||||
|
||||
@@ -96,9 +96,16 @@
|
||||
* MetricsRegistry::meter(). The registry builds that meter in its
|
||||
* constructor, before any subsystem exists, and guarantees it is never
|
||||
* empty while the registry is enabled (a no-op meter stands in if the
|
||||
* pipeline failed to build). So a call site holds a valid instrument from
|
||||
* its first call and needs no check of its own; the only branch on the
|
||||
* hot path is the isEnabled() gate.
|
||||
* pipeline failed to build, and again after stop()). So a call site holds
|
||||
* a valid instrument from its first call and needs no check of its own.
|
||||
* The only branch on the hot path is the recording() gate, which is false
|
||||
* once stop() has torn the pipeline down; without that gate a Record on a
|
||||
* stale SDK instrument would deref a dangling AggregationConfig.
|
||||
*
|
||||
* @note Static-init safety: Meter::CreateXxx is declared noexcept in the
|
||||
* OTel API (opentelemetry/metrics/meter.h), so the function-local static
|
||||
* that caches the instrument cannot throw during first-call construction.
|
||||
* A throw there would call std::terminate.
|
||||
*
|
||||
* @note The OBSERVABLE registration macros are the opposite: call them
|
||||
* EAGERLY, exactly once, from constructor/init code -- never from a hot
|
||||
@@ -136,7 +143,7 @@
|
||||
#define XRPL_METRIC_COUNTER_INC(app, name, description) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
static auto const xrpl_counter_ = \
|
||||
xrpl_mr_->meter()->CreateUInt64Counter((name), (description)); \
|
||||
@@ -151,7 +158,7 @@
|
||||
#define XRPL_METRIC_COUNTER_INC_LABELED(app, name, description, ...) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
static auto const xrpl_counter_ = \
|
||||
xrpl_mr_->meter()->CreateUInt64Counter((name), (description)); \
|
||||
@@ -164,7 +171,7 @@
|
||||
#define XRPL_METRIC_COUNTER_ADD(app, name, description, amount) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
static auto const xrpl_counter_ = \
|
||||
xrpl_mr_->meter()->CreateUInt64Counter((name), (description)); \
|
||||
@@ -177,7 +184,7 @@
|
||||
#define XRPL_METRIC_COUNTER_ADD_LABELED(app, name, description, amount, ...) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
static auto const xrpl_counter_ = \
|
||||
xrpl_mr_->meter()->CreateUInt64Counter((name), (description)); \
|
||||
@@ -194,7 +201,7 @@
|
||||
#define XRPL_METRIC_UPDOWN_ADD(app, name, description, amount) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
static auto const xrpl_updown_ = \
|
||||
xrpl_mr_->meter()->CreateInt64UpDownCounter((name), (description)); \
|
||||
@@ -207,7 +214,7 @@
|
||||
#define XRPL_METRIC_UPDOWN_ADD_LABELED(app, name, description, amount, ...) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
static auto const xrpl_updown_ = \
|
||||
xrpl_mr_->meter()->CreateInt64UpDownCounter((name), (description)); \
|
||||
@@ -218,7 +225,7 @@
|
||||
#define XRPL_METRIC_HISTOGRAM_RECORD(app, name, description, value) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
static auto const xrpl_hist_ = \
|
||||
xrpl_mr_->meter()->CreateDoubleHistogram((name), (description)); \
|
||||
@@ -231,7 +238,7 @@
|
||||
#define XRPL_METRIC_HISTOGRAM_RECORD_LABELED(app, name, description, value, ...) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
static auto const xrpl_hist_ = \
|
||||
xrpl_mr_->meter()->CreateDoubleHistogram((name), (description)); \
|
||||
@@ -257,7 +264,7 @@
|
||||
#define XRPL_METRIC_GAUGE_RECORD(app, name, description, value) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
static auto const xrpl_gauge_ = \
|
||||
xrpl_mr_->meter()->CreateDoubleGauge((name), (description)); \
|
||||
@@ -269,7 +276,7 @@
|
||||
#define XRPL_METRIC_GAUGE_RECORD_LABELED(app, name, description, value, ...) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
static auto const xrpl_gauge_ = \
|
||||
xrpl_mr_->meter()->CreateDoubleGauge((name), (description)); \
|
||||
@@ -317,7 +324,7 @@
|
||||
#define XRPL_METRIC_OBSERVABLE_GAUGE_REGISTER(app, name, description, valueFn) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
auto xrpl_m_ = xrpl_mr_->meter(); \
|
||||
auto* xrpl_fn_ = new std::function<int64_t()>(valueFn); \
|
||||
@@ -342,7 +349,7 @@
|
||||
#define XRPL_METRIC_OBSERVABLE_COUNTER_REGISTER(app, name, description, valueFn) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
auto xrpl_m_ = xrpl_mr_->meter(); \
|
||||
auto* xrpl_fn_ = new std::function<int64_t()>(valueFn); \
|
||||
@@ -367,7 +374,7 @@
|
||||
#define XRPL_METRIC_OBSERVABLE_UPDOWN_REGISTER(app, name, description, valueFn) \
|
||||
do \
|
||||
{ \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->isEnabled()) \
|
||||
if (auto* xrpl_mr_ = (app).getMetricsRegistry(); xrpl_mr_ && xrpl_mr_->recording()) \
|
||||
{ \
|
||||
auto xrpl_m_ = xrpl_mr_->meter(); \
|
||||
auto* xrpl_fn_ = new std::function<int64_t()>(valueFn); \
|
||||
|
||||
@@ -78,7 +78,6 @@
|
||||
#include <opentelemetry/context/context.h>
|
||||
#include <opentelemetry/exporters/otlp/otlp_http_metric_exporter_factory.h>
|
||||
#include <opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h>
|
||||
#include <opentelemetry/metrics/noop.h>
|
||||
#include <opentelemetry/metrics/observer_result.h>
|
||||
#include <opentelemetry/nostd/shared_ptr.h>
|
||||
#include <opentelemetry/nostd/variant.h>
|
||||
@@ -99,6 +98,7 @@
|
||||
#include <chrono>
|
||||
#include <cstddef>
|
||||
#include <cstdint>
|
||||
#include <exception>
|
||||
#include <memory>
|
||||
#include <sstream>
|
||||
#include <string>
|
||||
@@ -248,14 +248,10 @@ void
|
||||
MetricsRegistry::disablePipeline(std::string_view reason)
|
||||
{
|
||||
provider_.reset();
|
||||
// meter_ becomes a no-op meter, which keeps the invariant the
|
||||
// XRPL_METRIC_* macros rely on: an enabled registry always has a meter,
|
||||
// so every call site gets an instrument (a no-op one here) with no check
|
||||
// of its own. Through the base pointer, as Telemetry::getMeter() does:
|
||||
// the no-op provider's override hides the base class's defaulted overload.
|
||||
opentelemetry::nostd::shared_ptr<opentelemetry::metrics::MeterProvider> const noop(
|
||||
new opentelemetry::metrics::NoopMeterProvider());
|
||||
meter_ = noop->GetMeter(std::string(kMeterName), std::string(kMeterVersion));
|
||||
// A no-op meter keeps the invariant the XRPL_METRIC_* macros rely on: an
|
||||
// enabled registry always has a meter, so every call site gets an inert
|
||||
// instrument here with no check of its own.
|
||||
meter_ = noopMeter(kMeterName);
|
||||
JLOG(journal_.error()) << "MetricsRegistry: metrics pipeline failed to initialise, "
|
||||
"continuing without native metrics: "
|
||||
<< reason;
|
||||
@@ -278,23 +274,26 @@ MetricsRegistry::startAsyncGauges()
|
||||
// same-named instruments, and a call after stop() would register on a
|
||||
// provider that is gone. Checked before the pipeline, so a call after
|
||||
// stop() is reported as what it is and not as a build failure.
|
||||
if (phase_ != Phase::Ready)
|
||||
auto const currentPhase = phase_.load(std::memory_order_relaxed);
|
||||
if (currentPhase != Phase::Ready)
|
||||
{
|
||||
JLOG(journal_.warn()) << "MetricsRegistry: startAsyncGauges() called "
|
||||
<< (phase_ == Phase::Stopped ? "after stop()" : "twice")
|
||||
<< (currentPhase == Phase::Stopped ? "after stop()" : "twice")
|
||||
<< "; ignored";
|
||||
return;
|
||||
}
|
||||
|
||||
// The pipeline failed to build: the meter is a no-op, so registering
|
||||
// gauges on it would only log a success that is not one.
|
||||
// gauges on it would only log a success that is not one. phase_ stays
|
||||
// at Ready, so a second call lands here again and logs the same message.
|
||||
// Idempotent.
|
||||
if (!provider_)
|
||||
{
|
||||
JLOG(journal_.warn()) << "MetricsRegistry: startAsyncGauges() without a pipeline; "
|
||||
"no gauges registered";
|
||||
return;
|
||||
}
|
||||
phase_ = Phase::GaugesArmed;
|
||||
phase_.store(Phase::GaugesArmed, std::memory_order_relaxed);
|
||||
|
||||
registerAsyncGauges();
|
||||
|
||||
@@ -462,9 +461,12 @@ void
|
||||
MetricsRegistry::stop()
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
// Idempotent: the destructor calls this after run() or the Application
|
||||
// destructor already did.
|
||||
phase_ = Phase::Stopped;
|
||||
// Store Stopped with release ordering BEFORE the pipeline goes away.
|
||||
// Every recording thread reads phase_ through recording() with acquire
|
||||
// ordering, so any record that has not yet passed the gate will see
|
||||
// Stopped and skip. Idempotent: destructor calls this after run() or
|
||||
// ~ApplicationImp already did.
|
||||
phase_.store(Phase::Stopped, std::memory_order_release);
|
||||
if (!provider_)
|
||||
return;
|
||||
|
||||
@@ -477,11 +479,23 @@ MetricsRegistry::stop()
|
||||
// to detach first.
|
||||
callbacksDetached_.store(true, std::memory_order_release);
|
||||
|
||||
// meter_ is left alone on purpose. Job threads are still running here and
|
||||
// may be inside a macro, so writing meter_ would race with their read.
|
||||
// The recording() gate is what keeps them off the dying pipeline: only the
|
||||
// macros read meter_, and none of them does so once phase_ is Stopped.
|
||||
//
|
||||
// SDK teardown order: Shutdown() stops the PeriodicExportingMetricReader
|
||||
// thread (so no further gauge callbacks fire) and performs the final
|
||||
// collect-and-export drain itself. The trailing ForceFlush() is a
|
||||
// redundant safety net (a no-op once the reader is shut down), then
|
||||
// reset() destroys the provider.
|
||||
//
|
||||
// provider_.reset() destroys MeterProvider -> MeterContext -> ViewRegistry
|
||||
// -> each View -> its shared_ptr<AggregationConfig>. Live SDK
|
||||
// SyncMetricStorage instances cached in call-site statics still hold a
|
||||
// raw AggregationConfig pointer; a Record with a NEW attribute set after
|
||||
// this point would fire the factory lambda and deref that dangling
|
||||
// pointer, and a late meter()->CreateXxx would return null.
|
||||
provider_->Shutdown();
|
||||
provider_->ForceFlush();
|
||||
provider_.reset();
|
||||
@@ -498,7 +512,7 @@ void
|
||||
MetricsRegistry::recordRpcStarted([[maybe_unused]] std::string_view method)
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (!enabled_ || !rpcStartedCounter_)
|
||||
if (!recording() || !rpcStartedCounter_)
|
||||
return;
|
||||
rpcStartedCounter_->Add(1, {{"method", std::string(method)}});
|
||||
#endif
|
||||
@@ -510,7 +524,7 @@ MetricsRegistry::recordRpcFinished(
|
||||
[[maybe_unused]] std::int64_t durationUs)
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (!enabled_ || !rpcFinishedCounter_)
|
||||
if (!recording() || !rpcFinishedCounter_)
|
||||
return;
|
||||
rpcFinishedCounter_->Add(1, {{"method", std::string(method)}});
|
||||
if (rpcDurationHistogram_)
|
||||
@@ -529,7 +543,7 @@ MetricsRegistry::recordRpcErrored(
|
||||
[[maybe_unused]] std::int64_t durationUs)
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (!enabled_ || !rpcErroredCounter_)
|
||||
if (!recording() || !rpcErroredCounter_)
|
||||
return;
|
||||
rpcErroredCounter_->Add(1, {{"method", std::string(method)}});
|
||||
if (rpcDurationHistogram_)
|
||||
@@ -552,7 +566,7 @@ MetricsRegistry::recordJobQueued(
|
||||
[[maybe_unused]] std::string_view jobName)
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (!enabled_ || !jobQueuedCounter_)
|
||||
if (!recording() || !jobQueuedCounter_)
|
||||
return;
|
||||
jobQueuedCounter_->Add(
|
||||
1,
|
||||
@@ -568,7 +582,7 @@ MetricsRegistry::recordJobStarted(
|
||||
[[maybe_unused]] std::int64_t queuedDurUs)
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (!enabled_ || !jobStartedCounter_)
|
||||
if (!recording() || !jobStartedCounter_)
|
||||
return;
|
||||
// Build the attribute pair once: both the counter and the histogram
|
||||
// must carry the identical label set or they cannot be joined.
|
||||
@@ -595,7 +609,7 @@ MetricsRegistry::recordJobFinished(
|
||||
[[maybe_unused]] std::int64_t runningDurUs)
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (!enabled_ || !jobFinishedCounter_)
|
||||
if (!recording() || !jobFinishedCounter_)
|
||||
return;
|
||||
std::string const handler(sanitiseHandler(jobName));
|
||||
jobFinishedCounter_->Add(1, {{kJobTypeLabel, std::string(jobType)}, {kHandlerLabel, handler}});
|
||||
@@ -1732,7 +1746,7 @@ void
|
||||
MetricsRegistry::incrementLedgersClosed()
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (enabled_ && ledgersClosedCounter_)
|
||||
if (recording() && ledgersClosedCounter_)
|
||||
ledgersClosedCounter_->Add(1);
|
||||
#endif
|
||||
}
|
||||
@@ -1741,7 +1755,7 @@ void
|
||||
MetricsRegistry::incrementValidationsSent()
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (enabled_ && validationsSentCounter_)
|
||||
if (recording() && validationsSentCounter_)
|
||||
validationsSentCounter_->Add(1);
|
||||
#endif
|
||||
}
|
||||
@@ -1750,7 +1764,7 @@ void
|
||||
MetricsRegistry::incrementValidationsChecked()
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (enabled_ && validationsCheckedCounter_)
|
||||
if (recording() && validationsCheckedCounter_)
|
||||
validationsCheckedCounter_->Add(1);
|
||||
#endif
|
||||
}
|
||||
@@ -1759,7 +1773,7 @@ void
|
||||
MetricsRegistry::incrementStateChanges()
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (enabled_ && stateChangesCounter_)
|
||||
if (recording() && stateChangesCounter_)
|
||||
stateChangesCounter_->Add(1);
|
||||
#endif
|
||||
}
|
||||
@@ -1768,7 +1782,7 @@ void
|
||||
MetricsRegistry::incrementLedgerHistoryMismatch(std::string_view reason)
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (enabled_ && ledgerHistoryMismatchCounter_)
|
||||
if (recording() && ledgerHistoryMismatchCounter_)
|
||||
ledgerHistoryMismatchCounter_->Add(1, {{"reason", std::string(reason)}});
|
||||
#endif
|
||||
}
|
||||
@@ -1777,7 +1791,7 @@ void
|
||||
MetricsRegistry::incrementTxqExpired()
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (enabled_ && txqExpiredCounter_)
|
||||
if (recording() && txqExpiredCounter_)
|
||||
txqExpiredCounter_->Add(1);
|
||||
#endif
|
||||
}
|
||||
@@ -1786,7 +1800,7 @@ void
|
||||
MetricsRegistry::incrementTxqDropped(std::string_view reason)
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
if (enabled_ && txqDroppedCounter_)
|
||||
if (recording() && txqDroppedCounter_)
|
||||
txqDroppedCounter_->Add(1, {{"reason", std::string(reason)}});
|
||||
#endif
|
||||
}
|
||||
|
||||
@@ -461,6 +461,11 @@ public:
|
||||
/**
|
||||
* Flush pending metrics and shut down the pipeline.
|
||||
*
|
||||
* Stores `Phase::Stopped` first so `recording()` reads false on every
|
||||
* later record call, then destroys the SDK provider. meter_ is not
|
||||
* touched: record threads may still be running, and the gate is what
|
||||
* keeps them off the dying pipeline. Idempotent.
|
||||
*
|
||||
* @pre `detachCallbacks()` should have been called earlier in the
|
||||
* shutdown sequence; otherwise there is a narrow race between
|
||||
* the final reader-thread tick and the destruction of
|
||||
@@ -478,6 +483,28 @@ public:
|
||||
return enabled_;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return true when a record call is safe to run.
|
||||
*
|
||||
* False when the registry is disabled, or after stop() has torn down the
|
||||
* export pipeline. After stop() the SDK's SyncMetricStorage still holds a
|
||||
* raw pointer to an AggregationConfig owned by a destroyed View, so a
|
||||
* record with a first-seen attribute set would fire the factory lambda
|
||||
* and deref that dangling pointer. Every XRPL_METRIC_* macro reads this
|
||||
* once before touching an instrument.
|
||||
*
|
||||
* One acquire atomic load in the hot path.
|
||||
*/
|
||||
[[nodiscard]] bool
|
||||
recording() const noexcept
|
||||
{
|
||||
#ifdef XRPL_ENABLE_TELEMETRY
|
||||
return enabled_ && phase_.load(std::memory_order_acquire) != Phase::Stopped;
|
||||
#else
|
||||
return enabled_;
|
||||
#endif
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------
|
||||
// Synchronous instrument recording (called from PerfLog hot paths)
|
||||
// -----------------------------------------------------------------
|
||||
@@ -849,11 +876,11 @@ public:
|
||||
* counters/histograms can be declared at their call site instead of as
|
||||
* MetricsRegistry members.
|
||||
*
|
||||
* Invariant: never empty while isEnabled() is true. The constructor sets
|
||||
* Invariant: never empty while recording() is true. The constructor sets
|
||||
* it to the real meter, or to a no-op meter when the pipeline failed to
|
||||
* build, so a call site creates its instrument with no check of its own.
|
||||
* Empty only when the registry is disabled, which the macros gate on
|
||||
* first.
|
||||
* build, and never writes it again, so reads need no lock. After stop()
|
||||
* the meter's SDK context is gone; the macros gate on recording() first,
|
||||
* so no caller reaches it then.
|
||||
*
|
||||
* @return The shared Meter.
|
||||
*/
|
||||
@@ -950,13 +977,18 @@ private:
|
||||
* Where the registry is in its life. Construction ends in `Ready`;
|
||||
* startAsyncGauges() moves to `GaugesArmed`; stop() to `Stopped`. A call
|
||||
* that does not fit the current phase logs a warning and does nothing.
|
||||
*
|
||||
* After `Stopped` the SDK pipeline is gone. recording() reads false, so
|
||||
* no macro touches meter_ or a cached instrument.
|
||||
*/
|
||||
enum class Phase { Ready, GaugesArmed, Stopped };
|
||||
|
||||
/**
|
||||
* Current phase; written only from the Application lifecycle thread.
|
||||
* Current phase. Written from the Application lifecycle thread with
|
||||
* release ordering; read from record threads via `recording()` with
|
||||
* acquire ordering, so no record starts once stop() has stored `Stopped`.
|
||||
*/
|
||||
Phase phase_{Phase::Ready};
|
||||
std::atomic<Phase> phase_{Phase::Ready};
|
||||
|
||||
/**
|
||||
* Set by detachCallbacks() during shutdown so every ObservableGauge
|
||||
|
||||
Reference in New Issue
Block a user