feat: Loki log backend — first non-file log source (vertical slice)#54
Open
HumanBean17 wants to merge 16 commits into
Open
feat: Loki log backend — first non-file log source (vertical slice)#54HumanBean17 wants to merge 16 commits into
HumanBean17 wants to merge 16 commits into
Conversation
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>
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
agctl logs *previously reached only local NDJSON files. This PR ships the first non-file log backend — a built-intype: lokisource — 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
LogBackendpatterns 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.py—LokiBackend(5-methodLogBackend):query_rangefetch with full HTTP error mapping; client-side filtering; two-phaseawait_one(timestamp high-water, no double-count); poll-basedfollow(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).NdjsonFileBackendnow delegates to it (−115 lines of triplicated logic, behavior-preserving — its existing suite stays green at the identical count).agctl/config/models.py—LogSourcegainsurl/query/options(backend-specific escape hatch, mirroringDatabaseConnection.options) andextra="forbid"(so a misplaced top-levelusername:/token:fails loud instead of silently breaking auth).agctl/clients/log_client.py—BUILTIN_BACKENDS = {"file": NdjsonFileBackend, "loki": LokiBackend}.pyproject.toml— newloki = ["httpx>=0.27", "jq>=1.6"]extra. httpx is lazy-imported inside methods; a missing extra surfaces asConfigError(exit 2) pointing atpip install 'agctl[loki]', never a crash.How it works
The user authors the LogQL
queryin config (server-side filtering); agctl's--level/--logger/--message/--matchapply client-side over fetched entries — identical semantics to the file backend, no fragile LogQL generation.scanreports honest two-signal truncation (matched > limitor Loki hit its fetch cap).followpollsquery_rangewith an advancingstart(no websocket dependency).tail_linesis ignored (a documented file-backend hint).Error mapping mirrors
HttpClient: 400 / non-stream result →ConfigError(2); 401/403/5xx/ConnectError→ConnectionFailure(2);ReadTimeout→OperationTimeout(1). The command layer (logs_commands.py) is unchanged — Loki is transparent behindLogClient; the one-JSON-envelope contract is preserved.Testing
log_common+LokiBackend). httpx is injected via a duck-typedhttp_clientseam, so unit tests run without httpx installed; transport-exception tests usepytest.importorskip("httpx")+MockTransport.tests/integration/test_logs_loki.py+require_loki/_start_lokiin conftest) — pushes a line to agrafana/lokicontainer 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 +LogBackendprotocol 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