Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ jobs:

- name: Install CLI tools
run: |
for tool in mbake ty ruff; do uv tool install ${tool}; done
for tool in mbake ty; do uv tool install ${tool}; done
# Pin ruff to the same version as the dev dependency group in
# pyproject.toml so the linter behaves identically across the uv tool,
# the synced virtualenv, and developer PATH. Bump both sites together.
uv tool install ruff==0.15.12
npm install -g markdownlint-cli2

- name: Validate Makefile
Expand Down
23 changes: 15 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,14 @@ MDLINT ?= $(shell which markdownlint)
NIXIE ?= $(shell which nixie)
MDFORMAT_ALL ?= $(shell which mdformat-all)
UV ?= $(shell command -v uv 2>/dev/null || printf '%s/.local/bin/uv' "$$HOME")
TOOLS = $(MDFORMAT_ALL) ruff ty $(MDLINT) $(NIXIE) $(UV)
# Pin Ruff so `make` invokes the same version as the `ruff==` dev dependency
# in pyproject.toml and the `uv tool install ruff==` step in
# .github/workflows/ci.yml. Bump all three sites together: a version mismatch
# causes version-skew lint failures because rule sets differ between Ruff
# releases.
RUFF_VERSION ?= 0.15.12
RUFF ?= $(UV) tool run --from ruff==$(RUFF_VERSION) ruff
Comment thread
coderabbitai[bot] marked this conversation as resolved.
TOOLS = $(MDFORMAT_ALL) ty $(MDLINT) $(NIXIE) $(UV)
PY_SOURCES := $(sort $(shell find lading scripts -type f -name '*.py' -print))
VENV_TOOLS = interrogate pytest
PYLINT_PYTHON ?= pypy
Expand Down Expand Up @@ -59,17 +66,17 @@ $(VENV_TOOLS): build ## Verify required CLI tools in venv
$(call ensure_tool_venv,$@)
endif

fmt: ruff $(MDFORMAT_ALL) ## Format sources
ruff format
ruff check --select I --fix
fmt: $(UV) $(MDFORMAT_ALL) ## Format sources
$(RUFF) format
$(RUFF) check --select I --fix
$(MDFORMAT_ALL)

check-fmt: ruff ## Verify formatting
ruff format --check
check-fmt: $(UV) ## Verify formatting
$(RUFF) format --check
# mdformat-all doesn't currently do checking

lint: ruff build $(UV) interrogate ## Run linters
ruff check
lint: build $(UV) interrogate ## Run linters
$(RUFF) check
$(UV) run interrogate --fail-under 100 lading
$(PYLINT) $(PYLINT_TARGETS)

Expand Down
67 changes: 67 additions & 0 deletions docs/adr/004-in-process-metrics-backend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# ADR-004: In-process metrics accumulator flushed at exit

## Status

Accepted.

## Date

2026-06-14

## Context and problem statement

`lading` needs to count operational events so maintainers can see how often a
release relied on a particular code path — for example, how often a crates.io
index-lookup failure was downgraded to a warning under
`--allow-unpublished-workspace-deps`, or how lockfile discovery, refresh, and
validation behaved during a `bump`.

A `lading` invocation is a short-lived command-line process, not a long-running
service. There is no scrape endpoint to expose and no daemon lifetime over
which a time series would accumulate. Adding an exporter such as
`prometheus_client` or `statsd` would introduce a runtime dependency and a
network target with no consumer, and would still need a flush at process exit
to be useful for a one-shot command. The logs a `lading` run already emits are
the established operational boundary.

## Decision

Record metrics in an in-process accumulator, `lading.utils.metrics`, and flush
them as a single structured JSON log line at interpreter exit.

- Labelled counters are recorded with `increment_counter(name, **labels)` and
duration aggregates with `observe_duration(name, seconds, **labels)`.
- `emit_summary` renders the accumulated counters and duration aggregates as
one `INFO` log line (`lading metrics summary: [...]`). Runs that record
nothing emit nothing.
- The flush is registered with `atexit` from explicit application bootstrap
(`lading.cli.main` calls `register_summary_atexit`) rather than as an
import-time side effect, so the exit-time behaviour is a visible lifecycle
decision. Registration is idempotent and sets its guard flag only after
`atexit.register` succeeds.
- The registry doubles as a deterministic test seam through `counter_value`,
`duration_stats`, `snapshot`, and `reset`.

