How to Review Pull Requests That Touch Logging and Observability Code

A thorough code review of logging and observability changes takes about 30 to 45 minutes when you follow a consistent order: read the baseline first, audit every log call for level, content, and sensitive data, then check traces and metrics before writing comments. Most reviewers skip the baseline step, which is why logging bugs survive review.

Picture a PR that adds structured logging to a payments service. The diff looks clean: a few log.info calls, a new span, a counter. But without knowing the existing log format, the production log level, and what fields the team correlates on, you can't tell whether those changes help or quietly break the on-call dashboard. That's the scenario these steps are built for.

Before you start, pull up the PR, the service's logging config, and your team's log aggregator in separate tabs. You don't need special tools, just access to the running system.

1. Read the logging context before you read the diff

A diff makes no sense without a baseline. Before scanning a single changed line, identify what the service already logs, at what levels, and where those logs go.

Pull up the log aggregator your team uses, whether that's Datadog, Splunk, CloudWatch, or something else, and search for the service name to confirm what's in production right now. Then open the logging config file: log4j2.xml, logging.yml, appsettings.json, or the equivalent. Check which levels are enabled per environment. A service that suppresses debug in production behaves differently from one that doesn't, and the PR may assume one or the other.

Note whether the codebase uses structured logging (JSON key-value fields) or plain-text messages. The two have different failure modes: plain-text logs break grep-based alerting when message wording changes; structured logs break correlation queries when field names drift.

You're done with this step when you can answer three questions: what format are logs emitted in, where do they go, and are debug logs suppressed in production?

2. Check every new log call for level, content, and context

Each new or changed log statement needs the right severity level, a message that's useful without a debugger, and enough structured fields to filter on later.

2a. Verify severity levels are accurate

A log.debug call that fires on every HTTP request in production will flood storage and mask real errors. Confirm that debug and trace calls are gated by environment or feature flag. info should mark meaningful state transitions, not internal loop iterations. warn and error should map to conditions that actually require attention.

2b. Confirm log messages carry enough context

A message like user action failed is unactionable at 2 a.m. Flag any log that omits the entity ID, operation name, or error type an on-call engineer would need to diagnose the problem without opening a debugger. The test is simple: could a new team member understand what happened from the message alone?

2c. Check structured fields for consistency

If the rest of the codebase uses a request_id field, a new log that spells it requestId breaks correlation queries across services. Name mismatches are easy to miss in a diff because the field name looks plausible in isolation. Cross-check new field names against one or two existing log statements in the same service.

3. Spot the security and privacy risks

Logging code is one of the most common places sensitive data leaks into plain-text log files. Catching it is part of doing a code review correctly on this kind of change.

Scan every log statement for parameters, request bodies, or objects that could contain passwords, tokens, PII, or payment data. Logging a full user object is a classic mistake, often introduced when a developer adds a quick debug log and forgets to remove it. Check that error handlers don't log raw exception messages that expose stack traces, internal paths, or database schema details to any log sink readable outside the team.

If the PR touches a log serializer or a custom formatter, treat it with the same scrutiny you'd give an auth change. A serializer that calls toString() on arbitrary objects can expose fields the author never intended to include. Ask yourself: what's the worst object that could flow through this code path?

You're done with this step when no new log statement writes a value the team hasn't explicitly decided is safe to store.

4. Review tracing and metrics instrumentation

Observability isn't only logs. If the PR adds or changes spans, traces, or metrics, those need the same line-by-line scrutiny as the log calls.

4a. Check span boundaries and naming

A span that wraps an entire HTTP handler hides where latency actually lives. Look for spans that are too coarse to be useful, and flag any span name that doesn't follow the team's existing convention, whether that's HTTP verb patterns, service.operation naming, or something else. Inconsistent span names break trace queries in tools like Jaeger, Tempo, or the APM layer in Datadog.

4b. Verify metric cardinality

A counter or histogram that uses a user ID or URL path as a label dimension creates unbounded cardinality. This can crash a self-hosted Prometheus instance or spike costs in a hosted metrics service. Confirm that every label value comes from a fixed, small set: HTTP status codes, operation names, boolean flags. If the PR introduces a label that could take thousands of values, flag it as a blocking issue.

4c. Confirm error rates are tracked

Any new code path that can fail should increment an error counter or set a span status to error. A PR that adds a retry loop with no failure metric is leaving a blind spot in the dashboard. Check that the happy path and the failure path both have coverage, not just the path the author was focused on.

5. Test your conclusions against the running service

A review comment is stronger when you can point to what a real log or trace will look like, not just what the code implies.

If you have access to a staging environment, reproduce the changed code path and check the log aggregator output against what you predicted from the diff. For metrics changes, open the existing dashboard for the service and confirm the new metric or label fits cleanly, or flag that a dashboard update is missing from the PR.

If you can't reproduce locally, ask the author to attach sample log output or a screenshot of the trace in the PR description. That's a reasonable request and saves a round of back-and-forth after merge.

You're done with this step when your comments reference observable evidence, not just code-level inference.

6. Write comments that are specific and actionable

A comment that says "this log looks wrong" blocks the author without helping them. Every review comment on logging code should name the exact risk and suggest a fix.

Use this format: what the current code does, why that's a problem in production, and one concrete alternative. Three sentences covers most cases. Keep that structure in mind whether you're flagging a token logged in plaintext or a span name that breaks an existing query.

Separate blocking issues from suggestions so the author knows what must change before merge. A token logged in plaintext is blocking. A log message that could be more descriptive is a suggestion. Mixing the two wastes everyone's time.

If the PR is for a team with a shared logging library or a wrapper around a third-party SDK, link to the internal docs in your comment rather than restating conventions from memory. It's more reliable, and it trains the author to find the source of truth themselves.

For more on what separates a comment that moves a PR forward from one that just registers an opinion, this breakdown of what makes a code review comment actionable covers the pattern in detail.

Frequently asked questions

Does the same process apply when reviewing a PR that adds a new external dependency for logging?

Mostly yes, but external integrations add a layer. When a PR swaps in a new logging SDK or adds an observability vendor's agent, check the vendor's data retention and export policies before approving, and confirm the new dependency doesn't change what fields get sent off-premise. The steps for reviewing a PR that introduces a new external API integration walk through that additional layer.

How do I review logging code in a stack I'm not familiar with?

Start with the config file and the aggregator output rather than the syntax. The risks, wrong severity levels, sensitive field exposure, cardinality problems, are the same across stacks. What changes is the library API and the config format. Read the library's docs for the log levels it defines, then apply the same checklist.

What's the fastest way to get better at catching logging bugs in review?

Repetition on real pull requests beats reading about it. Reviewing the same class of bug across multiple codebases builds the pattern recognition that makes these checks fast. Goodcatch's stack-specific tracks include logging and observability scenarios graded against what senior engineers actually catch, which gives you feedback on what you missed rather than just what you flagged.

Start practising code reviews on Goodcatch, free, no account required.