mirror of
https://github.com/XRPLF/rippled.git
synced 2026-09-28 07:48:01 +00:00
docs(telemetry): correct stale code citations and the log-correlation claim
Every citation below was checked against the file it names: - LedgerMaster.cpp:463 is fixIndex, not the ledger.store span; that guard is at :470 and the insert it wraps at :476 - LedgerMaster.cpp:987 is the tvc assignment, which sits BEFORE the tvc < minVal return at :988; the ledger.validate span opens at :1003 - ServerHandler.cpp:705 is inside makeJsonError; processRequest is at :718 - docker-compose.yml:71 and :75 are comments in the collector's volume block; the loki service is at :112 and its config command at :116 Two claims were also wrong rather than merely stale. Log-trace correlation is gated in CI, because the workflow passes no --skip-loki, and the separate check in integration-test.sh is run by no workflow at all. The Loki label note described the Grafana Cloud collector config rather than the local one: only the cloud variant sets job=xrpld, and the local config's own comment says to select on service_name. The dashboards carry 35 Loki queries, not 38.
This commit is contained in:
@@ -584,7 +584,7 @@ Fluentd or PerfLog change. Two pieces:
|
||||
> only an **allow-listed** set of resource attributes to indexed stream labels
|
||||
> (`service.name`, `service.namespace`, `service.instance.id`,
|
||||
> `deployment.environment`, the `k8s.*`/`cloud.*` keys); `job` is not on that
|
||||
> list, and this repo ships no Loki config override — `docker-compose.yml:75`
|
||||
> list, and this repo ships no Loki config override — `docker-compose.yml:116`
|
||||
> starts Loki with the image's built-in `/etc/loki/local-config.yaml`. `job`
|
||||
> therefore lands in **structured metadata**, which cannot appear in a stream
|
||||
> selector, so `{job="xrpld"}` returns an empty result rather than an error.
|
||||
|
||||
@@ -734,13 +734,18 @@ flowchart LR
|
||||
absent or the context is invalid (`Log.cpp:310-318`)
|
||||
- [x] Loki ingests xrpld logs via OTel Collector filelog receiver —
|
||||
`otel-collector-config.yaml:38` (`filelog`); `loki` service in
|
||||
`docker-compose.yml:71`
|
||||
`docker-compose.yml:112`
|
||||
- [x] Grafana Tempo → Loki one-click correlation works —
|
||||
`provisioning/datasources/tempo.yaml:32` (`tracesToLogs`)
|
||||
- [x] Grafana Loki → Tempo reverse lookup works via derived field —
|
||||
`provisioning/datasources/loki.yaml:16` (`derivedFields`)
|
||||
- [ ] Integration test verifies trace_id presence in logs — implemented in the
|
||||
Phase 10 harness, but CI runs it with `--skip-loki`, so it is not gated
|
||||
- [ ] Integration test verifies trace_id presence in logs — CI gates this
|
||||
through the Phase 10 harness's `validate_telemetry.py`, whose
|
||||
`log.trace_id_present` and `log.trace_id_cross_reference` checks run
|
||||
because the workflow passes no `--skip-loki`. That harness and
|
||||
`.github/workflows/telemetry-validation.yml` live on the Phase 10 branch,
|
||||
not here. `docker/telemetry/integration-test.sh:79-126` carries a separate
|
||||
trace_id-in-logs check that no workflow under `.github/workflows/` runs
|
||||
- [ ] No performance regression from trace_id injection (< 0.1% overhead) —
|
||||
needs the Phase 10 benchmark suite
|
||||
|
||||
|
||||
@@ -507,7 +507,7 @@ These are journal (`debug.log`) lines, not PerfLog lines — see §7.7.2.
|
||||
> **allow-listed** set of resource attributes to indexed stream labels
|
||||
> (`service.name`, `service.namespace`, `service.instance.id`,
|
||||
> `deployment.environment`, `k8s.*`, `cloud.*`), and `job` is not on it. This
|
||||
> repo mounts no Loki config override (`docker-compose.yml:75` uses the image's
|
||||
> repo mounts no Loki config override (`docker-compose.yml:116` uses the image's
|
||||
> built-in `local-config.yaml`), so `job` lands in **structured metadata** —
|
||||
> queryable only with a `|` filter after a selector, never as the selector
|
||||
> itself. A `{job="xrpld"}` query returns empty with no error, which is why this
|
||||
|
||||
@@ -150,7 +150,7 @@ Before Phases 1-9 can be considered production-ready, we need proof that:
|
||||
- HTTP: `rpc.http_request` → `rpc.process` → `rpc.command.*`
|
||||
- WebSocket: `rpc.ws_message` → `rpc.command.*` — **there is no
|
||||
`rpc.process` on the WS path**. `rpc.process` is created only in
|
||||
`ServerHandler::processRequest()` (`ServerHandler.cpp:705`), reached from
|
||||
`ServerHandler::processRequest()` (`ServerHandler.cpp:718`), reached from
|
||||
`processSession(Session, coro)`, i.e. HTTP only. Under WS-only load
|
||||
`rpc.process` never appears, and `rpc.command.*` parents directly to
|
||||
`rpc.ws_message`.
|
||||
@@ -276,7 +276,8 @@ Before Phases 1-9 can be considered production-ready, we need proof that:
|
||||
(totals computed dynamically from `expected_spans.json` /
|
||||
`expected_metrics.json`, not the stale 16 / 22 figures)
|
||||
- [ ] Log-trace correlation validated end-to-end (Loki ↔ Tempo) — implemented,
|
||||
but CI runs with `--skip-loki`, so it is not gated
|
||||
and gated in CI: the workflow passes no `--skip-loki`, so
|
||||
`validate_telemetry.py` builds and runs both log-correlation checks
|
||||
- [ ] All 14 harness-asserted Grafana dashboards render data (no empty panels);
|
||||
15 on disk
|
||||
- [ ] Benchmark shows < 3% CPU overhead, < 5MB memory overhead
|
||||
|
||||
@@ -631,20 +631,22 @@ curl -sG "http://localhost:3100/loki/api/v1/query" \
|
||||
|
||||
Expected: > 0 results.
|
||||
|
||||
> **Use `service_name`, not `job`.** The collector's `resource/logs` processor
|
||||
> applies an `upsert` to **both** `service.name=xrpld` and `job=xrpld`
|
||||
> (`otel-collector-config.yaml:57-70`), and its comment says the `job` attribute
|
||||
> is there so operators can paste `{job="xrpld"}`. That does not work: on OTLP
|
||||
> ingest Loki promotes only an allow-listed set of resource attributes to indexed
|
||||
> stream labels (`service.name` → `service_name`, plus `service.namespace`,
|
||||
> **Use `service_name`, not `job`.** The local stack's `resource/logs` processor
|
||||
> sets one key, `service.name=xrpld` (`otel-collector-config.yaml:84-86`); its
|
||||
> comment there explains that a custom `job` attribute is not promoted to a
|
||||
> stream label and tells you to select on `service_name`. Only the Grafana Cloud
|
||||
> variant also sets `job=xrpld` (`otel-collector-config.grafanacloud.yaml:73-75`).
|
||||
> Either way `{job="xrpld"}` does not work as a selector: on OTLP ingest Loki
|
||||
> promotes only an allow-listed set of resource attributes to indexed stream
|
||||
> labels (`service.name` → `service_name`, plus `service.namespace`,
|
||||
> `service.instance.id`, `deployment.environment`, `k8s.*`, `cloud.*`), and `job`
|
||||
> is not on the list. This repo mounts no Loki config override — the `loki`
|
||||
> service runs the image's built-in `/etc/loki/local-config.yaml`
|
||||
> (`docker-compose.yml:75`) — so `job` lands in **structured metadata**, which
|
||||
> (`docker-compose.yml:116`) — so `job` lands in **structured metadata**, which
|
||||
> cannot be a stream selector. `{job="xrpld"}` therefore returns **zero results
|
||||
> with no error**, which reads exactly like "logs are not being ingested". If
|
||||
> this query is empty, check `{service_name="xrpld"}` before debugging the
|
||||
> pipeline. All 38 Loki queries in the shipped dashboards select on
|
||||
> pipeline. All 35 Loki queries in the shipped dashboards select on
|
||||
> `service_name`; none uses `job`.
|
||||
|
||||
### Step 4: Verify Grafana Tempo-to-Loki correlation
|
||||
|
||||
@@ -985,7 +985,7 @@ flowchart TB
|
||||
(`tvc < minVal`) it returns early with no promotion — a built ledger that loses
|
||||
is abandoned ([LedgerMaster.cpp:980](../src/xrpld/app/ledger/detail/LedgerMaster.cpp#L980);
|
||||
[docs/consensus.md:50](consensus.md)). The `ledger.validate` span is emitted only
|
||||
inside `checkAccept` ([LedgerMaster.cpp:987](../src/xrpld/app/ledger/detail/LedgerMaster.cpp#L987)).
|
||||
inside `checkAccept` ([LedgerMaster.cpp:1003](../src/xrpld/app/ledger/detail/LedgerMaster.cpp#L1003)).
|
||||
- **validation-send guard**: broadcast only if
|
||||
`validating_ && isCompatible && !consensusFail && canValidateSeq(seq)` — silently
|
||||
suppressed for incompatible ledgers or an already-validated seq
|
||||
@@ -1152,8 +1152,8 @@ are pending a code fix:
|
||||
- **`ledger.acquire` / `ledger.store` / `ledger.validate` are not reliably roots
|
||||
either.** All three use `SpanGuard::span`
|
||||
([InboundLedger.cpp:113](../src/xrpld/app/ledger/detail/InboundLedger.cpp#L113),
|
||||
[LedgerMaster.cpp:463](../src/xrpld/app/ledger/detail/LedgerMaster.cpp#L463),
|
||||
[987](../src/xrpld/app/ledger/detail/LedgerMaster.cpp#L987)), which inherits the
|
||||
[LedgerMaster.cpp:470](../src/xrpld/app/ledger/detail/LedgerMaster.cpp#L470),
|
||||
[1003](../src/xrpld/app/ledger/detail/LedgerMaster.cpp#L1003)), which inherits the
|
||||
ambient span ([SpanGuard.cpp:233](../src/libxrpl/telemetry/SpanGuard.cpp#L233))
|
||||
rather than `freshRoot`
|
||||
([245](../src/libxrpl/telemetry/SpanGuard.cpp#L245)) — the same defect as
|
||||
|
||||
Reference in New Issue
Block a user