Skip to content

Commit 68ce8af

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 64dde14 commit 68ce8af

1 file changed

Lines changed: 97 additions & 0 deletions

File tree

AGENTS.md

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,103 @@ type
189189
PosixPath('.../repo')
190190
```
191191

192+
### Logging Standards
193+
194+
These rules guide future logging changes; existing code may not yet conform.
195+
196+
#### Logger setup
197+
198+
- Use `logging.getLogger(__name__)` in every module
199+
- Add `NullHandler` in library `__init__.py` files
200+
- Never configure handlers, levels, or formatters in library code — that's the application's job
201+
202+
#### Structured context via `extra`
203+
204+
Pass structured data on every log call where useful for filtering, searching, or test assertions.
205+
206+
**Core keys** (stable, scalar, safe at any log level):
207+
208+
| Key | Type | Context |
209+
|-----|------|---------|
210+
| `vcs_cmd` | `str` | VCS command line |
211+
| `vcs_type` | `str` | VCS type (git, svn, hg) |
212+
| `vcs_url` | `str` | repository URL |
213+
| `vcs_exit_code` | `int` | VCS process exit code |
214+
| `vcs_repo_path` | `str` | local repository path |
215+
| `vcspull_config_path` | `str` | workspace config file path |
216+
217+
**Heavy/optional keys** (DEBUG only, potentially large):
218+
219+
| Key | Type | Context |
220+
|-----|------|---------|
221+
| `vcs_stdout` | `list[str]` | VCS stdout lines (truncate or cap; `%(vcs_stdout)s` produces repr) |
222+
| `vcs_stderr` | `list[str]` | VCS stderr lines (same caveats) |
223+
224+
Treat established keys as compatibility-sensitive — downstream users may build dashboards and alerts on them. Change deliberately.
225+
226+
#### Key naming rules
227+
228+
- `snake_case`, not dotted; `vcs_` prefix
229+
- Prefer stable scalars; avoid ad-hoc objects
230+
- Heavy keys (`vcs_stdout`, `vcs_stderr`) are DEBUG-only; consider companion `vcs_stdout_len` fields or hard truncation (e.g. `stdout[:100]`)
231+
232+
#### Lazy formatting
233+
234+
`logger.debug("msg %s", val)` not f-strings. Two rationales:
235+
- Deferred string interpolation: skipped entirely when level is filtered
236+
- Aggregator message template grouping: `"Running %s"` is one signature grouped ×10,000; f-strings make each line unique
237+
238+
When computing `val` itself is expensive, guard with `if logger.isEnabledFor(logging.DEBUG)`.
239+
240+
#### stacklevel for wrappers
241+
242+
Increment for each wrapper layer so `%(filename)s:%(lineno)d` and OTel `code.filepath` point to the real caller. Verify whenever call depth changes.
243+
244+
#### LoggerAdapter for persistent context
245+
246+
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+.
247+
248+
#### Log levels
249+
250+
| Level | Use for | Examples |
251+
|-------|---------|----------|
252+
| `DEBUG` | Internal mechanics, VCS I/O | VCS command + stdout, URL parsing steps |
253+
| `INFO` | Repository lifecycle, user-visible operations | Repository cloned, sync completed |
254+
| `WARNING` | Recoverable issues, deprecation, user-actionable config | Deprecated VCS option, unrecognized remote |
255+
| `ERROR` | Failures that stop an operation | VCS command failed, invalid URL |
256+
257+
Config discovery noise belongs in `DEBUG`; only surprising/user-actionable config issues → `WARNING`.
258+
259+
#### Message style
260+
261+
- Lowercase, past tense for events: `"repository cloned"`, `"vcs command failed"`
262+
- No trailing punctuation
263+
- Keep messages short; put details in `extra`, not the message string
264+
265+
#### Exception logging
266+
267+
- Use `logger.exception()` only inside `except` blocks when you are **not** re-raising
268+
- Use `logger.error(..., exc_info=True)` when you need the traceback outside an `except` block
269+
- 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
270+
271+
#### Testing logs
272+
273+
Assert on `caplog.records` attributes, not string matching on `caplog.text`:
274+
- Scope capture: `caplog.at_level(logging.DEBUG, logger="vcspull.cli")`
275+
- Filter records rather than index by position: `[r for r in caplog.records if hasattr(r, "vcs_cmd")]`
276+
- Assert on schema: `record.vcs_exit_code == 0` not `"exit code 0" in caplog.text`
277+
- `caplog.record_tuples` cannot access extra fields — always use `caplog.records`
278+
279+
#### Avoid
280+
281+
- f-strings/`.format()` in log calls
282+
- Unguarded logging in hot loops (guard with `isEnabledFor()`)
283+
- Catch-log-reraise without adding new context
284+
- `print()` for diagnostics
285+
- Logging secret env var values (log key names only)
286+
- Non-scalar ad-hoc objects in `extra`
287+
- Requiring custom `extra` fields in format strings without safe defaults (missing keys raise `KeyError`)
288+
192289
### Testing
193290

194291
**Use functional tests only**: Write tests as standalone functions (`test_*`), not classes. Avoid `class TestFoo:` groupings - use descriptive function names and file organization instead. This applies to pytest tests, not doctests.

0 commit comments

Comments
 (0)