The metric contracts are documented in the developer guide; the
`publish.index_lookup_downgrade` counter carries `subcommand` (`package` or
`publish`) and `missing_crate` labels.

Label values, including `missing_crate`, are recorded verbatim rather than
bucketed, hashed, or capped. The accumulator is process-local and flushed once,
so its cardinality is bounded by the work a single run performs (at most the
number of distinct crates in that publish run), not by an unbounded external
key space. The crate name is the actionable detail an operator needs, so
collapsing it to a bucket such as `other` would remove the metric's value
without removing a real cost.

## Consequences

- Metrics are dependency-free and add no network target; they are visible only
in the run's logs, which is where short-lived CLI output is already
aggregated.
- The verbatim `missing_crate` label is acceptable for the one-shot,
log-flushed design. If a future change exports these metrics to a long-lived
time-series backend, the cardinality decision must be revisited and label
values bucketed or aggregated at the export boundary at that time.
- New metrics should be added to the accumulator and documented in the
developer guide alongside the existing counters.
3 changes: 3 additions & 0 deletions docs/contents.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ looking for project guidance, then follow the link that matches the task.

- [ADR-003: Use three-tier Python linting][adr-003] - accepted linting policy
for Ruff, Interrogate, and Pylint.
- [ADR-004: In-process metrics accumulator flushed at exit][adr-004] - accepted
design for the `lading.utils.metrics` backend and metric contracts.

## Reference documents

Expand All @@ -35,3 +37,4 @@ looking for project guidance, then follow the link that matches the task.
spies, fixtures, and process-boundary assertions.

