diff --git a/.github/scripts/otel-naming/README.md b/.github/scripts/otel-naming/README.md index 660b574ed0..50051e8e31 100644 --- a/.github/scripts/otel-naming/README.md +++ b/.github/scripts/otel-naming/README.md @@ -35,18 +35,29 @@ hardcoded allowlist: constants passed there (`xrpl.network.*`). A dotted key that is _declared_ in a header but never set as a resource attr is a span attribute in resource clothing — a Rule-A violation, even if it lives in the base `SpanNames.h`. +- **L1-metrics** — instrument names, label keys and bounded label values come + from the `namespace metric` / `namespace label` / `namespace lval` blocks of + every `*MetricNames.h`, read as `inline constexpr char NAME[] = "wire";`. + These headers deliberately do **not** use the `makeStr`/`StaticStr` DSL the + span headers use: the OTel C++ API takes `nostd::string_view`, which + constructs from `char const*` but has no constructor from + `std::string_view`, so a `StaticStr` will not compile in an instrument-name + or label-key position. ### Rules (each fails the build, when its inputs are present) -| Rule | Check | -| ---- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| A | No stray dotted span-attribute key (only the derived resource keys may be dotted). | -| G | Attribute keys are `lower_snake_case` (`^[a-z][a-z0-9_]*$` per dot-segment) — no camelCase, UPPERCASE, or spaces. | -| F | No string literals as attribute keys or span-name arguments in `setAttribute`/`addEvent`/`span`/`rootSpan`/`childSpan` (`rootSpan` shares `span`'s `(cat, prefix, name)` signature). Attribute _values_ are exempt (runtime data); `*SpanNames.h` definitions and test files are exempt. | -| B | Every collector `spanmetrics.dimensions` name exists in the L1 key set. | -| C | Every Tempo span-filter tag exists in the L1 key set. | -| D | Every dashboard label resolves to an L1 span attribute, a native-metric label (L6, emitted by MetricsRegistry), or a Prometheus/Grafana builtin. TraceQL scope prefixes (`span.`/`resource.`/…) are stripped before the L1 lookup. | -| E | No dotted `xrpl..` attribute key in the runbook (only the L1 resource attrs `xrpl.network.*` may be dotted). Span names, filenames, OTel-standard keys, and metric labels are not flagged. | +| Rule | Check | +| ---- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| A | No stray dotted span-attribute key (only the derived resource keys may be dotted). | +| G | Attribute keys are `lower_snake_case` (`^[a-z][a-z0-9_]*$` per dot-segment) — no camelCase, UPPERCASE, or spaces. | +| F | No string literals as attribute keys or span-name arguments in `setAttribute`/`addEvent`/`span`/`rootSpan`/`childSpan` (`rootSpan` shares `span`'s `(cat, prefix, name)` signature). Attribute _values_ are exempt (runtime data); `*SpanNames.h` definitions and test files are exempt. | +| B | Every collector `spanmetrics.dimensions` name exists in the L1 key set. | +| C | Every Tempo span-filter tag exists in the L1 key set. | +| D | Every dashboard label resolves to an L1 span attribute, a native-metric label (L6, emitted by MetricsRegistry), or a Prometheus/Grafana builtin. TraceQL scope prefixes (`span.`/`resource.`/…) are stripped before the L1 lookup. | +| E | No dotted `xrpl..` attribute key in the runbook (only the L1 resource attrs `xrpl.network.*` may be dotted). Span names, filenames, OTel-standard keys, and metric labels are not flagged. | +| I | No string literals as **metric** instrument names or label keys — the mirror of Rule F. Applies to the name passed to an `XRPL_METRIC_*` macro or a `meter->Create*` factory and to the label _keys_ in its label set. Label _values_, descriptions, `*MetricNames.h`, `MetricMacros.h` and test files are exempt. Scoped by metric **family** (first underscore segment): declaring a constant opts that family in, so the metric surface can be converted subsystem by subsystem. Unconverted families warn as Rule L. | +| J | Metric instrument names follow the suffix conventions: `lower_snake_case`, no `xrpld_`/`xrpl_` prefix (the exporter adds it), a counter ends `_total`, a histogram ends `_us`/`_ms`/`_seconds`, a gauge does not end `_total`. The instrument **kind** is read from the emit site, never guessed from words in the name — so a multi-series gauge carrying units in its label values (e.g. `nodestore_latency` observing `write_mean_us`) is not a violation. | +| K | Every metric named in `docker/telemetry/workload/expected_metrics.json` resolves to a declared constant, so a rename in code cannot leave the workload validator asserting a name nothing emits. PromQL selectors (`m{label="v"}`) and exporter-appended histogram suffixes (`_bucket`/`_count`/`_sum`) are normalized away first; groups fed by another emit path (`statsd_gauges`, `statsd_counters`, `spanmetrics`) are out of scope by design. | Rule F runs **unconditionally** (it is a purely syntactic check on the call-sites and needs no `*SpanNames.h`), so a code path that calls @@ -58,6 +69,7 @@ still caught. | Rule | Check | | ---- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | H | A namespace-qualified constant (e.g. `foo::bar::myKey`) used at a telemetry call-site is not defined in any `*SpanNames.h`. The constant should live in the proper header; defining it in-place bypasses rules A/G/F. Warns rather than fails — the argument may be a legitimately dynamic value, and the header may live on a later branch. Bare locals and `std::` names are not warned. | +| L | A literal metric name in a family that has no `*MetricNames.h` constants yet. Rule I's ratchet defers these instead of failing the build on the whole pre-existing metric surface at once; the warning keeps the outstanding conversion work visible rather than silently accepted. | ## Presence-gated diff --git a/.github/scripts/otel-naming/check_otel_naming.py b/.github/scripts/otel-naming/check_otel_naming.py index 2df6c43790..2847397afd 100644 --- a/.github/scripts/otel-naming/check_otel_naming.py +++ b/.github/scripts/otel-naming/check_otel_naming.py @@ -5,10 +5,12 @@ Usage: check_otel_naming.py This script takes no parameters and can be called from any directory inside the repository (it locates the repo root via `git rev-parse`). -Enforces the OpenTelemetry span-attribute naming convention documented in -CONTRIBUTING.md ("Telemetry span attribute naming") across every layer of the -telemetry pipeline. The `*SpanNames.h` constants are the single source of truth -(L1); every other layer must agree with them. +Enforces the OpenTelemetry naming conventions documented in CONTRIBUTING.md +("Telemetry span attribute naming" and "Telemetry metric naming") across every +layer of the telemetry pipeline. The `*SpanNames.h` constants are the single +source of truth for span attributes (L1) and the `*MetricNames.h` constants for +metric names and label keys (L1-metrics); every other layer must agree with +them. Design principles ----------------- @@ -49,6 +51,10 @@ Layers L5 runbook : docs/telemetry-runbook.md (attr tables) L6 metrics : MetricsRegistry.cpp instrument labels (native-metric label keys, a valid dashboard-label source besides L1) + L1-metrics : src/**/*MetricNames.h, include/**/*MetricNames.h (ground truth + for metric instrument names, label keys and bounded values) + L7 workload : docker/telemetry/workload/expected_metrics.json (asserted + metric names) Rules (each FAILS the build, when its inputs are present) --------------------------------------------------------- @@ -71,6 +77,24 @@ Rules (each FAILS the build, when its inputs are present) form -- xrpl.work.item/.branch/.node.role -- may be dotted). Span names, filenames, OTel-standard keys, and metric labels are not flagged. + I No string literals as METRIC instrument names or label keys. The mirror of + Rule F for metrics: the name passed to an XRPL_METRIC_* macro or to a + meter->Create* factory, and the label KEYS in its label set, must reference + a *MetricNames.h constant. Label VALUES are exempt (runtime data), as are + the descriptions, *MetricNames.h itself, MetricMacros.h, and test files. + Scoped by metric FAMILY (first underscore segment) so the metric surface + can be converted subsystem by subsystem -- declaring a constant for a + family opts that family into enforcement. See Rule L. + J Metric instrument names follow the naming and suffix conventions: + lower_snake_case, no xrpld_/xrpl_ prefix (the exporter adds it), a counter + ends in _total, a histogram ends in _us/_ms/_seconds, and a gauge does not + end in _total. The instrument KIND is read from the emit site, never + guessed from words in the name. + K Every metric named in expected_metrics.json resolves to a *MetricNames.h + constant, so a rename in code cannot leave the workload validator + asserting a name nothing emits. PromQL selectors and exporter-appended + histogram suffixes are normalized away first; groups fed by another emit + path (statsd_*, spanmetrics) are out of scope. Warnings (printed, but do NOT fail the build) ---------------------------------------------- @@ -79,11 +103,16 @@ Warnings (printed, but do NOT fail the build) *SpanNames.h (single source of truth); defining one in-place bypasses the naming rules. A warning (not a failure) because the argument may instead be a legitimately dynamic local (e.g. a computed span-name leaf). + L A literal metric name in a family that has no *MetricNames.h constants + yet. Rule I's ratchet defers these rather than failing the build on the + whole pre-existing metric surface at once; the warning is what keeps the + outstanding conversion work visible instead of silently accepted. Exit code is non-zero if any present-and-enforced rule finds a violation. Warnings never change the exit code. """ +import json import re import subprocess import sys @@ -474,6 +503,24 @@ def main() -> None: run_rule_d_dashboards(root, l1_keys, metric_labels, report) run_rule_e_runbook(root, l1_keys, report) + # --- Metric rules I/J/K -------------------------------------------------- + # The metric-name counterpart of the span rules above. Rule I is the mirror + # of Rule F (no literals at an emit site) and, like it, runs + # unconditionally because it is purely syntactic. Rules J and K are + # presence-gated on *MetricNames.h existing. + l1_metric_names, l1_metric_labels, _ = metric_constants(root) + if find_metricname_headers(root): + report.ok( + f"L1-metrics: {len(l1_metric_names)} instrument name(s) and " + f"{len(l1_metric_labels)} label key(s) from " + f"{len(find_metricname_headers(root))} *MetricNames.h header(s)" + ) + else: + report.skip("L1-metrics", "no *MetricNames.h present") + run_rule_i_metric_literals(root, report) + run_rule_j_metric_suffixes(root, report) + run_rule_k_expected_metrics(root, report) + report.render_and_exit() @@ -812,7 +859,13 @@ def metric_label_names(root: Path) -> Set[str]: Collects both the first label of a set and every subsequent one, so a multi-label instrument such as `{{"from", a}, {"to", b}}` contributes ALL - of its keys.""" + of its keys. + + Includes the keys declared as constants in `*MetricNames.h` (L1-metrics), + because Rule I requires call sites to reference a constant rather than a + literal — so once a subsystem is converted, its label keys no longer appear + as literals anywhere and a literal-only scan would under-derive the set and + make Rule D reject a dashboard querying a label the code really emits.""" labels: Set[str] = set() for base in ("src", "include"): for p in (root / base).rglob("*.cpp"): @@ -823,9 +876,84 @@ def metric_label_names(root: Path) -> Set[str]: continue labels |= set(METRIC_LABEL.findall(text)) labels |= set(METRIC_LABEL_NEXT.findall(text)) + labels |= metric_constants(root)[1] return labels +# --------------------------------------------------------------------------- +# L1-metrics: parse *MetricNames.h into the authoritative metric-name/label set +# --------------------------------------------------------------------------- + +# A metric-name constant definition: `inline constexpr char NAME[] = "wire";`. +# The metric headers use `constexpr char[]` rather than the `makeStr`/StaticStr +# DSL the span headers use, because the OTel C++ API takes nostd::string_view, +# which constructs from `char const*` but NOT from std::string_view. +METRIC_CONST_DEF = re.compile( + r"inline\s+constexpr\s+char\s+(\w+)\s*\[\s*\]\s*=\s*\"([^\"]*)\"\s*;" +) + + +def find_metricname_headers(root: Path) -> List[Path]: + """Every `*MetricNames.h` in the tree (the metric-side counterpart of + `find_spanname_headers`).""" + return sorted( + p + for p in list((root / "src").rglob("*MetricNames.h")) + + list((root / "include").rglob("*MetricNames.h")) + if p.is_file() + ) + + +def metric_constants(root: Path) -> Tuple[Set[str], Set[str], Set[str]]: + """Return (instrument_names, label_keys, label_values) declared across the + `*MetricNames.h` headers, split by which namespace block each constant sits + in: `namespace metric` -> instrument names, `namespace label` -> label keys, + `namespace lval` -> bounded label values. + + Comments are stripped first, so an illustrative `@code` example in a doc + comment cannot seed the authoritative set (same reasoning as + `strip_comments` for L1 spans). + + A constant in none of the three namespaces is ignored rather than guessed + at, keeping the derivation conservative in the same direction as L1.""" + names: Set[str] = set() + keys: Set[str] = set() + values: Set[str] = set() + for h in find_metricname_headers(root): + text = strip_comments(read_source(h)) + for ns, bucket in ( + ("metric", names), + ("label", keys), + ("lval", values), + ): + for block in namespace_spans(text, ns): + for m in METRIC_CONST_DEF.finditer(block): + bucket.add(m.group(2)) + return names, keys, values + + +def namespace_spans(text: str, want: str) -> List[str]: + """Return the body text of each `namespace { ... }` block, brace + matched so nested namespaces (e.g. `lval::dns_resolve`) are contained in + their parent's span. Generalizes `attr_namespace_spans`, which is hardcoded + to `attr`.""" + spans: List[str] = [] + for opener in NS_OPEN.finditer(text): + if opener.group(1).split("::")[-1] != want: + continue + i = opener.end() + depth, start = 1, i + while i < len(text) and depth > 0: + c = text[i] + if c == "{": + depth += 1 + elif c == "}": + depth -= 1 + i += 1 + spans.append(text[start : i - 1]) + return spans + + # Identity labels stamped by EXTERNAL infrastructure the OTel pipeline in this # repo does not own: the perf-iac harness attaches these to every metric it # scrapes so dashboards can filter by which build/role produced a series. They @@ -942,5 +1070,429 @@ def run_rule_e_runbook(root: Path, l1_keys: Set[str], report: Report) -> None: report.ok("E: runbook attribute references consistent with L1") +# --------------------------------------------------------------------------- +# Metric rules I / J / K +# --------------------------------------------------------------------------- + +# A metric emit site whose name argument must be a constant. Two families: +# * the XRPL_METRIC_* call-site macros, where arg0 is the app/registry and +# arg1 is the instrument name; +# * the OTel meter factories, where arg0 is the instrument name. +# Matching the macro by name (rather than by receiver, as CALLSITE does) is +# sufficient because the XRPL_METRIC_ prefix is unambiguous in this repo. +METRIC_MACRO_CALL = re.compile(r"\bXRPL_METRIC_([A-Z_]+)\s*\(") +METRIC_FACTORY_CALL = re.compile( + r"\bCreate(?:UInt64|Int64|Double)" + r"(?:Counter|UpDownCounter|Histogram|Gauge|ObservableGauge|" + r"ObservableCounter|ObservableUpDownCounter)\s*\(" +) +# A `{ KEY , ...` label pair opener inside an already-isolated argument list. +# KEY is what Rule I checks; the VALUE after the comma is exempt (runtime data). +METRIC_PAIR_KEY = re.compile(r"\{\s*(\"(?:[^\"\\]|\\.)*\"|[A-Za-z_][\w:]*)\s*,") + +# The unit/kind suffix convention every instrument name must satisfy (Rule J). +# A cumulative counter reads correctly under rate() only if a reader can tell it +# from a gauge, and a duration is ambiguous unless it carries its unit -- the +# OTel `unit` argument is not surfaced on the Prometheus metric name. +METRIC_COUNTER_SUFFIX = "_total" +METRIC_DURATION_SUFFIXES = ("_us", "_ms", "_seconds") + + +def metric_calls(text: str): + """Yield (kind, name_index, arglist, lineno) for every metric emit site. + + `kind` is the macro suffix (e.g. `COUNTER_INC_LABELED`) for a macro call, or + the factory method name for a `meter->Create*` call. `name_index` is which + top-level argument holds the instrument name.""" + for m in METRIC_MACRO_CALL.finditer(text): + arglist = balanced_arglist(text, m.end()) + yield m.group(1), 1, arglist, text.count("\n", 0, m.start()) + 1 + for m in METRIC_FACTORY_CALL.finditer(text): + arglist = balanced_arglist(text, m.end()) + name = m.group(0).rstrip("( \t\n") + yield name, 0, arglist, text.count("\n", 0, m.start()) + 1 + + +def balanced_arglist(text: str, start: int) -> str: + """Return the argument-list text of a call whose '(' ends at `start`-1, + balancing nesting and ignoring parens inside string literals.""" + i, depth, in_str, esc = start, 1, False, False + while i < len(text) and depth > 0: + c = text[i] + if in_str: + if esc: + esc = False + elif c == "\\": + esc = True + elif c == '"': + in_str = False + elif c == '"': + in_str = True + elif c == "(": + depth += 1 + elif c == ")": + depth -= 1 + i += 1 + return text[start : i - 1] + + +def run_rule_i_metric_literals(root: Path, report: Report) -> None: + """Rule I: no string literal as a metric instrument NAME or label KEY. + + The metric-side mirror of Rule F. An instrument name or label key passed to + an `XRPL_METRIC_*` macro or to a `meter->Create*` factory must reference a + `*MetricNames.h` constant, so a rename is one edit instead of six and a typo + is a compile error instead of a metric that silently never appears. + + Exempt, for the same reasons Rule F exempts them: + * label VALUES -- the argument after a label key is runtime data; + * the instrument DESCRIPTION -- prose, not naming surface; + * `*MetricNames.h` itself -- that is where the strings are defined; + * `MetricMacros.h` -- the macro definitions, whose `(name)` is a parameter; + * test code -- tests pass arbitrary literals to exercise the API, and + asserting on a literal is what proves a constant's value is unchanged. + + Scope -- a RATCHET, not a flag day. The rule fires only on a metric whose + FAMILY already has constants declared in a `*MetricNames.h` (matched on the + name's first underscore segment, e.g. `sync_`, `jobq_`, `unl_`). The metric + surface predates this rule by many phases, and failing the build on ~27 + pre-existing instruments at once would force one un-reviewable mega-change. + Declaring a constant for a family is what opts that family in, so: + * a NEW metric in an already-converted family is caught immediately; + * a literal left behind in a family being converted is caught; + * an untouched legacy family stays green until someone converts it, at + which point the whole family is enforced. + A literal whose family has no constants at all is reported as a Rule L + warning instead (non-fatal), so the remaining work stays visible. + + Presence-gated on `*MetricNames.h` existing at all; with no metric header + in the tree the rule has no families to enforce and is skipped.""" + if not find_metricname_headers(root): + report.skip("I", "no *MetricNames.h present") + return + names, keys, _ = metric_constants(root) + owned = metric_prefixes(names) + found = False + deferred: List[Tuple[str, str]] = [] + + def report_literal(rel: object, lineno: int, token: str, wire: str) -> None: + nonlocal found + # A label key is enforced once ANY label-key constant exists, because + # the label vocabulary is shared across families -- `outcome` means the + # same thing everywhere, so there is no per-family ratchet for it. + in_scope = ( + any(wire.startswith(p) for p in owned) if token.startswith("name") else True + ) + if in_scope: + found = True + report.violation( + "I", f"{rel}:{lineno}", token, "use a *MetricNames.h constant" + ) + else: + deferred.append((f"{rel}:{lineno}", token)) + + for path in sorted(iter_sources(root)): + if path.name.endswith("MetricNames.h") or path.name == "MetricMacros.h": + continue + if is_test_path(path): + continue + text = strip_comments(read_source(path)) + rel = path.relative_to(root) + for kind, name_idx, arglist, lineno in metric_calls(text): + args = split_top_level_args(arglist) + if len(args) > name_idx: + lit = STRING_LITERAL.search(args[name_idx]) + if lit: + report_literal( + rel, lineno, f'name "{lit.group(1)}" ({kind})', lit.group(1) + ) + # Label KEYS live in the label-set initializer, e.g. + # `{{"outcome", v}, {"reason", w}}`. Scanned over the WHOLE argument + # list rather than per split argument, because + # `split_top_level_args` balances only parentheses -- a brace- + # enclosed label set contains top-level commas and would be split + # apart mid-pair, hiding every key from the pattern. + # + # Scanning the whole list is safe: METRIC_PAIR_KEY requires an + # opening `{` before the key, which the instrument name and the + # description never have. Only the key position is checked; the + # VALUE after it is runtime data and exempt. + if keys: + for pm in METRIC_PAIR_KEY.finditer(arglist): + key = pm.group(1) + if not key.startswith('"'): + continue + report_literal(rel, lineno, f"label {key} ({kind})", key.strip('"')) + for loc, token in deferred: + report.warning("L", loc, token, "metric family not yet converted") + if not found: + report.ok("I: no string-literal names/label keys in converted metric families") + + +def iter_sources(root: Path) -> List[Path]: + """Every C++ source/header under src/ and include/.""" + return [ + p + for base in ("src", "include") + for ext in ("*.h", "*.cpp") + for p in (root / base).rglob(ext) + if p.is_file() + ] + + +def instrument_kinds(root: Path, wire_by_symbol: Dict[str, str]) -> Dict[str, str]: + """Map each declared instrument's WIRE name to its OTel instrument kind, by + looking at how the emit sites actually create it. + + The kind is what decides which suffix is correct, so it must be read from + the emit site rather than guessed from the name -- guessing from words like + "latency" mislabels a multi-series gauge whose units live in its label + VALUES (e.g. `nodestore_latency` observing `write_mean_us`), which is a + legitimate shape, not a violation. + + Returns one of `counter`, `histogram`, `gauge`, `updown` per wire name. + A name whose emit site is not found is absent from the result, so Rule J + checks only the shape-independent rules for it.""" + kinds: Dict[str, str] = {} + for path in iter_sources(root): + if path.name == "MetricMacros.h" or path.name.endswith("MetricNames.h"): + continue + if is_test_path(path): + continue + text = strip_comments(read_source(path)) + for kind, name_idx, arglist, _ in metric_calls(text): + args = split_top_level_args(arglist) + if len(args) <= name_idx: + continue + arg = args[name_idx].strip() + lit = re.fullmatch(r'"((?:[^"\\]|\\.)*)"', arg) + if lit: + wire: Optional[str] = lit.group(1) + else: + ident = IDENTIFIER_ARG.match(arg) + wire = ( + wire_by_symbol.get(ident.group(1).split("::")[-1]) + if ident + else None + ) + if wire is None: + continue + kinds[wire] = classify_instrument_kind(kind) + return kinds + + +def classify_instrument_kind(kind: str) -> str: + """Normalise a macro suffix (`COUNTER_INC_LABELED`) or a factory method + name (`CreateInt64ObservableGauge`) to one of counter/histogram/gauge/ + updown. Order matters: `UpDown` and `Observable` are checked before the + bare `Counter` substring they both contain.""" + if "HISTOGRAM" in kind or "Histogram" in kind: + return "histogram" + if "UPDOWN" in kind or "UpDown" in kind: + return "updown" + if "GAUGE" in kind or "Gauge" in kind: + return "gauge" + if "COUNTER" in kind or "Counter" in kind: + return "counter" + return "other" + + +def symbol_wire_names(root: Path) -> Dict[str, str]: + """Map each `*MetricNames.h` constant's C++ symbol name to its wire string, + so an emit site referencing `metric::dnsResolveTotal` can be resolved back + to `dns_resolve_total`.""" + out: Dict[str, str] = {} + for h in find_metricname_headers(root): + for m in METRIC_CONST_DEF.finditer(strip_comments(read_source(h))): + out[m.group(1)] = m.group(2) + return out + + +def run_rule_j_metric_suffixes(root: Path, report: Report) -> None: + """Rule J: instrument names follow the naming and unit/kind suffix rules. + + Checked against the `namespace metric` constants in `*MetricNames.h`, which + Rule I makes the only place an instrument name can be spelled. Enforced: + + * lower_snake_case (same shape as Rule G for span attributes); + * no `xrpld_`/`xrpl_` prefix -- the Prometheus exporter adds the namespace + itself, so a name carrying it would emit `xrpld_xrpld_*` on the wire; + * a monotonic COUNTER ends in `_total`, so `rate()` over it reads + correctly and a reader can tell it from a gauge; + * a HISTOGRAM ends in `_us`, `_ms` or `_seconds`: histograms in this repo + are all durations, and the OTel `unit` argument is not surfaced on the + Prometheus metric name, so an unlabelled duration is ambiguous; + * a GAUGE does not end in `_total`, which is reserved for counters. + + The instrument KIND comes from the emit site (see `instrument_kinds`), not + from words in the name, so a multi-series gauge that carries its units in + its label values is not mistaken for a mis-suffixed duration. + + Presence-gated: skipped when no `*MetricNames.h` exists.""" + if not find_metricname_headers(root): + report.skip("J", "no *MetricNames.h present") + return + names, _, _ = metric_constants(root) + if not names: + report.skip("J", "no metric instrument-name constants declared") + return + kinds = instrument_kinds(root, symbol_wire_names(root)) + found = False + + def flag(name: str, expected: str) -> None: + nonlocal found + found = True + report.violation("J", "*MetricNames.h", name, expected) + + for name in sorted(names): + if not SNAKE_SEGMENT.match(name): + flag(name, "must be lower_snake_case") + continue + if name.startswith(("xrpld_", "xrpl_")): + flag(name, "drop the prefix; the exporter adds it") + continue + kind = kinds.get(name) + if kind == "counter" and not name.endswith(METRIC_COUNTER_SUFFIX): + flag(name, "counter must end in _total") + elif kind == "histogram" and not name.endswith(METRIC_DURATION_SUFFIXES): + flag(name, "histogram needs _us/_ms/_seconds suffix") + elif kind == "gauge" and name.endswith(METRIC_COUNTER_SUFFIX): + flag(name, "_total is reserved for counters") + if not found: + report.ok(f"J: {len(names)} metric instrument name(s) follow the naming rules") + + +def run_rule_k_expected_metrics(root: Path, report: Report) -> None: + """Rule K: every metric named in the workload's `expected_metrics.json` + exists as a `namespace metric` constant. + + The layer that cannot reference a C++ constant, so it is validated against + one instead. This is the check that catches the failure mode this rule set + exists for: a metric renamed in code while the workload validator still + asserts on the old name, which fails as "metric never appeared" at runtime + rather than at review time. + + Scope. Only groups whose metrics come from the OTel instrument API are + checked. The file also inventories `beast::insight` statsd gauges/counters + (`statsd_gauges`, `statsd_counters`) and collector-derived spanmetrics, whose + names are minted by a different emit path -- `formatName()` lowercasing an + insight metric, or the spanmetrics connector -- and so have no + `*MetricNames.h` constant to resolve to by design. Checking them would fail + the build on metrics this convention does not govern. Within the OTel groups + the family ratchet still applies, so an unconverted family is out of scope + too. Presence-gated on both layers.""" + path = root / "docker" / "telemetry" / "workload" / "expected_metrics.json" + if not path.is_file(): + report.skip("K", "expected_metrics.json not present") + return + if not find_metricname_headers(root): + report.skip("K", "no *MetricNames.h to validate against") + return + names, _, _ = metric_constants(root) + try: + doc = json.loads(read_source(path)) + except json.JSONDecodeError as exc: + report.violation("K", str(path.relative_to(root)), str(exc), "valid JSON") + return + declared = { + base_metric_name(e) for e in expected_metric_names(doc, NON_OTEL_METRIC_GROUPS) + } + # Only entries inside a metric FAMILY the constants already own are checked. + # Anything else in the file predates *MetricNames.h, so demanding a constant + # for it would fail the build on unrelated pre-existing metrics rather than + # on the drift this rule targets. + owned = metric_prefixes(names) + unknown = sorted( + n for n in declared if n not in names and any(n.startswith(p) for p in owned) + ) + for n in unknown: + report.violation( + "K", + str(path.relative_to(root)), + n, + "no matching *MetricNames.h constant", + ) + if not unknown: + checked = sum(1 for n in declared if n in names) + report.ok(f"K: {checked} expected-metric name(s) resolve to constants") + + +# Suffixes the Prometheus exporter appends to a histogram instrument. The +# instrument name declared in code is the stem, so they are stripped before the +# constant lookup. +HISTOGRAM_SUFFIXES = ("_bucket", "_count", "_sum") + + +def base_metric_name(entry: str) -> str: + """Reduce an `expected_metrics.json` entry to the bare instrument name. + + Entries are written the way an operator would query them, not the way the + code declares them, so two forms have to be normalized away: + * a PromQL label selector -- `sync_state{metric="ledgers_behind"}`; + * an exporter-appended histogram suffix -- `..._ms_bucket`/`_count`/`_sum`, + which the SDK adds and the code never names. + Without this the rule would reject every entry in the file, which is a + checker bug rather than a naming violation.""" + name = entry.split("{", 1)[0].strip() + for suffix in HISTOGRAM_SUFFIXES: + if name.endswith(suffix): + return name[: -len(suffix)] + return name + + +def metric_prefixes(names: Set[str]) -> Set[str]: + """The first underscore-delimited segment of each declared instrument name, + e.g. {`sync_`, `jobq_`, `unl_`}. Used by Rule K to decide whether an entry + in `expected_metrics.json` belongs to a family this repo's constants own -- + so the rule flags a stale name inside a converted family without demanding a + constant for every unrelated metric in the file.""" + return {n.split("_", 1)[0] + "_" for n in names if "_" in n} + + +# Groups in expected_metrics.json whose names are NOT minted by the OTel +# instrument API, and therefore have no *MetricNames.h constant by design: +# statsd_gauges / statsd_counters -- beast::insight metrics, whose wire names +# come from formatName() lowercasing an insight metric path; +# spanmetrics -- synthesised by the collector's spanmetrics connector from +# span names, not declared in C++ at all. +NON_OTEL_METRIC_GROUPS = frozenset({"statsd_gauges", "statsd_counters", "spanmetrics"}) + + +def expected_metric_names( + doc: object, skip_groups: frozenset = frozenset() +) -> Set[str]: + """Collect metric names from `expected_metrics.json`. + + The file groups metrics by source (`spanmetrics`, `statsd_gauges`, + `phase9_nodestore`, ...), each group holding a `metrics` LIST OF STRINGS + alongside a prose `description`. Both that shape and a + `[{"metric": "..."}]` shape are accepted, and the walk is generic, so a + layout change degrades to "checks fewer names" rather than silently + checking none. `description` is skipped explicitly -- it is prose, and + letting it through would feed whole sentences to the family match. + + `skip_groups` drops whole top-level groups (see NON_OTEL_METRIC_GROUPS).""" + out: Set[str] = set() + + def walk(node: object, in_metrics: bool = False) -> None: + if isinstance(node, dict): + for key, val in node.items(): + if key == "description" or key in skip_groups: + continue + if key in ("metric", "name") and isinstance(val, str): + out.add(val) + else: + walk(val, in_metrics or key == "metrics") + elif isinstance(node, list): + for item in node: + if in_metrics and isinstance(item, str): + out.add(item) + else: + walk(item, in_metrics) + + walk(doc) + return out + + if __name__ == "__main__": main() diff --git a/.github/scripts/otel-naming/test_check_otel_naming.py b/.github/scripts/otel-naming/test_check_otel_naming.py index 890648cefe..451d4eff97 100644 --- a/.github/scripts/otel-naming/test_check_otel_naming.py +++ b/.github/scripts/otel-naming/test_check_otel_naming.py @@ -1,5 +1,9 @@ #!/usr/bin/env python3 +# cspell:ignore ISTOGRAM +# The all-caps macro name XRPL_METRIC_HISTOGRAM_RECORD trips cspell's +# compound-word splitter, which emits the subword "ISTOGRAM"; ignore it here. + """Unit tests for check_otel_naming.py. Stdlib-only (unittest), matching the dependency-free policy of the check itself. @@ -897,5 +901,471 @@ class RuleEReportTuple(unittest.TestCase): shutil.rmtree(d) +# A minimal *MetricNames.h body: the three namespaces the metric rules read. +def _metric_header( + metric_body: str = "", label_body: str = "", lval_body: str = "" +) -> str: + return ( + "#pragma once\n" + "namespace xrpl::telemetry {\n" + f"namespace metric {{\n{metric_body}\n}}\n" + f"namespace label {{\n{label_body}\n}}\n" + f"namespace lval {{\n{lval_body}\n}}\n" + "}\n" + ) + + +def _mc(symbol: str, wire: str) -> str: + """One `inline constexpr char SYM[] = "wire";` declaration.""" + return f'inline constexpr char {symbol}[] = "{wire}";' + + +class MetricConstantExtraction(unittest.TestCase): + """L1-metrics: instrument names / label keys / label values are read from + the right namespace, and comments never seed the set.""" + + def _run(self, header_text): + d = Path(tempfile.mkdtemp()) + try: + _write(d / "src" / "xrpld" / "telemetry" / "MetricNames.h", header_text) + return chk.metric_constants(d) + finally: + shutil.rmtree(d) + + def test_splits_by_namespace(self): + names, keys, vals = self._run( + _metric_header( + _mc("dnsResolveTotal", "dns_resolve_total"), + _mc("outcome", "outcome"), + _mc("resolved", "resolved"), + ) + ) + self.assertEqual(names, {"dns_resolve_total"}) + self.assertEqual(keys, {"outcome"}) + self.assertEqual(vals, {"resolved"}) + + def test_nested_lval_namespace_included(self): + # `namespace lval { namespace dns_resolve { ... } }` — the inner block is + # inside the outer's brace-matched span, so its values must be collected. + _, _, vals = self._run( + _metric_header( + lval_body="namespace dns_resolve {\n" + + _mc("resolved", "resolved") + + "\n}\n" + ) + ) + self.assertEqual(vals, {"resolved"}) + + def test_commented_constant_not_collected(self): + names, _, _ = self._run( + _metric_header( + "// " + _mc("ghost", "ghost_total") + "\n" + _mc("real", "real_total") + ) + ) + self.assertEqual(names, {"real_total"}) + + def test_block_commented_constant_not_collected(self): + names, _, _ = self._run( + _metric_header("/* " + _mc("ghost", "ghost_total") + " */\n") + ) + self.assertEqual(names, set()) + + def test_no_header_empty_sets(self): + d = Path(tempfile.mkdtemp()) + try: + (d / "src").mkdir() + self.assertEqual(chk.metric_constants(d), (set(), set(), set())) + finally: + shutil.rmtree(d) + + +class RuleIMetricLiterals(unittest.TestCase): + """Rule I: literal instrument names / label keys at a metric emit site are + flagged, but only inside a metric FAMILY that already has constants.""" + + def _run(self, rel_path, source, header=None): + d = Path(tempfile.mkdtemp()) + try: + _write( + d / "src" / "xrpld" / "telemetry" / "MetricNames.h", + ( + header + if header is not None + else _metric_header( + _mc("syncState", "sync_state"), _mc("outcome", "outcome") + ) + ), + ) + _write(d / rel_path, source) + report = chk.Report() + chk.run_rule_i_metric_literals(d, report) + return ( + sorted(v[2] for v in report.violations), + sorted(w[2] for w in report.warnings), + ) + finally: + shutil.rmtree(d) + + # ----- positive: a literal in a converted family must FAIL ----- + def test_literal_macro_name_flagged(self): + v, _ = self._run( + "src/xrpld/Foo.cpp", + 'XRPL_METRIC_COUNTER_INC(app, "sync_acquire_total", "d");\n', + ) + self.assertEqual(v, ['name "sync_acquire_total" (COUNTER_INC)']) + + def test_literal_factory_name_flagged(self): + v, _ = self._run( + "src/xrpld/Foo.cpp", + 'meter_->CreateInt64ObservableGauge("sync_thing", "d");\n', + ) + self.assertEqual(v, ['name "sync_thing" (CreateInt64ObservableGauge)']) + + def test_literal_label_key_flagged(self): + v, _ = self._run( + "src/xrpld/Foo.cpp", + 'XRPL_METRIC_COUNTER_INC_LABELED(app, metric::syncState, "d", ' + '{{"outcome", std::string(x)}});\n', + ) + self.assertEqual(v, ['label "outcome" (COUNTER_INC_LABELED)']) + + def test_multiline_call_flagged(self): + v, _ = self._run( + "src/xrpld/Foo.cpp", + 'XRPL_METRIC_COUNTER_INC(\n app,\n "sync_x_total",\n "d");\n', + ) + self.assertEqual(v, ['name "sync_x_total" (COUNTER_INC)']) + + # ----- negative: things Rule I must NOT flag ----- + def test_constant_name_accepted(self): + v, _ = self._run( + "src/xrpld/Foo.cpp", + 'XRPL_METRIC_COUNTER_INC(app, metric::syncState, "d");\n', + ) + self.assertEqual(v, []) + + def test_label_value_exempt(self): + # The VALUE after a constant key is runtime data and must not be flagged. + v, _ = self._run( + "src/xrpld/Foo.cpp", + 'XRPL_METRIC_COUNTER_INC_LABELED(app, metric::syncState, "d", ' + '{{label::outcome, std::string("resolved")}});\n', + ) + self.assertEqual(v, []) + + def test_description_exempt(self): + # arg2 is prose, not naming surface. + v, _ = self._run( + "src/xrpld/Foo.cpp", + 'XRPL_METRIC_COUNTER_INC(app, metric::syncState, "Some description");\n', + ) + self.assertEqual(v, []) + + def test_test_path_exempt(self): + v, _ = self._run( + "src/tests/libxrpl/telemetry/MetricMacros.cpp", + 'XRPL_METRIC_COUNTER_INC(app, "sync_lit_total", "d");\n', + ) + self.assertEqual(v, []) + + def test_metricnames_header_exempt(self): + v, _ = self._run( + "src/xrpld/telemetry/OtherMetricNames.h", + 'XRPL_METRIC_COUNTER_INC(app, "sync_lit_total", "d");\n', + ) + self.assertEqual(v, []) + + def test_macro_definition_header_exempt(self): + v, _ = self._run( + "src/xrpld/telemetry/MetricMacros.h", + 'XRPL_METRIC_COUNTER_INC(app, "sync_lit_total", "d");\n', + ) + self.assertEqual(v, []) + + # ----- the ratchet: an unconverted family warns (L) instead of failing ----- + def test_unconverted_family_warns_not_fails(self): + v, w = self._run( + "src/xrpld/Foo.cpp", + 'meter_->CreateDoubleObservableGauge("txq_metrics", "d");\n', + ) + self.assertEqual(v, []) + self.assertEqual(w, ['name "txq_metrics" (CreateDoubleObservableGauge)']) + + def test_skip_when_no_metric_header(self): + d = Path(tempfile.mkdtemp()) + try: + _write( + d / "src" / "xrpld" / "Foo.cpp", + 'XRPL_METRIC_COUNTER_INC(app, "x_total", "d");\n', + ) + report = chk.Report() + chk.run_rule_i_metric_literals(d, report) + self.assertEqual(report.violations, []) + self.assertTrue(any("I" in s for s in report.skips)) + finally: + shutil.rmtree(d) + + +class RuleJMetricSuffixes(unittest.TestCase): + """Rule J: instrument names follow the kind-appropriate suffix rules, with + the KIND read from the emit site rather than guessed from the name.""" + + def _run(self, metric_body, emit_source=""): + d = Path(tempfile.mkdtemp()) + try: + _write( + d / "src" / "xrpld" / "telemetry" / "MetricNames.h", + _metric_header(metric_body), + ) + if emit_source: + _write(d / "src" / "xrpld" / "Foo.cpp", emit_source) + report = chk.Report() + chk.run_rule_j_metric_suffixes(d, report) + return sorted((v[2], v[3]) for v in report.violations) + finally: + shutil.rmtree(d) + + # ----- positive ----- + def test_counter_without_total_flagged(self): + self.assertEqual( + self._run( + _mc("dnsResolve", "dns_resolve"), + 'XRPL_METRIC_COUNTER_INC(app, metric::dnsResolve, "d");\n', + ), + [("dns_resolve", "counter must end in _total")], + ) + + def test_histogram_without_unit_flagged(self): + self.assertEqual( + self._run( + _mc("dialLatency", "overlay_dial_latency"), + 'XRPL_METRIC_HISTOGRAM_RECORD(app, metric::dialLatency, "d", v);\n', + ), + [("overlay_dial_latency", "histogram needs _us/_ms/_seconds suffix")], + ) + + def test_gauge_with_total_flagged(self): + self.assertEqual( + self._run( + _mc("syncTotal", "sync_total"), + 'meter_->CreateInt64ObservableGauge(metric::syncTotal, "d");\n', + ), + [("sync_total", "_total is reserved for counters")], + ) + + def test_prefixed_name_flagged(self): + self.assertEqual( + self._run(_mc("prefixed", "xrpld_sync_state")), + [("xrpld_sync_state", "drop the prefix; the exporter adds it")], + ) + + def test_non_snake_case_flagged(self): + self.assertEqual( + self._run(_mc("camel", "syncState")), + [("syncState", "must be lower_snake_case")], + ) + + # ----- negative ----- + def test_conforming_counter_accepted(self): + self.assertEqual( + self._run( + _mc("dnsResolveTotal", "dns_resolve_total"), + 'XRPL_METRIC_COUNTER_INC(app, metric::dnsResolveTotal, "d");\n', + ), + [], + ) + + def test_conforming_histogram_accepted(self): + self.assertEqual( + self._run( + _mc("dialMs", "overlay_dial_latency_ms"), + 'XRPL_METRIC_HISTOGRAM_RECORD(app, metric::dialMs, "d", v);\n', + ), + [], + ) + + def test_gauge_named_latency_is_not_flagged(self): + # The regression this rule's kind-awareness exists for: a multi-series + # GAUGE whose units live in its label VALUES (nodestore_latency + # observing write_mean_us) must not be read as a mis-suffixed duration. + self.assertEqual( + self._run( + _mc("nodestoreLatency", "nodestore_latency"), + 'meter_->CreateInt64ObservableGauge(metric::nodestoreLatency, "d");\n', + ), + [], + ) + + def test_unknown_kind_only_shape_checked(self): + # With no emit site found, the kind is unknown, so only the + # shape-independent rules apply -- a bare gauge-ish name is fine. + self.assertEqual(self._run(_mc("syncState", "sync_state")), []) + + def test_skip_when_no_header(self): + d = Path(tempfile.mkdtemp()) + try: + (d / "src").mkdir() + report = chk.Report() + chk.run_rule_j_metric_suffixes(d, report) + self.assertEqual(report.violations, []) + self.assertTrue(any("J" in s for s in report.skips)) + finally: + shutil.rmtree(d) + + +class RuleKExpectedMetrics(unittest.TestCase): + """Rule K: a name in expected_metrics.json inside a converted family must + resolve to a *MetricNames.h constant.""" + + def _run(self, json_text, metric_body): + d = Path(tempfile.mkdtemp()) + try: + _write( + d / "src" / "xrpld" / "telemetry" / "MetricNames.h", + _metric_header(metric_body), + ) + _write( + d / "docker" / "telemetry" / "workload" / "expected_metrics.json", + json_text, + ) + report = chk.Report() + chk.run_rule_k_expected_metrics(d, report) + return sorted(v[2] for v in report.violations), report.skips + finally: + shutil.rmtree(d) + + def test_stale_name_in_converted_family_flagged(self): + # `sync_` is owned (sync_state is declared), so a sibling that resolves + # to nothing is the rename-drift this rule exists to catch. + v, _ = self._run( + '{"metrics": [{"metric": "sync_stale_total"}]}', + _mc("syncState", "sync_state"), + ) + self.assertEqual(v, ["sync_stale_total"]) + + def test_declared_name_accepted(self): + v, _ = self._run( + '{"metrics": [{"metric": "sync_state"}]}', _mc("syncState", "sync_state") + ) + self.assertEqual(v, []) + + def test_unowned_family_not_flagged(self): + # `txq_` has no constants, so its entries are out of scope rather than + # a failure -- the ratchet again. + v, _ = self._run( + '{"metrics": [{"metric": "txq_metrics"}]}', _mc("syncState", "sync_state") + ) + self.assertEqual(v, []) + + def test_name_key_also_read(self): + v, _ = self._run( + '{"metrics": [{"name": "sync_stale_total"}]}', + _mc("syncState", "sync_state"), + ) + self.assertEqual(v, ["sync_stale_total"]) + + def test_nested_structure_walked(self): + v, _ = self._run( + '{"groups": {"a": {"items": [{"metric": "sync_stale_total"}]}}}', + _mc("syncState", "sync_state"), + ) + self.assertEqual(v, ["sync_stale_total"]) + + def test_promql_selector_stripped(self): + # Entries are written as an operator would query them; the selector must + # be stripped before the constant lookup or every entry would fail. + v, _ = self._run( + '{"g": {"metrics": ["sync_state{metric=\\"ledgers_behind\\"}"]}}', + _mc("syncState", "sync_state"), + ) + self.assertEqual(v, []) + + def test_histogram_suffix_stripped(self): + # _bucket/_count/_sum are appended by the exporter, never named in code. + v, _ = self._run( + '{"g": {"metrics": ["overlay_dial_latency_ms_bucket"]}}', + _mc("dialMs", "overlay_dial_latency_ms"), + ) + self.assertEqual(v, []) + + def test_statsd_group_skipped(self): + # beast::insight names come from formatName(), not the OTel API, so a + # statsd group must not be held to a *MetricNames.h constant. + v, _ = self._run( + '{"statsd_gauges": {"metrics": ["sync_not_a_constant"]}}', + _mc("syncState", "sync_state"), + ) + self.assertEqual(v, []) + + def test_spanmetrics_group_skipped(self): + v, _ = self._run( + '{"spanmetrics": {"metrics": ["sync_span_calls_total"]}}', + _mc("syncState", "sync_state"), + ) + self.assertEqual(v, []) + + def test_description_prose_not_treated_as_metric(self): + v, _ = self._run( + '{"g": {"description": "sync_ stuff about metrics", "metrics": []}}', + _mc("syncState", "sync_state"), + ) + self.assertEqual(v, []) + + def test_malformed_json_reported(self): + v, _ = self._run("{not json", _mc("syncState", "sync_state")) + self.assertEqual(len(v), 1) + + def test_skip_when_file_absent(self): + d = Path(tempfile.mkdtemp()) + try: + _write( + d / "src" / "xrpld" / "telemetry" / "MetricNames.h", + _metric_header(_mc("syncState", "sync_state")), + ) + report = chk.Report() + chk.run_rule_k_expected_metrics(d, report) + self.assertEqual(report.violations, []) + self.assertTrue(any("K" in s for s in report.skips)) + finally: + shutil.rmtree(d) + + +class InstrumentKindClassification(unittest.TestCase): + """classify_instrument_kind: `UpDown`/`Observable` are checked before the + bare `Counter` substring they both contain.""" + + def test_macro_kinds(self): + for kind, want in ( + ("COUNTER_INC", "counter"), + ("COUNTER_ADD_LABELED", "counter"), + ("HISTOGRAM_RECORD", "histogram"), + ("UPDOWN_ADD", "updown"), + ("OBSERVABLE_GAUGE_REGISTER", "gauge"), + ): + self.assertEqual(chk.classify_instrument_kind(kind), want, kind) + + def test_factory_kinds(self): + for kind, want in ( + ("CreateUInt64Counter", "counter"), + ("CreateDoubleHistogram", "histogram"), + ("CreateInt64UpDownCounter", "updown"), + ("CreateInt64ObservableGauge", "gauge"), + ("CreateInt64ObservableCounter", "counter"), + ("CreateInt64ObservableUpDownCounter", "updown"), + ): + self.assertEqual(chk.classify_instrument_kind(kind), want, kind) + + +class MetricPrefixFamilies(unittest.TestCase): + def test_first_segment_is_the_family(self): + self.assertEqual( + chk.metric_prefixes({"sync_state", "jobq_backlog", "unl_quorum"}), + {"sync_", "jobq_", "unl_"}, + ) + + def test_name_without_underscore_has_no_family(self): + self.assertEqual(chk.metric_prefixes({"uptime"}), set()) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d15548e239..778babf012 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -414,13 +414,65 @@ python .github/scripts/otel-naming/check_otel_naming.py See [.github/scripts/otel-naming/README.md](.github/scripts/otel-naming/README.md) for the full rule list. +## Telemetry metric naming + +The metric-side counterpart of the span rules above. Metric instrument names and +metric label keys are duplicated across the emit site, the instrument +registration, the unit test, `expected_metrics.json`, the dashboard PromQL and +the runbook, so a rename touches six places and a typo in any one of them fails +silently at runtime — a metric that never appears, or a label that never joins. +The constants in the `*MetricNames.h` headers are the single source of truth for +the C++ layers; a CI check validates the layers that cannot reference a constant. + +1. Instrument names are bare `lower_snake_case` with **no `xrpld_` prefix**. The + Prometheus exporter adds the namespace itself, so a name carrying it emits + `xrpld_xrpld_*` on the wire. +2. A monotonic counter ends in `_total`, so `rate()` over it reads correctly and + a reader can tell it from a gauge at a glance. +3. A duration carries its unit as the suffix — `_us`, `_ms` or `_seconds`. The + unit belongs in the name because the OTel `unit` argument is not surfaced on + the Prometheus metric name. +4. A gauge that snapshots current state takes no suffix (`jobq_backlog`, + `sync_state`), and never `_total`. +5. Label keys are `lower_snake_case` and must have **bounded** cardinality. A + multi-series gauge discriminates its readings with the `metric` label rather + than minting one instrument per reading. +6. Label **values** are declared as constants only when the code picks them from + a fixed set (`namespace lval`). A value derived from runtime data — a peer + address, a ledger hash — must never become a label on a metric. + +Always reference the `*MetricNames.h` constants for instrument names and label +keys — never pass a string literal. (Label _values_ may be runtime data.) Note +that these headers use `constexpr char[]`, not the `makeStr`/`StaticStr` DSL the +`*SpanNames.h` headers use: the OTel C++ API takes `nostd::string_view`, which +constructs from `char const*` but has no constructor from `std::string_view`, so +`StaticStr` does not compile in an instrument-name or label-key position. + +Enforcement is by the same script as the span rules, whose metric rules are: + +- **I** — no string literal as an instrument name or label key at an emit site + (the mirror of Rule F). Scoped by metric _family_ (the first underscore + segment) so conversion can proceed subsystem by subsystem: declaring a + constant opts that family in. An unconverted family is reported as a + non-fatal **L** warning, keeping the remaining work visible. +- **J** — the suffix conventions above. The instrument _kind_ is read from the + emit site, not guessed from the name, so a multi-series gauge whose units live + in its label values is not mistaken for a mis-suffixed duration. +- **K** — every metric named in `docker/telemetry/workload/expected_metrics.json` + resolves to a declared constant. This is the check that catches a metric + renamed in code while the workload validator still asserts the old name. + Groups whose names come from a different emit path (`statsd_gauges`, + `statsd_counters` from `beast::insight`, and collector-derived `spanmetrics`) + are out of scope by design. + ## Adding a new OTel metric See `src/xrpld/telemetry/MetricMacros.h` for the call-site macros covering every OTel instrument kind (Counter, UpDownCounter, Histogram, Gauge, and their -Observable/async counterparts) and the "Adding a New Metric" section in -[docs/telemetry-runbook.md](docs/telemetry-runbook.md) for the walkthrough and a -need-to-macro lookup table. +Observable/async counterparts), `src/xrpld/telemetry/MetricNames.h` for the name +and label constants to reference (and the rules above), and the "Adding a New +Metric" section in [docs/telemetry-runbook.md](docs/telemetry-runbook.md) for the +walkthrough and a need-to-macro lookup table. ## Contracts and instrumentation diff --git a/docs/telemetry-runbook.md b/docs/telemetry-runbook.md index 378083dddd..8e80edd981 100644 --- a/docs/telemetry-runbook.md +++ b/docs/telemetry-runbook.md @@ -1508,23 +1508,48 @@ Use the call-site macros in `src/xrpld/telemetry/MetricMacros.h` -- no | Last-value snapshot (not a distribution) | `XRPL_METRIC_GAUGE_RECORD` [+ `_LABELED`] -- requires an ABI v2 opentelemetry-cpp build; this repo currently builds ABI v1, so use the observable-gauge row below instead | | Value your own code already tracks, sampled on a timer | `XRPL_METRIC_OBSERVABLE_GAUGE_REGISTER` / `_COUNTER_REGISTER` / `_UPDOWN_REGISTER` | +First declare the name in `src/xrpld/telemetry/MetricNames.h` -- the emit site +must reference a constant, never a string literal, and CI Rule I enforces that +for any metric family that already has constants: + +```cpp +// in src/xrpld/telemetry/MetricNames.h, namespace metric: +inline constexpr char myNewThingTotal[] = "my_new_thing_total"; +inline constexpr char myInFlightRequests[] = "my_in_flight_requests"; +inline constexpr char myThingSize[] = "my_thing_size"; +``` + +Then emit against it: + ```cpp #include +#include // Monotonic counter: -XRPL_METRIC_COUNTER_INC(app_, "my_new_thing_total", "Description of what this counts"); +XRPL_METRIC_COUNTER_INC( + app_, metric::myNewThingTotal, "Description of what this counts"); // Value that can go up and down, e.g. in-flight work (no _total suffix -- that // is reserved for monotonic counters; an UpDownCounter is a current value): -XRPL_METRIC_UPDOWN_ADD(app_, "my_in_flight_requests", "Currently executing", 1); // on start -XRPL_METRIC_UPDOWN_ADD(app_, "my_in_flight_requests", "Currently executing", -1); // on finish +XRPL_METRIC_UPDOWN_ADD(app_, metric::myInFlightRequests, "Currently executing", 1); +XRPL_METRIC_UPDOWN_ADD(app_, metric::myInFlightRequests, "Currently executing", -1); + +// Labelled: the KEY is a constant too, and so is the VALUE when it comes from a +// fixed set. Only runtime data stays a plain expression. +XRPL_METRIC_COUNTER_INC_LABELED( + app_, + metric::myNewThingTotal, + "Description of what this counts", + {{label::outcome, std::string(lval::dns_resolve::resolved)}}); // Sampled from your own state, on the OTel export timer (register ONCE, in init code): -XRPL_METRIC_OBSERVABLE_GAUGE_REGISTER(app_, "my_thing_size", "Current size", +XRPL_METRIC_OBSERVABLE_GAUGE_REGISTER(app_, metric::myThingSize, "Current size", [this] { return static_cast(myThing_.size()); }); ``` -Counters use a `_total` suffix by convention. A histogram whose values can +Naming rules (counter `_total`, duration `_us`/`_ms`/`_seconds`, no `xrpld_` +prefix, bounded label cardinality) are listed in CONTRIBUTING.md -> +"Telemetry metric naming" and enforced by CI Rules I/J/K. A histogram whose values can exceed ~10,000 units (e.g. a microsecond duration beyond 10ms) still needs one line added to `addMicrosecondHistogramView()` in `MetricsRegistry.cpp` -- the only case that still touches a central file. There is no way to read a metric's diff --git a/src/tests/libxrpl/telemetry/MetricMacros.cpp b/src/tests/libxrpl/telemetry/MetricMacros.cpp index d29b784994..9b661186d1 100644 --- a/src/tests/libxrpl/telemetry/MetricMacros.cpp +++ b/src/tests/libxrpl/telemetry/MetricMacros.cpp @@ -27,6 +27,7 @@ #include #include +#include #include #include @@ -533,7 +534,7 @@ TEST(MetricMacros, counter_inc_labeled_does_not_crash) app, "test_macro_labeled_counter_total", "Test labeled counter for macro unit test", - {{"reason", std::string("unit_test")}}); + {{telemetry::label::reason, std::string("unit_test")}}); // Instrument was created exactly once at this single call site. EXPECT_EQ(app.registry().meterCalls(), 1); @@ -582,7 +583,7 @@ TEST(MetricMacros, updown_add_labeled_does_not_crash) "test_macro_updown_labeled_total", "Test labeled updown for macro unit test", -1, - {{"reason", std::string("unit_test")}}); + {{telemetry::label::reason, std::string("unit_test")}}); EXPECT_EQ(app.registry().meterCalls(), 1); } @@ -688,9 +689,9 @@ TEST(MetricMacros, dns_resolve_records_exact_counts_and_latency) { XRPL_METRIC_COUNTER_INC_LABELED( app, - "dns_resolve_total", + telemetry::metric::dnsResolveTotal, "Peer hostname resolutions, by outcome", - {{"outcome", std::string(resolved ? "resolved" : "empty")}}); + {{telemetry::label::outcome, std::string(resolved ? "resolved" : "empty")}}); } // Two latency samples with known values: 1.5 ms + 2.5 ms = 4.0 ms. @@ -698,7 +699,7 @@ TEST(MetricMacros, dns_resolve_records_exact_counts_and_latency) { XRPL_METRIC_HISTOGRAM_RECORD( app, - "dns_resolve_latency_ms", + telemetry::metric::dnsResolveLatencyMs, "Time taken to resolve a configured peer hostname, in milliseconds", ms); } @@ -740,9 +741,9 @@ TEST(MetricMacros, overlay_connect_records_exact_counts_per_outcome) auto const bump = [&app](char const* outcome) { XRPL_METRIC_COUNTER_INC_LABELED( app, - "overlay_connect_total", + telemetry::metric::overlayConnectTotal, "Outbound peer connection attempts, by terminal outcome", - {{"outcome", std::string(outcome)}}); + {{telemetry::label::outcome, std::string(outcome)}}); }; bump("connected"); bump("tcp_fail"); @@ -756,7 +757,7 @@ TEST(MetricMacros, overlay_connect_records_exact_counts_per_outcome) { XRPL_METRIC_HISTOGRAM_RECORD( app, - "overlay_dial_latency_ms", + telemetry::metric::overlayDialLatencyMs, "Time from starting an outbound peer dial to its terminal outcome, in milliseconds", ms); } @@ -792,9 +793,9 @@ TEST(MetricMacros, handshake_negotiation_fail_keeps_reasons_distinct) { XRPL_METRIC_COUNTER_INC_LABELED( app, - "handshake_negotiation_fail_total", + telemetry::metric::handshakeNegotiationFailTotal, "Peer handshake negotiations rejected, by reason", - {{"reason", std::string(reason)}}); + {{telemetry::label::reason, std::string(reason)}}); } auto const data = provider.collect(); @@ -832,9 +833,10 @@ TEST(MetricMacros, unl_fetch_total_keys_series_on_site_and_outcome_pair) auto const bump = [&app](char const* site, char const* outcome) { XRPL_METRIC_COUNTER_INC_LABELED( app, - "unl_fetch_total", + telemetry::metric::unlFetchTotal, "Validator list fetch attempts, by site and outcome", - {{"site", std::string(site)}, {"outcome", std::string(outcome)}}); + {{telemetry::label::site, std::string(site)}, + {telemetry::label::outcome, std::string(outcome)}}); }; constexpr char const* kSiteA = "https://a.example.com/vl.json"; @@ -901,7 +903,7 @@ TEST(MetricMacros, unl_quorum_gauge_observes_exact_trusted_keys_and_quorum) // deregisters the callback (ObservableInstrument's destructor calls // CleanupCallback), which is why the real registry holds it in a member. auto gauge = provider.meter()->CreateInt64ObservableGauge( - "unl_quorum", "Trusted UNL key count vs required quorum"); + telemetry::metric::unlQuorum, "Trusted UNL key count vs required quorum"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* self = static_cast(state); @@ -909,7 +911,7 @@ TEST(MetricMacros, unl_quorum_gauge_observes_exact_trusted_keys_and_quorum) auto observe = [&](char const* name, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", name}}); + ->Observe(value, {{telemetry::label::metric, name}}); }; observe("trusted_keys", self->trustedKeys); observe("quorum", self->quorum); @@ -941,14 +943,15 @@ TEST(MetricMacros, clock_skew_gauge_observes_exact_negative_offset) std::int64_t offsetSeconds = -3; auto gauge = provider.meter()->CreateInt64ObservableGauge( - "clock_close_offset_seconds", "Network close time offset from the local clock, in seconds"); + telemetry::metric::clockCloseOffsetSeconds, + "Network close time offset from the local clock, in seconds"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* value = static_cast(state); auto observe = [&](char const* name, std::int64_t v) { opentelemetry::nostd::get>>(result) - ->Observe(v, {{"metric", name}}); + ->Observe(v, {{telemetry::label::metric, name}}); }; observe("offset", *value); }, @@ -979,35 +982,35 @@ TEST(MetricMacros, sync_diagnostics_metrics_emit_nothing_when_registry_disabled) XRPL_METRIC_COUNTER_INC_LABELED( app, - "dns_resolve_total", + telemetry::metric::dnsResolveTotal, "Peer hostname resolutions, by outcome", - {{"outcome", std::string("resolved")}}); + {{telemetry::label::outcome, std::string("resolved")}}); XRPL_METRIC_HISTOGRAM_RECORD( app, - "dns_resolve_latency_ms", + telemetry::metric::dnsResolveLatencyMs, "Time taken to resolve a configured peer hostname, in milliseconds", 1.5); XRPL_METRIC_COUNTER_INC_LABELED( app, - "overlay_connect_total", + telemetry::metric::overlayConnectTotal, "Outbound peer connection attempts, by terminal outcome", - {{"outcome", std::string("connected")}}); + {{telemetry::label::outcome, std::string("connected")}}); XRPL_METRIC_HISTOGRAM_RECORD( app, - "overlay_dial_latency_ms", + telemetry::metric::overlayDialLatencyMs, "Time from starting an outbound peer dial to its terminal outcome, in milliseconds", 10.0); XRPL_METRIC_COUNTER_INC_LABELED( app, - "handshake_negotiation_fail_total", + telemetry::metric::handshakeNegotiationFailTotal, "Peer handshake negotiations rejected, by reason", - {{"reason", std::string("wrong_network")}}); + {{telemetry::label::reason, std::string("wrong_network")}}); XRPL_METRIC_COUNTER_INC_LABELED( app, - "unl_fetch_total", + telemetry::metric::unlFetchTotal, "Validator list fetch attempts, by site and outcome", - {{"site", std::string("https://a.example.com/vl.json")}, - {"outcome", std::string("accepted")}}); + {{telemetry::label::site, std::string("https://a.example.com/vl.json")}, + {telemetry::label::outcome, std::string("accepted")}}); auto const data = provider.collect(); @@ -1061,9 +1064,9 @@ TEST(MetricMacros, state_changes_total_keys_series_on_from_to_pair) auto const transition = [&app](char const* from, char const* to) { XRPL_METRIC_COUNTER_INC_LABELED( app, - "state_changes_total", + telemetry::metric::stateChangesTotal, "Total operating mode changes", - {{"from", std::string(from)}, {"to", std::string(to)}}); + {{telemetry::label::from, std::string(from)}, {telemetry::label::to, std::string(to)}}); }; // A clean climb: disconnected -> connected -> syncing -> full, once each. @@ -1142,8 +1145,8 @@ TEST(MetricMacros, sync_state_gauge_observes_exact_stuck_node_values) // Keep the instrument alive for the whole test: destroying the handle // deregisters the callback, which is why the real registry holds a member. - auto gauge = - provider.meter()->CreateInt64ObservableGauge("sync_state", "Sync-pipeline health signals"); + auto gauge = provider.meter()->CreateInt64ObservableGauge( + telemetry::metric::syncState, "Sync-pipeline health signals"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* self = static_cast(state); @@ -1151,7 +1154,7 @@ TEST(MetricMacros, sync_state_gauge_observes_exact_stuck_node_values) auto observe = [&](char const* name, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", name}}); + ->Observe(value, {{telemetry::label::metric, name}}); }; observe("initial_full_duration_us", self->initialFullDurationUs); observe("network_ledger_gate", self->networkLedgerGate); @@ -1211,7 +1214,7 @@ TEST(MetricMacros, stall_events_counter_observes_exact_cumulative_count) std::int64_t stallEpisodes = 3; auto counter = provider.meter()->CreateInt64ObservableCounter( - "server_stall_events_total", "Total server main-loop stall episodes"); + telemetry::metric::serverStallEventsTotal, "Total server main-loop stall episodes"); counter->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* value = static_cast(state); @@ -1250,9 +1253,10 @@ TEST(MetricMacros, state_changes_total_emits_nothing_when_registry_disabled) XRPL_METRIC_COUNTER_INC_LABELED( app, - "state_changes_total", + telemetry::metric::stateChangesTotal, "Total operating mode changes", - {{"from", std::string("connected")}, {"to", std::string("full")}}); + {{telemetry::label::from, std::string("connected")}, + {telemetry::label::to, std::string("full")}}); auto const data = provider.collect(); @@ -1299,9 +1303,9 @@ TEST(MetricMacros, acquire_source_splits_local_and_network) auto const acquire = [&app](bool localComplete) { XRPL_METRIC_COUNTER_INC_LABELED( app, - "sync_acquire_source_total", + telemetry::metric::syncAcquireSourceTotal, "Ledger acquires by where the data came from", - {{"source", std::string(localComplete ? "local" : "network")}}); + {{telemetry::label::source, std::string(localComplete ? "local" : "network")}}); }; // One satisfied locally, two needing the network. acquire(true); @@ -1341,7 +1345,7 @@ TEST(MetricMacros, acquire_no_progress_counts_only_stalled_timeouts) { XRPL_METRIC_COUNTER_INC( app, - "sync_acquire_no_progress_total", + telemetry::metric::syncAcquireNoProgressTotal, "Ledger-acquire timeouts where no new node arrived"); } }; @@ -1390,10 +1394,10 @@ TEST(MetricMacros, addnode_outcomes_record_exact_batch_tallies) return; XRPL_METRIC_COUNTER_ADD_LABELED( app, - "sync_addnode_total", + telemetry::metric::syncAddnodeTotal, "SHAMap nodes received during ledger acquire, by outcome", static_cast(count), - {{"outcome", std::string(outcome)}}); + {{telemetry::label::outcome, std::string(outcome)}}); }; emit("good", good); emit("duplicate", duplicate); @@ -1460,7 +1464,8 @@ TEST(MetricMacros, sync_acquire_gauge_observes_exact_stuck_acquire_values) // Keep the instrument alive for the whole test: destroying the handle // deregisters the callback, which is why the real registry holds a member. auto gauge = provider.meter()->CreateInt64ObservableGauge( - "sync_acquire", "Aggregate ledger-acquire progress across in-flight acquires"); + telemetry::metric::syncAcquire, + "Aggregate ledger-acquire progress across in-flight acquires"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* self = static_cast(state); @@ -1468,7 +1473,7 @@ TEST(MetricMacros, sync_acquire_gauge_observes_exact_stuck_acquire_values) auto observe = [&](char const* name, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", name}}); + ->Observe(value, {{telemetry::label::metric, name}}); }; observe("missing_state_nodes_max", self->maxMissingStateNodes); observe("missing_tx_nodes_max", self->maxMissingTxNodes); @@ -1527,7 +1532,8 @@ TEST(MetricMacros, shamap_cache_hit_rate_gauge_normalizes_to_unit_fraction) float rawHitRatePercent = 90.0F; auto gauge = provider.meter()->CreateDoubleObservableGauge( - "shamap_cache_hit_rate", "SHAMap tree-node cache hit rate (0.0-1.0), by cache"); + telemetry::metric::shamapCacheHitRate, + "SHAMap tree-node cache hit rate (0.0-1.0), by cache"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* raw = static_cast(state); @@ -1535,7 +1541,8 @@ TEST(MetricMacros, shamap_cache_hit_rate_gauge_normalizes_to_unit_fraction) opentelemetry::nostd::get< opentelemetry::nostd::shared_ptr>>( result) - ->Observe(static_cast(*raw / 100.0F), {{"metric", "treenode"}}); + ->Observe( + static_cast(*raw / 100.0F), {{telemetry::label::metric, "treenode"}}); }, &rawHitRatePercent); @@ -1580,17 +1587,19 @@ TEST(MetricMacros, acquire_counters_emit_nothing_when_registry_disabled) XRPL_METRIC_COUNTER_INC_LABELED( app, - "sync_acquire_source_total", + telemetry::metric::syncAcquireSourceTotal, "Ledger acquires by where the data came from", - {{"source", std::string("network")}}); + {{telemetry::label::source, std::string("network")}}); XRPL_METRIC_COUNTER_INC( - app, "sync_acquire_no_progress_total", "Ledger-acquire timeouts where no new node arrived"); + app, + telemetry::metric::syncAcquireNoProgressTotal, + "Ledger-acquire timeouts where no new node arrived"); XRPL_METRIC_COUNTER_ADD_LABELED( app, - "sync_addnode_total", + telemetry::metric::syncAddnodeTotal, "SHAMap nodes received during ledger acquire, by outcome", static_cast(5), - {{"outcome", std::string("good")}}); + {{telemetry::label::outcome, std::string("good")}}); auto const data = provider.collect(); @@ -1643,7 +1652,8 @@ TEST(MetricMacros, jobq_backlog_gauge_separates_waiting_running_and_deferred) // Keep the instrument alive for the whole test: destroying the handle // deregisters the callback, which is why the real registry holds a member. auto gauge = provider.meter()->CreateInt64ObservableGauge( - "jobq_backlog", "JobQueue occupancy per job type (waiting/running/deferred)"); + telemetry::metric::jobqBacklog, + "JobQueue occupancy per job type (waiting/running/deferred)"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* counts = static_cast const*>(state); @@ -1651,7 +1661,9 @@ TEST(MetricMacros, jobq_backlog_gauge_separates_waiting_running_and_deferred) auto observe = [&](char const* field, std::string const& jobType, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", field}, {"job_type", jobType}}); + ->Observe( + value, + {{telemetry::label::metric, field}, {telemetry::label::jobType, jobType}}); }; for (auto const& count : *counts) { @@ -1746,14 +1758,15 @@ TEST(MetricMacros, jobq_saturation_gauge_observes_exact_pool_exhaustion_values) JobQueue::WorkerSaturation observed{.runningTasks = 6, .workerThreads = 6, .totalWaiting = 12}; auto gauge = provider.meter()->CreateInt64ObservableGauge( - "jobq_saturation", "Worker-pool saturation: tasks in flight, worker threads, jobs queued"); + telemetry::metric::jobqSaturation, + "Worker-pool saturation: tasks in flight, worker threads, jobs queued"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* self = static_cast(state); auto observe = [&](char const* field, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", field}}); + ->Observe(value, {{telemetry::label::metric, field}}); }; observe("running_tasks", self->runningTasks); observe("worker_threads", self->workerThreads); @@ -1847,7 +1860,8 @@ TEST(MetricMacros, peer_ledger_supply_gauge_names_a_gap_no_peer_can_fill) // Keep the instrument alive for the whole test: destroying the handle // deregisters the callback, which is why the real registry holds a member. auto gauge = provider.meter()->CreateInt64ObservableGauge( - "peer_ledger_supply", "Peer coverage of the ledger sequence this node needs"); + telemetry::metric::peerLedgerSupply, + "Peer coverage of the ledger sequence this node needs"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* self = static_cast(state); @@ -1855,7 +1869,7 @@ TEST(MetricMacros, peer_ledger_supply_gauge_names_a_gap_no_peer_can_fill) auto observe = [&](char const* field, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", field}}); + ->Observe(value, {{telemetry::label::metric, field}}); }; observe("peers_reporting", self->peersReporting); observe("peers_serving_validated", self->peersServingValidated); @@ -1919,14 +1933,15 @@ TEST(MetricMacros, peer_ledger_supply_gauge_reads_zero_window_as_unknown) .supplyMaxSeq = 5000}; auto gauge = provider.meter()->CreateInt64ObservableGauge( - "peer_ledger_supply", "Peer coverage of the ledger sequence this node needs"); + telemetry::metric::peerLedgerSupply, + "Peer coverage of the ledger sequence this node needs"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* self = static_cast(state); auto observe = [&](char const* field, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", field}}); + ->Observe(value, {{telemetry::label::metric, field}}); }; observe("peers_reporting", self->peersReporting); observe("peers_serving_validated", self->peersServingValidated); @@ -1996,7 +2011,8 @@ TEST(MetricMacros, slot_census_gauge_names_each_bootstrap_fault_exactly) .livecache = 12}; auto gauge = provider.meter()->CreateInt64ObservableGauge( - "peerfinder_slot_census", "PeerFinder slots, connection attempts and address caches"); + telemetry::metric::peerfinderSlotCensus, + "PeerFinder slots, connection attempts and address caches"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* self = static_cast(state); @@ -2004,7 +2020,7 @@ TEST(MetricMacros, slot_census_gauge_names_each_bootstrap_fault_exactly) auto observe = [&](char const* field, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", field}}); + ->Observe(value, {{telemetry::label::metric, field}}); }; observe("out_active", self->outActive); observe("out_max", self->outMax); @@ -2079,14 +2095,15 @@ TEST(MetricMacros, slot_census_gauge_reports_every_field_even_when_idle) .livecache = 0}; auto gauge = provider.meter()->CreateInt64ObservableGauge( - "peerfinder_slot_census", "PeerFinder slots, connection attempts and address caches"); + telemetry::metric::peerfinderSlotCensus, + "PeerFinder slots, connection attempts and address caches"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* self = static_cast(state); auto observe = [&](char const* field, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", field}}); + ->Observe(value, {{telemetry::label::metric, field}}); }; observe("out_active", self->outActive); observe("out_max", self->outMax); @@ -2170,14 +2187,15 @@ TEST(MetricMacros, amendment_block_gauge_observes_exact_countdown_and_sentinel) .warned = false, .expectedEpochSeconds = {}, .nowEpochSeconds = kNowEpochSeconds}; auto gauge = provider.meter()->CreateInt64ObservableGauge( - "amendment_block", "Amendment-block warning and seconds until the node stops validating"); + telemetry::metric::amendmentBlock, + "Amendment-block warning and seconds until the node stops validating"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* self = static_cast(state); auto observe = [&](char const* field, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", field}}); + ->Observe(value, {{telemetry::label::metric, field}}); }; observe("warned", self->warned ? 1 : 0); @@ -2246,14 +2264,15 @@ TEST(MetricMacros, amendment_block_gauge_clamps_past_due_and_carries_no_amendmen .nowEpochSeconds = kNowEpochSeconds}; auto gauge = provider.meter()->CreateInt64ObservableGauge( - "amendment_block", "Amendment-block warning and seconds until the node stops validating"); + telemetry::metric::amendmentBlock, + "Amendment-block warning and seconds until the node stops validating"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* self = static_cast(state); auto observe = [&](char const* field, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", field}}); + ->Observe(value, {{telemetry::label::metric, field}}); }; observe("warned", self->warned ? 1 : 0); std::int64_t secondsToBlock = -1; @@ -2315,9 +2334,10 @@ TEST(MetricMacros, peer_disconnect_total_keys_series_on_reason_and_direction_pai auto const bump = [&app](char const* reason, char const* direction) { XRPL_METRIC_COUNTER_INC_LABELED( app, - "peer_disconnect_total", + telemetry::metric::peerDisconnectTotal, "Peer disconnects, by cause and connection direction", - {{"reason", std::string(reason)}, {"direction", std::string(direction)}}); + {{telemetry::label::reason, std::string(reason)}, + {telemetry::label::direction, std::string(direction)}}); }; // Two OUR-FAULT reasons: this node could not keep up with what it owed the @@ -2394,9 +2414,10 @@ TEST(MetricMacros, peer_disconnect_total_does_not_merge_directions_for_one_reaso auto const bump = [&app](char const* reason, char const* direction) { XRPL_METRIC_COUNTER_INC_LABELED( app, - "peer_disconnect_total", + telemetry::metric::peerDisconnectTotal, "Peer disconnects, by cause and connection direction", - {{"reason", std::string(reason)}, {"direction", std::string(direction)}}); + {{telemetry::label::reason, std::string(reason)}, + {telemetry::label::direction, std::string(direction)}}); }; // One read_error each way. If direction did not key the series, this would @@ -2463,9 +2484,9 @@ TEST(MetricMacros, peer_accept_total_keys_series_on_outcome) auto const bump = [&app](char const* outcome) { XRPL_METRIC_COUNTER_INC_LABELED( app, - "peer_accept_total", + telemetry::metric::peerAcceptTotal, "Inbound peer connection attempts, by terminal outcome", - {{"outcome", std::string(outcome)}}); + {{telemetry::label::outcome, std::string(outcome)}}); }; // accepted x2, no_slot x3, handshake_error x1 -- three distinct @@ -2520,9 +2541,10 @@ TEST(MetricMacros, serve_refused_total_keys_series_on_request_and_reason_pair) auto const bump = [&app](char const* request, char const* reason) { XRPL_METRIC_COUNTER_INC_LABELED( app, - "serve_refused_total", + telemetry::metric::serveRefusedTotal, "Peer data requests this node declined to serve, by request kind and cause", - {{"request", std::string(request)}, {"reason", std::string(reason)}}); + {{telemetry::label::request, std::string(request)}, + {telemetry::label::reason, std::string(reason)}}); }; // Same request kind, two different reasons -> two series. @@ -2605,7 +2627,7 @@ TEST(MetricMacros, ledger_jump_total_accumulates_on_one_unlabelled_series) { XRPL_METRIC_COUNTER_INC( app, - "ledger_jump_total", + telemetry::metric::ledgerJumpTotal, "Forced jumps of the last closed ledger to a divergent chain"); } @@ -2631,7 +2653,9 @@ TEST(MetricMacros, ledger_jump_total_accumulates_on_one_unlabelled_series) // A fourth jump advances the SAME series to exactly 4 rather than creating a // second one, which is what "no labels" has to mean over time. XRPL_METRIC_COUNTER_INC( - app, "ledger_jump_total", "Forced jumps of the last closed ledger to a divergent chain"); + app, + telemetry::metric::ledgerJumpTotal, + "Forced jumps of the last closed ledger to a divergent chain"); auto const fourth = provider.collect(); ASSERT_EQ(fourth.at("ledger_jump_total").size(), 1u); EXPECT_EQ(counterValue(fourth, "ledger_jump_total", otel_sdk::PointAttributes{}), 4); @@ -2650,21 +2674,25 @@ TEST(MetricMacros, sync_supply_counters_emit_nothing_when_registry_disabled) XRPL_METRIC_COUNTER_INC_LABELED( app, - "peer_disconnect_total", + telemetry::metric::peerDisconnectTotal, "Peer disconnects, by cause and connection direction", - {{"reason", std::string("large_sendq")}, {"direction", std::string("outbound")}}); + {{telemetry::label::reason, std::string("large_sendq")}, + {telemetry::label::direction, std::string("outbound")}}); XRPL_METRIC_COUNTER_INC_LABELED( app, - "peer_accept_total", + telemetry::metric::peerAcceptTotal, "Inbound peer connection attempts, by terminal outcome", - {{"outcome", std::string("accepted")}}); + {{telemetry::label::outcome, std::string("accepted")}}); XRPL_METRIC_COUNTER_INC_LABELED( app, - "serve_refused_total", + telemetry::metric::serveRefusedTotal, "Peer data requests this node declined to serve, by request kind and cause", - {{"request", std::string("ledger")}, {"reason", std::string("sendq_full")}}); + {{telemetry::label::request, std::string("ledger")}, + {telemetry::label::reason, std::string("sendq_full")}}); XRPL_METRIC_COUNTER_INC( - app, "ledger_jump_total", "Forced jumps of the last closed ledger to a divergent chain"); + app, + telemetry::metric::ledgerJumpTotal, + "Forced jumps of the last closed ledger to a divergent chain"); auto const data = provider.collect(); @@ -2699,15 +2727,15 @@ TEST(MetricMacros, ledger_replay_fallback_counter_separates_stages_by_exact_coun { XRPL_METRIC_COUNTER_INC_LABELED( app, - "ledger_replay_fallback_total", + telemetry::metric::ledgerReplayFallbackTotal, "Replay sub-acquires that fell back to a full ledger acquire", - {{"stage", std::string("skiplist")}}); + {{telemetry::label::stage, std::string("skiplist")}}); } XRPL_METRIC_COUNTER_INC_LABELED( app, - "ledger_replay_fallback_total", + telemetry::metric::ledgerReplayFallbackTotal, "Replay sub-acquires that fell back to a full ledger acquire", - {{"stage", std::string("delta")}}); + {{telemetry::label::stage, std::string("delta")}}); auto const data = provider.collect(); @@ -2743,9 +2771,9 @@ TEST(MetricMacros, ledger_replay_outcome_counter_records_each_terminal_state_exa { XRPL_METRIC_COUNTER_INC_LABELED( app, - "ledger_replay_outcome_total", + telemetry::metric::ledgerReplayOutcomeTotal, "Ledger replay tasks by terminal outcome", - {{"outcome", std::string(outcome)}}); + {{telemetry::label::outcome, std::string(outcome)}}); } }; record("success", 3); @@ -2782,14 +2810,14 @@ TEST(MetricMacros, ledger_replay_counters_emit_nothing_when_disabled) XRPL_METRIC_COUNTER_INC_LABELED( app, - "ledger_replay_fallback_total", + telemetry::metric::ledgerReplayFallbackTotal, "Replay sub-acquires that fell back to a full ledger acquire", - {{"stage", std::string("skiplist")}}); + {{telemetry::label::stage, std::string("skiplist")}}); XRPL_METRIC_COUNTER_INC_LABELED( app, - "ledger_replay_outcome_total", + telemetry::metric::ledgerReplayOutcomeTotal, "Ledger replay tasks by terminal outcome", - {{"outcome", std::string("timeout")}}); + {{telemetry::label::outcome, std::string("timeout")}}); auto const data = provider.collect(); @@ -2829,14 +2857,15 @@ TEST(MetricMacros, nodestore_latency_gauge_observes_exact_derived_means) .fetchDurationUs = 1'000'000}; // 1 s over 1000 fetches -> 1000 us auto gauge = provider.meter()->CreateInt64ObservableGauge( - "nodestore_latency", "NodeStore mean store/fetch latency in microseconds, with counts"); + telemetry::metric::nodestoreLatency, + "NodeStore mean store/fetch latency in microseconds, with counts"); gauge->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto const* self = static_cast(state); auto observe = [&](char const* field, std::int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", field}}); + ->Observe(value, {{telemetry::label::metric, field}}); }; observe("write_count", static_cast(self->storeCount)); observe("read_count", static_cast(self->fetchCount)); @@ -2942,7 +2971,7 @@ TEST(MetricMacros, consensus_round_duration_records_exact_values) { XRPL_METRIC_HISTOGRAM_RECORD( app, - "consensus_round_duration_ms", + telemetry::metric::consensusRoundDurationMs, "Wall-clock duration of a completed consensus round in milliseconds", ms); } @@ -2976,7 +3005,7 @@ TEST(MetricMacros, consensus_round_duration_emits_nothing_when_registry_disabled XRPL_METRIC_HISTOGRAM_RECORD( app, - "consensus_round_duration_ms", + telemetry::metric::consensusRoundDurationMs, "Wall-clock duration of a completed consensus round in milliseconds", 3100); diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index bab81168fd..aaf7e17c83 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -533,7 +534,7 @@ RCLConsensus::Adaptor::makeAcceptSpan(Result const& result) // transaction. XRPL_METRIC_HISTOGRAM_RECORD( app_, - "consensus_round_duration_ms", + telemetry::metric::consensusRoundDurationMs, "Wall-clock duration of a completed consensus round in milliseconds", result.roundTime.read().count()); span->setAttribute(cs::attr::quorum, static_cast(app_.getValidators().quorum())); diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp index 5da372f319..dd50e14ff6 100644 --- a/src/xrpld/app/ledger/detail/InboundLedger.cpp +++ b/src/xrpld/app/ledger/detail/InboundLedger.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -160,9 +161,12 @@ InboundLedger::init(ScopedLockType& collectionLock) // ("nothing was local, every node must come over the wire"). XRPL_METRIC_COUNTER_INC_LABELED( app_, - "sync_acquire_source_total", + telemetry::metric::syncAcquireSourceTotal, "Ledger acquires by where the data came from", - {{"source", std::string(complete_ ? "local" : "network")}}); + {{telemetry::label::source, + std::string( + complete_ ? telemetry::lval::acquire_source::local + : telemetry::lval::acquire_source::network)}}); if (!complete_) { @@ -500,7 +504,7 @@ InboundLedger::onTimer(bool wasProgress, ScopedLockType&) // signature of a sync that will never complete. XRPL_METRIC_COUNTER_INC( app_, - "sync_acquire_no_progress_total", + telemetry::metric::syncAcquireNoProgressTotal, "Ledger-acquire timeouts where no new node arrived"); // addPeers triggers if the reason is not HISTORY @@ -1517,14 +1521,14 @@ InboundLedger::recordBatchOutcome(SHAMapAddNode const& san) return; XRPL_METRIC_COUNTER_ADD_LABELED( app_, - "sync_addnode_total", + telemetry::metric::syncAddnodeTotal, "SHAMap nodes received during ledger acquire, by outcome", static_cast(count), - {{"outcome", std::string(outcome)}}); + {{telemetry::label::outcome, std::string(outcome)}}); }; - emit("good", san.getGood()); - emit("duplicate", san.getDuplicate()); - emit("invalid", san.getBad()); + emit(telemetry::lval::addnode::good, san.getGood()); + emit(telemetry::lval::addnode::duplicate, san.getDuplicate()); + emit(telemetry::lval::addnode::invalid, san.getBad()); return san.getGood(); } diff --git a/src/xrpld/app/ledger/detail/LedgerDeltaAcquire.cpp b/src/xrpld/app/ledger/detail/LedgerDeltaAcquire.cpp index 9dc8cf5339..4cb449a08f 100644 --- a/src/xrpld/app/ledger/detail/LedgerDeltaAcquire.cpp +++ b/src/xrpld/app/ledger/detail/LedgerDeltaAcquire.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -115,9 +116,10 @@ LedgerDeltaAcquire::trigger(std::size_t limit, ScopedLockType& sl) // Emitted once, on the transition into fallback. XRPL_METRIC_COUNTER_INC_LABELED( app_, - "ledger_replay_fallback_total", + telemetry::metric::ledgerReplayFallbackTotal, "Replay sub-acquires that fell back to a full ledger acquire", - {{"stage", std::string("delta")}}); + {{telemetry::label::stage, + std::string(telemetry::lval::replay_fallback::delta)}}); fallBack_ = true; } diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp index e6432c10af..b238bbbffc 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -1061,9 +1062,9 @@ LedgerMaster::checkAccept(std::shared_ptr const& ledger) // OTel callback ever acquires mutex_. XRPL_METRIC_COUNTER_INC_LABELED( app_, - "ledger_quorum_shortfall_total", + telemetry::metric::ledgerQuorumShortfallTotal, "Pre-accept gate rejections because trusted validations were below quorum", - {{"stage", std::string("pre_accept")}}); + {{telemetry::label::stage, std::string(telemetry::lval::quorum_shortfall::preAccept)}}); return; } diff --git a/src/xrpld/app/ledger/detail/LedgerReplayTask.cpp b/src/xrpld/app/ledger/detail/LedgerReplayTask.cpp index 59d713bbec..98f08ca1d5 100644 --- a/src/xrpld/app/ledger/detail/LedgerReplayTask.cpp +++ b/src/xrpld/app/ledger/detail/LedgerReplayTask.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -213,7 +214,7 @@ LedgerReplayTask::tryAdvance(ScopedLockType& sl) // Terminal success. Emitted once per task: the loop above is guarded by // isDone() at every entry point, so a completed task cannot re-enter // and double-count. - recordOutcome("success"); + recordOutcome(telemetry::lval::replay_outcome::success); } catch (std::runtime_error const&) { @@ -222,7 +223,7 @@ LedgerReplayTask::tryAdvance(ScopedLockType& sl) // A delta failed to build on top of its parent, so the replayed range // cannot be reconstructed. Previously not logged at all here, only // reflected in failed_. - recordOutcome("build_failed"); + recordOutcome(telemetry::lval::replay_outcome::buildFailed); } } @@ -235,9 +236,9 @@ LedgerReplayTask::recordOutcome(char const* outcome) const // indistinguishable from one that was never attempted. XRPL_METRIC_COUNTER_INC_LABELED( app_, - "ledger_replay_outcome_total", + telemetry::metric::ledgerReplayOutcomeTotal, "Ledger replay tasks by terminal outcome", - {{"outcome", std::string(outcome)}}); + {{telemetry::label::outcome, std::string(outcome)}}); } void @@ -259,7 +260,7 @@ LedgerReplayTask::updateSkipList( // so the task is abandoned before any delta is fetched. A distinct // outcome from a timeout: this one indicates a peer served an // inconsistent skip list, not a slow or absent peer. - recordOutcome("parameter_failed"); + recordOutcome(telemetry::lval::replay_outcome::parameterFailed); return; } } @@ -282,7 +283,7 @@ LedgerReplayTask::onTimer(bool progress, ScopedLockType& sl) // The task ran out of retries waiting for its deltas. This is the // outcome that pairs with the fallback counters: the sub-acquires gave // up, and so did the task above them. - recordOutcome("timeout"); + recordOutcome(telemetry::lval::timeout); } else { diff --git a/src/xrpld/app/ledger/detail/SkipListAcquire.cpp b/src/xrpld/app/ledger/detail/SkipListAcquire.cpp index cca54d5fcb..c69ae82d76 100644 --- a/src/xrpld/app/ledger/detail/SkipListAcquire.cpp +++ b/src/xrpld/app/ledger/detail/SkipListAcquire.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -111,9 +112,10 @@ SkipListAcquire::trigger(std::size_t limit, ScopedLockType& sl) // call below, which re-runs on every later trigger. XRPL_METRIC_COUNTER_INC_LABELED( app_, - "ledger_replay_fallback_total", + telemetry::metric::ledgerReplayFallbackTotal, "Replay sub-acquires that fell back to a full ledger acquire", - {{"stage", std::string("skiplist")}}); + {{telemetry::label::stage, + std::string(telemetry::lval::replay_fallback::skiplist)}}); fallBack_ = true; } diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 15a1da17d9..796d6816d9 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -2172,7 +2173,7 @@ NetworkOPsImp::switchLastClosedLedger(std::shared_ptr const& newLC // unbounded as label values, and the log line above already carries them. XRPL_METRIC_COUNTER_INC( registry_.get(), - "ledger_jump_total", + telemetry::metric::ledgerJumpTotal, "Forced jumps of the last closed ledger to a divergent chain"); clearNeedNetworkLedger(); @@ -2739,9 +2740,10 @@ NetworkOPsImp::setMode(OperatingMode om) // names, which keeps them identical to the ones server_info reports. XRPL_METRIC_COUNTER_INC_LABELED( registry_.get(), - "state_changes_total", + telemetry::metric::stateChangesTotal, "Total operating mode changes", - {{"from", strOperatingMode(prevMode, false)}, {"to", strOperatingMode(om, false)}}); + {{telemetry::label::from, strOperatingMode(prevMode, false)}, + {telemetry::label::to, strOperatingMode(om, false)}}); JLOG(journal_.info()) << "STATE->" << strOperatingMode(); pubServer(); diff --git a/src/xrpld/app/misc/detail/ValidatorSite.cpp b/src/xrpld/app/misc/detail/ValidatorSite.cpp index c40b00042c..f7667e7f90 100644 --- a/src/xrpld/app/misc/detail/ValidatorSite.cpp +++ b/src/xrpld/app/misc/detail/ValidatorSite.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -394,10 +395,10 @@ ValidatorSite::reportFetchOutcome( // the time series stable when a site redirects. XRPL_METRIC_COUNTER_INC_LABELED( app_, - "unl_fetch_total", + telemetry::metric::unlFetchTotal, "Validator list fetch attempts, by site and outcome", - {{"site", std::string(sites_[siteIdx].loadedResource->uri)}, - {"outcome", std::string(outcome)}}); + {{telemetry::label::site, std::string(sites_[siteIdx].loadedResource->uri)}, + {telemetry::label::outcome, std::string(outcome)}}); } void @@ -593,7 +594,7 @@ ValidatorSite::onSiteFetch( { JLOG(j_.warn()) << "Problem retrieving from " << sites_[siteIdx].activeResource->uri << " " << endpoint << " " << ec.value() << ":" << ec.message(); - onError("fetch error", true, "fetch_error"); + onError("fetch error", true, telemetry::lval::unl_fetch::fetchError); } else { @@ -629,7 +630,7 @@ ValidatorSite::onSiteFetch( JLOG(j_.warn()) << "Request for validator list at " << sites_[siteIdx].activeResource->uri << " " << endpoint << " returned bad status: " << res.result_int(); - onError("bad result code", true, "bad_status"); + onError("bad result code", true, telemetry::lval::unl_fetch::badStatus); } } } @@ -658,14 +659,14 @@ ValidatorSite::onTextFetch( std::scoped_lock const lockSites{sitesMutex_}; { // Both failures share one catch, so the label is set where detected. - std::string_view outcome = "parse_error"; + std::string_view outcome = telemetry::lval::unl_fetch::parseError; try { if (ec) { JLOG(j_.warn()) << "Problem retrieving from " << sites_[siteIdx].activeResource->uri << " " << ec.value() << ": " << ec.message(); - outcome = "fetch_error"; + outcome = telemetry::lval::unl_fetch::fetchError; throw std::runtime_error{"fetch error"}; } diff --git a/src/xrpld/overlay/detail/ConnectAttempt.cpp b/src/xrpld/overlay/detail/ConnectAttempt.cpp index 08084c7148..6b3877b079 100644 --- a/src/xrpld/overlay/detail/ConnectAttempt.cpp +++ b/src/xrpld/overlay/detail/ConnectAttempt.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -166,7 +167,7 @@ ConnectAttempt::reportOutcome(std::string_view outcome) // comma in a non-variadic macro argument, which the preprocessor splits. XRPL_METRIC_HISTOGRAM_RECORD( app_, - "overlay_dial_latency_ms", + telemetry::metric::overlayDialLatencyMs, "Time from starting an outbound peer dial to its terminal outcome, in milliseconds", std::chrono::duration_cast( std::chrono::steady_clock::now() - dialStart_) @@ -175,9 +176,9 @@ ConnectAttempt::reportOutcome(std::string_view outcome) XRPL_METRIC_COUNTER_INC_LABELED( app_, - "overlay_connect_total", + telemetry::metric::overlayConnectTotal, "Outbound peer connection attempts, by terminal outcome", - {{"outcome", std::string(outcome)}}); + {{telemetry::label::outcome, std::string(outcome)}}); // End the span with the SAME outcome value the counter just recorded, from // the same funnel, so the two can never disagree. The first-call-wins guard diff --git a/src/xrpld/overlay/detail/Handshake.cpp b/src/xrpld/overlay/detail/Handshake.cpp index fcd359d158..74e4bb0b38 100644 --- a/src/xrpld/overlay/detail/Handshake.cpp +++ b/src/xrpld/overlay/detail/Handshake.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -263,9 +264,9 @@ throwNegotiationFailure(Application& app, char const* reason, std::string const& { XRPL_METRIC_COUNTER_INC_LABELED( app, - "handshake_negotiation_fail_total", + telemetry::metric::handshakeNegotiationFailTotal, "Peer handshake negotiations rejected, by reason", - {{"reason", std::string(reason)}}); + {{telemetry::label::reason, std::string(reason)}}); throw std::runtime_error(message); } @@ -284,7 +285,10 @@ verifyHandshake( if (auto const iter = headers.find("Server-Domain"); iter != headers.end()) { if (!isProperlyFormedTomlDomain(iter->value())) - throwNegotiationFailure(app, "invalid_server_domain", "Invalid server domain"); + { + throwNegotiationFailure( + app, telemetry::lval::handshake_fail::invalidServerDomain, "Invalid server domain"); + } } if (auto const iter = headers.find("Network-ID"); iter != headers.end()) @@ -292,10 +296,20 @@ verifyHandshake( std::uint32_t nid = 0; if (!beast::lexicalCastChecked(nid, iter->value())) - throwNegotiationFailure(app, "invalid_network_id", "Invalid peer network identifier"); + { + throwNegotiationFailure( + app, + telemetry::lval::handshake_fail::invalidNetworkId, + "Invalid peer network identifier"); + } if (networkID && nid != *networkID) - throwNegotiationFailure(app, "wrong_network", "Peer is on a different network"); + { + throwNegotiationFailure( + app, + telemetry::lval::handshake_fail::wrongNetwork, + "Peer is on a different network"); + } } if (auto const iter = headers.find("Network-Time"); iter != headers.end()) @@ -308,7 +322,10 @@ verifyHandshake( // It's not an error for the header field to not be present but if // it is present and it contains junk data, that is an error. - throwNegotiationFailure(app, "invalid_clock_timestamp", "Invalid peer clock timestamp"); + throwNegotiationFailure( + app, + telemetry::lval::handshake_fail::invalidClockTimestamp, + "Invalid peer clock timestamp"); }(); using namespace std::chrono; @@ -328,7 +345,10 @@ verifyHandshake( auto const offset = calculateOffset(netTime, ourTime); if (abs(offset) > tolerance) - throwNegotiationFailure(app, "clock_skew", "Peer clock is too far off"); + { + throwNegotiationFailure( + app, telemetry::lval::handshake_fail::clockSkew, "Peer clock is too far off"); + } } PublicKey const publicKey = [&headers, &app] { @@ -339,14 +359,19 @@ verifyHandshake( if (pk) { if (publicKeyType(*pk) != KeyType::Secp256k1) + { throwNegotiationFailure( - app, "unsupported_key_type", "Unsupported public key type"); + app, + telemetry::lval::handshake_fail::unsupportedKeyType, + "Unsupported public key type"); + } return *pk; } } - throwNegotiationFailure(app, "bad_public_key", "Bad node public key"); + throwNegotiationFailure( + app, telemetry::lval::handshake_fail::badPublicKey, "Bad node public key"); }(); // This check gets two birds with one stone: @@ -359,16 +384,29 @@ verifyHandshake( auto const iter = headers.find("Session-Signature"); if (iter == headers.end()) - throwNegotiationFailure(app, "no_session_signature", "No session signature specified"); + { + throwNegotiationFailure( + app, + telemetry::lval::handshake_fail::noSessionSignature, + "No session signature specified"); + } auto sig = base64Decode(iter->value()); if (!verifyDigest(publicKey, sharedValue, makeSlice(sig), false)) - throwNegotiationFailure(app, "session_verify_failed", "Failed to verify session"); + { + throwNegotiationFailure( + app, + telemetry::lval::handshake_fail::sessionVerifyFailed, + "Failed to verify session"); + } } if (publicKey == app.nodeIdentity().first) - throwNegotiationFailure(app, "self_connection", "Self connection"); + { + throwNegotiationFailure( + app, telemetry::lval::handshake_fail::selfConnection, "Self connection"); + } if (auto const iter = headers.find("Local-IP"); iter != headers.end()) { @@ -376,13 +414,16 @@ verifyHandshake( auto const localIp = boost::asio::ip::make_address(std::string_view(iter->value()), ec); if (ec) - throwNegotiationFailure(app, "invalid_local_ip", "Invalid Local-IP"); + { + throwNegotiationFailure( + app, telemetry::lval::handshake_fail::invalidLocalIp, "Invalid Local-IP"); + } if (beast::IP::isPublic(remote) && remote != localIp) { throwNegotiationFailure( app, - "local_ip_mismatch", + telemetry::lval::handshake_fail::localIpMismatch, "Incorrect Local-IP: " + remote.to_string() + " instead of " + localIp.to_string()); } } @@ -393,7 +434,10 @@ verifyHandshake( auto const remoteIp = boost::asio::ip::make_address(std::string_view(iter->value()), ec); if (ec) - throwNegotiationFailure(app, "invalid_remote_ip", "Invalid Remote-IP"); + { + throwNegotiationFailure( + app, telemetry::lval::handshake_fail::invalidRemoteIp, "Invalid Remote-IP"); + } if (beast::IP::isPublic(remote) && !beast::IP::isUnspecified(publicIp)) { @@ -403,7 +447,7 @@ verifyHandshake( { throwNegotiationFailure( app, - "remote_ip_mismatch", + telemetry::lval::handshake_fail::remoteIpMismatch, "Incorrect Remote-IP: " + publicIp.to_string() + " instead of " + remoteIp.to_string()); } diff --git a/src/xrpld/overlay/detail/OverlayImpl.cpp b/src/xrpld/overlay/detail/OverlayImpl.cpp index e33baddb0c..6a1a2760bd 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.cpp +++ b/src/xrpld/overlay/detail/OverlayImpl.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -241,7 +242,7 @@ OverlayImpl::onHandoff( if (ec) { JLOG(journal.debug()) << remoteEndpoint << " failed: " << ec.message(); - reportAcceptOutcome("local_endpoint_fail"); + reportAcceptOutcome(telemetry::lval::peer_accept::localEndpointFail); return handoff; } @@ -249,7 +250,7 @@ OverlayImpl::onHandoff( resourceManager_.newInboundEndpoint(beast::IPAddressConversion::fromAsio(remoteEndpoint)); if (consumer.disconnect(journal)) { - reportAcceptOutcome("resource_limit"); + reportAcceptOutcome(telemetry::lval::peer_accept::resourceLimit); return handoff; } @@ -262,7 +263,7 @@ OverlayImpl::onHandoff( // connection refused either IP limit exceeded or self-connect handoff.moved = false; JLOG(journal.debug()) << "Peer " << remoteEndpoint << " refused, " << to_string(result); - reportAcceptOutcome("no_slot"); + reportAcceptOutcome(telemetry::lval::peer_accept::noSlot); return handoff; } @@ -277,7 +278,7 @@ OverlayImpl::onHandoff( handoff.moved = false; handoff.response = makeRedirectResponse(slot, request, remoteEndpoint.address()); handoff.keepAlive = beast::rfc2616::isKeepAlive(request); - reportAcceptOutcome("not_peer_request"); + reportAcceptOutcome(telemetry::lval::peer_accept::notPeerRequest); return handoff; } } @@ -290,7 +291,7 @@ OverlayImpl::onHandoff( handoff.response = makeErrorResponse( slot, request, remoteEndpoint.address(), "Unable to agree on a protocol version"); handoff.keepAlive = false; - reportAcceptOutcome("protocol_mismatch"); + reportAcceptOutcome(telemetry::lval::peer_accept::protocolMismatch); return handoff; } @@ -302,7 +303,7 @@ OverlayImpl::onHandoff( handoff.response = makeErrorResponse(slot, request, remoteEndpoint.address(), "Incorrect security cookie"); handoff.keepAlive = false; - reportAcceptOutcome("bad_cookie"); + reportAcceptOutcome(telemetry::lval::peer_accept::badCookie); return handoff; } @@ -332,7 +333,7 @@ OverlayImpl::onHandoff( handoff.moved = false; handoff.response = makeRedirectResponse(slot, request, remoteEndpoint.address()); handoff.keepAlive = false; - reportAcceptOutcome("slot_refused"); + reportAcceptOutcome(telemetry::lval::peer_accept::slotRefused); return handoff; } } @@ -365,7 +366,7 @@ OverlayImpl::onHandoff( // Only after run() is the peer genuinely accepted. Anything that threw // above is reported as a handshake error by the catch below instead. - reportAcceptOutcome("accepted"); + reportAcceptOutcome(telemetry::lval::peer_accept::accepted); return handoff; } catch (std::exception const& e) @@ -377,7 +378,7 @@ OverlayImpl::onHandoff( handoff.moved = false; handoff.response = makeErrorResponse(slot, request, remoteEndpoint.address(), e.what()); handoff.keepAlive = false; - reportAcceptOutcome("handshake_error"); + reportAcceptOutcome(telemetry::lval::peer_accept::handshakeError); return handoff; } } @@ -636,7 +637,7 @@ OverlayImpl::reportDnsResolve(std::chrono::steady_clock::time_point start, bool // comma in a non-variadic macro argument, which the preprocessor splits. XRPL_METRIC_HISTOGRAM_RECORD( app_, - "dns_resolve_latency_ms", + telemetry::metric::dnsResolveLatencyMs, "Time taken to resolve a configured peer hostname, in milliseconds", std::chrono::duration_cast( std::chrono::steady_clock::now() - start) @@ -645,9 +646,12 @@ OverlayImpl::reportDnsResolve(std::chrono::steady_clock::time_point start, bool XRPL_METRIC_COUNTER_INC_LABELED( app_, - "dns_resolve_total", + telemetry::metric::dnsResolveTotal, "Peer hostname resolutions, by outcome", - {{"outcome", std::string(resolved ? "resolved" : "empty")}}); + {{telemetry::label::outcome, + std::string( + resolved ? telemetry::lval::dns_resolve::resolved + : telemetry::lval::dns_resolve::empty)}}); } void @@ -655,9 +659,9 @@ OverlayImpl::reportAcceptOutcome(char const* outcome) { XRPL_METRIC_COUNTER_INC_LABELED( app_, - "peer_accept_total", + telemetry::metric::peerAcceptTotal, "Inbound peer connection attempts, by terminal outcome", - {{"outcome", std::string(outcome)}}); + {{telemetry::label::outcome, std::string(outcome)}}); } PeerLedgerSupply diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 3f7320370e..98132100be 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -232,7 +233,7 @@ PeerImp::run() if (!closed) { - self->setDisconnectReason("malformed_handshake"); + self->setDisconnectReason(telemetry::lval::disconnect::malformedHandshake); self->fail("Malformed handshake data (1)"); } } @@ -243,14 +244,14 @@ PeerImp::run() if (!previous) { - self->setDisconnectReason("malformed_handshake"); + self->setDisconnectReason(telemetry::lval::disconnect::malformedHandshake); self->fail("Malformed handshake data (2)"); } } if (previous && !closed) { - self->setDisconnectReason("malformed_handshake"); + self->setDisconnectReason(telemetry::lval::disconnect::malformedHandshake); self->fail("Malformed handshake data (3)"); } @@ -285,7 +286,7 @@ PeerImp::stop() // Overlay-wide shutdown, not a fault with this peer. Distinguished so // a clean restart does not look like a wave of peer failures. - self->setDisconnectReason("stopping"); + self->setDisconnectReason(telemetry::lval::disconnect::stopping); self->close(); }); } @@ -415,7 +416,7 @@ PeerImp::charge(Resource::Charge const& fee, std::string const& context) // Set inside the latch, so only the one worker that wins the // exchange writes it. This is the node's own backpressure, not // a peer or network fault. - self->setDisconnectReason("charge_resources"); + self->setDisconnectReason(telemetry::lval::disconnect::chargeResources); self->fail("charge: Resources"); } } @@ -651,10 +652,11 @@ PeerImp::close() // "ping_timeout", "read_error"), and the two need opposite responses. XRPL_METRIC_COUNTER_INC_LABELED( app_, - "peer_disconnect_total", + telemetry::metric::peerDisconnectTotal, "Peer disconnects, by cause and connection direction", - {{"reason", std::string(disconnectReason_)}, - {"direction", std::string(inbound_ ? "inbound" : "outbound")}}); + {{telemetry::label::reason, std::string(disconnectReason_)}, + {telemetry::label::direction, + std::string(inbound_ ? telemetry::lval::inbound : telemetry::lval::outbound)}}); JLOG((inbound_ ? journal_.debug() : journal_.info())) << "close: Closed"; } @@ -664,9 +666,10 @@ PeerImp::reportServeRefusal(char const* request, char const* reason) { XRPL_METRIC_COUNTER_INC_LABELED( app_, - "serve_refused_total", + telemetry::metric::serveRefusedTotal, "Peer data requests this node declined to serve, by request kind and cause", - {{"request", std::string(request)}, {"reason", std::string(reason)}}); + {{telemetry::label::request, std::string(request)}, + {telemetry::label::reason, std::string(reason)}}); } void @@ -762,7 +765,7 @@ PeerImp::onTimer(error_code const& ec) // This should never happen JLOG(journal_.error()) << "onTimer: " << ec.message(); - setDisconnectReason("timer_error"); + setDisconnectReason(telemetry::lval::disconnect::timerError); close(); return; } @@ -771,7 +774,7 @@ PeerImp::onTimer(error_code const& ec) { // Our own send queue never drained: this node could not keep up with // what it owed the peer, so it is local backpressure, not a peer fault. - setDisconnectReason("large_sendq"); + setDisconnectReason(telemetry::lval::disconnect::largeSendq); fail("Large send queue"); return; } @@ -791,7 +794,7 @@ PeerImp::onTimer(error_code const& ec) overlay_.peerFinder().onFailure(slot_); // The peer is on a different chain, or we cannot tell: a topology // signal, not a fault on either side. - setDisconnectReason("not_useful"); + setDisconnectReason(telemetry::lval::disconnect::notUseful); fail("Not useful"); return; } @@ -800,7 +803,7 @@ PeerImp::onTimer(error_code const& ec) // Already waiting for PONG if (lastPingSeq_) { - setDisconnectReason("ping_timeout"); + setDisconnectReason(telemetry::lval::disconnect::pingTimeout); fail("Ping Timeout"); return; } @@ -842,7 +845,7 @@ PeerImp::onShutdown(error_code ec) // The TLS shutdown handshake finished. First-wins means the reason set by // whoever asked for the graceful close is kept; "shutdown" only lands when // the teardown started here, i.e. a clean close with no earlier cause. - setDisconnectReason("shutdown"); + setDisconnectReason(telemetry::lval::disconnect::shutdown); close(); } @@ -858,7 +861,7 @@ PeerImp::doAccept() // the shared value successfully in OverlayImpl if (!sharedValue) { - setDisconnectReason("shared_value"); + setDisconnectReason(telemetry::lval::disconnect::sharedValue); fail("makeSharedValue: Unexpected failure"); return; } @@ -908,7 +911,7 @@ PeerImp::doAccept() if (ec == boost::asio::error::operation_aborted) return; - setDisconnectReason("write_error"); + setDisconnectReason(telemetry::lval::disconnect::writeError); fail("onWriteResponse", ec); return; } @@ -918,7 +921,7 @@ PeerImp::doAccept() doProtocolStart(); return; } - setDisconnectReason("write_error"); + setDisconnectReason(telemetry::lval::disconnect::writeError); fail("Failed to write header"); return; })); @@ -995,12 +998,12 @@ PeerImp::onReadMessage(error_code ec, std::size_t bytesTransferred) JLOG(journal_.info()) << "EOF"; // The peer closed its side cleanly. Counted apart from a read // error because it is normal peer churn, not a fault. - setDisconnectReason("graceful"); + setDisconnectReason(telemetry::lval::disconnect::graceful); gracefulClose(); return; } - setDisconnectReason("read_error"); + setDisconnectReason(telemetry::lval::disconnect::readError); fail("onReadMessage", ec); return; } @@ -1030,7 +1033,7 @@ PeerImp::onReadMessage(error_code ec, std::size_t bytesTransferred) if (ec) { - setDisconnectReason("read_error"); + setDisconnectReason(telemetry::lval::disconnect::readError); fail("onReadMessage", ec); return; } @@ -1067,7 +1070,7 @@ PeerImp::onWriteMessage(error_code ec, std::size_t bytesTransferred) if (ec == boost::asio::error::operation_aborted) return; - setDisconnectReason("write_error"); + setDisconnectReason(telemetry::lval::disconnect::writeError); fail("onWriteMessage", ec); return; } @@ -2601,7 +2604,8 @@ PeerImp::onMessage(std::shared_ptr const& m) if (sendQueue_.size() >= Tuning::kDropSendQueue) { JLOG(pJournal_.debug()) << "GetObject: Large send queue"; - reportServeRefusal("object", "sendq_full"); + reportServeRefusal( + telemetry::lval::serve_request::object, telemetry::lval::serve_refused::sendqFull); return; } @@ -2960,7 +2964,8 @@ PeerImp::doFetchPack(std::shared_ptr const& packet) // A fetch pack is how a syncing peer catches up in bulk, so refusing // one directly slows that peer's sync. Counted separately from the // ledger path because the shed threshold is a different one. - reportServeRefusal("fetchpack", "load_shed"); + reportServeRefusal( + telemetry::lval::serve_request::fetchpack, telemetry::lval::serve_refused::loadShed); return; } @@ -3537,7 +3542,7 @@ PeerImp::processLedgerRequest(std::shared_ptr const& m) { if (sharedMap = getTxSet(m); !sharedMap) { - reportServeRefusal("txset", "not_found"); + reportServeRefusal(telemetry::lval::serve_request::txset, telemetry::lval::notFound); return; } map = sharedMap.get(); @@ -3557,19 +3562,21 @@ PeerImp::processLedgerRequest(std::shared_ptr const& m) if (sendQueue_.size() >= Tuning::kDropSendQueue) { JLOG(pJournal_.debug()) << "processLedgerRequest: Large send queue"; - reportServeRefusal("ledger", "sendq_full"); + reportServeRefusal( + telemetry::lval::serve_request::ledger, telemetry::lval::serve_refused::sendqFull); return; } if (app_.getFeeTrack().isLoadedLocal() && !cluster()) { JLOG(pJournal_.debug()) << "processLedgerRequest: Too busy"; - reportServeRefusal("ledger", "load_shed"); + reportServeRefusal( + telemetry::lval::serve_request::ledger, telemetry::lval::serve_refused::loadShed); return; } if (ledger = getLedger(m); !ledger) { - reportServeRefusal("ledger", "not_found"); + reportServeRefusal(telemetry::lval::serve_request::ledger, telemetry::lval::notFound); return; } @@ -3602,7 +3609,9 @@ PeerImp::processLedgerRequest(std::shared_ptr const& m) default: // This case should not be possible here JLOG(pJournal_.error()) << "processLedgerRequest: Invalid ledger info type"; - reportServeRefusal("ledger", "bad_type"); + reportServeRefusal( + telemetry::lval::serve_request::ledger, + telemetry::lval::serve_refused::badType); return; } } @@ -3610,7 +3619,8 @@ PeerImp::processLedgerRequest(std::shared_ptr const& m) if (map == nullptr) { JLOG(pJournal_.warn()) << "processLedgerRequest: Unable to find map"; - reportServeRefusal("ledger", "no_map"); + reportServeRefusal( + telemetry::lval::serve_request::ledger, telemetry::lval::serve_refused::noMap); return; } @@ -3699,7 +3709,10 @@ PeerImp::processLedgerRequest(std::shared_ptr const& m) // The map was found but produced no nodes to return, so the requester // gets nothing back and will have to ask someone else. Emitted here, // after the node loop, rather than inside it -- one call per request. - reportServeRefusal(itype == protocol::liTS_CANDIDATE ? "txset" : "ledger", "empty_reply"); + reportServeRefusal( + itype == protocol::liTS_CANDIDATE ? telemetry::lval::serve_request::txset + : telemetry::lval::serve_request::ledger, + telemetry::lval::serve_refused::emptyReply); return; } diff --git a/src/xrpld/overlay/detail/PeerImp.h b/src/xrpld/overlay/detail/PeerImp.h index c486e38954..67db379055 100644 --- a/src/xrpld/overlay/detail/PeerImp.h +++ b/src/xrpld/overlay/detail/PeerImp.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -210,7 +211,7 @@ private: * never peer-supplied data -- so the label's cardinality is bounded * by the code. */ - char const* disconnectReason_{"unknown"}; + char const* disconnectReason_{telemetry::lval::disconnect::unknown}; std::shared_ptr const slot_; boost::beast::multi_buffer readBuffer_; @@ -510,7 +511,7 @@ private: { // Only the first cause is kept: fail() sites frequently run before // close(), and a later generic reason must not mask the real one. - if (disconnectReason_ == std::string_view{"unknown"}) + if (disconnectReason_ == std::string_view{telemetry::lval::disconnect::unknown}) disconnectReason_ = reason; } diff --git a/src/xrpld/telemetry/MetricNames.h b/src/xrpld/telemetry/MetricNames.h new file mode 100644 index 0000000000..ad2ecbf1d1 --- /dev/null +++ b/src/xrpld/telemetry/MetricNames.h @@ -0,0 +1,695 @@ +#pragma once + +/** + * Compile-time OTel metric name constants for the sync-diagnostics signals. + * + * The metric-side counterpart of the `*SpanNames.h` headers: one constant per + * emitted string, so a rename is one edit and a typo is a compile error rather + * than a metric that silently never appears. Each instrument name, label key + * and bounded label value added by the sync-diagnostics work is declared here + * exactly once and referenced from every C++ user of it -- the emit site, the + * gauge registration in MetricsRegistry.cpp, and the unit test. + * + * Layer map -- who references these constants: + * + * +-------------------------------------------------------------+ + * | MetricNames.h (this file, L1-metrics) | + * | namespace metric namespace label namespace lval | + * +-------------------------------------------------------------+ + * ^ ^ ^ + * | | | + * +--------------+ +-------------------+ +---------------------+ + * | emit sites | | MetricsRegistry | | unit test | + * | XRPL_METRIC_ | | Create*Gauge + | | tests/libxrpl/ | + * | macros under | | AddCallback | | telemetry/ | + * | src/xrpld/ | | observe(...) | | MetricMacros.cpp | + * +--------------+ +-------------------+ +---------------------+ + * + * Layers that CANNOT reference a C++ constant -- the collector config, the + * dashboard PromQL, `expected_metrics.json` and the runbook -- are held to + * these same strings by `.github/scripts/otel-naming/check_otel_naming.py` + * instead. + * + * Where this is documented: + * - CONTRIBUTING.md -> "Telemetry metric naming" is the authoritative rule + * list, alongside the sibling span-attribute convention. + * - `.github/scripts/otel-naming/README.md` describes the enforcing rules + * I (no literals), J (suffix conventions) and K (expected_metrics.json). + * - docs/telemetry-runbook.md -> "Adding a New Metric" is the walkthrough for + * adding one, and shows the declare-then-emit pattern. + * + * Naming rules (enforced by the checker's Rule J): + * - Bare `lower_snake_case`. No `xrpld_` prefix in code: the Prometheus + * exporter adds the namespace prefix itself, so writing it here would + * produce `xrpld_xrpld_*` on the wire. + * - A monotonic counter ends in `_total`, so `rate()` over it reads correctly + * and a reader can tell it from a gauge at a glance. + * - A duration carries its unit as the suffix: `_us`, `_ms` or `_seconds`. + * The unit belongs in the name because the OTel `unit` argument is not + * surfaced on the Prometheus metric name. + * - A gauge that is a snapshot of current state takes no suffix + * (`jobq_backlog`, `sync_state`). + * - Label VALUES are declared here only when they come from a fixed set that + * the code itself writes (`namespace lval`), which is what keeps series + * cardinality bounded. A value derived from runtime data -- a site URI, a + * peer address, a ledger hash -- is deliberately NOT declared here and must + * never become a label on a metric. + * + * Why `constexpr char[]` and not the `makeStr`/`StaticStr` DSL that + * `SpanNames.h` uses: the OTel C++ API takes `nostd::string_view`, which in + * this build (`OPENTELEMETRY_STL_VERSION` unset, so the back-ported class is + * used) constructs only from `char const*`, `std::string` or + * `(char const*, size)`. It has NO constructor from `std::string_view`, so + * neither `StaticStr` (which converts to `std::string_view`) nor a + * `constexpr std::string_view` compiles in an instrument-name or label-key + * position -- both were tried and both fail with "no viable conversion". + * A `constexpr char[]` decays to `char const*` and binds directly. This also + * matches the precedent already in MetricsRegistry.cpp + * (`kJobQueuedDurationUs`), which this header absorbs. + * + * Example usage -- a labelled counter at an emit site: + * @code + * XRPL_METRIC_COUNTER_INC_LABELED( + * app_, + * metric::dnsResolveTotal, + * "Peer hostname resolutions, by outcome", + * {{label::outcome, + * std::string( + * resolved ? lval::dns_resolve::resolved : lval::dns_resolve::empty)}}); + * @endcode + * + * Example usage -- an observable gauge and its sub-metric discriminators: + * @code + * syncStateGauge_ = meter_->CreateInt64ObservableGauge( + * metric::syncState, "Sync-pipeline health signals"); + * // ... inside the callback: + * observe(lval::sync_state::ledgersBehind, ops.getLedgersBehindNetwork()); + * @endcode + * + * Example usage -- edge case: a value that must NOT be a constant. The site + * URI is runtime data, so only the KEY is named here; declaring the value + * would imply a bounded set that does not exist: + * @code + * XRPL_METRIC_COUNTER_INC_LABELED( + * app_, metric::unlFetchTotal, "...", + * {{label::site, std::string(sites_[siteIdx].loadedResource->uri)}, + * {label::outcome, std::string(outcome)}}); + * @endcode + * + * @note Header-only and dependency-free: nothing here includes an OTel or an + * xrpld header, so `src/tests/libxrpl/telemetry/MetricMacros.cpp` can + * include it even though `xrpl_tests` links only `xrpl.libxrpl`. The + * constants are `inline constexpr`, so they contribute no symbol to link + * against. + * @note Not guarded by `XRPL_ENABLE_TELEMETRY`, for the same reason + * `SpanNames.h` is not: call sites name these constants even where the + * macros expand to no-ops, and the compiler elides any constant whose + * only uses are in dead code. + * @note Every constant is a compile-time value with no mutable state, so all + * of them are safe to read concurrently from any thread. + */ + +namespace xrpl::telemetry { + +/** + * Instrument names -- the metric name as it reaches the OTel meter. + * + * Grouped by the subsystem that emits them, matching how the sync-diagnostics + * work was staged. A name is declared here whether it is created lazily by an + * `XRPL_METRIC_*` macro at a call site or eagerly by a `meter_->Create*` call + * in MetricsRegistry.cpp, because the dashboards cannot tell the two apart. + */ +namespace metric { + +// ===== Bootstrap: getting a fresh node its first peers and its first UNL ===== + +/** + * Time to resolve one configured peer hostname. + */ +inline constexpr char dnsResolveLatencyMs[] = "dns_resolve_latency_ms"; +/** + * Peer hostname resolutions, split by whether any address came back. + */ +inline constexpr char dnsResolveTotal[] = "dns_resolve_total"; +/** + * Time from starting an outbound dial to its terminal outcome. + */ +inline constexpr char overlayDialLatencyMs[] = "overlay_dial_latency_ms"; +/** + * Outbound peer connection attempts, by terminal outcome. + */ +inline constexpr char overlayConnectTotal[] = "overlay_connect_total"; +/** + * Peer handshakes this node rejected, by reason. + */ +inline constexpr char handshakeNegotiationFailTotal[] = "handshake_negotiation_fail_total"; +/** + * Validator-list fetch attempts, by site and outcome. + */ +inline constexpr char unlFetchTotal[] = "unl_fetch_total"; +/** + * Trusted UNL key count against the required quorum. + * + * A gauge, not a counter: both series are current state, and the useful read + * is the difference between them. + */ +inline constexpr char unlQuorum[] = "unl_quorum"; +/** + * Network close-time offset from the local clock. + */ +inline constexpr char clockCloseOffsetSeconds[] = "clock_close_offset_seconds"; + +// ===== Sync state: why this node is not FULL yet ============================= + +/** + * Operating-mode transitions, labelled with the mode pair. + */ +inline constexpr char stateChangesTotal[] = "state_changes_total"; +/** + * Four sync-pipeline health signals, split by the `metric` label. + */ +inline constexpr char syncState[] = "sync_state"; +/** + * Main-loop stall episodes. Cumulative, so `rate()` gives episodes/sec. + */ +inline constexpr char serverStallEventsTotal[] = "server_stall_events_total"; + +// ===== Acquire / SHAMap: is ledger data actually arriving? =================== + +/** + * Aggregate ledger-acquire progress across all in-flight acquires. + */ +inline constexpr char syncAcquire[] = "sync_acquire"; +/** + * SHAMap tree-node cache hit rate, 0.0-1.0. + */ +inline constexpr char shamapCacheHitRate[] = "shamap_cache_hit_rate"; +/** + * Ledger acquires by where the data came from (local store vs network). + */ +inline constexpr char syncAcquireSourceTotal[] = "sync_acquire_source_total"; +/** + * Acquire timeouts where not one new node arrived. + */ +inline constexpr char syncAcquireNoProgressTotal[] = "sync_acquire_no_progress_total"; +/** + * SHAMap nodes received during an acquire, by per-node outcome. + */ +inline constexpr char syncAddnodeTotal[] = "sync_addnode_total"; + +// ===== JobQueue: is the worker pool the bottleneck? ========================== + +/** + * Instantaneous JobQueue occupancy, per job type and per state. + */ +inline constexpr char jobqBacklog[] = "jobq_backlog"; +/** + * Worker-pool saturation: tasks in flight, threads, and jobs queued. + */ +inline constexpr char jobqSaturation[] = "jobq_saturation"; + +// ===== Quorum and publish: can this node accept and publish a ledger? ======= + +/** + * Pre-accept quorum gate and publish lag, split by the `metric` label. + */ +inline constexpr char ledgerQuorumPublish[] = "ledger_quorum_publish"; +/** + * Pre-accept gate rejections for being below quorum. + */ +inline constexpr char ledgerQuorumShortfallTotal[] = "ledger_quorum_shortfall_total"; + +// ===== Back-fill persistence: is history repair making progress? ============= + +/** + * Replay sub-acquires that fell back to a full ledger acquire, by stage. + */ +inline constexpr char ledgerReplayFallbackTotal[] = "ledger_replay_fallback_total"; +/** + * Ledger replay tasks by terminal outcome. + */ +inline constexpr char ledgerReplayOutcomeTotal[] = "ledger_replay_outcome_total"; +/** + * Forced jumps of the last closed ledger to a divergent chain. + */ +inline constexpr char ledgerJumpTotal[] = "ledger_jump_total"; + +// ===== Peer supply: what this node's peers can and will serve =============== + +/** + * Peer coverage of the ledger sequence range this node still needs. + */ +inline constexpr char peerLedgerSupply[] = "peer_ledger_supply"; +/** + * Inbound peer connection attempts, by terminal outcome. + */ +inline constexpr char peerAcceptTotal[] = "peer_accept_total"; +/** + * Peer disconnects, by cause and connection direction. + */ +inline constexpr char peerDisconnectTotal[] = "peer_disconnect_total"; +/** + * Peer data requests this node declined to serve, by kind and cause. + */ +inline constexpr char serveRefusedTotal[] = "serve_refused_total"; +/** + * PeerFinder slots, dials in flight, and address-cache sizes. + */ +inline constexpr char peerfinderSlotCensus[] = "peerfinder_slot_census"; +/** + * Amendment-block warning and the countdown to this node ceasing to validate. + */ +inline constexpr char amendmentBlock[] = "amendment_block"; +/** + * NodeStore mean store/fetch latency, with the operation counts. + */ +inline constexpr char nodestoreLatency[] = "nodestore_latency"; + +// ===== Consensus ============================================================= + +/** + * Wall-clock duration of a completed consensus round. + */ +inline constexpr char consensusRoundDurationMs[] = "consensus_round_duration_ms"; + +// ===== Pre-existing instruments pulled in by the family ratchet ============== +// +// These predate the sync-diagnostics work. They are declared here because the +// checker's Rule I enforces literal-freedom per metric FAMILY (first +// underscore segment), and each of these shares a family with a name above -- +// `ledger_`, `nodestore_`, `server_`, `peer_`, `state_`. Leaving them as +// literals would either weaken the rule to per-name (letting a typo'd sibling +// through) or require an exemption list. Declaring them is the honest option: +// no behaviour changes, and the next author editing these families finds the +// constant rather than inventing a second spelling. +// +// The remaining unconverted families are reported as Rule L warnings, so the +// outstanding work stays visible rather than silently accepted. + +/** + * Built-vs-validated ledger mismatches, by reason. + */ +inline constexpr char ledgerHistoryMismatchTotal[] = "ledger_history_mismatch_total"; +/** + * Ledger fee and economy readings. + */ +inline constexpr char ledgerEconomy[] = "ledger_economy"; +/** + * NodeStore I/O counters, queue depth and write load. + */ +inline constexpr char nodestoreState[] = "nodestore_state"; +/** + * Server-level health readings. + */ +inline constexpr char serverInfo[] = "server_info"; +/** + * Peer-network quality readings. + */ +inline constexpr char peerQuality[] = "peer_quality"; +/** + * Node state and operating-mode tracking. + */ +inline constexpr char stateTracking[] = "state_tracking"; + +} // namespace metric + +/** + * Label keys -- the dimension names attached to a metric datapoint. + * + * Every key here is bounded by design: the values it can take are either a + * fixed set declared in `namespace lval` below, or a small enumeration the + * code derives (an operating mode, a job type). A key whose values are + * unbounded runtime data would mint one time series per distinct value, so no + * such key is declared. + */ +namespace label { + +/** + * Sub-metric discriminator on a multi-series gauge. + * + * The pattern every observable gauge in MetricsRegistry.cpp already uses: one + * instrument carries several related readings, told apart by this label rather + * than by being separate instruments. Pre-dates the sync-diagnostics work; + * named here because the new gauges are its heaviest users. + */ +inline constexpr char metric[] = "metric"; +/** + * Job type, as produced by `JobTypes::name()`. + */ +inline constexpr char jobType[] = "job_type"; +/** + * Terminal result of a bounded operation. + */ +inline constexpr char outcome[] = "outcome"; +/** + * Cause of a rejection, refusal or teardown. + */ +inline constexpr char reason[] = "reason"; +/** + * Configured validator-list site URI. The one runtime-valued key here. + */ +inline constexpr char site[] = "site"; +/** + * Operating mode a transition started from. + */ +inline constexpr char from[] = "from"; +/** + * Operating mode a transition ended at. + */ +inline constexpr char to[] = "to"; +/** + * Where acquired ledger data came from. + */ +inline constexpr char source[] = "source"; +/** + * Which stage of a multi-step pipeline the event belongs to. + */ +inline constexpr char stage[] = "stage"; +/** + * Connection direction, inbound or outbound. + */ +inline constexpr char direction[] = "direction"; +/** + * Which kind of peer data request is being described. + */ +inline constexpr char request[] = "request"; + +} // namespace label + +/** + * Bounded label values -- the fixed value sets the code itself writes. + * + * Nested by the instrument (or the gauge) that owns the set, because the same + * word means different things in different sets and a flat namespace would let + * two of them collide. A value is declared here only when the code chooses it + * from a fixed list; anything derived from runtime data stays out. + */ +namespace lval { + +// ===== Shared outcome/direction slugs ======================================= + +/** + * Values shared by more than one instrument. Declared once so two instruments + * that mean the same thing cannot spell it differently. + */ +inline constexpr char timeout[] = "timeout"; +inline constexpr char notFound[] = "not_found"; +inline constexpr char inbound[] = "inbound"; +inline constexpr char outbound[] = "outbound"; + +/** + * `dns_resolve_total` outcomes: did the resolver return any address? + */ +namespace dns_resolve { +inline constexpr char resolved[] = "resolved"; +inline constexpr char empty[] = "empty"; +} // namespace dns_resolve + +/** + * `peer_accept_total` outcomes -- the nine exits of the inbound-accept path. + * + * Every exit records one of these, so the counter's total equals the number of + * inbound attempts and an unexplained gap is impossible. + */ +namespace peer_accept { +inline constexpr char localEndpointFail[] = "local_endpoint_fail"; +inline constexpr char resourceLimit[] = "resource_limit"; +inline constexpr char noSlot[] = "no_slot"; +inline constexpr char notPeerRequest[] = "not_peer_request"; +inline constexpr char protocolMismatch[] = "protocol_mismatch"; +inline constexpr char badCookie[] = "bad_cookie"; +inline constexpr char slotRefused[] = "slot_refused"; +inline constexpr char accepted[] = "accepted"; +inline constexpr char handshakeError[] = "handshake_error"; +} // namespace peer_accept + +/** + * `handshake_negotiation_fail_total` reasons -- one per rejection point in + * the handshake verifier. + * + * These separate a peer misconfiguration this node should tolerate + * (`wrong_network`, `self_connection`) from a local misconfiguration an + * operator must fix (`clock_skew`, `local_ip_mismatch`), which is the whole + * point of splitting the counter by reason. + */ +namespace handshake_fail { +inline constexpr char invalidServerDomain[] = "invalid_server_domain"; +inline constexpr char invalidNetworkId[] = "invalid_network_id"; +inline constexpr char wrongNetwork[] = "wrong_network"; +inline constexpr char invalidClockTimestamp[] = "invalid_clock_timestamp"; +inline constexpr char clockSkew[] = "clock_skew"; +inline constexpr char unsupportedKeyType[] = "unsupported_key_type"; +inline constexpr char badPublicKey[] = "bad_public_key"; +inline constexpr char noSessionSignature[] = "no_session_signature"; +inline constexpr char sessionVerifyFailed[] = "session_verify_failed"; +inline constexpr char selfConnection[] = "self_connection"; +inline constexpr char invalidLocalIp[] = "invalid_local_ip"; +inline constexpr char localIpMismatch[] = "local_ip_mismatch"; +inline constexpr char invalidRemoteIp[] = "invalid_remote_ip"; +inline constexpr char remoteIpMismatch[] = "remote_ip_mismatch"; +} // namespace handshake_fail + +/** + * `unl_fetch_total` outcomes for the transport-level failures. + * + * The success path instead labels with `to_string(ListDisposition)`, whose + * values are owned by the protocol enum and are therefore not restated here -- + * duplicating them would create a second place to update when a disposition is + * added. + */ +namespace unl_fetch { +inline constexpr char fetchError[] = "fetch_error"; +inline constexpr char badStatus[] = "bad_status"; +inline constexpr char parseError[] = "parse_error"; +} // namespace unl_fetch + +/** + * `unl_quorum` sub-metrics: the trusted-key count and the bar it must clear. + */ +namespace unl_quorum { +inline constexpr char trustedKeys[] = "trusted_keys"; +inline constexpr char quorum[] = "quorum"; +} // namespace unl_quorum + +/** + * `clock_close_offset_seconds` sub-metric. + */ +namespace clock_offset { +inline constexpr char offset[] = "offset"; +} // namespace clock_offset + +/** + * `sync_state` sub-metrics -- the four "why am I not FULL" signals. + * + * `initial_full_duration_us` reads zero until the node first reaches FULL, + * which is the state this gauge exists to make visible rather than a missing + * value. + */ +namespace sync_state { +inline constexpr char initialFullDurationUs[] = "initial_full_duration_us"; +inline constexpr char networkLedgerGate[] = "network_ledger_gate"; +inline constexpr char serverStallSeconds[] = "server_stall_seconds"; +inline constexpr char ledgersBehind[] = "ledgers_behind"; +} // namespace sync_state + +/** + * `sync_acquire` sub-metrics -- aggregate acquire progress. + * + * The two `missing_*_max` series are the stuck-detector: flat and non-zero + * across collection ticks means the acquire will never finish, while shrinking + * means slow but alive. `in_flight` is the context that tells idle from stuck. + */ +namespace sync_acquire { +inline constexpr char missingStateNodesMax[] = "missing_state_nodes_max"; +inline constexpr char missingTxNodesMax[] = "missing_tx_nodes_max"; +inline constexpr char receivedDataDepth[] = "received_data_depth"; +inline constexpr char inFlight[] = "in_flight"; +} // namespace sync_acquire + +/** + * `shamap_cache_hit_rate` sub-metric: which cache the rate describes. + */ +namespace shamap_cache { +inline constexpr char treenode[] = "treenode"; +} // namespace shamap_cache + +/** + * `sync_acquire_source_total` sources: served locally or fetched from peers. + */ +namespace acquire_source { +inline constexpr char local[] = "local"; +inline constexpr char network[] = "network"; +} // namespace acquire_source + +/** + * `sync_addnode_total` outcomes -- the per-node verdict on received SHAMap data. + * + * The split is what separates real progress (`good`) from wasted bandwidth + * (`duplicate`) and a misbehaving peer (`invalid`); traffic-level metrics show + * all three as healthy throughput. + */ +namespace addnode { +inline constexpr char good[] = "good"; +inline constexpr char duplicate[] = "duplicate"; +inline constexpr char invalid[] = "invalid"; +} // namespace addnode + +/** + * `jobq_backlog` sub-metrics -- the three occupancy states of a job type. + * + * `deferred` has no other exposure anywhere: a job held back by its type's + * concurrency limit counts as neither waiting nor running. + */ +namespace jobq_backlog { +inline constexpr char waiting[] = "waiting"; +inline constexpr char running[] = "running"; +inline constexpr char deferred[] = "deferred"; +} // namespace jobq_backlog + +/** + * `jobq_saturation` sub-metrics: the numerator, denominator and the backlog. + */ +namespace jobq_saturation { +inline constexpr char runningTasks[] = "running_tasks"; +inline constexpr char workerThreads[] = "worker_threads"; +inline constexpr char totalWaiting[] = "total_waiting"; +} // namespace jobq_saturation + +/** + * `peer_ledger_supply` sub-metrics: who can serve what this node needs. + */ +namespace peer_supply { +inline constexpr char peersReporting[] = "peers_reporting"; +inline constexpr char peersServingValidated[] = "peers_serving_validated"; +inline constexpr char peersServingNext[] = "peers_serving_next"; +inline constexpr char supplyMinSeq[] = "supply_min_seq"; +inline constexpr char supplyMaxSeq[] = "supply_max_seq"; +} // namespace peer_supply + +/** + * `peerfinder_slot_census` sub-metrics -- slots, dials and address caches. + * + * `connecting` non-zero while `out_active` stays under `out_max` is the + * "starting dials and never completing them" case; both caches at zero on a + * fresh node means there is nothing left to dial at all. + */ +namespace slot_census { +inline constexpr char outActive[] = "out_active"; +inline constexpr char outMax[] = "out_max"; +inline constexpr char inActive[] = "in_active"; +inline constexpr char inMax[] = "in_max"; +inline constexpr char connecting[] = "connecting"; +inline constexpr char fixedConfigured[] = "fixed_configured"; +inline constexpr char fixedActive[] = "fixed_active"; +inline constexpr char bootcache[] = "bootcache"; +inline constexpr char livecache[] = "livecache"; +} // namespace slot_census + +/** + * `amendment_block` sub-metrics: the warning flag and the countdown. + */ +namespace amendment_block { +inline constexpr char warned[] = "warned"; +inline constexpr char secondsToBlock[] = "seconds_to_block"; +} // namespace amendment_block + +/** + * `nodestore_latency` sub-metrics: mean latency per direction, with counts. + */ +namespace nodestore_latency { +inline constexpr char writeCount[] = "write_count"; +inline constexpr char readCount[] = "read_count"; +inline constexpr char writeMeanUs[] = "write_mean_us"; +inline constexpr char readMeanUs[] = "read_mean_us"; +} // namespace nodestore_latency + +/** + * `ledger_quorum_publish` sub-metrics: the gate, and how late publish is. + */ +namespace quorum_publish { +inline constexpr char trustedValidationTally[] = "trusted_validation_tally"; +inline constexpr char quorumTarget[] = "quorum_target"; +inline constexpr char timeToFirstValidatedUs[] = "time_to_first_validated_us"; +inline constexpr char publishLag[] = "publish_lag"; +} // namespace quorum_publish + +/** + * `ledger_quorum_shortfall_total` stage: which gate did the rejecting. + */ +namespace quorum_shortfall { +inline constexpr char preAccept[] = "pre_accept"; +} // namespace quorum_shortfall + +/** + * `ledger_replay_fallback_total` stages: which sub-acquire gave up. + */ +namespace replay_fallback { +inline constexpr char skiplist[] = "skiplist"; +inline constexpr char delta[] = "delta"; +} // namespace replay_fallback + +/** + * `ledger_replay_outcome_total` outcomes -- the four terminal states of a + * replay task. `timeout` is the shared slug above. + */ +namespace replay_outcome { +inline constexpr char success[] = "success"; +inline constexpr char buildFailed[] = "build_failed"; +inline constexpr char parameterFailed[] = "parameter_failed"; +} // namespace replay_outcome + +/** + * `peer_disconnect_total` reasons -- why a peer connection closed. + * + * The split separates our-fault backpressure (`large_sendq`, + * `charge_resources`) from a topology or network fault (`not_useful`, + * `ping_timeout`, `read_error`); the two call for opposite responses. + * `unknown` is the initial value and appears when a teardown path set no + * cause, so an unattributed disconnect is visible rather than absent. + */ +namespace disconnect { +inline constexpr char unknown[] = "unknown"; +inline constexpr char malformedHandshake[] = "malformed_handshake"; +inline constexpr char stopping[] = "stopping"; +inline constexpr char chargeResources[] = "charge_resources"; +inline constexpr char timerError[] = "timer_error"; +inline constexpr char largeSendq[] = "large_sendq"; +inline constexpr char notUseful[] = "not_useful"; +inline constexpr char pingTimeout[] = "ping_timeout"; +inline constexpr char shutdown[] = "shutdown"; +inline constexpr char sharedValue[] = "shared_value"; +inline constexpr char writeError[] = "write_error"; +inline constexpr char graceful[] = "graceful"; +inline constexpr char readError[] = "read_error"; +} // namespace disconnect + +/** + * `serve_refused_total` request kinds: what the peer had asked for. + */ +namespace serve_request { +inline constexpr char object[] = "object"; +inline constexpr char fetchpack[] = "fetchpack"; +inline constexpr char txset[] = "txset"; +inline constexpr char ledger[] = "ledger"; +} // namespace serve_request + +/** + * `serve_refused_total` reasons: why this node would not answer. + * `not_found` is the shared slug above. + * + * `empty_reply` is the subtle one: the map WAS found, but the reply loop + * produced no nodes, so the requester still gets nothing and must ask another + * peer. Counting it as served would make a node that answers every request + * with an empty payload look healthy. + */ +namespace serve_refused { +inline constexpr char sendqFull[] = "sendq_full"; +inline constexpr char loadShed[] = "load_shed"; +inline constexpr char badType[] = "bad_type"; +inline constexpr char noMap[] = "no_map"; +inline constexpr char emptyReply[] = "empty_reply"; +} // namespace serve_refused + +} // namespace lval + +} // namespace xrpl::telemetry diff --git a/src/xrpld/telemetry/MetricsRegistry.cpp b/src/xrpld/telemetry/MetricsRegistry.cpp index aba1a313dc..27b3e35c2b 100644 --- a/src/xrpld/telemetry/MetricsRegistry.cpp +++ b/src/xrpld/telemetry/MetricsRegistry.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include @@ -369,7 +370,7 @@ MetricsRegistry::initSyncInstruments() }, this); ledgerHistoryMismatchCounter_ = meter_->CreateUInt64Counter( - "ledger_history_mismatch_total", "Total built-vs-validated ledger mismatches by reason"); + metric::ledgerHistoryMismatchTotal, "Total built-vs-validated ledger mismatches by reason"); txqExpiredCounter_ = meter_->CreateUInt64Counter( "txq_expired_total", "Total transactions expired out of the transaction queue"); txqDroppedCounter_ = meter_->CreateUInt64Counter( @@ -815,7 +816,7 @@ MetricsRegistry::registerNodeStoreGauge() // libxrpl nodestore code — the MetricsRegistry reads the existing atomic // counters from Database via its public accessors. nodeStoreGauge_ = meter_->CreateInt64ObservableGauge( - "nodestore_state", "NodeStore I/O counters, queue depth, and write load"); + metric::nodestoreState, "NodeStore I/O counters, queue depth, and write load"); nodeStoreGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -894,7 +895,7 @@ MetricsRegistry::registerServerInfoGauge() { // --- Task 9.7a: Server info gauges --- serverInfoGauge_ = - meter_->CreateInt64ObservableGauge("server_info", "Server-level health metrics"); + meter_->CreateInt64ObservableGauge(metric::serverInfo, "Server-level health metrics"); serverInfoGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1146,7 +1147,7 @@ MetricsRegistry::registerPeerQualityGauge() // Uses Peer::json() to read latency and version since those accessors // are not on the abstract Peer interface (they live on PeerImp). peerQualityGauge_ = - meter_->CreateDoubleObservableGauge("peer_quality", "Peer network quality metrics"); + meter_->CreateDoubleObservableGauge(metric::peerQuality, "Peer network quality metrics"); peerQualityGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1297,8 +1298,8 @@ void MetricsRegistry::registerLedgerEconomyGauge() { // --- Task 7.11: Ledger economy gauges --- - ledgerEconomyGauge_ = - meter_->CreateDoubleObservableGauge("ledger_economy", "Ledger fee and economy metrics"); + ledgerEconomyGauge_ = meter_->CreateDoubleObservableGauge( + metric::ledgerEconomy, "Ledger fee and economy metrics"); ledgerEconomyGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1364,7 +1365,7 @@ MetricsRegistry::registerStateTrackingGauge() { // --- Task 7.12: State tracking gauges --- stateTrackingGauge_ = - meter_->CreateDoubleObservableGauge("state_tracking", "Node state and mode tracking"); + meter_->CreateDoubleObservableGauge(metric::stateTracking, "Node state and mode tracking"); stateTrackingGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1567,7 +1568,7 @@ MetricsRegistry::registerUnlQuorumGauge() // with the trusted-key count in one instrument is what makes the // "can this node ever validate?" comparison a single query. unlQuorumGauge_ = meter_->CreateInt64ObservableGauge( - "unl_quorum", "Trusted UNL key count vs required quorum"); + metric::unlQuorum, "Trusted UNL key count vs required quorum"); unlQuorumGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1587,7 +1588,9 @@ MetricsRegistry::registerUnlQuorumGauge() // Trusted master keys currently in effect. Zero means no // usable UNL: quorum can never be met. - observe("trusted_keys", static_cast(validators.trustedKeyCount())); + observe( + lval::unl_quorum::trustedKeys, + static_cast(validators.trustedKeyCount())); // Validations required for a ledger to be fully validated. // ValidatorList disables quorum by returning SIZE_MAX when too @@ -1599,7 +1602,7 @@ MetricsRegistry::registerUnlQuorumGauge() // truthful signal. auto const quorum = validators.quorum(); observe( - "quorum", + lval::unl_quorum::quorum, quorum == std::numeric_limits::max() ? std::numeric_limits::max() : static_cast(quorum)); @@ -1620,7 +1623,8 @@ MetricsRegistry::registerClockSkewGauge() // network, which delays consensus participation. server_info hides // this below 60 s, so export it continuously instead. clockSkewGauge_ = meter_->CreateInt64ObservableGauge( - "clock_close_offset_seconds", "Network close time offset from the local clock, in seconds"); + metric::clockCloseOffsetSeconds, + "Network close time offset from the local clock, in seconds"); clockSkewGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1637,7 +1641,9 @@ MetricsRegistry::registerClockSkewGauge() }; // Negative when the local clock runs ahead of the network. - observe("offset", static_cast(app.getTimeKeeper().closeOffset().count())); + observe( + lval::clock_offset::offset, + static_cast(app.getTimeKeeper().closeOffset().count())); } catch (...) // NOLINT(bugprone-empty-catch) { @@ -1654,7 +1660,7 @@ MetricsRegistry::registerSyncStateGauge() // Four values that previously lived only in a log line or in server_info // JSON. All four are cheap reads pulled on the ~10 s reader tick. syncStateGauge_ = - meter_->CreateInt64ObservableGauge("sync_state", "Sync-pipeline health signals"); + meter_->CreateInt64ObservableGauge(metric::syncState, "Sync-pipeline health signals"); syncStateGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1675,21 +1681,23 @@ MetricsRegistry::registerSyncStateGauge() // Time to first FULL. Zero means the node has not synced yet, // which is exactly the case this signal exists to expose. observe( - "initial_full_duration_us", + lval::sync_state::initialFullDurationUs, static_cast(ops.getInitialSyncDurationUs())); // 1 = still waiting for a full network ledger. While this is // set the node refuses transactions and cannot reach FULL. - observe("network_ledger_gate", ops.isNeedNetworkLedger() ? 1 : 0); + observe(lval::sync_state::networkLedgerGate, ops.isNeedNetworkLedger() ? 1 : 0); // Current main-loop stall duration; 0 when healthy. observe( - "server_stall_seconds", + lval::sync_state::serverStallSeconds, static_cast(app.getLoadManager().getCurrentStallSeconds())); // Distance from the network tip, floored at zero by the // accessor. - observe("ledgers_behind", static_cast(ops.getLedgersBehindNetwork())); + observe( + lval::sync_state::ledgersBehind, + static_cast(ops.getLedgersBehindNetwork())); } catch (...) // NOLINT(bugprone-empty-catch) { @@ -1709,7 +1717,7 @@ MetricsRegistry::registerStallEventsCounter() // Kept out of the sync_state gauge because a cumulative total needs // counter aggregation for rate() to be meaningful. stallEventsObservable_ = meter_->CreateInt64ObservableCounter( - "server_stall_events_total", "Total server main-loop stall episodes"); + metric::serverStallEventsTotal, "Total server main-loop stall episodes"); stallEventsObservable_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1738,7 +1746,7 @@ MetricsRegistry::registerSyncAcquireGauge() // acquired, which is unbounded. The per-ledger view lives on the // ledger.acquire span instead. syncAcquireGauge_ = meter_->CreateInt64ObservableGauge( - "sync_acquire", "Aggregate ledger-acquire progress across in-flight acquires"); + metric::syncAcquire, "Aggregate ledger-acquire progress across in-flight acquires"); syncAcquireGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1761,15 +1769,20 @@ MetricsRegistry::registerSyncAcquireGauge() // Flat and non-zero across ticks = this acquire will never // finish. Shrinking = slow but alive. observe( - "missing_state_nodes_max", static_cast(progress.maxMissingStateNodes)); - observe("missing_tx_nodes_max", static_cast(progress.maxMissingTxNodes)); + lval::sync_acquire::missingStateNodesMax, + static_cast(progress.maxMissingStateNodes)); + observe( + lval::sync_acquire::missingTxNodesMax, + static_cast(progress.maxMissingTxNodes)); // Deep stash = arriving data outpaces processing. - observe("received_data_depth", static_cast(progress.receivedDataDepth)); + observe( + lval::sync_acquire::receivedDataDepth, + static_cast(progress.receivedDataDepth)); // Context for the three above: zero everywhere with zero // in-flight acquires is idle, not healthy. - observe("in_flight", static_cast(progress.inFlight)); + observe(lval::sync_acquire::inFlight, static_cast(progress.inFlight)); } catch (...) // NOLINT(bugprone-empty-catch) { @@ -1786,7 +1799,7 @@ MetricsRegistry::registerCacheHitRateDetailGauge() // The memory layer above the node store: a miss here is what causes a // node-store read, which the NuDB hit-ratio panel then measures. shamapCacheHitRateGauge_ = meter_->CreateDoubleObservableGauge( - "shamap_cache_hit_rate", "SHAMap tree-node cache hit rate (0.0-1.0), by cache"); + metric::shamapCacheHitRate, "SHAMap tree-node cache hit rate (0.0-1.0), by cache"); shamapCacheHitRateGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1802,7 +1815,8 @@ MetricsRegistry::registerCacheHitRateDetailGauge() auto const rate = app.getNodeFamily().getTreeNodeCache()->getHitRate() / 100.0F; opentelemetry::nostd::get>>(result) - ->Observe(static_cast(rate), {{"metric", "treenode"}}); + ->Observe( + static_cast(rate), {{label::metric, lval::shamap_cache::treenode}}); } catch (...) // NOLINT(bugprone-empty-catch) { @@ -1821,7 +1835,7 @@ MetricsRegistry::registerJobQueueBacklogGauge() // has no other exposure: a job held back by its type's concurrency limit // counts as neither waiting nor running anywhere else. jobQueueBacklogGauge_ = meter_->CreateInt64ObservableGauge( - "jobq_backlog", "JobQueue occupancy per job type (waiting/running/deferred)"); + metric::jobqBacklog, "JobQueue occupancy per job type (waiting/running/deferred)"); jobQueueBacklogGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1834,7 +1848,7 @@ MetricsRegistry::registerJobQueueBacklogGauge() auto observe = [&](char const* field, std::string const& jobType, int64_t value) { opentelemetry::nostd::get>>(result) - ->Observe(value, {{"metric", field}, {"job_type", jobType}}); + ->Observe(value, {{label::metric, field}, {label::jobType, jobType}}); }; // One snapshot under one lock acquire, so the three fields of @@ -1846,9 +1860,9 @@ MetricsRegistry::registerJobQueueBacklogGauge() // the same one the job_*_total counters already use, so the // two label sets join. auto const& jobType = JobTypes::name(count.type); - observe("waiting", jobType, count.waiting); - observe("running", jobType, count.running); - observe("deferred", jobType, count.deferred); + observe(lval::jobq_backlog::waiting, jobType, count.waiting); + observe(lval::jobq_backlog::running, jobType, count.running); + observe(lval::jobq_backlog::deferred, jobType, count.deferred); } } catch (...) // NOLINT(bugprone-empty-catch) @@ -1867,7 +1881,8 @@ MetricsRegistry::registerJobQueueSaturationGauge() // leaving it to look like an independent fault in every subsystem whose // jobs are queued behind it. jobQueueSaturationGauge_ = meter_->CreateInt64ObservableGauge( - "jobq_saturation", "Worker-pool saturation: tasks in flight, worker threads, jobs queued"); + metric::jobqSaturation, + "Worker-pool saturation: tasks in flight, worker threads, jobs queued"); jobQueueSaturationGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1886,16 +1901,16 @@ MetricsRegistry::registerJobQueueSaturationGauge() // One reading feeds all three series so the ratio and the // backlog describe the same instant. auto const saturation = app.getJobQueue().getWorkerSaturation(); - observe("running_tasks", saturation.runningTasks); + observe(lval::jobq_saturation::runningTasks, saturation.runningTasks); // The denominator for the ratio panel. Derived at startup from // [workers], node size and hardware concurrency, so it cannot // be hardcoded in a dashboard. - observe("worker_threads", saturation.workerThreads); + observe(lval::jobq_saturation::workerThreads, saturation.workerThreads); // Ratio at 1.0 alone is a busy pool; ratio at 1.0 with a // non-zero backlog is an exhausted one. - observe("total_waiting", saturation.totalWaiting); + observe(lval::jobq_saturation::totalWaiting, saturation.totalWaiting); } catch (...) // NOLINT(bugprone-empty-catch) { @@ -1914,7 +1929,7 @@ MetricsRegistry::registerPeerLedgerSupplyGauge() // want" looked exactly like "my peers are slow" -- two faults with // completely different fixes. peerLedgerSupplyGauge_ = meter_->CreateInt64ObservableGauge( - "peer_ledger_supply", "Peer coverage of the ledger sequence this node needs"); + metric::peerLedgerSupply, "Peer coverage of the ledger sequence this node needs"); peerLedgerSupplyGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1937,17 +1952,17 @@ MetricsRegistry::registerPeerLedgerSupplyGauge() // The denominator. Zero serving out of zero reporting is // silence; zero out of many is a real supply gap. - observe("peers_reporting", supply.peersReporting); - observe("peers_serving_validated", supply.peersServingValidated); + observe(lval::peer_supply::peersReporting, supply.peersReporting); + observe(lval::peer_supply::peersServingValidated, supply.peersServingValidated); // The verdict: zero here while peers_reporting is non-zero // means waiting cannot finish the sync. - observe("peers_serving_next", supply.peersServingNext); + observe(lval::peer_supply::peersServingNext, supply.peersServingNext); // The window the peer set covers, so an operator can tell a // request for discarded history from one for an unreached tip. - observe("supply_min_seq", supply.supplyMinSeq); - observe("supply_max_seq", supply.supplyMaxSeq); + observe(lval::peer_supply::supplyMinSeq, supply.supplyMinSeq); + observe(lval::peer_supply::supplyMaxSeq, supply.supplyMaxSeq); } catch (...) // NOLINT(bugprone-empty-catch) { @@ -1965,7 +1980,7 @@ MetricsRegistry::registerSlotCensusGauge() // counts are exported today, which cannot distinguish "not dialling", // "dialling and failing" and "nothing to dial". slotCensusGauge_ = meter_->CreateInt64ObservableGauge( - "peerfinder_slot_census", "PeerFinder slots, connection attempts and address caches"); + metric::peerfinderSlotCensus, "PeerFinder slots, connection attempts and address caches"); slotCensusGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -1985,23 +2000,23 @@ MetricsRegistry::registerSlotCensusGauge() // and capacity can be compared against each other. auto const census = app.getOverlay().getSlotCensus(); - observe("out_active", census.outActive); - observe("out_max", census.outMax); - observe("in_active", census.inActive); - observe("in_max", census.inMax); + observe(lval::slot_census::outActive, census.outActive); + observe(lval::slot_census::outMax, census.outMax); + observe(lval::slot_census::inActive, census.inActive); + observe(lval::slot_census::inMax, census.inMax); // Dials in flight. Non-zero while out_active stays under // out_max is the "starting and never completing" case. - observe("connecting", census.connecting); + observe(lval::slot_census::connecting, census.connecting); // fixed_active below fixed_configured names a configured peer // that cannot be reached. - observe("fixed_configured", census.fixedConfigured); - observe("fixed_active", census.fixedActive); + observe(lval::slot_census::fixedConfigured, census.fixedConfigured); + observe(lval::slot_census::fixedActive, census.fixedActive); // Both at zero on a fresh node means there is nothing to dial. - observe("bootcache", census.bootcache); - observe("livecache", census.livecache); + observe(lval::slot_census::bootcache, census.bootcache); + observe(lval::slot_census::livecache, census.livecache); } catch (...) // NOLINT(bugprone-empty-catch) { @@ -2018,7 +2033,8 @@ MetricsRegistry::registerAmendmentBlockGauge() // The existing validator_health{metric="amendment_blocked"} reports the // terminal state, when nothing can be done. This is the window before it. amendmentBlockGauge_ = meter_->CreateInt64ObservableGauge( - "amendment_block", "Amendment-block warning and seconds until the node stops validating"); + metric::amendmentBlock, + "Amendment-block warning and seconds until the node stops validating"); amendmentBlockGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -2036,7 +2052,7 @@ MetricsRegistry::registerAmendmentBlockGauge() // An unsupported amendment has reached majority. Until now this // only surfaced as an admin-only server_info warning. - observe("warned", app.getOPs().isAmendmentWarned() ? 1 : 0); + observe(lval::amendment_block::warned, app.getOPs().isAmendmentWarned() ? 1 : 0); // Seconds until that amendment activates. -1 means nothing is // pending: a distinct healthy value rather than an absent @@ -2056,7 +2072,7 @@ MetricsRegistry::registerAmendmentBlockGauge() // overdue by an amount worth charting. secondsToBlock = std::max(expectedSecs - nowSecs, 0); } - observe("seconds_to_block", secondsToBlock); + observe(lval::amendment_block::secondsToBlock, secondsToBlock); } catch (...) // NOLINT(bugprone-empty-catch) { @@ -2082,7 +2098,8 @@ MetricsRegistry::registerNodeStoreLatencyGauge() // ledger write. This reads four atomics per ~10 s tick instead. The // trade-off is that percentiles are unavailable -- see the header comment. nodeStoreLatencyGauge_ = meter_->CreateInt64ObservableGauge( - "nodestore_latency", "NodeStore mean store/fetch latency in microseconds, with counts"); + metric::nodestoreLatency, + "NodeStore mean store/fetch latency in microseconds, with counts"); nodeStoreLatencyGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { auto* self = static_cast(state); @@ -2109,8 +2126,8 @@ MetricsRegistry::registerNodeStoreLatencyGauge() // Counts are always observed, including zero: that is what // separates "nothing written yet" from "writes are instant". - observe("write_count", static_cast(storeCount)); - observe("read_count", static_cast(fetchCount)); + observe(lval::nodestore_latency::writeCount, static_cast(storeCount)); + observe(lval::nodestore_latency::readCount, static_cast(fetchCount)); // A mean needs a non-zero denominator, and it needs a // numerator that was actually measured. Both are required, and @@ -2128,9 +2145,17 @@ MetricsRegistry::registerNodeStoreLatencyGauge() // stays 0. Omitting the mean makes that a visible data gap // instead of a false "writes take 0 us" line on the panel. if (storeCount > 0 && storeDurationUs > 0) - observe("write_mean_us", static_cast(storeDurationUs / storeCount)); + { + observe( + lval::nodestore_latency::writeMeanUs, + static_cast(storeDurationUs / storeCount)); + } if (fetchCount > 0 && fetchDurationUs > 0) - observe("read_mean_us", static_cast(fetchDurationUs / fetchCount)); + { + observe( + lval::nodestore_latency::readMeanUs, + static_cast(fetchDurationUs / fetchCount)); + } } catch (...) // NOLINT(bugprone-empty-catch) { @@ -2149,7 +2174,7 @@ MetricsRegistry::registerLedgerQuorumPublishGauge() // one validated (quorum short), or validate correctly and never publish // (pipeline behind). Both used to be trace-log-only or not derivable at all. ledgerQuorumPublishGauge_ = meter_->CreateInt64ObservableGauge( - "ledger_quorum_publish", + metric::ledgerQuorumPublish, "Pre-accept quorum gate and publish lag (tally vs quorum, first-validated, lag)"); ledgerQuorumPublishGauge_->AddCallback( [](opentelemetry::metrics::ObserverResult result, void* state) { @@ -2171,21 +2196,25 @@ MetricsRegistry::registerLedgerQuorumPublishGauge() // The pair that separates "slow" from "stuck". A tally climbing // toward the target will get there; a tally flat below it never // will, and no acquire or peer panel says which is happening. - observe("trusted_validation_tally", ledgerMaster.getTrustedValidationTally()); + observe( + lval::quorum_publish::trustedValidationTally, + ledgerMaster.getTrustedValidationTally()); // What the last gate evaluation actually required, as opposed to // unl_quorum{quorum} which is what the trusted list configures. // Already clamped against the SIZE_MAX "quorum disabled" // sentinel by LedgerMaster, so this never wraps negative. - observe("quorum_target", ledgerMaster.getQuorumTarget()); + observe(lval::quorum_publish::quorumTarget, ledgerMaster.getQuorumTarget()); // One-shot: a value is the time the first ledger took to pass // the gate, and 0 means it never has. Not a trend. - observe("time_to_first_validated_us", ledgerMaster.getTimeToFirstValidatedUs()); + observe( + lval::quorum_publish::timeToFirstValidatedUs, + ledgerMaster.getTimeToFirstValidatedUs()); // Validated but not yet published. pubLedgerSeq_ was never // exported, so this gap was not derivable from any other series. - observe("publish_lag", ledgerMaster.getPublishLag()); + observe(lval::quorum_publish::publishLag, ledgerMaster.getPublishLag()); } catch (...) // NOLINT(bugprone-empty-catch) {