Skip to content

feat: Loki log backend — first non-file log source (vertical slice)#54

Open
HumanBean17 wants to merge 16 commits into
mainfrom
feat/loki-log-backend
Open

feat: Loki log backend — first non-file log source (vertical slice)#54
HumanBean17 wants to merge 16 commits into
mainfrom
feat/loki-log-backend

Conversation

@HumanBean17

Copy link
Copy Markdown
Owner

Summary

agctl logs * previously reached only local NDJSON files. This PR ships the first non-file log backend — a built-in type: loki source — and the shared foundation every later backend (docker, journald, syslog, ELK, CloudWatch) will inherit.

Approach: a deliberate vertical slice (one backend, end-to-end) that hardens the LogBackend patterns once, so the remaining five inherit a proven shape rather than each re-discovering the gaps. Spec: docs/superpowers/specs/active/2026-07-22-loki-log-backend-design.md; plan: docs/superpowers/plans/active/2026-07-22-loki-log-backend-plan.md.

What's in it

  • agctl/clients/log_backends/loki.pyLokiBackend (5-method LogBackend): query_range fetch with full HTTP error mapping; client-side filtering; two-phase await_one (timestamp high-water, no double-count); poll-based follow (dedup + stop-event + transient-error tolerance).
  • agctl/clients/log_common.py (new, extracted) — shared slot-mapping normalization (normalize_dict), per-entry filtering (entry_matches), schema inference (infer_schema). NdjsonFileBackend now delegates to it (−115 lines of triplicated logic, behavior-preserving — its existing suite stays green at the identical count).
  • agctl/config/models.pyLogSource gains url / query / options (backend-specific escape hatch, mirroring DatabaseConnection.options) and extra="forbid" (so a misplaced top-level username:/token: fails loud instead of silently breaking auth).
  • agctl/clients/log_client.pyBUILTIN_BACKENDS = {"file": NdjsonFileBackend, "loki": LokiBackend}.
  • pyproject.toml — new loki = ["httpx>=0.27", "jq>=1.6"] extra. httpx is lazy-imported inside methods; a missing extra surfaces as ConfigError (exit 2) pointing at pip install 'agctl[loki]', never a crash.

How it works

The user authors the LogQL query in config (server-side filtering); agctl's --level/--logger/--message/--match apply client-side over fetched entries — identical semantics to the file backend, no fragile LogQL generation. scan reports honest two-signal truncation (matched > limit or Loki hit its fetch cap). follow polls query_range with an advancing start (no websocket dependency). tail_lines is ignored (a documented file-backend hint).

logs:
  sources:
    order-service:
      type: loki
      url: "${LOKI_URL}"
      query: '{app="order-service"}'
      service: order-service
      options:
        username: "${LOKI_USER:-}"      # basic auth (mutually exclusive with token)
        password: "${LOKI_PASSWORD:-}"
        token: "${LOKI_TOKEN:-}"        # bearer
        org_id: "${LOKI_ORG_ID:-}"      # X-Scope-OrgID
        verify_tls: true
        fetch_limit: 500                # > agctl --limit, for filtering headroom
        direction: forward

Error mapping mirrors HttpClient: 400 / non-stream result → ConfigError (2); 401/403/5xx/ConnectErrorConnectionFailure (2); ReadTimeoutOperationTimeout (1). The command layer (logs_commands.py) is unchanged — Loki is transparent behind LogClient; the one-JSON-envelope contract is preserved.

Testing

  • 1732 unit tests pass / 32 skip (full suite green throughout — file-backend refactor preserved byte-for-byte; +~50 new tests for log_common + LokiBackend). httpx is injected via a duck-typed http_client seam, so unit tests run without httpx installed; transport-exception tests use pytest.importorskip("httpx") + MockTransport.
  • Self-skipping live integration test (tests/integration/test_logs_loki.py + require_loki/_start_loki in conftest) — pushes a line to a grafana/loki container and queries it back. Skips cleanly without Docker / AGCTL_TEST_LIVE; the green path runs in CI.

Deliberately deferred (non-goals for this slice)

LogQL filter push-down · websocket /tail · mTLS/client certs · the other five backends (docker/journald/syslog/ELK/CloudWatch) · a shared remote-HTTP base class + LogBackend protocol refactor (extract after a second remote backend proves the shared surface — YAGNI).

Process