[adr-003]: adr/003-three-tier-python-linting.md
[adr-004]: adr/004-in-process-metrics-backend.md
56 changes: 46 additions & 10 deletions docs/developers-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,25 @@ make lint
```

The target is deliberately three-tiered. Ruff runs first because it is fast,
handles broad style and correctness checks, and imports the stricter lint policy
used by `leynos/episodic`. If Ruff passes, the target runs `interrogate` with
`--fail-under 100` across `lading` to enforce **100% docstring coverage**. If
`interrogate` passes, the final tier runs Pylint through the pinned
handles broad style and correctness checks, and imports the stricter lint
policy used by `leynos/episodic`. If Ruff passes, the target runs `interrogate`
with `--fail-under 100` across `lading` to enforce **100% docstring coverage**.
If `interrogate` passes, the final tier runs Pylint through the pinned
`pylint-pypy-shim` tool under PyPy. The final tier is focused on rule families
that complement Ruff, especially logging format safety, pattern matching checks,
selected simplification checks, deprecated standard-library usage, file hygiene,
and design-size limits. [ADR-003](adr/003-three-tier-python-linting.md) records
the policy decision.
that complement Ruff, especially logging format safety, pattern matching
checks, selected simplification checks, deprecated standard-library usage, file
hygiene, and design-size limits.
[ADR-003](adr/003-three-tier-python-linting.md) records the policy decision.

The relevant Makefile variables are:

- `RUFF_VERSION` — pinned Ruff version; defaults to `0.15.12`. Keep it in sync
with the `ruff==` dev dependency in `pyproject.toml` and the
`uv tool install ruff==` step in `.github/workflows/ci.yml`, bumping all
three together to avoid version-skew lint failures.
- `RUFF` — the pinned Ruff command
(`uv tool run --from ruff==$(RUFF_VERSION) ruff`) that the `fmt`,
`check-fmt`, and `lint` targets invoke.
- `PYLINT_PYTHON` — Python executable used by `uv tool run`; defaults to `pypy`.
- `PYLINT_TARGETS` — directories passed to Pylint; defaults to
`lading scripts tests`.
Expand Down Expand Up @@ -503,8 +510,8 @@ The index-lookup handling is split across the adapter and decision helper:
fatal. If the parsed name is not in the publish plan, the failure is fatal
with guidance to publish or index that dependency first. The helper checks
projected availability by comparing publish-order positions and raises for
out-of-plan, self, or late dependencies. If the parsed name is in the plan
and `allow_unpublished_workspace_deps` is set, the helper logs a warning and
out-of-plan, self, or late dependencies. If the parsed name is in the plan and
`allow_unpublished_workspace_deps` is set, the helper logs a warning and
continues; otherwise it raises with guidance to enable the dry-run
unpublished workspace dependency override or follow the staged-publish
workaround.
Expand Down Expand Up @@ -610,6 +617,35 @@ a formatted message that includes all four values. Using a single function for
message construction keeps the error format consistent across the packaging and
publish phases and makes snapshot testing straightforward.

### In-process metrics (`lading.utils.metrics`)

`lading.utils.metrics` is a process-local metrics accumulator. Counters are
recorded with `increment_counter(name, **labels)` and flushed as a single
structured JSON log line at interpreter exit (`emit_summary`, registered via
`atexit`); runs that record no metrics emit nothing. The module deliberately
avoids exporter dependencies such as `prometheus_client` or `statsd`: a lading
invocation is a short-lived CLI process whose logs are already aggregated, so
the summary line is the operational boundary. Tests use `counter_value`,
`duration_stats`, `snapshot`, and `reset` as deterministic seams.
[ADR-004](adr/004-in-process-metrics-backend.md) records this backend decision,
including why label values such as `missing_crate` are kept verbatim rather
than bucketed.

Defined metrics:

| Metric | Labels | Incremented when |
| -------------------------------- | ----------------------------- | --------------------------------------------------------------------------------------------------------------------- |
| `publish.index_lookup_downgrade` | `subcommand`, `missing_crate` | `_handle_index_missing_version` downgrades a crates.io index-lookup failure to a warning (in-plan, override enabled). |
| `lockfile.discovered` | (none) | Incremented by the number of tracked lockfiles each `discover_tracked_lockfiles` call returns. |
| `lockfile.refresh` | `outcome` | One increment per `refresh_lockfile` invocation; `outcome` is `success` or `failure`. |
| `lockfile.refresh.duration` | (none) | Duration observation around each `cargo generate-lockfile` invocation. |
| `lockfile.validate` | `outcome` | One increment per `validate_lockfile_freshness` call; `outcome` is `fresh`, `stale`, or `failed`. |
| `lockfile.validate.duration` | (none) | Duration observation around each `cargo metadata --locked` probe. |

Duration metrics aggregate a count and total seconds per label set via
`observe_duration` / `duration_stats` and appear in the exit summary with
`count` and `total_seconds` fields.

### Command runners (`lading.runtime`)

`lading.runtime` owns the shared `CommandRunner` protocol and the production
Expand Down
26 changes: 26 additions & 0 deletions docs/users-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ missing dependency is **not** in the publish plan, or when it appears **after**
the current crate in `publish.order`, the failure is still treated as an error.
Fix the explicit `publish.order` so foundational crates come before dependants,
or remove `publish.order` and rely on dependency-derived topological sorting.
Each such downgrade is counted and surfaced in the metrics summary emitted at
exit; see [Observability](#observability).

## Configuration reference (`lading.toml`)

Expand Down Expand Up @@ -291,6 +293,30 @@ or run the relevant Cargo command yourself before committing the bump.
`"per-crate"`. Controls how `[patch.crates-io]` is edited in the staged
workspace before packaging.

### Observability

When `lading` runs, a structured JSON summary may appear in the log output at
`INFO` level just before the process exits. The flush is process-wide — any
command can emit it — and reports whichever metrics that run recorded. For
example, a `publish` run that downgraded an index-lookup failure emits:

```plaintext
lading metrics summary: [{"metric": "publish.index_lookup_downgrade", "labels": {"missing_crate": "...", "subcommand": "..."}, "value": 1}]
```

Each entry records a counter name, the label values that identify it, and the
accumulated count for the current invocation. The summary line is omitted
entirely when no metrics were recorded (quiet runs stay quiet).
Comment thread
coderabbitai[bot] marked this conversation as resolved.

#### `publish.index_lookup_downgrade`

Incremented on each downgrade event when a crates.io index-lookup failure for a
sibling workspace dependency is downgraded to a warning because
`allow_unpublished_workspace_deps` is enabled. Labels:

- `subcommand` — the Cargo subcommand that failed (`package` or `publish`).
- `missing_crate` — the name of the workspace dependency absent from the index.

### `[preflight]`

- `test_exclude`: array of strings, default `[]`. Crate names to exclude from
Expand Down
6 changes: 5 additions & 1 deletion lading/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

from . import commands, config
from .runtime import CommandRunner, subprocess_runner
from .utils import normalise_workspace_root
from .utils import metrics, normalise_workspace_root
from .workspace import WorkspaceGraph, WorkspaceModelError, load_workspace
from .workspace import metadata as metadata_module

Expand Down Expand Up @@ -298,6 +298,10 @@ def main(argv: cabc.Sequence[str] | None = None) -> int:
if argv is None:
argv = sys.argv[1:]
_configure_logging()
# Flush the accumulated metrics summary when this CLI process exits.
# Registered here in bootstrap so the lifecycle is explicit rather than
# an import-time side effect of lading.utils.metrics.
metrics.register_summary_atexit()
workspace_override, remaining = _extract_workspace_override(list(argv))
workspace_root = normalise_workspace_root(workspace_override)
if not remaining:
Expand Down
20 changes: 20 additions & 0 deletions lading/commands/lockfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@
import collections.abc as cabc
import dataclasses as dc
import logging
import time
import typing as typ
from pathlib import Path

from lading.exceptions import LadingError
from lading.utils import metrics
from lading.utils.process import append_detail, command_detail, with_detail

if typ.TYPE_CHECKING:
Expand All @@ -43,6 +45,13 @@
LOGGER = logging.getLogger(__name__)
_ManifestExists = cabc.Callable[[Path], bool]

# Metric names (issue #91); documented in docs/developers-guide.md.
DISCOVERED_LOCKFILES_METRIC = "lockfile.discovered"
REFRESH_METRIC = "lockfile.refresh"
REFRESH_DURATION_METRIC = "lockfile.refresh.duration"
VALIDATE_METRIC = "lockfile.validate"
VALIDATE_DURATION_METRIC = "lockfile.validate.duration"


class LockfileRefreshError(LadingError):
"""Raised when Cargo cannot regenerate a lockfile."""
Expand Down Expand Up @@ -153,6 +162,10 @@ def discover_tracked_lockfiles(
if error_result is not None:
return error_result
lockfiles = _lockfiles_with_manifests(stdout, workspace_root, manifest_exists)
if lockfiles:
# Skip a zero-amount increment: it would create a 0-valued counter key
# and force an otherwise-silent exit summary (quiet runs stay quiet).
metrics.increment_counter(DISCOVERED_LOCKFILES_METRIC, amount=len(lockfiles))
LOGGER.info(
"Discovered %d tracked lockfile(s) with adjacent manifests in %s",
len(lockfiles),
Expand Down Expand Up @@ -189,13 +202,17 @@ def refresh_lockfile(
"""
lockfile_path = manifest_path.parent / "Cargo.lock"
LOGGER.info("Refreshing %s", lockfile_path)
started_at = time.perf_counter()
exit_code, stdout, stderr = runner(
("cargo", "generate-lockfile", "--manifest-path", str(manifest_path)),
cwd=manifest_path.parent,
)
metrics.observe_duration(REFRESH_DURATION_METRIC, time.perf_counter() - started_at)
if exit_code != 0:
metrics.increment_counter(REFRESH_METRIC, outcome="failure")
message = with_detail(f"Failed to refresh {lockfile_path}", stdout, stderr)
raise LockfileRefreshError(message)
metrics.increment_counter(REFRESH_METRIC, outcome="success")
LOGGER.info("Refreshed %s", lockfile_path)
return lockfile_path

Expand All @@ -221,6 +238,7 @@ def validate_lockfile_freshness(
because Cargo says it needs updating under ``--locked``, or failed for
another reason.
"""
started_at = time.perf_counter()
exit_code, stdout, stderr = runner(
(
"cargo",
Expand All @@ -232,12 +250,14 @@ def validate_lockfile_freshness(
),
cwd=manifest_path.parent,
)
metrics.observe_duration(VALIDATE_DURATION_METRIC, time.perf_counter() - started_at)
detail = command_detail(stdout, stderr)
is_fresh = exit_code == 0
is_stale = _is_lockfile_stale_detail(detail)
state = "fresh"
if not is_fresh:
state = "stale" if is_stale else "failed"
metrics.increment_counter(VALIDATE_METRIC, outcome=state)
LOGGER.info(
"Validated lockfile freshness for %s: %s",
manifest_path,
Expand Down
Loading
Loading