diff --git a/OpenTelemetryPlan/05-configuration-reference.md b/OpenTelemetryPlan/05-configuration-reference.md index 8aaf1b4e31..d1b9656632 100644 --- a/OpenTelemetryPlan/05-configuration-reference.md +++ b/OpenTelemetryPlan/05-configuration-reference.md @@ -18,7 +18,7 @@ The authoritative `[telemetry]` example lives in `cfg/xrpld-example.cfg`. Teleme | Option | Type | Default | Description | | -------------------------- | ------ | --------------------------------- | ---------------------------------------------------------------------------------------------------------- | | `enabled` | 0 or 1 | `0` | Enable/disable telemetry | -| `endpoint` | string | `http://localhost:4318/v1/traces` | OTLP/HTTP collector endpoint | +| `traces_endpoint` | string | `http://localhost:4318/v1/traces` | Full OTLP/HTTP URL for spans, used verbatim | | `use_tls` | 0 or 1 | `0` | Enable TLS for exporter connection | | `tls_ca_cert` | string | `""` | Path to CA certificate file | | `tls_client_cert` | string | `""` | Client cert (PEM) for mTLS; empty = one-way; if `enabled=1`, needs key + `use_tls=1` or startup fails | diff --git a/OpenTelemetryPlan/09-data-collection-reference.md b/OpenTelemetryPlan/09-data-collection-reference.md index 3fb1b0d349..be92d8c872 100644 --- a/OpenTelemetryPlan/09-data-collection-reference.md +++ b/OpenTelemetryPlan/09-data-collection-reference.md @@ -1058,7 +1058,7 @@ endpoint=http://localhost:4318/v1/metrics ```ini [telemetry] enabled=1 -endpoint=http://otel-collector:4318/v1/traces +traces_endpoint=http://otel-collector:4318/v1/traces trace_peer=0 batch_size=1024 max_queue_size=4096 diff --git a/docker/telemetry/TESTING.md b/docker/telemetry/TESTING.md index d6404c0ee1..01a3fc90cb 100644 --- a/docker/telemetry/TESTING.md +++ b/docker/telemetry/TESTING.md @@ -34,8 +34,10 @@ The binary is at `.build/xrpld`. ## Test 1: Single-Node Standalone (Quick Verification) -This test verifies RPC and transaction spans in standalone mode. Consensus -spans will not fire because standalone mode does not run consensus. +This test verifies RPC and transaction spans in standalone mode, plus the +consensus spans that a simulated round still produces. The proposal, voting +and peer-facing consensus spans do not fire — see the expected-spans table at +the end of this test for which do and which do not. ### Step 1: Start the observability stack @@ -113,32 +115,7 @@ curl -s http://localhost:5005 -d '{"method":"ledger_accept"}' ### Step 5: Verify traces in Tempo -Wait 5 seconds for the batch export, then: - -```bash -TEMPO="http://localhost:3200" - -# Check xrpld service is registered -curl -s "$TEMPO/api/v2/search/tag/resource.service.name/values" | jq '.tagValues[].value' - -# Check RPC spans -curl -s "$TEMPO/api/search" \ - --data-urlencode 'q={resource.service.name="xrpld" && name="rpc.http_request"}' \ - --data-urlencode 'limit=5' | jq '.traces | length' - -curl -s "$TEMPO/api/search" \ - --data-urlencode 'q={resource.service.name="xrpld" && name="rpc.process"}' \ - --data-urlencode 'limit=5' | jq '.traces | length' - -curl -s "$TEMPO/api/search" \ - --data-urlencode 'q={resource.service.name="xrpld" && name="rpc.command.server_info"}' \ - --data-urlencode 'limit=5' | jq '.traces | length' - -# Check transaction spans -curl -s "$TEMPO/api/search" \ - --data-urlencode 'q={resource.service.name="xrpld" && name="tx.process"}' \ - --data-urlencode 'limit=5' | jq '.traces | length' -``` +Wait 5 seconds for the batch export, then see the "Verification Queries" section below. Its span loop is a superset of what standalone mode produces, so compare its output against the "Expected spans (standalone mode)" table above rather than running a second, narrower set of queries here. Or open Grafana Explore with Tempo datasource: http://localhost:3000 @@ -152,23 +129,24 @@ kill $(pgrep -f 'xrpld.*xrpld-telemetry') docker compose -f docker/telemetry/docker-compose.yml down # Clean xrpld data -rm -rf data/ +rm -rf docker/telemetry/data/ ``` ### Expected spans (standalone mode) -| Span Name | Expected | Notes | -| --------------------------- | -------- | ----------------------------- | -| `rpc.http_request` | Yes | Every HTTP RPC call | -| `rpc.process` | Yes | Every RPC processing | -| `rpc.command.server_info` | Yes | server_info RPC | -| `rpc.command.server_state` | Yes | server_state RPC | -| `rpc.command.ledger` | Yes | ledger RPC | -| `rpc.command.submit` | Yes | submit RPC | -| `rpc.command.ledger_accept` | Yes | ledger_accept RPC | -| `tx.process` | Yes | Transaction submission | -| `tx.receive` | No | No peers in standalone | -| `consensus.*` | No | Consensus disabled standalone | +| Span Name | Expected | Notes | +| ---------------------------------------------------------------------------------------------------------- | -------- | ------------------------------------------------- | +| `rpc.http_request` | Yes | Every HTTP RPC call | +| `rpc.process` | Yes | Every RPC processing | +| `rpc.command.server_info` | Yes | server_info RPC | +| `rpc.command.server_state` | Yes | server_state RPC | +| `rpc.command.ledger` | Yes | ledger RPC | +| `rpc.command.submit` | Yes | submit RPC | +| `rpc.command.ledger_accept` | Yes | ledger_accept RPC | +| `tx.process` | Yes | Transaction submission | +| `tx.receive` | No | No peers in standalone | +| `consensus.round`, `.phase.open`, `.ledger_close`, `.accept`, `.accept.apply` | Yes | `ledger_accept` drives a simulated round | +| `consensus.establish`, `.update_positions`, `.check`, `.proposal.*`, `.validation.receive`, `.mode_change` | No | `simulate` jumps straight to `Accepted`; no peers | --- @@ -185,17 +163,11 @@ Run the integration test script: bash docker/telemetry/integration-test.sh ``` -The script will: +It checks prerequisites, clears the previous run, brings up the observability stack, generates six validator key pairs and their node configs, starts the nodes, waits for consensus and then for a validated ledger, exercises RPC and submits a transaction, verifies traces in Tempo and both the spanmetrics and the StatsD-derived metrics in Prometheus, then prints a summary and leaves the stack running. -1. Start the observability stack -2. Generate 6 validator key pairs -3. Create config files for each node -4. Start all 6 nodes -5. Wait for consensus ("proposing" state) -6. Exercise RPC, submit transactions -7. Verify all span categories in Tempo -8. Verify spanmetrics in Prometheus -9. Print results and leave the stack running +The script announces each step as it runs, so read its `Step N:` headers for the authoritative sequence — they are not restated here, because a numbered copy of them drifts as soon as a step is added. + +Its Tempo checks cover the RPC, transaction, consensus, ledger and peer span categories from a fixed list, which is narrower than the loop in the "Verification Queries" section below. ### Manual @@ -231,7 +203,7 @@ Kill the temporary node: ```bash kill $TEMP_PID -rm -rf data/ +rm -rf docker/telemetry/data/ ``` #### Step 3: Create node configs @@ -284,7 +256,7 @@ online_delete=256 [telemetry] enabled=1 -endpoint=http://localhost:4318/v1/traces +traces_endpoint=http://localhost:4318/v1/traces batch_size=512 batch_delay_ms=2000 max_queue_size=2048 @@ -359,9 +331,11 @@ curl -s http://localhost:5005 -d '{ "Amount": "10000000" } }] -}' +}' | jq .result.engine_result ``` +Expected result: `"tesSUCCESS"`, the same as Test 1 Step 4. + Wait 15 seconds for consensus and batch export. #### Step 8: Verify in Tempo @@ -372,28 +346,30 @@ See the "Verification Queries" section below. ## Expected Span Catalog -All 16 production span names: +A smoke-test subset: the spans a short local run reliably produces, and how to trigger each. This is not the full catalog — the authoritative span list, with the attributes each span carries, is [docs/telemetry-runbook.md § Span Reference](../../docs/telemetry-runbook.md#span-reference). -| Span Name | Source File | Key Attributes | How to Trigger | -| --------------------------- | ----------------- | ---------------------------------------------------------------------------------------- | ------------------------- | -| `rpc.http_request` | ServerHandler.cpp | -- | Any HTTP RPC call | -| `rpc.ws_upgrade` | ServerHandler.cpp | -- | WebSocket upgrade | -| `rpc.ws_message` | ServerHandler.cpp | -- | WebSocket RPC message | -| `rpc.process` | ServerHandler.cpp | -- | RPC processing | -| `rpc.command.` | RPCHandler.cpp | `xrpl.rpc.command`, `xrpl.rpc.version`, `xrpl.rpc.role` | Any RPC command | -| `tx.process` | NetworkOPs.cpp | `xrpl.tx.hash`, `xrpl.tx.local`, `xrpl.tx.path` | Submit transaction | -| `tx.receive` | PeerImp.cpp | `xrpl.peer.id` | Peer relays transaction | -| `consensus.proposal.send` | RCLConsensus.cpp | `xrpl.consensus.round` | Consensus proposing phase | -| `consensus.ledger_close` | RCLConsensus.cpp | `xrpl.consensus.ledger.seq`, `xrpl.consensus.mode` | Ledger close event | -| `consensus.accept` | RCLConsensus.cpp | `xrpl.consensus.proposers`, `xrpl.consensus.round_time_ms` | Ledger accepted | -| `consensus.validation.send` | RCLConsensus.cpp | `xrpl.consensus.ledger.seq`, `xrpl.consensus.proposing` | Validation sent | -| `consensus.accept.apply` | RCLConsensus.cpp | `xrpl.consensus.close_time`, `close_time_correct`, `close_resolution_ms`, `state` | Ledger apply + close time | -| `tx.apply` | BuildLedger.cpp | `xrpl.ledger.tx_count`, `xrpl.ledger.tx_failed` | Ledger close (tx set) | -| `ledger.build` | BuildLedger.cpp | `xrpl.ledger.seq`, `xrpl.ledger.close_time`, `close_time_correct`, `close_resolution_ms` | Ledger build | -| `ledger.validate` | LedgerMaster.cpp | `xrpl.ledger.seq`, `xrpl.ledger.validations` | Ledger validated | -| `ledger.store` | LedgerMaster.cpp | `xrpl.ledger.seq` | Ledger stored | -| `peer.proposal.receive` | PeerImp.cpp | `xrpl.peer.id`, `xrpl.peer.proposal.trusted` | Peer sends proposal | -| `peer.validation.receive` | PeerImp.cpp | `xrpl.peer.id`, `xrpl.peer.validation.trusted` | Peer sends validation | +Attributes are deliberately not repeated here. Keeping a second copy is how this table came to list attribute keys that no longer exist anywhere in the code. + +| Span Name | Source File | How to Trigger | +| --------------------------- | ----------------- | ------------------------- | +| `rpc.http_request` | ServerHandler.cpp | Any HTTP RPC call | +| `rpc.ws_upgrade` | ServerHandler.cpp | WebSocket upgrade | +| `rpc.ws_message` | ServerHandler.cpp | WebSocket RPC message | +| `rpc.process` | ServerHandler.cpp | RPC processing | +| `rpc.command.` | RPCHandler.cpp | Any RPC command | +| `tx.process` | NetworkOPs.cpp | Submit transaction | +| `tx.receive` | PeerImp.cpp | Peer relays transaction | +| `consensus.proposal.send` | RCLConsensus.cpp | Consensus proposing phase | +| `consensus.ledger_close` | RCLConsensus.cpp | Ledger close event | +| `consensus.accept` | RCLConsensus.cpp | Ledger accepted | +| `consensus.validation.send` | RCLConsensus.cpp | Validation sent | +| `consensus.accept.apply` | RCLConsensus.cpp | Ledger apply + close time | +| `tx.apply` | BuildLedger.cpp | Ledger close (tx set) | +| `ledger.build` | BuildLedger.cpp | Ledger build | +| `ledger.validate` | LedgerMaster.cpp | Ledger validated | +| `ledger.store` | LedgerMaster.cpp | Ledger stored | +| `peer.proposal.receive` | PeerImp.cpp | Peer sends proposal | +| `peer.validation.receive` | PeerImp.cpp | Peer sends validation | --- @@ -433,22 +409,24 @@ Base URL: `http://localhost:9090` ```bash PROM="http://localhost:9090" -# Span call counts (from spanmetrics connector) -curl -s "$PROM/api/v1/query?query=traces_span_metrics_calls_total" | +# Span call counts (from the spanmetrics connector). The span_ prefix is the +# connector's `namespace: "span"` in otel-collector-config.yaml; drop that +# setting and these become traces_span_metrics_*. +curl -s "$PROM/api/v1/query?query=span_calls_total" | jq '.data.result[] | {span: .metric.span_name, count: .value[1]}' # Latency histogram -curl -s "$PROM/api/v1/query?query=traces_span_metrics_duration_milliseconds_count" | +curl -s "$PROM/api/v1/query?query=span_duration_milliseconds_count" | jq '.data.result[] | {span: .metric.span_name, count: .value[1]}' # RPC calls by command -curl -s "$PROM/api/v1/query?query=traces_span_metrics_calls_total{span_name=~\"rpc.command.*\"}" | - jq '.data.result[] | {command: .metric["xrpl.rpc.command"], count: .value[1]}' +curl -s "$PROM/api/v1/query?query=span_calls_total{span_name=~\"rpc.command.*\"}" | + jq '.data.result[] | {command: .metric.command, count: .value[1]}' # Deployment-tier labels present on metrics (set by the collector's # resource/tier processor and promoted via resource_to_telemetry_conversion). # Expect deployment_environment and xrpl_network_type on each series. -curl -s "$PROM/api/v1/query?query=traces_span_metrics_calls_total" | +curl -s "$PROM/api/v1/query?query=span_calls_total" | jq '.data.result[0].metric | {deployment_environment, xrpl_network_type, service_name}' ``` diff --git a/docker/telemetry/docker-compose.yml b/docker/telemetry/docker-compose.yml index 19dde68bee..98f6965c3f 100644 --- a/docker/telemetry/docker-compose.yml +++ b/docker/telemetry/docker-compose.yml @@ -18,7 +18,7 @@ # Configure xrpld to export traces by adding to xrpld.cfg: # [telemetry] # enabled=1 -# endpoint=http://localhost:4318/v1/traces +# traces_endpoint=http://localhost:4318/v1/traces services: # One-shot init for the collector's offset store. Docker creates a fresh diff --git a/docker/telemetry/integration-test.sh b/docker/telemetry/integration-test.sh index 79729bcb57..66c5921fba 100755 --- a/docker/telemetry/integration-test.sh +++ b/docker/telemetry/integration-test.sh @@ -383,7 +383,7 @@ ${IPS_FIXED} [telemetry] enabled=1 service_instance_id=Node-${i} -endpoint=http://localhost:4318/v1/traces +traces_endpoint=http://localhost:4318/v1/traces exporter=otlp_http batch_size=512 batch_delay_ms=2000 @@ -607,15 +607,18 @@ log "--- Spanmetrics ---" log "Waiting 20s for Prometheus scrape cycle..." sleep 20 -calls_count=$(curl -sf "$PROM/api/v1/query?query=traces_span_metrics_calls_total" | +# Names come from the spanmetrics connector's `namespace: "span"` in +# otel-collector-config.yaml. Without that namespace the connector emits +# traces_span_metrics_*, so these queries must move whenever it changes. +calls_count=$(curl -sf "$PROM/api/v1/query?query=span_calls_total" | jq '.data.result | length' 2>/dev/null || echo 0) if [ "$calls_count" -gt 0 ]; then - ok "Prometheus: traces_span_metrics_calls_total ($calls_count series)" + ok "Prometheus: span_calls_total ($calls_count series)" else - fail "Prometheus: traces_span_metrics_calls_total (0 series)" + fail "Prometheus: span_calls_total (0 series)" fi -duration_count=$(curl -sf "$PROM/api/v1/query?query=traces_span_metrics_duration_milliseconds_count" | +duration_count=$(curl -sf "$PROM/api/v1/query?query=span_duration_milliseconds_count" | jq '.data.result | length' 2>/dev/null || echo 0) if [ "$duration_count" -gt 0 ]; then ok "Prometheus: duration histogram ($duration_count series)" @@ -650,23 +653,30 @@ check_otel_metric() { fi } +# Names are what OTelCollector::formatName() produces: the beast::insight +# name lowercased with '.' and ' ' mapped to '_', any group() segment kept, and +# no prefix. The [insight] prefix knob is logged at startup and never applied on +# this path, and the collector's prometheus exporter sets no namespace, so a +# name carrying a product prefix or capitals cannot match any exported series. + # 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_jobq_job_count" +check_otel_metric "ledgermaster_validated_ledger_age" +check_otel_metric "ledgermaster_published_ledger_age" +check_otel_metric "jobq_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" +# Overlay traffic — one series per TrafficCount category; "total" is the +# aggregate category. +check_otel_metric "total_bytes_in" # Verify StatsD receiver is NOT required (no statsd receiver in pipeline) log "" diff --git a/docker/telemetry/xrpld-telemetry.cfg b/docker/telemetry/xrpld-telemetry.cfg index b7346f4bf4..abf49c0f4a 100644 --- a/docker/telemetry/xrpld-telemetry.cfg +++ b/docker/telemetry/xrpld-telemetry.cfg @@ -50,7 +50,7 @@ data/logs/devnet/debug.log [telemetry] enabled=1 service_instance_id=xrpld-standalone -endpoint=http://localhost:4318/v1/traces +traces_endpoint=http://localhost:4318/v1/traces exporter=otlp_http batch_size=512 batch_delay_ms=5000 diff --git a/docs/telemetry-runbook.md b/docs/telemetry-runbook.md index 21dd89481a..6c8cef4275 100644 --- a/docs/telemetry-runbook.md +++ b/docs/telemetry-runbook.md @@ -31,7 +31,7 @@ Add to your `xrpld.cfg`: ```ini [telemetry] enabled=1 -endpoint=http://localhost:4318/v1/traces +traces_endpoint=http://localhost:4318/v1/traces ``` ### 3. Build with telemetry support @@ -47,7 +47,7 @@ cmake --build --preset default | Option | Default | Description | | -------------------------- | --------------------------------- | --------------------------------------------------------- | | `enabled` | `0` | Master switch for telemetry | -| `endpoint` | `http://localhost:4318/v1/traces` | OTLP/HTTP endpoint | +| `traces_endpoint` | `http://localhost:4318/v1/traces` | Full OTLP/HTTP URL for spans, used verbatim | | `service_name` | `xrpld` | OpenTelemetry service name resource attribute | | `service_instance_id` | node public key | OpenTelemetry service instance ID resource attribute | | `trace_rpc` | `1` | Enable RPC request tracing | @@ -174,18 +174,18 @@ hash); `tx.preflight` is stateless and omits both. ### Ledger Spans -| Span Name | Source File | Attributes | Description | -| ----------------- | -------------------- | ------------------------------------- | ----------------------------- | -| `ledger.build` | BuildLedger.cpp:31 | `ledger_seq`, `tx_count`, `tx_failed` | Ledger build during consensus | -| `ledger.validate` | LedgerMaster.cpp:915 | `ledger_seq`, `validations` | Ledger promoted to validated | -| `ledger.store` | LedgerMaster.cpp:409 | `ledger_seq` | Ledger stored in history | +| Span Name | Source File | Attributes | Description | +| ----------------- | ---------------- | ----------------------------------------------------------------------- | ----------------------------- | +| `ledger.build` | BuildLedger.cpp | `ledger_seq`, `close_time`, `close_time_correct`, `close_resolution_ms` | Ledger build during consensus | +| `ledger.validate` | LedgerMaster.cpp | `ledger_seq`, `validations` | Ledger promoted to validated | +| `ledger.store` | LedgerMaster.cpp | `ledger_seq` | Ledger stored in history | ### Peer Spans -| Span Name | Source File | Attributes | Description | -| ------------------------- | ---------------- | ------------------------------- | ----------------------------- | -| `peer.proposal.receive` | PeerImp.cpp:1667 | `peer_id`, `proposal_trusted` | Proposal received from peer | -| `peer.validation.receive` | PeerImp.cpp:2264 | `peer_id`, `validation_trusted` | Validation received from peer | +| Span Name | Source File | Attributes | Description | +| ------------------------- | ----------- | ----------------------------------------------------------------- | ----------------------------- | +| `peer.proposal.receive` | PeerImp.cpp | `peer_id`, `proposal_trusted` | Proposal received from peer | +| `peer.validation.receive` | PeerImp.cpp | `peer_id`, `ledger_hash`, `full_validation`, `validation_trusted` | Validation received from peer | Both peer receive spans are `kConsumer` inbound entry points started as fresh trace roots. They never inherit an ambient span left active on the peer thread, @@ -461,7 +461,7 @@ all its normal attributes, it just lacks a cross-node parent link. {name=~"tx\\..*"} | tx_hash = "" # Find all spans in a cross-node consensus trace -{rootServiceName="xrpld"} | consensus_round_id = 92345679 +{rootServiceName="xrpld"} | consensus_round_id = "" # Compare latency between sender and receiver for validations {name="consensus.validation.send" || name="consensus.validation.receive"} @@ -473,6 +473,8 @@ The OTel Collector's spanmetrics connector automatically derives RED (Rate, Erro ### Generated Metric Names +These names are deliberately generic: the connector emits **one** metric family covering every span, not a metric per span. Which span a series belongs to comes from the `span_name` label, and the rest of the breakdown from the `dimensions` list in `otel-collector-config.yaml`. So a query always names the span in a label selector rather than in the metric name — `span_calls_total{span_name="ledger.build"}`, never a `ledger_build_calls_total`. + | Prometheus Metric | Type | Description | | ----------------------------------- | --------- | ---------------------------- | | `span_calls_total` | Counter | Total span invocations | @@ -480,6 +482,19 @@ The OTel Collector's spanmetrics connector automatically derives RED (Rate, Erro | `span_duration_milliseconds_count` | Histogram | Latency observation count | | `span_duration_milliseconds_sum` | Histogram | Cumulative latency | +Only one part of those names is ours to choose. Reading a name left to right: + +| Part | Set by | +| ------------------------------------- | ------------------------------------------------------------------------------------ | +| `span_` | the connector's `namespace: "span"` in `otel-collector-config.yaml` — **our choice** | +| `calls`, `duration` | the spanmetrics connector's own metric names | +| `_milliseconds` | the Prometheus exporter, expanding the metric's declared unit | +| `_total`, `_bucket`, `_count`, `_sum` | Prometheus conventions for counters and histograms | + +`_milliseconds` rather than `_ms` is therefore not a style decision taken here. The config declares the histogram in milliseconds (`buckets: [1ms, 5ms, ...]`) and never contains the string `milliseconds`; the exporter writes the unit out in full when it translates OTLP to Prometheus. Shortening it would mean renaming the series after export, which would break every dashboard and leave the exported name and the queried name disagreeing. + +Drop the `namespace` setting and these become `traces_span_metrics_*` instead — the connector's default. Any query, dashboard panel or test that names one of these metrics has to move with that setting. + ### Metric Labels Every metric carries these standard labels: diff --git a/include/xrpl/beast/insight/Collector.h b/include/xrpl/beast/insight/Collector.h index c4ba20e3f4..e95d805cc2 100644 --- a/include/xrpl/beast/insight/Collector.h +++ b/include/xrpl/beast/insight/Collector.h @@ -116,9 +116,8 @@ public: * @param unit What the samples measure. */ virtual Event - makeEvent(std::string const& name, Unit unit) + makeEvent(std::string const& name, [[maybe_unused]] Unit unit) { - (void)unit; return makeEvent(name); } @@ -130,7 +129,7 @@ public: return makeEvent(prefix + "." + name); } - Event + [[nodiscard]] Event makeEvent(std::string const& prefix, std::string const& name, Unit unit) { if (prefix.empty()) diff --git a/include/xrpl/beast/insight/OTelCollector.h b/include/xrpl/beast/insight/OTelCollector.h index b93b9bbf50..a84e55cf60 100644 --- a/include/xrpl/beast/insight/OTelCollector.h +++ b/include/xrpl/beast/insight/OTelCollector.h @@ -39,14 +39,27 @@ #include #include +#include namespace beast::insight { +/** + * Instrumentation scope this collector fetches its Meter under. + * + * Must equal xrpl::telemetry::kMeterName and kMeterVersion, or instruments land + * on a different scope than the views. Duplicated because beast sits below the + * telemetry module and cannot include its header; Telemetry.cpp static_asserts + * the two agree. + */ +inline constexpr std::string_view kOTelMeterName{"xrpld"}; +inline constexpr std::string_view kOTelMeterVersion{"1.0.0"}; + /** * @brief A Collector that exports metrics via OpenTelemetry OTLP/HTTP. * - * Replaces StatsD-based metric collection with native OTel Metrics SDK - * instruments. Each beast::insight instrument maps to an OTel equivalent: + * Selected by `[insight] server=otel`, as an alternative to StatsDCollector: + * it exports through the native OTel Metrics SDK rather than the StatsD wire + * format. Each beast::insight instrument maps to an OTel equivalent: * * - Counter -> OTel Counter * - Gauge -> OTel ObservableGauge (async callback) @@ -124,7 +137,7 @@ public: * @param journal Journal for logging. * @return Shared pointer to the created Collector. */ - static std::shared_ptr + [[nodiscard]] static std::shared_ptr // NOLINTNEXTLINE(readability-identifier-naming) New(std::string const& endpoint, std::string const& prefix, diff --git a/include/xrpl/beast/insight/Unit.h b/include/xrpl/beast/insight/Unit.h index 3993663b9b..5697155ec5 100644 --- a/include/xrpl/beast/insight/Unit.h +++ b/include/xrpl/beast/insight/Unit.h @@ -7,24 +7,24 @@ namespace beast::insight { /** * @brief What an Event's samples measure. * - * `Event` documents itself as carrying "a millisecond time, or other integral - * value", but both backends used to assume the first case: the OTel bridge - * declared every instrument with unit `ms`, and StatsD tagged every sample - * `|ms`. A size metric therefore exported under a `_milliseconds` name and - * inherited a latency bucket ladder, which censored a quarter of its samples - * and pinned its p95 to a constant. - * - * Naming the unit at creation time is what lets the OTel bridge pick both the - * instrument unit and the matching bucket ladder: + * `Event` carries "a millisecond time, or other integral value", so the unit + * cannot be inferred from the sample. Naming it at creation time is what lets + * the OTel bridge pick both the instrument unit and the matching bucket + * ladder: * * makeEvent("time", Unit::Millis) --> OTel unit "ms" --> millisecond ladder * makeEvent("size", Unit::Bytes) --> OTel unit "By" --> byte ladder * - * The StatsD backend deliberately ignores this and keeps emitting `|ms` for - * every Event. That path is out of service -- its UDP port is commented out of - * the compose file and the integration test fails if anything is listening on - * 8125 -- so changing its wire format would alter a legacy contract for no - * local benefit and with no way to verify it. + * Without an explicit unit every instrument declares `ms`, so a size metric + * exports under a `_milliseconds` name and inherits a latency bucket ladder. + * For RPC response sizes that ladder censors about a quarter of the samples + * and pins the p95 to a constant. + * + * The StatsD backend deliberately ignores this and emits `|ms` for every + * Event. That path is out of service -- its UDP port is commented out of the + * compose file and the integration test fails if anything is listening on + * 8125 -- so changing its wire format would alter an external protocol + * contract for no local benefit and with no way to verify it. * * @note Adding a member requires extending otelUnitCode(), which switches * exhaustively so a new member is a compile error rather than a silent @@ -53,7 +53,7 @@ enum class Unit : std::uint8_t { * @param unit The unit to translate. * @return A static, null-terminated UCUM code. */ -constexpr char const* +[[nodiscard]] constexpr char const* otelUnitCode(Unit unit) noexcept { switch (unit) @@ -78,7 +78,7 @@ otelUnitCode(Unit unit) noexcept * @param unit The unit to describe. * @return A static, null-terminated description. */ -constexpr char const* +[[nodiscard]] constexpr char const* otelUnitDescription(Unit unit) noexcept { switch (unit) diff --git a/include/xrpl/consensus/Consensus.h b/include/xrpl/consensus/Consensus.h index 78750656a4..aced7555ea 100644 --- a/include/xrpl/consensus/Consensus.h +++ b/include/xrpl/consensus/Consensus.h @@ -1689,8 +1689,9 @@ Consensus::updateOurPositions(std::unique_ptr const& // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above using namespace telemetry; // Child of the establish span via its captured context (establishSpan_ is - // a thread-free SpanGuard, so parent explicitly via its context). Null - // context (establish not started) yields a null guard, same as before. + // a thread-free SpanGuard, so parent explicitly via its context). A null + // context — the establish phase has not started — yields a null guard, so + // the setAttribute calls below are no-ops. auto span = SpanGuard::childSpan(consensus::span::updatePositions, establishSpanContext_); span.setAttribute( consensus::span::attr::convergePercent, static_cast(convergePercent_)); diff --git a/include/xrpl/consensus/ConsensusSpanLabels.h b/include/xrpl/consensus/ConsensusSpanLabels.h index cc66639c41..21b867d128 100644 --- a/include/xrpl/consensus/ConsensusSpanLabels.h +++ b/include/xrpl/consensus/ConsensusSpanLabels.h @@ -3,10 +3,11 @@ /** * Enum-to-label mappings for consensus span attribute values. * - * Split from ConsensusSpanNames.h so that header stays dependency-free like - * its siblings: the span-name and attribute-key constants are included by - * overlay and app translation units that have no use for the consensus - * enums, while these mappings are needed only by Consensus.h. + * These mappings live in their own header so ConsensusSpanNames.h stays + * dependency-free like its siblings: the span-name and attribute-key + * constants are included by overlay and app translation units that have no + * use for the consensus enums, while these mappings are needed only by + * Consensus.h. * * ConsensusSpanNames.h (constants only, no domain deps) * ^ diff --git a/include/xrpl/consensus/ConsensusSpanNames.h b/include/xrpl/consensus/ConsensusSpanNames.h index fc56adcd44..f71b332457 100644 --- a/include/xrpl/consensus/ConsensusSpanNames.h +++ b/include/xrpl/consensus/ConsensusSpanNames.h @@ -59,10 +59,10 @@ * | | * | +-- consensus.accept.apply [jtACCEPT thread, child of accept] * | Created: Adaptor::doAccept() - * | Attrs: ledger_seq, close_time, close_time_correct, + * | Attrs: ledger_seq, close_time_ripple_epoch_s, close_time_correct, * | close_resolution_ms, consensus_state, proposing, round_time_ms, - * | parent_close_time, close_time_self, close_time_vote_bins, - * | resolution_direction, tx_count + * | parent_close_time_ripple_epoch_s, close_time_self_ripple_epoch_s, + * | close_time_vote_bins, resolution_direction, tx_count * | Events: tx.included (per tx, attrs: tx_id) * | * +~~~ consensus.validation.send [jtACCEPT thread, linked] @@ -140,8 +140,8 @@ namespace attr { * concept, same key, distinguished by span name (not an emitter prefix). */ using ::xrpl::telemetry::attr::closeResolutionMs; -using ::xrpl::telemetry::attr::closeTime; using ::xrpl::telemetry::attr::closeTimeCorrect; +using ::xrpl::telemetry::attr::closeTimeRippleEpochS; using ::xrpl::telemetry::attr::fullValidation; using ::xrpl::telemetry::attr::ledgerHash; using ::xrpl::telemetry::attr::ledgerSeq; @@ -232,8 +232,20 @@ inline constexpr auto positionHashPrefix = makeStr("position_hash_prefix"); * "consensus_state" — domain-qualified (collides with other domains' state). */ inline constexpr auto consensusState = makeStr("consensus_state"); -inline constexpr auto parentCloseTime = makeStr("parent_close_time"); -inline constexpr auto closeTimeSelf = makeStr("close_time_self"); +/** + * Close-time instants, both NetClock readings in whole seconds since the XRP + * Ledger epoch (2000-01-01T00:00:00Z) — see `closeTimeRippleEpochS` in + * SpanNames.h for why the epoch is spelled into the key. + * + * `parentCloseTimeRippleEpochS` is the previous ledger's close time; + * `closeTimeSelfRippleEpochS` is this node's own close-time vote for the round, + * so the pair shows how far the node's position sat from the ledger it built on. + * + * `closeTimeVoteBins` is not a time: it holds the number of distinct close-time + * positions seen from peers this round. + */ +inline constexpr auto parentCloseTimeRippleEpochS = makeStr("parent_close_time_ripple_epoch_s"); +inline constexpr auto closeTimeSelfRippleEpochS = makeStr("close_time_self_ripple_epoch_s"); inline constexpr auto closeTimeVoteBins = makeStr("close_time_vote_bins"); inline constexpr auto resolutionDirection = makeStr("resolution_direction"); inline constexpr auto convergePercent = makeStr("converge_percent"); @@ -275,6 +287,14 @@ inline constexpr auto disputesCount = makeStr("disputes_count"); */ inline constexpr auto proposalTrusted = makeStr("proposal_trusted"); inline constexpr auto validationTrusted = makeStr("validation_trusted"); + +/** + * "validation_status" — which exit the inbound validation took. Set once per + * exit, so a dropped validation (microseconds) is separable from a queued one + * (job wait plus checkValidation). Without it the span name reports two + * unrelated latency distributions and every quantile over it is meaningless. + */ +inline constexpr auto validationStatus = makeStr("validation_status"); } // namespace attr // ===== Event names =========================================================== @@ -340,6 +360,10 @@ inline constexpr auto closeAnomaly = makeStr("anomaly"); inline constexpr auto closeOthersClosed = makeStr("others_closed"); inline constexpr auto closeIdle = makeStr("idle"); inline constexpr auto closeNormal = makeStr("normal"); +// validation_status values, one per exit of the inbound validation path. +inline constexpr auto validationQueued = makeStr("queued"); +inline constexpr auto validationDroppedDiverged = makeStr("dropped_diverged"); +inline constexpr auto validationDroppedLoad = makeStr("dropped_load"); } // namespace val } // namespace xrpl::telemetry::consensus::span diff --git a/include/xrpl/telemetry/HistogramBuckets.h b/include/xrpl/telemetry/HistogramBuckets.h index 0f453d54a3..9d67052205 100644 --- a/include/xrpl/telemetry/HistogramBuckets.h +++ b/include/xrpl/telemetry/HistogramBuckets.h @@ -11,10 +11,8 @@ namespace xrpl::telemetry::buckets { * @file HistogramBuckets.h * @brief Explicit histogram bucket edges for xrpld's OTel instruments. * - * One header owns every ladder so a reviewer sees all of them at once and a - * test can assert their invariants. The alternative -- file-local - * `namespace {}` constants at each registration site -- is unreachable from - * any test and lets the ladders drift apart. + * One header owns every ladder, so a reviewer sees all of them together and + * a test can assert their invariants. * * Why a ladder is worth this much care: when a quantile falls in the `+Inf` * bucket, Prometheus returns the *second-highest* edge, not `+Inf`. A @@ -68,11 +66,10 @@ namespace xrpl::telemetry::buckets { * **This list must contain every representable edge of the collector's * spanmetrics ladder, and may extend above it.** Agreement over the shared * range is deliberate: it lets a span-derived latency panel and a native - * histogram panel be read on the same scale. `check_bucket_parity.py` - * machine-checks the containment, because a ladder that agrees only by - * convention drifts the first time one side is extended alone, and a top edge - * below the collector's censors every quantile above it. Add a collector edge, - * add it here too. + * histogram panel be read on the same scale. Drop an edge the collector + * carries and every quantile above it reads back as the top edge instead of + * failing. `check_bucket_parity.py` enforces the containment -- add a + * collector edge, add it here too. * * The sub-millisecond edges the collector carries (0.01 to 0.5 ms) are * deliberately absent. `beast::insight::Event` rounds every duration up to @@ -82,13 +79,12 @@ namespace xrpl::telemetry::buckets { * * The 60 s and 120 s edges exceed the collector's 30 s top on purpose, * because jobs outlive spans: the updatepaths job type was measured - * averaging about 60 s, so a 30 s ceiling would censor its quantiles just - * as 5 s censors them today. All these Events share one ladder, so its - * ceiling has to cover the slowest member rather than the typical one. + * averaging about 60 s, so a 30 s ceiling would censor its quantiles. All + * these Events share one ladder, so its ceiling has to cover the slowest + * member rather than the typical one. * - * The 2, 3 and 4 s edges subdivide the 1 s to 5 s span, so second-scale work - * resolves to about a second rather than being interpolated across a single - * four-second-wide bucket. + * The 2, 3 and 4 s edges resolve second-scale work, which a single + * four-second-wide bucket can only interpolate across. */ inline constexpr std::array kMillisecondBuckets{ 1.0, @@ -148,7 +144,7 @@ inline constexpr std::array kByteBuckets{ * @return true when the ladder is non-empty, starts at or above zero, and * every later edge is strictly greater than its predecessor. */ -constexpr bool +[[nodiscard]] constexpr bool isAscendingNonNegative(std::span ladder) noexcept { if (ladder.empty() || ladder.front() < 0.0) @@ -171,7 +167,7 @@ static_assert(isAscendingNonNegative(kByteBuckets)); * @param ladder Bucket upper bounds. * @return A vector holding the same edges in the same order. */ -inline std::vector +[[nodiscard]] inline std::vector toVector(std::span ladder) { return std::vector(ladder.begin(), ladder.end()); diff --git a/include/xrpl/telemetry/SpanGuard.h b/include/xrpl/telemetry/SpanGuard.h index 95c471f677..9ccfcca607 100644 --- a/include/xrpl/telemetry/SpanGuard.h +++ b/include/xrpl/telemetry/SpanGuard.h @@ -410,8 +410,10 @@ public: * follows-from link. Use to stitch sequential * top-level spans (e.g. consecutive consensus * rounds). Ignored if nullptr or invalid. + * @return An active guard, or a null guard when the category is + * disabled or hashSize is under 16. */ - static SpanGuard + [[nodiscard]] static SpanGuard hashSpan( TraceCategory const cat, std::string_view const name, @@ -431,8 +433,10 @@ public: * @param parentSpanId Pointer to 8 bytes of parent span ID. * @param parentSpanSize Size of parent span ID buffer (must be 8). * @param traceFlags Trace flags from remote context. + * @return An active guard, or a null guard when the category is + * disabled, hashSize is under 16, or parentSpanSize is not 8. */ - static SpanGuard + [[nodiscard]] static SpanGuard hashSpan( TraceCategory const cat, std::string_view const name, diff --git a/include/xrpl/telemetry/SpanNames.h b/include/xrpl/telemetry/SpanNames.h index b848f96c03..c24f0b254b 100644 --- a/include/xrpl/telemetry/SpanNames.h +++ b/include/xrpl/telemetry/SpanNames.h @@ -133,8 +133,19 @@ inline constexpr auto ledgerSeq = makeStr("ledger_seq"); /** * Shared close-time attrs — bare names, reused by consensus and ledger. + * + * `closeTimeRippleEpochS` carries a NetClock reading: whole seconds since the + * XRP Ledger epoch (2000-01-01T00:00:00Z), never the Unix epoch. The key names + * both the unit and the epoch because neither is recoverable from the value. + * A consumer rendering it as wall-clock time must first add kEpochOffset + * (946684800 seconds, see basics/chrono.h); read as a Unix timestamp instead, + * it lands roughly 30 years early. + * + * `closeResolutionMs` is a duration, not an instant — the granularity the + * close time is rounded to. NetClock resolution is whole seconds, so this + * value is always a multiple of 1000. */ -inline constexpr auto closeTime = makeStr("close_time"); +inline constexpr auto closeTimeRippleEpochS = makeStr("close_time_ripple_epoch_s"); inline constexpr auto closeTimeCorrect = makeStr("close_time_correct"); inline constexpr auto closeResolutionMs = makeStr("close_resolution_ms"); /** diff --git a/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h index e5e0d897ad..8f915b6b41 100644 --- a/include/xrpl/telemetry/Telemetry.h +++ b/include/xrpl/telemetry/Telemetry.h @@ -148,7 +148,7 @@ public: * Get the global Telemetry instance. * @return Pointer to the active instance, or nullptr if not started. */ - static Telemetry* + [[nodiscard]] static Telemetry* getInstance() { return instance.load(std::memory_order_acquire); @@ -196,9 +196,10 @@ public: std::string serviceInstanceId; /** - * OTLP/HTTP endpoint URL where spans are sent. + * Full OTLP/HTTP URL where spans are sent, including the signal path. + * Used verbatim: no other endpoint is derived from it. */ - std::string exporterEndpoint = "http://localhost:4318/v1/traces"; + std::string tracesEndpoint = "http://localhost:4318/v1/traces"; /** * Whether to use TLS for the exporter connection. @@ -309,10 +310,9 @@ public: * @param id The node's base58-encoded public key or custom identifier. */ virtual void - setServiceInstanceId(std::string const& id) + setServiceInstanceId([[maybe_unused]] std::string const& id) { // Default no-op for NullTelemetry implementations. - (void)id; } /** @@ -378,7 +378,7 @@ public: * @param name Tracer name used to identify the instrumentation library. * @return A shared pointer to the Tracer. */ - virtual opentelemetry::nostd::shared_ptr + [[nodiscard]] virtual opentelemetry::nostd::shared_ptr getTracer(std::string_view name = kTracerName) = 0; /** @@ -393,7 +393,7 @@ public: * @param name Meter name used to identify the instrumentation scope. * @return A shared pointer to the Meter. */ - virtual opentelemetry::nostd::shared_ptr + [[nodiscard]] virtual opentelemetry::nostd::shared_ptr getMeter(std::string_view name = kMeterName) = 0; /** @@ -411,7 +411,7 @@ public: * - kConsumer: async message receive * @return A shared pointer to the new Span. */ - virtual opentelemetry::nostd::shared_ptr + [[nodiscard]] virtual opentelemetry::nostd::shared_ptr startSpan( std::string_view name, opentelemetry::trace::SpanKind kind = opentelemetry::trace::SpanKind::kInternal) = 0; @@ -427,7 +427,7 @@ public: * @param kind The span kind (defaults to kInternal). * @return A shared pointer to the new Span. */ - virtual opentelemetry::nostd::shared_ptr + [[nodiscard]] virtual opentelemetry::nostd::shared_ptr startSpan( std::string_view name, opentelemetry::context::Context const& parentContext, @@ -486,7 +486,7 @@ makeTelemetrySetup( * @param networkId The network identifier from [network_id] config. * @return "mainnet" (0), "testnet" (1), "devnet" (2), or "unknown". */ -std::string +[[nodiscard]] std::string networkTypeFromId(std::uint32_t networkId); } // namespace xrpl::telemetry diff --git a/include/xrpl/telemetry/TraceContextPropagator.h b/include/xrpl/telemetry/TraceContextPropagator.h index 9933e79292..54305eb355 100644 --- a/include/xrpl/telemetry/TraceContextPropagator.h +++ b/include/xrpl/telemetry/TraceContextPropagator.h @@ -43,7 +43,7 @@ namespace xrpl::telemetry { * @return An OTel Context with the extracted parent span, or an empty * context if the protobuf fields are missing or invalid. */ -inline opentelemetry::context::Context +[[nodiscard]] inline opentelemetry::context::Context extractFromProtobuf(protocol::TraceContext const& proto) { namespace trace = opentelemetry::trace; diff --git a/include/xrpl/telemetry/TraceContextValidation.h b/include/xrpl/telemetry/TraceContextValidation.h index e69b2d17ca..c299ba6cf9 100644 --- a/include/xrpl/telemetry/TraceContextValidation.h +++ b/include/xrpl/telemetry/TraceContextValidation.h @@ -57,7 +57,7 @@ namespace xrpl::telemetry { * @param traceId The raw trace_id bytes from a protobuf TraceContext. * @return true if usable as a trace identifier, false otherwise. */ -inline bool +[[nodiscard]] inline bool isValidTraceId(std::string const& traceId) { return traceId.size() == 16 && std::ranges::any_of(traceId, [](char c) { return c != 0; }); @@ -69,7 +69,7 @@ isValidTraceId(std::string const& traceId) * @param spanId The raw span_id bytes from a protobuf TraceContext. * @return true if usable as a span identifier, false otherwise. */ -inline bool +[[nodiscard]] inline bool isValidSpanId(std::string const& spanId) { return spanId.size() == 8 && std::ranges::any_of(spanId, [](char c) { return c != 0; }); @@ -86,7 +86,7 @@ isValidSpanId(std::string const& spanId) * @param tc The protobuf TraceContext received from a peer. * @return true if both ids are present and valid, false otherwise. */ -inline bool +[[nodiscard]] inline bool isValidTraceContext(protocol::TraceContext const& tc) { return tc.has_trace_id() && isValidTraceId(tc.trace_id()) && tc.has_span_id() && diff --git a/src/libxrpl/beast/insight/OTelCollector.cpp b/src/libxrpl/beast/insight/OTelCollector.cpp index be0311ca78..b81c1824bf 100644 --- a/src/libxrpl/beast/insight/OTelCollector.cpp +++ b/src/libxrpl/beast/insight/OTelCollector.cpp @@ -4,10 +4,9 @@ * * Compiled only when XRPL_ENABLE_TELEMETRY is defined (via CMake * telemetry=ON). Maps beast::insight instruments to OTel SDK instruments - * created on the GLOBAL Meter published by the telemetry module. This class - * is an adapter only: it owns no export pipeline. The MeterProvider, - * PeriodicExportingMetricReader, OTLP exporter and histogram view all live in - * xrpl::telemetry::Telemetry. + * created on the GLOBAL Meter published by the telemetry module. It owns no + * export pipeline of its own: 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. @@ -44,6 +43,7 @@ #include #include #include +#include #include #include @@ -62,9 +62,12 @@ #include #include #include +#include #include #include +#include #include +#include #include #include @@ -176,11 +179,10 @@ private: * The instrument's declared unit is what selects its bucket ladder: the * histogram views registered in Telemetry.cpp match on unit, so a `ms` * instrument gets the millisecond ladder and a `By` instrument the byte - * ladder. The edges themselves live in xrpl/telemetry/HistogramBuckets.h -- - * do not restate them here. An earlier version of this comment listed - * `[1, 5, ..., 1000, 5000] ms` as "matching the SpanMetrics connector"; that - * was true when written and silently became false when the connector's - * ladder was extended, which is why the edges now have one owner. + * ladder. The edges themselves live in xrpl/telemetry/HistogramBuckets.h, + * which is their single owner -- do not restate them here. An edge list copied + * into a comment reads as authoritative and goes stale the moment the + * collector's SpanMetrics ladder is extended, with nothing to flag the drift. * * Thread safety: OTel Histogram::Record() is thread-safe by specification. */ @@ -272,7 +274,7 @@ public: * @brief Return the current gauge value for the OTel callback. * @return The most recently set/incremented value. */ - int64_t + [[nodiscard]] int64_t currentValue() const; OTelGaugeImpl& @@ -285,10 +287,13 @@ public: gaugeCallback(opentelemetry::metrics::ObserverResult result, void* state); /** - * Create the observable instrument and register the callback, once. + * Create the observable instrument and register the callback. * * Called when the collector is told collection is ready, because the * callback reads live application state. + * + * Idempotent. Arming twice would register the callback twice, so callers + * need not check; onCollectionReady() iterates a snapshot and may re-arm. */ void arm(); @@ -380,7 +385,7 @@ private: //------------------------------------------------------------------------------ /** - * @brief Main OTel Collector implementation (adapter over the global Meter). + * @brief Main OTel Collector implementation. * * Obtains its Meter from the GLOBAL MeterProvider owned and published by the * telemetry module (xrpl::telemetry::Telemetry), rather than building its own @@ -389,7 +394,7 @@ private: * * 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. + * the thin adapter that lets beast::insight callers reach it. * * Class diagram: * @@ -447,8 +452,8 @@ public: * the global telemetry pipeline is authoritative for * the actual export endpoint. Used only in the startup * log line. - * @param prefix Legacy metric-name prefix. Not applied to metric - * names; used only in the startup log line. + * @param prefix Metric-name prefix. Not applied to metric names; + * used only in the startup log line. * @param instanceId Value for the service.instance.id resource attribute. * When empty, the attribute is omitted. * @param serviceName Value for the service.name resource attribute. @@ -552,7 +557,7 @@ public: * @brief Get the OTel Meter instance for creating instruments. * @return Shared pointer to the OTel Meter. */ - opentelemetry::nostd::shared_ptr const& + [[nodiscard]] opentelemetry::nostd::shared_ptr const& otelMeter() const; /** @@ -565,8 +570,8 @@ public: * @param name Raw metric name from beast::insight callers. * @return Export-ready metric name. */ - static std::string - formatName(std::string const& name); + [[nodiscard]] static std::string + formatName(std::string_view name); private: /** @@ -650,8 +655,10 @@ OTelCounterImpl::OTelCounterImpl( void OTelCounterImpl::increment(value_type amount) { - // OTel counters require non-negative values. beast::insight CounterImpl - // uses int64_t, so clamp negative values to 0 and cast to uint64_t. + // OTel counters take unsigned deltas only. Assert to catch a decrementing + // caller; skip the Add so a release build under-counts instead of wrapping. + XRPL_ASSERT( + amount >= 0, "beast::insight::detail::OTelCounterImpl::increment : non-negative amount"); if (amount > 0) counter_->Add(static_cast(amount)); } @@ -738,20 +745,25 @@ OTelGaugeImpl::~OTelGaugeImpl() void OTelGaugeImpl::set(value_type value) { - value_.store(static_cast(value), std::memory_order_relaxed); + // value_type is uint64_t, the gauge reports int64_t. Clamp instead of + // wrapping to a negative, which increment() would then floor to 0. + constexpr auto kMax = static_cast(std::numeric_limits::max()); + value_.store(static_cast(std::min(value, kMax)), std::memory_order_relaxed); } void OTelGaugeImpl::increment(difference_type amount) { - // Use compare-exchange loop to safely clamp to [0, MAX]. + // Saturate in [0, INT64_MAX]. Signed overflow is UB, so check the headroom + // before adding. A negative amount cannot underflow: current is never + // negative, so the lowest sum is 0 + INT64_MIN. + constexpr auto kMax = std::numeric_limits::max(); int64_t current = value_.load(std::memory_order_relaxed); int64_t desired = 0; do { - desired = current + amount; - // Clamp to 0 on underflow. - desired = std::max(desired, int64_t{0}); + desired = + (amount > 0 && current > kMax - amount) ? kMax : std::max(current + amount, int64_t{0}); } while (!value_.compare_exchange_weak(current, desired, std::memory_order_relaxed)); } @@ -785,23 +797,19 @@ OTelMeterImpl::increment(value_type amount) OTelCollectorImp::OTelCollectorImp( std::string const& endpoint, std::string prefix, - std::string const& instanceId, - std::string const& serviceName, - std::string const& networkType, + // instanceId/serviceName/networkType are accepted so the New() signature + // stays uniform for callers, but they are not read here: the telemetry + // module owns the resource attributes for the shared metrics pipeline. + [[maybe_unused]] std::string const& instanceId, + [[maybe_unused]] std::string const& serviceName, + [[maybe_unused]] std::string const& networkType, 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. + // real exporter. It is logged here purely as a startup diagnostic. journal_.info() << "OTelCollector starting: endpoint=" << endpoint << " prefix=" << prefix_; } @@ -810,14 +818,10 @@ OTelCollectorImp::OTelCollectorImp( // 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. + // export under one resource identity. The scope must match the telemetry + // module's; see kOTelMeterName in the header. otelMeter_ = metrics_api::Provider::GetMeterProvider()->GetMeter( - std::string{"xrpld"}, std::string{"1.0.0"}); + std::string{kOTelMeterName}, std::string{kOTelMeterVersion}); if (journal_.info()) { @@ -827,12 +831,8 @@ OTelCollectorImp::OTelCollectorImp( OTelCollectorImp::~OTelCollectorImp() { - if (journal_.info()) - { - journal_.info() << "OTelCollector shutting down"; - } - // No pipeline teardown here: the telemetry module owns the global - // MeterProvider lifecycle (ForceFlush/Shutdown happen in Telemetry::stop()). + // Nothing to tear down: the telemetry module owns the global MeterProvider, + // so ForceFlush and Shutdown happen in Telemetry::stop(). if (journal_.info()) { journal_.info() << "OTelCollector stopped"; @@ -998,25 +998,16 @@ OTelCollectorImp::otelMeter() const } std::string -OTelCollectorImp::formatName(std::string const& name) +OTelCollectorImp::formatName(std::string_view name) { - // Produce a clean, lowercase, Prometheus-compatible metric name. - // No prefix — the OTel resource (service.name) identifies the service. - // Dots and spaces become underscores; everything lowercased. - std::string result; - result.reserve(name.size()); - for (char const c : name) - { - if (c == '.' || c == ' ') - { - result += '_'; - } - else - { - result += static_cast(std::tolower(static_cast(c))); - } - } - return result; + // Lowercase, with '.' and ' ' mapped to '_'. No prefix: the service.name + // resource attribute identifies the service. + return name | std::views::transform([](char c) { + return (c == '.' || c == ' ') + ? '_' + : static_cast(std::tolower(static_cast(c))); + }) | + std::ranges::to(); } } // namespace detail diff --git a/src/libxrpl/beast/insight/StatsDCollector.cpp b/src/libxrpl/beast/insight/StatsDCollector.cpp index bc2640ca77..55d8c48e86 100644 --- a/src/libxrpl/beast/insight/StatsDCollector.cpp +++ b/src/libxrpl/beast/insight/StatsDCollector.cpp @@ -167,6 +167,9 @@ private: std::string name_; GaugeImpl::value_type lastValue_{0}; GaugeImpl::value_type value_{0}; + // Start dirty so the initial value (0) is emitted on the first flush. + // Without this, gauges whose value never changes from 0 would never + // appear in downstream metric stores (e.g. Prometheus via StatsD). bool dirty_{true}; }; @@ -599,9 +602,6 @@ StatsDEventImpl::doNotify(EventImpl::value_type const& value) StatsDGaugeImpl::StatsDGaugeImpl(std::string name, std::shared_ptr impl) : impl_(std::move(impl)), name_(std::move(name)) { - // Start dirty so the initial value (0) is emitted on the first flush. - // Without this, gauges whose value never changes from 0 would never - // appear in downstream metric stores (e.g. Prometheus via StatsD). impl_->add(*this); } diff --git a/src/libxrpl/telemetry/NullTelemetry.cpp b/src/libxrpl/telemetry/NullTelemetry.cpp index baa418b19a..e0f00ab3bf 100644 --- a/src/libxrpl/telemetry/NullTelemetry.cpp +++ b/src/libxrpl/telemetry/NullTelemetry.cpp @@ -116,7 +116,7 @@ public: } #ifdef XRPL_ENABLE_TELEMETRY - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr getTracer(std::string_view) override { static auto noopTracer = opentelemetry::nostd::shared_ptr( @@ -124,14 +124,14 @@ public: return noopTracer; } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan(std::string_view, opentelemetry::trace::SpanKind) override { return opentelemetry::nostd::shared_ptr( new opentelemetry::trace::NoopSpan(nullptr)); } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan( std::string_view, opentelemetry::context::Context const&, diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp index fc8928c8ed..0fe590b358 100644 --- a/src/libxrpl/telemetry/Telemetry.cpp +++ b/src/libxrpl/telemetry/Telemetry.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -77,6 +78,24 @@ namespace xrpl::telemetry { +// beast cannot include this header, so it duplicates the meter scope. Fail the +// build if the copies drift: instruments would land off the views' scope. +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. + */ +constexpr auto kMetricExportInterval = std::chrono::milliseconds{1000}; +constexpr auto kMetricExportTimeout = std::chrono::milliseconds{500}; + namespace { namespace trace_api = opentelemetry::trace; @@ -244,7 +263,7 @@ public: return setup_.consensusTraceStrategy; } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr getTracer(std::string_view) override { static auto noopTracer = @@ -252,7 +271,7 @@ public: return noopTracer; } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr getMeter(std::string_view name) override { // Serve a meter from a process-wide noop provider, mirroring the @@ -262,13 +281,13 @@ public: return noopProvider->GetMeter(std::string(name), std::string(kMeterVersion)); } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan(std::string_view, trace_api::SpanKind) override { return opentelemetry::nostd::shared_ptr(new trace_api::NoopSpan(nullptr)); } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan(std::string_view, opentelemetry::context::Context const&, trace_api::SpanKind) override { @@ -366,74 +385,102 @@ 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. + * + * @return Exporter pointed at the metrics endpoint, with the same TLS + * options the trace exporter uses. + */ + [[nodiscard]] auto + makeMetricExporter() const + { + otlp_http::OtlpHttpMetricExporterOptions opts; + opts.url = signalEndpoint(setup_.tracesEndpoint, kMetricsPath); + if (setup_.useTls) + { + opts.ssl_ca_cert_path = setup_.tlsCertPath; + opts.ssl_client_cert_path = setup_.tlsClientCertPath; + opts.ssl_client_key_path = setup_.tlsClientKeyPath; + } + return otlp_http::OtlpHttpMetricExporterFactory::Create(opts); + } + + /** + * @brief Register one histogram view, selected by instrument unit. + * + * The unit is the selector, so an instrument gets the ladder matching what + * it measures and a byte count is never bucketed on a latency ladder. + * + * The view name stays EMPTY: a non-empty one renames every matching + * histogram to it and collapses them into a single series. The meter + * selector must match kMeterName, or the view never applies and + * instruments fall back to the SDK default ladder (ceiling 10,000). + * + * @param unitCode OTel unit code to select on, e.g. "ms". + * @param boundaries Bucket upper bounds, from HistogramBuckets.h. + * @param description Description recorded on the view. + */ + void + addUnitView( + std::string const& unitCode, + std::vector boundaries, + std::string const& description) + { + auto selector = metrics_sdk::InstrumentSelectorFactory::Create( + metrics_sdk::InstrumentType::kHistogram, "*", unitCode); + auto meterSelector = + metrics_sdk::MeterSelectorFactory::Create(std::string(kMeterName), "", ""); + auto config = std::make_shared(); + config->boundaries_ = std::move(boundaries); + auto view = metrics_sdk::ViewFactory::Create( + "", description, metrics_sdk::AggregationType::kHistogram, std::move(config)); + meterProvider_->AddView(std::move(selector), std::move(meterSelector), std::move(view)); + } + 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. - 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); + readerOpts.export_interval_millis = kMetricExportInterval; + readerOpts.export_timeout_millis = kMetricExportTimeout; auto reader = metrics_sdk::PeriodicExportingMetricReaderFactory::Create( - std::move(metricExporter), readerOpts); + makeMetricExporter(), readerOpts); - // Create MeterProvider with the shared resource, then attach reader. meterProvider_ = metrics_sdk::MeterProviderFactory::Create( std::make_unique(), makeResource()); meterProvider_->AddMetricReader(std::move(reader)); - // One histogram view per unit. The unit is the selector, so an - // instrument gets the ladder that fits what it measures -- a byte - // count gets the byte ladder instead of a latency one. Edges come from - // HistogramBuckets.h, which owns every ladder. - // - // Both views keep the "*" name pattern and an EMPTY view name: a - // non-empty view name would rename every matching histogram to it and - // collapse them into a single series. - // - // The meter selector MUST match the meter name used by getMeter() and - // the beast OTelCollector, or a view never applies and instruments - // fall back to the SDK default ladder (ceiling 10,000). - auto const addUnitView = [this]( - std::string const& unitCode, - std::vector boundaries, - std::string const& description) { - auto selector = metrics_sdk::InstrumentSelectorFactory::Create( - metrics_sdk::InstrumentType::kHistogram, "*", unitCode); - auto meterSelector = - metrics_sdk::MeterSelectorFactory::Create(std::string(kMeterName), "", ""); - auto config = std::make_shared(); - config->boundaries_ = std::move(boundaries); - auto view = metrics_sdk::ViewFactory::Create( - "", description, metrics_sdk::AggregationType::kHistogram, std::move(config)); - meterProvider_->AddView(std::move(selector), std::move(meterSelector), std::move(view)); - }; - addUnitView( beast::insight::otelUnitCode(beast::insight::Unit::Millis), buckets::toVector(buckets::kMillisecondBuckets), @@ -443,8 +490,8 @@ class TelemetryImpl : public Telemetry buckets::toVector(buckets::kByteBuckets), "Size buckets, 512 B to 1 MiB"); - // Publish as the global meter provider so developers (and the beast - // OTelCollector shim) reach the same pipeline. + // Publish globally so both direct-API and beast-sourced metrics ride + // one pipeline. metrics_api::Provider::SetMeterProvider( opentelemetry::nostd::shared_ptr(meterProvider_)); } @@ -488,12 +535,12 @@ public: void start() override { - JLOG(journal_.info()) << "Telemetry starting: endpoint=" << setup_.exporterEndpoint + JLOG(journal_.info()) << "Telemetry starting: traces_endpoint=" << setup_.tracesEndpoint << " sampling=" << setup_.samplingRatio; // Configure OTLP HTTP exporter otlp_http::OtlpHttpExporterOptions exporterOpts; - exporterOpts.url = setup_.exporterEndpoint; + exporterOpts.url = signalEndpoint(setup_.tracesEndpoint, kTracesPath); if (setup_.useTls) { exporterOpts.ssl_ca_cert_path = setup_.tlsCertPath; @@ -655,7 +702,7 @@ public: return setup_.consensusTraceStrategy; } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr getTracer(std::string_view name = kTracerName) override { if (!sdkProvider_) @@ -665,7 +712,7 @@ public: return sdkProvider_->GetTracer(std::string(name)); } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr getMeter(std::string_view name = kMeterName) override { if (!meterProvider_) @@ -676,7 +723,7 @@ public: return meterProvider_->GetMeter(std::string(name), std::string(kMeterVersion)); } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan(std::string_view name, trace_api::SpanKind kind) override { auto tracer = getTracer(); @@ -685,7 +732,7 @@ public: return tracer->StartSpan(std::string(name), opts); } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan( std::string_view name, opentelemetry::context::Context const& parentContext, diff --git a/src/libxrpl/telemetry/TelemetryConfig.cpp b/src/libxrpl/telemetry/TelemetryConfig.cpp index be97045332..142b86b317 100644 --- a/src/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/libxrpl/telemetry/TelemetryConfig.cpp @@ -35,7 +35,7 @@ namespace key { constexpr char const* enabled = "enabled"; constexpr char const* serviceName = "service_name"; constexpr char const* serviceInstanceId = "service_instance_id"; -constexpr char const* endpoint = "endpoint"; +constexpr char const* tracesEndpoint = "traces_endpoint"; constexpr char const* useTls = "use_tls"; constexpr char const* tlsCaCert = "tls_ca_cert"; constexpr char const* tlsClientCert = "tls_client_cert"; @@ -60,7 +60,7 @@ constexpr char const* traceLedger = "trace_ledger"; */ namespace dflt { constexpr char const* serviceName = "xrpld"; -constexpr char const* endpoint = "http://localhost:4318/v1/traces"; +constexpr char const* tracesEndpoint = "http://localhost:4318/v1/traces"; constexpr std::uint32_t batchSize = 512u; constexpr std::uint32_t batchDelayMs = 5000u; constexpr std::uint32_t maxQueueSize = 2048u; @@ -136,7 +136,7 @@ makeTelemetrySetup( setup.serviceVersion = version; setup.serviceInstanceId = section.valueOr(key::serviceInstanceId, nodePublicKey); - setup.exporterEndpoint = section.valueOr(key::endpoint, dflt::endpoint); + setup.tracesEndpoint = section.valueOr(key::tracesEndpoint, dflt::tracesEndpoint); setup.useTls = section.valueOr(key::useTls, 0) != 0; setup.tlsCertPath = section.valueOr(key::tlsCaCert, ""); diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index 4eb43d1596..4e6dbf33b2 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include @@ -1624,126 +1625,140 @@ Transactor::operator()() trapTransaction(*trap); } - auto result = ctx_.preclaimResult; - if (isTesSuccess(result)) - result = apply(); - - // No transaction can return temUNKNOWN from apply, - // and it can't be passed in from a preclaim. - XRPL_ASSERT(result != temUNKNOWN, "xrpl::Transactor::operator() : result is not temUNKNOWN"); - - if (auto stream = j_.trace()) - stream << "preclaim result: " << transToken(result); - - auto fee = ctx_.tx.getFieldAmount(sfFee).xrp(); - bool const canApply = std::invoke([&result, &fee, this] { - bool canApplyTmp = isTesSuccess(result); - - if (ctx_.size() > kOversizeMetaDataCap) - result = tecOVERSIZE; - - if (isTecClaim(result) && ((view().flags() & TapFailHard) != 0u)) - { - // If the TapFailHard flag is set, a tec result - // must not do anything - ctx_.discard(); - canApplyTmp = false; - } - else if ( - (result == tecOVERSIZE) || (result == tecKILLED) || (result == tecINCOMPLETE) || - (result == tecEXPIRED) || (isTecClaimHardFail(result, view().flags()))) - { - // This is and must remain the only place where `canApplyTmp` can change from false to - // true. Changing from true to false is no problem. - std::tie(result, fee, canApplyTmp) = processPersistentChanges(result, fee); - } - return canApplyTmp; - }); - - // Every exit from this function funnels through here, so this is also where - // the apply span records its outcome: each return path reports the engine - // result and whether the transaction was applied. - auto const logger = [this, &span]( - TER result, - bool canApply, - std::optional&& metadata = std::nullopt) -> ApplyResult { - JLOG(j_.trace()) << (canApply ? "applied " : "not applied ") << transToken(result); - - // Also guarded: transToken() is a lookup returning a string, and this - // funnel runs on every exit path. - if (span) - { - span.setAttribute( - telemetry::tx_apply_span::attr::terResult, transToken(result).c_str()); - span.setAttribute(telemetry::tx_apply_span::attr::applied, canApply); - // Mark the span as errored when the transaction was not applied or - // the engine result is not a success, so failed applies surface in - // span-status error counts alongside preflight and preclaim. - if (!canApply || !isTesSuccess(result)) - span.setError(transToken(result)); - } - - return {result, canApply, std::move(metadata)}; - }; - - if (!canApply) - return logger(result, canApply); - - // First invariant pass: both protocol and transaction-specific - // checks run against the transaction's tentative outcome. If it - // does not return tecINVARIANT_FAILED, we can proceed to apply the - // tx. - result = checkInvariants(result, fee, InvariantScope::Full); - if (result == tecINVARIANT_FAILED) + try { - // Fee-claim reset: roll the transaction's effects back so that - // only the fee deduction remains. This is the reset referenced - // by InvariantScope::ProtocolOnly. - auto const resetResult = reset(fee); - if (!isTesSuccess(resetResult.first)) - result = resetResult.first; + auto result = ctx_.preclaimResult; + if (isTesSuccess(result)) + result = apply(); - fee = resetResult.second; + // No transaction can return temUNKNOWN from apply, + // and it can't be passed in from a preclaim. + XRPL_ASSERT( + result != temUNKNOWN, "xrpl::Transactor::operator() : result is not temUNKNOWN"); - // Re-check invariants against the post-reset (fee-claim only) - // state. The transaction's effects are gone, so the - // transaction-specific invariants no longer apply and only the - // protocol invariants are re-run. A failure here escalates to - // tefINVARIANT_FAILED and excludes the tx from the ledger. - if (isTesSuccess(result) || isTecClaim(result)) - result = checkInvariants(result, fee, InvariantScope::ProtocolOnly); + if (auto stream = j_.trace()) + stream << "preclaim result: " << transToken(result); + + auto fee = ctx_.tx.getFieldAmount(sfFee).xrp(); + bool const canApply = std::invoke([&result, &fee, this] { + bool canApplyTmp = isTesSuccess(result); + + if (ctx_.size() > kOversizeMetaDataCap) + result = tecOVERSIZE; + + if (isTecClaim(result) && ((view().flags() & TapFailHard) != 0u)) + { + // If the TapFailHard flag is set, a tec result + // must not do anything + ctx_.discard(); + canApplyTmp = false; + } + else if ( + (result == tecOVERSIZE) || (result == tecKILLED) || (result == tecINCOMPLETE) || + (result == tecEXPIRED) || (isTecClaimHardFail(result, view().flags()))) + { + // This is and must remain the only place where `canApplyTmp` can change from false + // to true. Changing from true to false is no problem. + std::tie(result, fee, canApplyTmp) = processPersistentChanges(result, fee); + } + return canApplyTmp; + }); + + // Each return path funnels through here, so this is also where the apply + // span records its outcome: the engine result and whether the transaction + // was applied. A throw bypasses it and ends the span with no outcome. + auto const logger = [this, &span]( + TER result, + bool canApply, + std::optional&& metadata = std::nullopt) -> ApplyResult { + JLOG(j_.trace()) << (canApply ? "applied " : "not applied ") << transToken(result); + + // Also guarded: transToken() is a lookup returning a string, and this + // funnel runs on every return path. + if (span) + { + span.setAttribute( + telemetry::tx_apply_span::attr::terResult, transToken(result).c_str()); + span.setAttribute(telemetry::tx_apply_span::attr::applied, canApply); + // Mark the span as errored when the engine result is not a success, + // so failed applies surface alongside preflight and preclaim. Not + // keyed on `canApply`: a dry run reports tesSUCCESS with canApply + // false, and that is not a failure. + if (!isTesSuccess(result)) + span.setError(transToken(result)); + } + + return {result, canApply, std::move(metadata)}; + }; + + if (!canApply) + return logger(result, canApply); + + // First invariant pass: both protocol and transaction-specific + // checks run against the transaction's tentative outcome. If it + // does not return tecINVARIANT_FAILED, we can proceed to apply the + // tx. + result = checkInvariants(result, fee, InvariantScope::Full); + if (result == tecINVARIANT_FAILED) + { + // Fee-claim reset: roll the transaction's effects back so that + // only the fee deduction remains. This is the reset referenced + // by InvariantScope::ProtocolOnly. + auto const resetResult = reset(fee); + if (!isTesSuccess(resetResult.first)) + result = resetResult.first; + + fee = resetResult.second; + + // Re-check invariants against the post-reset (fee-claim only) + // state. The transaction's effects are gone, so the + // transaction-specific invariants no longer apply and only the + // protocol invariants are re-run. A failure here escalates to + // tefINVARIANT_FAILED and excludes the tx from the ledger. + if (isTesSuccess(result) || isTecClaim(result)) + result = checkInvariants(result, fee, InvariantScope::ProtocolOnly); + } + + // We ran through the invariant checker, which can, in some cases, + // return a tef error code. Don't apply the transaction in that case. + if (!isTecClaim(result) && !isTesSuccess(result)) + return logger(result, false); + + std::optional metadata; + + // Transaction succeeded fully or (retries are not allowed and the + // transaction could claim a fee) + + // The transactor and invariant checkers guarantee that this will + // *never* trigger but if it, somehow, happens, don't allow a tx + // that charges a negative fee. + if (fee < beast::kZero) + Throw("fee charged is negative!"); + + // Charge whatever fee they specified. The fee has already been + // deducted from the balance of the account that issued the + // transaction. We just need to account for it in the ledger + // header. + if (!view().open() && fee != beast::kZero) + ctx_.destroyXRP(fee); + + // Once we call apply, we will no longer be able to look at view() + metadata = ctx_.apply(result); + + if ((ctx_.flags() & TapDryRun) != 0u) + return logger(result, false, std::move(metadata)); + + return logger(result, canApply, std::move(metadata)); + } + catch (std::exception const& e) + { + // The caller's doApply() maps this to tefEXCEPTION. Record it on the + // span before unwinding so per-stage error counts include exceptions. + span.setAttribute( + telemetry::tx_apply_span::attr::terResult, transToken(tefEXCEPTION).c_str()); + span.recordException(e); + throw; } - - // We ran through the invariant checker, which can, in some cases, - // return a tef error code. Don't apply the transaction in that case. - if (!isTecClaim(result) && !isTesSuccess(result)) - return logger(result, false); - - std::optional metadata; - - // Transaction succeeded fully or (retries are not allowed and the - // transaction could claim a fee) - - // The transactor and invariant checkers guarantee that this will - // *never* trigger but if it, somehow, happens, don't allow a tx - // that charges a negative fee. - if (fee < beast::kZero) - Throw("fee charged is negative!"); - - // Charge whatever fee they specified. The fee has already been - // deducted from the balance of the account that issued the - // transaction. We just need to account for it in the ledger - // header. - if (!view().open() && fee != beast::kZero) - ctx_.destroyXRP(fee); - - // Once we call apply, we will no longer be able to look at view() - metadata = ctx_.apply(result); - - if ((ctx_.flags() & TapDryRun) != 0u) - return logger(result, false, std::move(metadata)); - - return logger(result, canApply, std::move(metadata)); } } // namespace xrpl diff --git a/src/libxrpl/tx/applySteps.cpp b/src/libxrpl/tx/applySteps.cpp index 45758bf616..3ec6c25aa8 100644 --- a/src/libxrpl/tx/applySteps.cpp +++ b/src/libxrpl/tx/applySteps.cpp @@ -314,6 +314,10 @@ invokePreclaim(PreclaimContext const& ctx) { span.setAttribute( telemetry::tx_apply_span::attr::terResult, transToken(preclaimTer).c_str()); + // Mark the span as errored when preclaim rejects the transaction so + // failed stages surface in span-status error counts. + if (!isTesSuccess(preclaimTer)) + span.setError(transToken(preclaimTer)); } return preclaimTer; } diff --git a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp index 36ab40b1b6..6cec7a5c86 100644 --- a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp +++ b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp @@ -99,7 +99,7 @@ TEST(SpanGuardFactory, consensus_close_time_attributes) auto span = telemetry::SpanGuard::span( telemetry::TraceCategory::Consensus, telemetry::seg::consensus, "accept.apply"); span.setAttribute("ledger_seq", static_cast(42)); - span.setAttribute("close_time", static_cast(780000000)); + span.setAttribute("close_time_ripple_epoch_s", static_cast(780000000)); span.setAttribute("close_time_correct", true); span.setAttribute("close_resolution_ms", static_cast(30000)); span.setAttribute("consensus_state", std::string("finished")); diff --git a/src/tests/libxrpl/telemetry/TelemetryConfig.cpp b/src/tests/libxrpl/telemetry/TelemetryConfig.cpp index 56a3ef4447..556d2a3710 100644 --- a/src/tests/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/tests/libxrpl/telemetry/TelemetryConfig.cpp @@ -115,7 +115,7 @@ TEST(TelemetryConfig, setup_defaults) EXPECT_EQ(s.serviceName, "xrpld"); EXPECT_TRUE(s.serviceVersion.empty()); EXPECT_TRUE(s.serviceInstanceId.empty()); - EXPECT_EQ(s.exporterEndpoint, "http://localhost:4318/v1/traces"); + EXPECT_EQ(s.tracesEndpoint, "http://localhost:4318/v1/traces"); EXPECT_FALSE(s.useTls); EXPECT_TRUE(s.tlsCertPath.empty()); EXPECT_DOUBLE_EQ(s.samplingRatio, 1.0); @@ -159,7 +159,7 @@ TEST(TelemetryConfig, parse_full_section) section.set("service_name", "my-rippled"); section.set("service_instance_id", "custom-id"); section.set("exporter", "otlp_http"); - section.set("endpoint", "http://collector:4318/v1/traces"); + section.set("traces_endpoint", "http://collector:4318/v1/traces"); section.set("use_tls", "1"); section.set("tls_ca_cert", caCert); section.set("batch_size", "256"); @@ -176,7 +176,7 @@ TEST(TelemetryConfig, parse_full_section) EXPECT_TRUE(setup.enabled); EXPECT_EQ(setup.serviceName, "my-rippled"); EXPECT_EQ(setup.serviceInstanceId, "custom-id"); - EXPECT_EQ(setup.exporterEndpoint, "http://collector:4318/v1/traces"); + EXPECT_EQ(setup.tracesEndpoint, "http://collector:4318/v1/traces"); EXPECT_TRUE(setup.useTls); EXPECT_EQ(setup.tlsCertPath, caCert); EXPECT_EQ(setup.batchSize, 256u); diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index a0e2f6f6d3..460a447631 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -635,7 +635,8 @@ RCLConsensus::Adaptor::doAccept( : telemetry::SpanGuard::childSpan(cs::acceptApply, roundSpanContext_); doAcceptSpan.setAttribute(cs::attr::ledgerSeq, static_cast(prevLedger.seq()) + 1); doAcceptSpan.setAttribute( - cs::attr::closeTime, static_cast(consensusCloseTime.time_since_epoch().count())); + cs::attr::closeTimeRippleEpochS, + static_cast(consensusCloseTime.time_since_epoch().count())); doAcceptSpan.setAttribute(cs::attr::closeTimeCorrect, closeTimeCorrect); doAcceptSpan.setAttribute( cs::attr::closeResolutionMs, @@ -648,10 +649,10 @@ RCLConsensus::Adaptor::doAccept( doAcceptSpan.setAttribute( cs::attr::roundTimeMs, static_cast(result.roundTime.read().count())); doAcceptSpan.setAttribute( - cs::attr::parentCloseTime, + cs::attr::parentCloseTimeRippleEpochS, static_cast(prevLedger.closeTime().time_since_epoch().count())); doAcceptSpan.setAttribute( - cs::attr::closeTimeSelf, + cs::attr::closeTimeSelfRippleEpochS, static_cast(rawCloseTimes.self.time_since_epoch().count())); doAcceptSpan.setAttribute( cs::attr::closeTimeVoteBins, static_cast(rawCloseTimes.peers.size())); diff --git a/src/xrpld/app/ledger/detail/BuildLedger.cpp b/src/xrpld/app/ledger/detail/BuildLedger.cpp index 482721bf6a..e2433f9c1a 100644 --- a/src/xrpld/app/ledger/detail/BuildLedger.cpp +++ b/src/xrpld/app/ledger/detail/BuildLedger.cpp @@ -188,6 +188,12 @@ applyTransactions( // If there are any transactions left, we must have // tried them in at least one final pass XRPL_ASSERT(txns.empty() || !certainRetry, "xrpl::applyTransactions : retry transactions"); + // Repeated from the parent ledger.build span on purpose: TraceQL cannot + // reach a parent's attributes from a child, so without it no query can + // select this span by ledger. `view` is the accumulator over the ledger + // being built and copies its header, so this is the same sequence number + // the parent reports. + applySpan.setAttribute(ledger_span::attr::ledgerSeq, static_cast(view.seq())); applySpan.setAttribute(ledger_span::attr::txCount, static_cast(count)); applySpan.setAttribute(ledger_span::attr::txFailed, static_cast(failed.size())); return count; diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp index 5e02b4307a..0c2ffff083 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -455,8 +455,9 @@ bool LedgerMaster::storeLedger(std::shared_ptr ledger) { using namespace telemetry; - auto span = SpanGuard::span(TraceCategory::Ledger, seg::ledger, ledger_span::op::store); - span.setAttribute(ledger_span::attr::ledgerSeq, static_cast(ledger->header().seq)); + auto storeSpan = SpanGuard::span(TraceCategory::Ledger, seg::ledger, ledger_span::op::store); + storeSpan.setAttribute( + ledger_span::attr::ledgerSeq, static_cast(ledger->header().seq)); bool const validated = ledger->header().validated; // Returns true if we already had the ledger @@ -978,55 +979,65 @@ LedgerMaster::checkAccept(std::shared_ptr const& ledger) return; } - using namespace telemetry; - auto valSpan = SpanGuard::span(TraceCategory::Ledger, seg::ledger, ledger_span::op::validate); - valSpan.setAttribute(ledger_span::attr::ledgerSeq, static_cast(ledger->header().seq)); - valSpan.setAttribute(ledger_span::attr::validations, static_cast(tvc)); - - JLOG(journal_.info()) << "Advancing accepted ledger to " << ledger->header().seq - << " with >= " << minVal << " validations"; - - ledger->setValidated(); - ledger->setFull(); - setValidLedger(ledger); - if (!pubLedger_) + // Scoped so ledger.validate measures only the promotion itself. The + // flag-ledger upgrade-warning check below runs on one ledger in 256 and + // reads every trusted validation of the parent, so leaving it inside would + // make every 256th span a duration outlier for work that is not part of + // promoting a ledger. tryAdvance() stays inside: it only sets a flag and + // posts a job, so it adds no measurable time. { - pendSaveValidated(app_, ledger, true, true); - setPubLedger(ledger); - app_.getOrderBookDB().setup(ledger); - } + using namespace telemetry; + auto validateSpan = + SpanGuard::span(TraceCategory::Ledger, seg::ledger, ledger_span::op::validate); + validateSpan.setAttribute( + ledger_span::attr::ledgerSeq, static_cast(ledger->header().seq)); + validateSpan.setAttribute(ledger_span::attr::validations, static_cast(tvc)); - std::uint32_t const base = app_.getFeeTrack().getLoadBase(); - auto fees = app_.getValidations().fees(ledger->header().hash, base); - { - auto fees2 = app_.getValidations().fees(ledger->header().parentHash, base); - fees.reserve(fees.size() + fees2.size()); - std::ranges::copy(fees2, std::back_inserter(fees)); - } - std::uint32_t fee = 0; - if (!fees.empty()) - { - std::ranges::sort(fees); - if (auto stream = journal_.debug()) + JLOG(journal_.info()) << "Advancing accepted ledger to " << ledger->header().seq + << " with >= " << minVal << " validations"; + + ledger->setValidated(); + ledger->setFull(); + setValidLedger(ledger); + if (!pubLedger_) { - std::stringstream s; - s << "Received fees from validations: (" << fees.size() << ") "; - for (auto const fee1 : fees) - { - s << " " << fee1; - } - stream << s.str(); + pendSaveValidated(app_, ledger, true, true); + setPubLedger(ledger); + app_.getOrderBookDB().setup(ledger); } - fee = fees[fees.size() / 2]; // median - } - else - { - fee = base; - } - app_.getFeeTrack().setRemoteFee(fee); + std::uint32_t const base = app_.getFeeTrack().getLoadBase(); + auto fees = app_.getValidations().fees(ledger->header().hash, base); + { + auto fees2 = app_.getValidations().fees(ledger->header().parentHash, base); + fees.reserve(fees.size() + fees2.size()); + std::ranges::copy(fees2, std::back_inserter(fees)); + } + std::uint32_t fee = 0; + if (!fees.empty()) + { + std::ranges::sort(fees); + if (auto stream = journal_.debug()) + { + std::stringstream s; + s << "Received fees from validations: (" << fees.size() << ") "; + for (auto const fee1 : fees) + { + s << " " << fee1; + } + stream << s.str(); + } + fee = fees[fees.size() / 2]; // median + } + else + { + fee = base; + } - tryAdvance(); + app_.getFeeTrack().setRemoteFee(fee); + + tryAdvance(); + } if (ledger->seq() % 256 == 0) { diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 1f96a0d629..0bce9a252c 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1962,10 +1962,12 @@ PeerImp::onMessage(std::shared_ptr const& m) { using namespace telemetry; // root: inbound peer message entry point (kConsumer); must not inherit - // any span left active on this peer thread. - auto span = + // any span left active on this peer thread. Named after the span it holds, + // peer.proposal.receive, to keep it distinct from `proposalSpan` below, + // which holds the consensus-level span handed to the job worker. + auto proposalReceiveSpan = ScopedSpanGuard::freshRoot(TraceCategory::Peer, seg::peer, peer_span::op::proposalReceive); - span.setAttribute(peer_span::attr::peerId, static_cast(id_)); + proposalReceiveSpan.setAttribute(peer_span::attr::peerId, static_cast(id_)); protocol::TMProposeSet const& set = *m; @@ -1993,7 +1995,7 @@ PeerImp::onMessage(std::shared_ptr const& m) // every time a spam packet is received PublicKey const publicKey{makeSlice(set.nodepubkey())}; auto const isTrusted = app_.getValidators().trusted(publicKey); - span.setAttribute(peer_span::attr::proposalTrusted, isTrusted); + proposalReceiveSpan.setAttribute(peer_span::attr::proposalTrusted, isTrusted); // If the operator has specified that untrusted proposals be dropped then // this happens here I.e. before further wasting CPU verifying the signature @@ -2567,10 +2569,12 @@ PeerImp::onMessage(std::shared_ptr const& m) { using namespace telemetry; // root: inbound peer message entry point (kConsumer); must not inherit - // any span left active on this peer thread. - auto valSpan = ScopedSpanGuard::freshRoot( + // any span left active on this peer thread. Named after the span it holds, + // peer.validation.receive, to keep it distinct from the consensus-level + // span handed to the job worker below. + auto validationReceiveSpan = ScopedSpanGuard::freshRoot( TraceCategory::Peer, seg::peer, peer_span::op::validationReceive); - valSpan.setAttribute(peer_span::attr::peerId, static_cast(id_)); + validationReceiveSpan.setAttribute(peer_span::attr::peerId, static_cast(id_)); if (m->validation().size() < 50) { @@ -2611,11 +2615,11 @@ PeerImp::onMessage(std::shared_ptr const& m) // false when telemetry is compiled out, switched off in the config, or // the Peer trace category is disabled; a span that exists but was // sampled out still pays. - if (valSpan) + if (validationReceiveSpan) { - valSpan.setAttribute( + validationReceiveSpan.setAttribute( peer_span::attr::ledgerHash, to_string(val->getLedgerHash()).c_str()); - valSpan.setAttribute(peer_span::attr::fullValidation, val->isFull()); + validationReceiveSpan.setAttribute(peer_span::attr::fullValidation, val->isFull()); } if (!isCurrent( @@ -2633,7 +2637,7 @@ PeerImp::onMessage(std::shared_ptr const& m) // suppression for 30 seconds to avoid doing a relatively expensive // lookup every time a spam packet is received auto const isTrusted = app_.getValidators().trusted(val->getSignerPublic()); - valSpan.setAttribute(peer_span::attr::validationTrusted, isTrusted); + validationReceiveSpan.setAttribute(peer_span::attr::validationTrusted, isTrusted); // If the operator has specified that untrusted validations be // dropped then this happens here I.e. before further wasting CPU @@ -2702,12 +2706,29 @@ PeerImp::onMessage(std::shared_ptr const& m) static_cast(val->getSignTime().time_since_epoch().count())); } + // validation_status is set once on each exit below, not as a default + // here, to avoid OTel SDK attribute duplication. It is what separates + // the microsecond drop paths from the queued path, which also covers + // job wait and checkValidation. if (!isTrusted && (tracking_.load() == Tracking::Diverged)) { + if (span && *span) + { + span->setAttribute( + telemetry::consensus::span::attr::validationStatus, + telemetry::consensus::span::val::validationDroppedDiverged); + } JLOG(pJournal_.debug()) << "Dropping untrusted validation from diverged peer"; } else if (isTrusted || !app_.getFeeTrack().isLoadedLocal()) { + // Set before the handle is moved into the job below. + if (span && *span) + { + span->setAttribute( + telemetry::consensus::span::attr::validationStatus, + telemetry::consensus::span::val::validationQueued); + } std::string const name = isTrusted ? "ChkTrust" : "ChkUntrust"; std::weak_ptr const weak = shared_from_this(); @@ -2721,6 +2742,12 @@ PeerImp::onMessage(std::shared_ptr const& m) } else { + if (span && *span) + { + span->setAttribute( + telemetry::consensus::span::attr::validationStatus, + telemetry::consensus::span::val::validationDroppedLoad); + } JLOG(pJournal_.debug()) << "Dropping untrusted validation for load"; } } diff --git a/src/xrpld/overlay/detail/PeerSpanNames.h b/src/xrpld/overlay/detail/PeerSpanNames.h index 6212d8fca2..24f80e0adf 100644 --- a/src/xrpld/overlay/detail/PeerSpanNames.h +++ b/src/xrpld/overlay/detail/PeerSpanNames.h @@ -36,7 +36,14 @@ using ::xrpl::telemetry::attr::ledgerHash; using ::xrpl::telemetry::attr::peerId; /** - * Trust flag qualified by message type, shared with consensus.*.receive. + * Trust flag qualified by message type — whether the sending key is on this + * node's UNL. + * + * The literals match consensus::span::attr::proposalTrusted and + * ::validationTrusted, so the peer and consensus receive spans report on one + * spanmetrics dimension instead of two. Unlike the constants above these are + * declared here rather than re-exported from SpanNames.h, so the two spellings + * are only kept equal by hand: change one and the dimension splits silently. */ inline constexpr auto proposalTrusted = makeStr("proposal_trusted"); inline constexpr auto validationTrusted = makeStr("validation_trusted"); diff --git a/src/xrpld/rpc/detail/PathRequest.cpp b/src/xrpld/rpc/detail/PathRequest.cpp index 827393b071..1154109fc9 100644 --- a/src/xrpld/rpc/detail/PathRequest.cpp +++ b/src/xrpld/rpc/detail/PathRequest.cpp @@ -596,8 +596,8 @@ PathRequest::findPaths( // One `pathfind.discover` span wraps the entire per-source-asset loop so // that a single RPC call produces one discover span instead of N (one per // candidate source asset). Trade-off: per-asset discovery/ranking timing - // is no longer split into individual spans — span count and Tempo storage - // are bounded per RPC at the cost of per-asset visibility. + // is not measured separately — span count and Tempo storage are bounded + // per RPC at the cost of per-asset visibility. // // This is an unscoped guard: it takes the ambient span as its own parent, // but does not itself become the ambient parent. Adding per-asset child diff --git a/src/xrpld/telemetry/TxTracing.h b/src/xrpld/telemetry/TxTracing.h index 5baf01df2d..682f482f1b 100644 --- a/src/xrpld/telemetry/TxTracing.h +++ b/src/xrpld/telemetry/TxTracing.h @@ -32,8 +32,12 @@ namespace xrpl::telemetry { * trace_id is derived from txID[0:16]. If the incoming message carries * a protobuf TraceContext with a valid span_id, it is used as the * parent to preserve relay ordering. + * @param txID Transaction id; its first 16 bytes become the trace_id. + * @param msg The received message, read only for its trace context. + * @return An active guard, or a null guard when the Transactions category + * is disabled. Bind it: a discarded guard ends the span immediately. */ -inline SpanGuard +[[nodiscard]] inline SpanGuard txReceiveSpan(uint256 const& txID, [[maybe_unused]] protocol::TMTransaction const& msg) { #ifdef XRPL_ENABLE_TELEMETRY @@ -63,8 +67,11 @@ txReceiveSpan(uint256 const& txID, [[maybe_unused]] protocol::TMTransaction cons /** * Create a "tx.process" span for transaction processing in NetworkOPs. * trace_id is derived from txID[0:16]. + * @param txID Transaction id; its first 16 bytes become the trace_id. + * @return An active guard, or a null guard when the Transactions category + * is disabled. Bind it: a discarded guard ends the span immediately. */ -inline SpanGuard +[[nodiscard]] inline SpanGuard txProcessSpan(uint256 const& txID) { return SpanGuard::hashSpan( diff --git a/src/xrpld/telemetry/ValidationTracker.h b/src/xrpld/telemetry/ValidationTracker.h index 346e324ae2..278332ad8e 100644 --- a/src/xrpld/telemetry/ValidationTracker.h +++ b/src/xrpld/telemetry/ValidationTracker.h @@ -138,21 +138,21 @@ public: * Agreement percentage over the last 1 hour. * @return Percentage [0.0, 100.0], or 0.0 if no data. */ - double + [[nodiscard]] double agreementPct1h() const; /** * Agreement percentage over the last 24 hours. * @return Percentage [0.0, 100.0], or 0.0 if no data. */ - double + [[nodiscard]] double agreementPct24h() const; /** * Agreement percentage over the last 7 days. * @return Percentage [0.0, 100.0], or 0.0 if no data. */ - double + [[nodiscard]] double agreementPct7d() const; /** @} */ @@ -165,37 +165,37 @@ public: /** * Number of agreements in the 1-hour window. */ - uint64_t + [[nodiscard]] uint64_t agreements1h() const; /** * Number of misses in the 1-hour window. */ - uint64_t + [[nodiscard]] uint64_t missed1h() const; /** * Number of agreements in the 24-hour window. */ - uint64_t + [[nodiscard]] uint64_t agreements24h() const; /** * Number of misses in the 24-hour window. */ - uint64_t + [[nodiscard]] uint64_t missed24h() const; /** * Number of agreements in the 7-day window. */ - uint64_t + [[nodiscard]] uint64_t agreements7d() const; /** * Number of misses in the 7-day window. */ - uint64_t + [[nodiscard]] uint64_t missed7d() const; /** @} */ @@ -208,25 +208,25 @@ public: /** * Total agreements since process start. */ - uint64_t + [[nodiscard]] uint64_t totalAgreements() const; /** * Total misses since process start. */ - uint64_t + [[nodiscard]] uint64_t totalMissed() const; /** * Total validations this node sent. */ - uint64_t + [[nodiscard]] uint64_t totalValidationsSent() const; /** * Total network validations observed for comparison. */ - uint64_t + [[nodiscard]] uint64_t totalValidationsChecked() const; /** @} */ diff --git a/src/xrpld/telemetry/detail/ValidationTracker.cpp b/src/xrpld/telemetry/detail/ValidationTracker.cpp index 94cd517d89..a29fde9c7f 100644 --- a/src/xrpld/telemetry/detail/ValidationTracker.cpp +++ b/src/xrpld/telemetry/detail/ValidationTracker.cpp @@ -116,19 +116,11 @@ void ValidationTracker::evictOldPending(TimePoint now) { auto const cutoff = now - kLateRepairWindow; - for (auto it = pending_.begin(); it != pending_.end();) - { - if (it->second.reconciled && it->second.recordTime < cutoff) - { - it = pending_.erase(it); - } - else - { - ++it; - } - } + std::erase_if(pending_, [cutoff](auto const& entry) { + return entry.second.reconciled && entry.second.recordTime < cutoff; + }); - // Hard trim if still over limit. The loop above already removed every + // Hard trim if still over limit. The pass 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) @@ -155,7 +147,7 @@ ValidationTracker::agreementPct1h() const if (window1h_.empty()) return 0.0; auto const agreed = static_cast( - std::count_if(window1h_.begin(), window1h_.end(), [](auto const& e) { return e.agreed; })); + std::ranges::count_if(window1h_, [](auto const& e) { return e.agreed; })); return (agreed / static_cast(window1h_.size())) * 100.0; } @@ -165,8 +157,8 @@ ValidationTracker::agreementPct24h() const std::scoped_lock const lock(mutex_); if (window24h_.empty()) return 0.0; - auto const agreed = static_cast(std::count_if( - window24h_.begin(), window24h_.end(), [](auto const& e) { return e.agreed; })); + auto const agreed = static_cast( + std::ranges::count_if(window24h_, [](auto const& e) { return e.agreed; })); return (agreed / static_cast(window24h_.size())) * 100.0; } @@ -175,7 +167,7 @@ ValidationTracker::agreements1h() const { std::scoped_lock const lock(mutex_); return static_cast( - std::count_if(window1h_.begin(), window1h_.end(), [](auto const& e) { return e.agreed; })); + std::ranges::count_if(window1h_, [](auto const& e) { return e.agreed; })); } uint64_t @@ -183,23 +175,23 @@ ValidationTracker::missed1h() const { std::scoped_lock const lock(mutex_); return static_cast( - std::count_if(window1h_.begin(), window1h_.end(), [](auto const& e) { return !e.agreed; })); + std::ranges::count_if(window1h_, [](auto const& e) { return !e.agreed; })); } uint64_t ValidationTracker::agreements24h() const { std::scoped_lock const lock(mutex_); - return static_cast(std::count_if( - window24h_.begin(), window24h_.end(), [](auto const& e) { return e.agreed; })); + return static_cast( + std::ranges::count_if(window24h_, [](auto const& e) { return e.agreed; })); } uint64_t ValidationTracker::missed24h() const { std::scoped_lock const lock(mutex_); - return static_cast(std::count_if( - window24h_.begin(), window24h_.end(), [](auto const& e) { return !e.agreed; })); + return static_cast( + std::ranges::count_if(window24h_, [](auto const& e) { return !e.agreed; })); } double @@ -209,7 +201,7 @@ ValidationTracker::agreementPct7d() const if (window7d_.empty()) return 0.0; auto const agreed = static_cast( - std::count_if(window7d_.begin(), window7d_.end(), [](auto const& e) { return e.agreed; })); + std::ranges::count_if(window7d_, [](auto const& e) { return e.agreed; })); return (agreed / static_cast(window7d_.size())) * 100.0; } @@ -218,7 +210,7 @@ ValidationTracker::agreements7d() const { std::scoped_lock const lock(mutex_); return static_cast( - std::count_if(window7d_.begin(), window7d_.end(), [](auto const& e) { return e.agreed; })); + std::ranges::count_if(window7d_, [](auto const& e) { return e.agreed; })); } uint64_t @@ -226,7 +218,7 @@ ValidationTracker::missed7d() const { std::scoped_lock const lock(mutex_); return static_cast( - std::count_if(window7d_.begin(), window7d_.end(), [](auto const& e) { return !e.agreed; })); + std::ranges::count_if(window7d_, [](auto const& e) { return !e.agreed; })); } uint64_t