AI Code Review for Logging and Observability Changes: The Checklist Before a Log Line Ships

The diff looks harmless. It writes to the one system everyone can read.

Nobody treats a logging pull request as dangerous. It adds a few logger.info calls, renames a metric so it matches the new naming scheme, wraps an outbound call in a span, nudges an alert threshold that has been paging people at 3 a.m. The tests pass, because tests do not assert on log output, and the author is usually doing the team a favour. What the diff does not show is where those lines go: into a log pipeline with its own retention policy, its own access list that is wider than the database's, and an export button; into a metrics system that bills per series; into dashboards and alert rules that reference the old names and will quietly go blank. Logging is the output path with the least review and the widest audience, and that combination is why it deserves a checklist.

This post is that checklist: eight questions, the failure each one prevents, what an AI reviewer can verify from the diff and the repository, what it can only ask, and the rules that make the questions run on every observability change without anyone remembering them. It follows the shape of our checklists for caching changes and error handling, and it leans on the same principle: the risk is rarely inside the changed lines.

Question 1: Does anything secret or personal reach a log line?

This is the one that becomes an incident report. A handler logs the request “for debugging” and the request carries an Authorization header. A retry helper logs the URL and the URL has a signed token in its query string. A payment module logs the form body on validation failure and the body has a card number in it. None of these is visible as a bug in the diff; each is a line that prints an object whose contents are defined somewhere else.

The pattern that hides best is the logged exception. An error object that wraps a failed HTTP call often carries the request it was made with, headers included, and logger.error('upstream failed', err) serialises the lot. A reviewer with repository context can follow the type of what is being logged, list its fields, and flag the ones that match credential, token, password, email, phone, address or card patterns, which a diff-only reviewer cannot do because the type is defined in a file the PR did not touch. What the reviewer cannot decide is whether a field counts as personal data under the regime the team works in; the right move is to name the field on the PR and let the team answer, which is the same split we drew in the data privacy post.

Question 2: What volume will this produce at the level it is set to?

A log line inside a loop over order items, at info, on a service that handles a thousand orders a second, is a bill, and sometimes a dropped-logs incident when the pipeline back-pressures. The reviewer checks three things: the level, the call site, and whether the line is gated. A debug line that was promoted to info to make a local investigation easier is the classic case; so is a line in a per-item path that should have been a single summary line after the loop. The reviewer can see the loop and the level from the diff. It cannot see the request rate or the cost per gigabyte, so the question on the PR is “how many times per second will this fire, and is that intended?”

Question 3: Are the structured fields the ones the rest of the codebase uses?

Structured logging only pays off if the keys are consistent. A new line that writes userId where every other line writes user_id, or writes the request ID as an integer where the rest of the service writes it as a string, produces two fields in the log index that look alike and cannot be queried together. String interpolation is the worse version of the same problem: a message built with the value inside it cannot be filtered on at all. The reviewer with the repository can look up how existing lines name the same concepts and propose the existing key, which is a small comment with a long payoff.

Question 4: Can any metric label take an unbounded set of values?

A counter labelled by user_id, order_id, request path with IDs in it, or a raw error message creates a new time series for every distinct value. Metrics systems price and degrade by series count, so this is the change that takes a dashboard from instant to unusable and the metrics bill from flat to vertical, a few days after the deploy. The reviewer checks every label added or changed and asks where its value comes from; an enum, a status code or a bounded route template is fine, anything derived from user data is not.

The second half of this question is renaming. Changing http_requests_total to http_server_requests_total is correct by the naming convention and breaks every dashboard panel and alert rule that used the old name, silently, because a missing series is not an error. When dashboards and alert rules live in the repository, as they increasingly do, the reviewer can search for the old name and list what references it. When they live in a SaaS console, it can only say “this metric was renamed; check dashboards and alerts”, which is still the comment that prevents the blank panel.

Question 5: Does trace context survive the new boundary?

