From 7b845392d4c2d236d0e797667003ba7359d12123 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 3 Sep 2026 16:39:04 +0100 Subject: [PATCH] feat(telemetry): give metrics its own endpoint key One [telemetry] key served both OTLP signals, and the metrics URL was derived from it by suffix-swap: strip a trailing slash, strip a known signal path if present, append the wanted one. Anything not ending /v1/traces therefore posted metrics to the traces path, and the OTLP version was pinned in code where an operator could not reach it. Adds metrics_endpoint alongside traces_endpoint. Both are full URLs used verbatim, so traces and metrics can go to different collectors, or to one whose OTLP paths are not the defaults. signalEndpoint(), kTracesPath and kMetricsPath are gone; nothing derives an endpoint from another. The startup log names both URLs, since with two independent endpoints there was otherwise no way to see where metrics were going. Also drops exporter=otlp_http from the shipped config and the test fixture. No branch in the chain reads an `exporter` key: it was a real Setup member in the first phase-1b implementation, removed when only OTLP/HTTP was wired up, and already deleted from TESTING.md once on the same grounds. --- OpenTelemetryPlan/06-implementation-phases.md | 7 ++-- .../09-data-collection-reference.md | 6 +-- docker/telemetry/integration-test.sh | 2 +- docker/telemetry/xrpld-telemetry.cfg | 2 +- include/xrpl/telemetry/Telemetry.h | 8 ++++ src/libxrpl/telemetry/Telemetry.cpp | 40 ++----------------- src/libxrpl/telemetry/TelemetryConfig.cpp | 4 ++ .../libxrpl/telemetry/TelemetryConfig.cpp | 4 +- 8 files changed, 27 insertions(+), 46 deletions(-) diff --git a/OpenTelemetryPlan/06-implementation-phases.md b/OpenTelemetryPlan/06-implementation-phases.md index afbe1099fa..78baf42202 100644 --- a/OpenTelemetryPlan/06-implementation-phases.md +++ b/OpenTelemetryPlan/06-implementation-phases.md @@ -512,13 +512,14 @@ graph LR server=otel # NEW: uses OTel OTLP metrics exporter # No prefix: it applies on the StatsD path only, not this one. -# Endpoint and auth inherited from [telemetry] section: +# Endpoints and auth come from the [telemetry] section: [telemetry] enabled=1 -endpoint=http://localhost:4318/v1/traces +traces_endpoint=http://localhost:4318/v1/traces +metrics_endpoint=http://localhost:4318/v1/metrics ``` -The `OTelCollector` reads the OTLP endpoint from `[telemetry]` config (replacing `/v1/traces` with `/v1/metrics` for the metrics exporter). No additional config keys needed. +Each signal has its own key and each URL is used verbatim, so an operator can send traces and metrics to different collectors, or to one whose OTLP paths are not the defaults. **Backward compatibility**: `server=statsd` continues to work exactly as before. diff --git a/OpenTelemetryPlan/09-data-collection-reference.md b/OpenTelemetryPlan/09-data-collection-reference.md index 786cc70db5..cbbdde7d17 100644 --- a/OpenTelemetryPlan/09-data-collection-reference.md +++ b/OpenTelemetryPlan/09-data-collection-reference.md @@ -546,9 +546,9 @@ name and maps `.` and space to `_`, and the only place the class reads `prefix_` is its startup log line. Exported names are therefore the lowercased raw names (`jobq_job_count`, `rpc_requests_total`) and the service is identified by the OTel resource `service.name`, not by a name prefix. `endpoint` is read from this section -but likewise reaches only that log line — the real exporter URL is derived inside -`Telemetry::initMetrics()` from `[telemetry] endpoint`, by swapping the trailing -`/v1/traces` for `/v1/metrics`. +but likewise reaches only that log line — the exporter URL comes from +`[telemetry] metrics_endpoint`, which `Telemetry::initMetrics()` uses verbatim. No +endpoint is derived from another. Fallback (StatsD). `StatsDCollector` is still selected by this value, but the stack in `docker/telemetry/` no longer receives it: using this path also requires diff --git a/docker/telemetry/integration-test.sh b/docker/telemetry/integration-test.sh index e055deaeca..d9a95dd063 100755 --- a/docker/telemetry/integration-test.sh +++ b/docker/telemetry/integration-test.sh @@ -331,7 +331,7 @@ ${IPS_FIXED} enabled=1 service_instance_id=Node-${i} traces_endpoint=http://localhost:4318/v1/traces -exporter=otlp_http +metrics_endpoint=http://localhost:4318/v1/metrics batch_size=512 batch_delay_ms=2000 max_queue_size=2048 diff --git a/docker/telemetry/xrpld-telemetry.cfg b/docker/telemetry/xrpld-telemetry.cfg index 5d217c46a9..4c816603e2 100644 --- a/docker/telemetry/xrpld-telemetry.cfg +++ b/docker/telemetry/xrpld-telemetry.cfg @@ -48,7 +48,7 @@ docker/telemetry/data/debug.log enabled=1 service_instance_id=xrpld-standalone traces_endpoint=http://localhost:4318/v1/traces -exporter=otlp_http +metrics_endpoint=http://localhost:4318/v1/metrics batch_size=512 batch_delay_ms=5000 max_queue_size=2048 diff --git a/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h index 8f915b6b41..19d56d3545 100644 --- a/include/xrpl/telemetry/Telemetry.h +++ b/include/xrpl/telemetry/Telemetry.h @@ -201,6 +201,14 @@ public: */ std::string tracesEndpoint = "http://localhost:4318/v1/traces"; + /** + * Full OTLP/HTTP URL where metrics are sent, including the signal path. + * Used verbatim and independent of tracesEndpoint, so an operator can + * point the two signals at different collectors, or at one whose OTLP + * paths are not the defaults. + */ + std::string metricsEndpoint = "http://localhost:4318/v1/metrics"; + /** * Whether to use TLS for the exporter connection. */ diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp index 0fe590b358..09638acd27 100644 --- a/src/libxrpl/telemetry/Telemetry.cpp +++ b/src/libxrpl/telemetry/Telemetry.cpp @@ -83,12 +83,6 @@ namespace xrpl::telemetry { static_assert(kMeterName == beast::insight::kOTelMeterName); static_assert(kMeterVersion == beast::insight::kOTelMeterVersion); -/** - * OTLP/HTTP path per signal, appended by signalEndpoint(). - */ -constexpr std::string_view kTracesPath{"/v1/traces"}; -constexpr std::string_view kMetricsPath{"/v1/metrics"}; - /** * Metric export cadence. The interval matches the 1 s scrape the dashboards * assume; the timeout bounds a stalled collector. @@ -385,35 +379,6 @@ class TelemetryImpl : public Telemetry * * @note Throws whatever the SDK factories throw; the constructor catches. */ - /** - * @brief Full OTLP/HTTP URL for one signal. - * - * `[telemetry] endpoint` is one setting but OTLP/HTTP has a path per - * signal, so both are derived from it by the same rule: drop a trailing - * slash, drop a signal path if one is already there, then append the path - * asked for. A bare host, a traces URL and a metrics URL therefore all - * yield the right endpoint for either signal. - * - * @param configured The `[telemetry] endpoint` value. - * @param signalPath Path to append, e.g. kTracesPath. - * @return Endpoint URL for that signal. - */ - [[nodiscard]] static std::string - signalEndpoint(std::string_view configured, std::string_view signalPath) - { - while (configured.ends_with('/')) - configured.remove_suffix(1); - - for (auto const known : {kTracesPath, kMetricsPath}) - { - if (configured.ends_with(known)) - { - configured.remove_suffix(known.size()); - break; - } - } - return std::string{configured} + std::string{signalPath}; - } /** * @brief Build the OTLP/HTTP metric exporter. @@ -425,7 +390,7 @@ class TelemetryImpl : public Telemetry makeMetricExporter() const { otlp_http::OtlpHttpMetricExporterOptions opts; - opts.url = signalEndpoint(setup_.tracesEndpoint, kMetricsPath); + opts.url = setup_.metricsEndpoint; if (setup_.useTls) { opts.ssl_ca_cert_path = setup_.tlsCertPath; @@ -536,11 +501,12 @@ public: start() override { JLOG(journal_.info()) << "Telemetry starting: traces_endpoint=" << setup_.tracesEndpoint + << " metrics_endpoint=" << setup_.metricsEndpoint << " sampling=" << setup_.samplingRatio; // Configure OTLP HTTP exporter otlp_http::OtlpHttpExporterOptions exporterOpts; - exporterOpts.url = signalEndpoint(setup_.tracesEndpoint, kTracesPath); + exporterOpts.url = setup_.tracesEndpoint; if (setup_.useTls) { exporterOpts.ssl_ca_cert_path = setup_.tlsCertPath; diff --git a/src/libxrpl/telemetry/TelemetryConfig.cpp b/src/libxrpl/telemetry/TelemetryConfig.cpp index 142b86b317..e8a68f3ab6 100644 --- a/src/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/libxrpl/telemetry/TelemetryConfig.cpp @@ -36,6 +36,7 @@ constexpr char const* enabled = "enabled"; constexpr char const* serviceName = "service_name"; constexpr char const* serviceInstanceId = "service_instance_id"; constexpr char const* tracesEndpoint = "traces_endpoint"; +constexpr char const* metricsEndpoint = "metrics_endpoint"; constexpr char const* useTls = "use_tls"; constexpr char const* tlsCaCert = "tls_ca_cert"; constexpr char const* tlsClientCert = "tls_client_cert"; @@ -61,6 +62,7 @@ constexpr char const* traceLedger = "trace_ledger"; namespace dflt { constexpr char const* serviceName = "xrpld"; constexpr char const* tracesEndpoint = "http://localhost:4318/v1/traces"; +constexpr char const* metricsEndpoint = "http://localhost:4318/v1/metrics"; constexpr std::uint32_t batchSize = 512u; constexpr std::uint32_t batchDelayMs = 5000u; constexpr std::uint32_t maxQueueSize = 2048u; @@ -137,6 +139,8 @@ makeTelemetrySetup( setup.serviceInstanceId = section.valueOr(key::serviceInstanceId, nodePublicKey); setup.tracesEndpoint = section.valueOr(key::tracesEndpoint, dflt::tracesEndpoint); + setup.metricsEndpoint = + section.valueOr(key::metricsEndpoint, dflt::metricsEndpoint); setup.useTls = section.valueOr(key::useTls, 0) != 0; setup.tlsCertPath = section.valueOr(key::tlsCaCert, ""); diff --git a/src/tests/libxrpl/telemetry/TelemetryConfig.cpp b/src/tests/libxrpl/telemetry/TelemetryConfig.cpp index 556d2a3710..137ac633b2 100644 --- a/src/tests/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/tests/libxrpl/telemetry/TelemetryConfig.cpp @@ -116,6 +116,7 @@ TEST(TelemetryConfig, setup_defaults) EXPECT_TRUE(s.serviceVersion.empty()); EXPECT_TRUE(s.serviceInstanceId.empty()); EXPECT_EQ(s.tracesEndpoint, "http://localhost:4318/v1/traces"); + EXPECT_EQ(s.metricsEndpoint, "http://localhost:4318/v1/metrics"); EXPECT_FALSE(s.useTls); EXPECT_TRUE(s.tlsCertPath.empty()); EXPECT_DOUBLE_EQ(s.samplingRatio, 1.0); @@ -158,8 +159,8 @@ TEST(TelemetryConfig, parse_full_section) section.set("enabled", "1"); section.set("service_name", "my-rippled"); section.set("service_instance_id", "custom-id"); - section.set("exporter", "otlp_http"); section.set("traces_endpoint", "http://collector:4318/v1/traces"); + section.set("metrics_endpoint", "http://collector:4318/v1/metrics"); section.set("use_tls", "1"); section.set("tls_ca_cert", caCert); section.set("batch_size", "256"); @@ -177,6 +178,7 @@ TEST(TelemetryConfig, parse_full_section) EXPECT_EQ(setup.serviceName, "my-rippled"); EXPECT_EQ(setup.serviceInstanceId, "custom-id"); EXPECT_EQ(setup.tracesEndpoint, "http://collector:4318/v1/traces"); + EXPECT_EQ(setup.metricsEndpoint, "http://collector:4318/v1/metrics"); EXPECT_TRUE(setup.useTls); EXPECT_EQ(setup.tlsCertPath, caCert); EXPECT_EQ(setup.batchSize, 256u);