Skip to content

Commit d14eb25

Browse files
committed
ai(rules[AGENTS]): Add logging standard conventions
why: Establish structured logging conventions, enabling OTel interop, pytest assertions on extra fields, and aggregator-friendly message templates. what: - Add Logging Standards section to AGENTS.md - Define vcs_ prefixed key vocabulary (core + heavy/optional) - Document lazy formatting, stacklevel, LoggerAdapter patterns - Specify log levels, message style, exception logging rules - Add testing guidance (caplog.records over caplog.text)
1 parent 741298d commit d14eb25

1 file changed

Lines changed: 96 additions & 0 deletions

File tree

AGENTS.md

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,102 @@ tests/test_cli.py
114114
- Ruff is the source of truth for lint rules; see `pyproject.toml` for enabled checks (E, F, I, UP, A, B, C4, COM, EM, Q, PTH, SIM, TRY, PERF, RUF, D, FA100).
115115
- Type checking is strict (`mypy --strict`); favor precise types and avoid `Any` unless necessary.
116116

117+
## Logging Standards
118+
119+
These rules guide future logging changes; existing code may not yet conform.
120+
121+
### Logger setup
122+
123+
- Use `logging.getLogger(__name__)` in every module
124+
- Add `NullHandler` in library `__init__.py` files
125+
- Never configure handlers, levels, or formatters in library code — that's the application's job
126+
127+
### Structured context via `extra`
128+
129+
Pass structured data on every log call where useful for filtering, searching, or test assertions.
130+
131+
**Core keys** (stable, scalar, safe at any log level):
132+
133+
| Key | Type | Context |
134+
|-----|------|---------|
135+
| `vcs_cmd` | `str` | VCS command line |
136+
| `vcs_type` | `str` | VCS type (git, svn, hg) |
137+
| `vcs_url` | `str` | repository URL |
138+
| `vcs_exit_code` | `int` | VCS process exit code |
139+
| `vcs_repo_path` | `str` | local repository path |
140+
141+
**Heavy/optional keys** (DEBUG only, potentially large):
142+
143+
| Key | Type | Context |
144+
|-----|------|---------|
145+
| `vcs_stdout` | `list[str]` | VCS stdout lines (truncate or cap; `%(vcs_stdout)s` produces repr) |
146+
| `vcs_stderr` | `list[str]` | VCS stderr lines (same caveats) |
147+
148+
Treat established keys as compatibility-sensitive — downstream users may build dashboards and alerts on them. Change deliberately.
149+
150+
### Key naming rules
151+
152+
- `snake_case`, not dotted; `vcs_` prefix
153+
- Prefer stable scalars; avoid ad-hoc objects
154+
- Heavy keys (`vcs_stdout`, `vcs_stderr`) are DEBUG-only; consider companion `vcs_stdout_len` fields or hard truncation (e.g. `stdout[:100]`)
155+
156+
### Lazy formatting
157+
158+
`logger.debug("msg %s", val)` not f-strings. Two rationales:
159+
- Deferred string interpolation: skipped entirely when level is filtered
160+
- Aggregator message template grouping: `"Running %s"` is one signature grouped ×10,000; f-strings make each line unique
161+
162+
When computing `val` itself is expensive, guard with `if logger.isEnabledFor(logging.DEBUG)`.
163+
164+
### stacklevel for wrappers
165+
166+
Increment for each wrapper layer so `%(filename)s:%(lineno)d` and OTel `code.filepath` point to the real caller. Verify whenever call depth changes.
167+
168+
### LoggerAdapter for persistent context
169+
170+
For objects with stable identity (Repository, Remote, Sync), use `LoggerAdapter` to avoid repeating the same `extra` on every call. Lead with the portable pattern (override `process()` to merge); `merge_extra=True` simplifies this on Python 3.13+.
171+
172+
### Log levels
173+
174+
| Level | Use for | Examples |
175+
|-------|---------|----------|
176+
| `DEBUG` | Internal mechanics, VCS I/O | VCS command + stdout, URL parsing steps |
177+
| `INFO` | Repository lifecycle, user-visible operations | Repository cloned, sync completed |
178+
| `WARNING` | Recoverable issues, deprecation, user-actionable config | Deprecated VCS option, unrecognized remote |
179+
| `ERROR` | Failures that stop an operation | VCS command failed, invalid URL |
180+
181+
Config discovery noise belongs in `DEBUG`; only surprising/user-actionable config issues → `WARNING`.
182+
183+
### Message style
184+
185+
- Lowercase, past tense for events: `"repository cloned"`, `"vcs command failed"`
186+
- No trailing punctuation
187+
- Keep messages short; put details in `extra`, not the message string
188+
189+
### Exception logging
190+
191+
- Use `logger.exception()` only inside `except` blocks when you are **not** re-raising
192+
- Use `logger.error(..., exc_info=True)` when you need the traceback outside an `except` block
193+
- Avoid `logger.exception()` followed by `raise` — this duplicates the traceback. Either add context via `extra` that would otherwise be lost, or let the exception propagate
194+
195+
### Testing logs
196+
197+
Assert on `caplog.records` attributes, not string matching on `caplog.text`:
198+
- Scope capture: `caplog.at_level(logging.DEBUG, logger="g.cli")`
199+
- Filter records rather than index by position: `[r for r in caplog.records if hasattr(r, "vcs_cmd")]`
200+
- Assert on schema: `record.vcs_exit_code == 0` not `"exit code 0" in caplog.text`
201+
- `caplog.record_tuples` cannot access extra fields — always use `caplog.records`
202+
203+
### Avoid
204+
205+
- f-strings/`.format()` in log calls
206+
- Unguarded logging in hot loops (guard with `isEnabledFor()`)
207+
- Catch-log-reraise without adding new context
208+
- `print()` for diagnostics
209+
- Logging secret env var values (log key names only)
210+
- Non-scalar ad-hoc objects in `extra`
211+
- Requiring custom `extra` fields in format strings without safe defaults (missing keys raise `KeyError`)
212+
117213
## Doctests
118214

119215
**All functions and methods MUST have working doctests.** Doctests serve as both documentation and tests.

0 commit comments

Comments
 (0)