Tracing changes fail in one direction: the span is there and the trace is in pieces. A PR that moves work onto a queue, a goroutine, a thread pool or a background job usually drops the context at the boundary, so the consumer starts a fresh trace with no parent. A span that is started in a function with three return paths and ended on one of them leaks. An exception caught and converted to a response without being recorded on the span leaves a trace that says the request succeeded. The reviewer can see each of these in the diff: the new boundary, the missing propagation call, the early return, the catch block. What it should ask is whether the trace is supposed to continue across that boundary, because for some fire-and-forget work a linked span is the intended design.

Question 6: Can untrusted input write its own log lines?

A log line that includes a user-supplied string unescaped lets that string contain a newline followed by a convincingly formatted fake entry. In a plain-text pipeline this is log forging; in a system that parses lines back into events it can also break the parser or inject fields. Structured loggers that JSON-encode values avoid it by construction, which is another reason to prefer them over interpolation. The reviewer looks for user-controlled values reaching a formatting call, and flags the ones that are not encoded.

Question 7: Why did the threshold change, and where is the runbook?

Alert rule changes are observability changes with the highest stakes and the least scrutiny. A threshold raised from 1% to 5% error rate makes the pager quieter and may be entirely right, or may be hiding a regression nobody wants to fix. A removed alert is a decision that a failure mode no longer matters. A new alert with no runbook link is a page that the person who receives it cannot act on. The reviewer can verify the structural things: the rule has a runbook annotation, the removal is mentioned in the description, the threshold change has a stated reason. It cannot verify that the reason is good, so it should quote the old and new values and the stated reason in one comment and let the on-call owner decide.

Question 8: What does this do to sampling and retention?

Two quieter changes belong on the list. Changing a sampling rate changes what every downstream consumer of those traces sees, including the SLO dashboards that are computed from them; a rate dropped from 10% to 1% to save money also makes rare errors invisible. Changing a retention setting, or routing a new category of log to a long-retention index, interacts with question 1: personal data that was tolerable for seven days is a different matter at ninety. The reviewer can flag both from the diff; the trade-off is the team's.

Turning the checklist into rules

The eight questions are too many for a human to ask on every PR, which is the argument for encoding them as review rules that run on any change touching a logger, a metrics client, a tracer, a dashboard file or an alert rule. PURA's custom rules are written in plain language, and for this category they look like this.

• For every added or changed log call, resolve the type of each logged value and block if any field matches credential, token, password, card, email, phone or address patterns, or if a whole request, response or exception object is logged.
• Flag any info or higher log call inside a loop or a per-item code path, and any debug line promoted to info, with the fire rate as a question.
• Flag structured field keys that do not match the keys used elsewhere in the repository for the same concept, and flag values interpolated into the message instead of passed as fields.
• Block any metric label whose value is derived from user input, an identifier or a free text string; flag any metric rename or removal and list every dashboard or alert file in the repository that references the old name.
• Flag new queues, threads, goroutines or background jobs that do not propagate trace context, spans with a return path that does not end them, and caught exceptions not recorded on the active span.
• Flag user-controlled values reaching a log formatting call without encoding.
• Require a runbook link on every new alert rule, a stated reason on every threshold change, and an explicit mention of every removed alert in the PR description.
• Flag changes to sampling rates and retention routing as decisions, with the old and new values quoted.

Because PURA is self-hosted on your own infrastructure and reviews with your own OpenAI, Anthropic or Google keys, the log formats, metric names and alert rules it reads never leave your boundary, which matters more for this category than most, since the rules themselves describe what your logs contain. The flat platform fee is metered by reviews, from Starter at $39 for 100 reviews to Pro at $199 for 1,000, with budgets per team, repository and provider, so a rule that runs on every logging change costs the same whether it fires or not. The point is not to make the reviewer decide whether 5% is the right error threshold; it is to make sure that question is asked, on the PR, before the pager goes quiet.

The one-paragraph version

A logging change writes to the system with the widest audience and the least review. Review it as a data-handling change: nothing secret or personal in a line, including through exceptions and request objects; a level and call site that will not flood the pipeline; keys that match the codebase; no unbounded metric labels and no silent renames; trace context across every new boundary; no injectable input; a reason and a runbook on every alert change; and sampling and retention treated as decisions. Six of those a reviewer with repository context can verify; two it must ask. Write all eight down as rules, and the next observability PR gets the review it has never had.