Spec → 9-task TDD plan → executed via subagent-driven-development (fresh implementer + task reviewer per task, one Important fix applied + re-reviewed, consolidated Minor cleanup). Docs synced via docs-watcher (DESIGN §2.1/§9.2, ARCHITECTURE §3/§8/§10/§11/§15, skills/agctl-config/reference/logs-sources.md).

🤖 Generated with Claude Code

HumanBean17 and others added 16 commits July 22, 2026 00:57
Adds the approved design for a built-in `type: loki` LogBackend and the shared
foundation every later backend inherits: an `options` escape hatch on LogSource
(fixing the strict-model blocker for all future backends) and a `log_common`
module (shared normalize/filter, extracted from the file backend). Client-side
filtering, poll-based follow, httpx lazy behind a new `loki` extra. No protocol
refactor; that waits for a second remote backend.

Co-Authored-By: Claude <noreply@anthropic.com>
Task-decomposed from the approved spec: extract log_common, behavior-preserving
file-backend refactor, LogSource url/query/options, LokiBackend skeleton+scan+
await_one+follow, built-in registration + loki extra, self-skipping integration
test. Sequential order respects all dependencies; unit tests run without httpx.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…o log_common

Co-Authored-By: Claude <noreply@anthropic.com>
Adds three fields to LogSource so remote log backends (Loki first) can carry
an endpoint (url), a query expression (query), and backend-specific extras
(options) — mirroring DatabaseConnection.options as the free-form escape
hatch. Existing file/journald sources stay valid (all three default empty).

Also adds model_config=ConfigDict(extra="forbid"): the model was NOT actually
strict before (Pydantic v2 default is extra="ignore"), so unknown top-level
keys were silently dropped. The brief's strictness contract requires them to
error; options remains the only place for arbitrary backend keys.

Co-Authored-By: Claude <noreply@anthropic.com>
…apping

Co-Authored-By: Claude <noreply@anthropic.com>
scan: one _fetch_entries call over [since, until] (defaults until=now,
since=until-1h), limit=fetch_limit, direction=options.direction. Applies
log_common.entry_matches client-side. truncated = (matched > limit) OR
(scanned == fetch_limit) -- two independent signals: client-side cap AND
server-cap. tail_lines accepted but ignored (file-backend hint).

sample_schema: _fetch_entries(start=now-1h, end=now, limit=sample_lines,
direction="backward") then log_common.infer_schema.

Adds _to_rfc3339_nano helper (UTC isoformat, naive treated as UTC).

Fold-in (Task 4 Minor #3): _resolve_http_client caches the lazy-built
httpx.Client on the instance for connection pooling; injected-client
path unchanged.

8 new tests (happy path, jq filter, both truncation signals, no-truncation,
default lookback, tail_lines ignored, sample_schema). Full suite
1714 passed / 32 skipped.

Co-Authored-By: Claude <noreply@anthropic.com>
…val robustness

Phase 2 of LokiBackend.await_one now fetches first (forward query_range
with start=last_ts), filters, returns on match, else sleeps and re-checks
the deadline — more responsive than the prior sleep-first sequence.
Dedup/high-water logic and deadline bounding are unchanged.

Also: normalize the high-water seed (window_start_ts) to a Z suffix so it
matches Loki's Z-suffixed entry timestamps for sound lexical comparison,
and guard poll_interval_ms == 0 via max(.,1) to avoid a busy-wait.

Tests: reworked test_await_one_poll_finds_match_after_one_sleep to a
delay-then-match scenario (first forward fetch no match, one sleep,
second forward fetch matches) so exactly-one-sleep still holds under
fetch-first; refreshed the timeout test docstring.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Add _start_loki (grafana/loki:3.0.0 on host-mapped 3100 with a 60s
log-wait strategy) and the require_loki fixture, mirroring the SR pattern.
The fixture probes <url>/ready and skips cleanly on any failure; under
AGCTL_TEST_LIVE=1 the session fixture spins Loki via testcontainers and
wires AGCTL_TEST_LOKI_URL.

test_logs_loki.py pushes one line via the Loki push API and asserts both
LogClient.scan and LogClient.await_one return the pushed entry with
message/fields parsed by normalize_dict. Self-skips on every absent-service
path (no flag, no Docker, no testcontainers) — never fails.

Co-Authored-By: Claude <noreply@anthropic.com>
Reflect the new built-in `type: loki` log backend (query_range over HTTP;
client-side filtering; poll-based follow; lazy httpx) and the LogSource
url/query/options fields (extra="forbid") at each doc's altitude:

DESIGN.md (WHAT/WHY):
- §2.1: logs schema now lists loki as a built-in alongside file, documents
  url/query/options, and adds a Loki source example.
- §9.2: built-in log backends are now file AND loki (entry point unchanged).

ARCHITECTURE.md (HOW):
- §3 module map: add clients/log_common.py + clients/log_backends/loki.py.
- §8 LogClient: BUILTIN_BACKENDS now {file, loki}; add LokiBackend paragraph
  (query_range, poll follow, two-phase await_one, error mapping, lazy httpx)
  and the shared log_common extraction note.
- §10 extension points / §11 extras table: note loki built-in + add loki row.
- §11 build: correct the logs_backends parenthetical (built-ins are hardcoded
  in BUILTIN_BACKENDS, not entry points).
- §15 known limitations: Loki poll-not-websocket, client-side filter, no mTLS,
  other backends not built-in.

skills/agctl-config/reference/logs-sources.md (OPERATIONAL):
- Recognize loki as a built-in type (not a custom plugin), add a Loki source
  example, and add gotchas (loki extra, poll-based follow, options under
  extra="forbid").

Co-Authored-By: Claude <noreply@anthropic.com>
…, cleanups

Apply the consolidated fix wave from the PR #54 whole-branch review.

Spec-required (TDD):
- validate_config: symmetric username/password auth check (a lone password
  was silently dropped by _build_auth -> confusing runtime 401; now a clean
  validate-time ConfigError).
- validate_config: enforce fetch_limit (int > 0), direction (forward/backward),
  and verify_tls (bool) per DESIGN §5.3.
- _start_loki: replace fragile log-banner wait strategy (unverified regex for
  grafana/loki:3.0.0 that silently burned the 60s budget and caused
  require_loki to skip even with AGCTL_TEST_LIVE=1) with an HTTP readiness
  gate polling /ready. Safe-skip invariant preserved; container is torn down
  on readiness timeout.

Doc-only / cleanups:
- LogSource docstring: note extra="forbid" is deliberate (auth keys are the
  highest-misplacement risk), stricter than DatabaseConnection.
- Delete dead _CONDITIONAL_SLOTS constant from log_common.
- Rename _to_rfc3339_nano -> _to_rfc3339_utc; emit Z suffix consistently so
  start/end/high-water seed agree with Loki's entry-timestamp format; drop the
  redundant per-call .replace() in await_one.
- Correct misleading timeout-test comment (forward fetch DOES run under
  fetch-first; it returns empty).
- Documenting comments: follow high-water advances on emitted entries only
  (contrast with await_one); per-request HTTP timeout not tunable via
  --timeout (v1 limitation).
- Coverage tests: _normalize_loki_line JSON-not-dict, _parse_streams
  malformed-pair skip and non-JSON-200 -> ConnectionFailure, scan direction
  plumbing, per-request timeout plumbing.

Tests: 1744 passed, 32 skipped (+12 new); integration test_logs_loki.py
still 2 skipped (no Docker in this env).

Co-Authored-By: Claude <noreply@anthropic.com>
…validation/precision nits)

Round-2 review fixes for PR #54.

Fix 1 (CRITICAL): logs tail leaked a raw traceback on Loki startup failures.
new_logs_client() (which calls validate_config) sat outside the startup try,
and _tail_run had no except around follow()'s first-fetch error propagation.
Move new_logs_client inside the startup try and wrap _tail_run with an
except-AgctlError that emits a structured logs.tail envelope + exit code
(one-envelope contract, DESIGN §10).

Fix 2 (IMPORTANT): follow advanced start only to the newest EMITTED timestamp,
silently dropping a later match sitting beyond fetch_limit when >fetch_limit
non-matching entries accumulated. Advance start to the newest FETCHED entry
each cycle (matching await_one); keep (timestamp, message) dedup as
defense-in-depth.

Fix 3 (MINOR): fetch_limit: true sneaked past validation as 1 (bool ⊆ int).
Tighten the check to reject bool.

Fix 4 (MINOR): _to_rfc3339_utc emitted microsecond precision while Loki entry
timestamps are nanosecond, breaking the lexical high-water compare. Emit
9-digit nanosecond precision.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant