Logging Middleware

[NickSavino]

logging.go has been created and injected as middleware, for each request, the following data is logged:

• Method (GET, POST, PUT, etc.)
• Request URI (e.g, api/workouts/types)
• Status Code
• Response time
• Response size in bytes
• Response class (server error, client error, redirect, success)

This will increase our observability and make debugging in production easier

[qodo-code-review]

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Implement HTTP logging middleware for observability

✨ Enhancement

Walkthroughs

Description

• Implemented logging middleware for all HTTP requests
• Captures method, URI, status code, response time, size
• Classifies responses by status (success, redirect, client/server error)
• Removed redundant logging from health check handler

Diagram

flowchart LR
  Request["HTTP Request"] -- "LoggingMiddleware" --> Handler["Handler"]
  Handler -- "Response" --> LoggingResponseWriter["LoggingResponseWriter<br/>Captures: status, bytes"]
  LoggingResponseWriter -- "Logs: method, URI,<br/>status, duration, size, class" --> Logger["Logger"]

Loading

File Changes

1. internal/http/middleware/logging.go ✨ Enhancement +75/-0

New logging middleware implementation

• Created new logging middleware package with LoggingMiddleware function
• Implemented custom loggingResponseWriter to intercept and track HTTP response metadata
• Added statusTextClass helper to classify responses by HTTP status code
• Logs request details including method, URI, status, duration, bytes written, and response class

internal/http/middleware/logging.go

---

2. internal/http/router.go ✨ Enhancement +1/-1

Integrate logging middleware into router

• Wrapped router's HTTP handler with LoggingMiddleware in Handler() method
• Middleware now intercepts all requests before reaching handlers

internal/http/router.go

---

3. internal/http/handlers/health.handler.go Miscellaneous +0/-2

Remove redundant logging from handler

• Removed redundant logger.Info() call from health check handler
• Logging now handled by middleware instead of individual handlers

internal/http/handlers/health.handler.go

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Format string re-interpreted ☑ 🐞 Bug ✓ Correctness

Description

LoggingMiddleware passes a fully-formatted message string into logger.Info/logger.Error, but the
logger treats its first argument as a format string and calls fmt.Sprintf again. If the request URI
contains '%' (common in percent-encoded paths/queries), log output will be corrupted (e.g.,
%!F(MISSING)), degrading production observability.

Code

internal/http/middleware/logging.go[R57-72]

+			msg := fmt.Sprintf(
+				"%s %s %d %s %dB class=%s",
+				r.Method,
+				r.URL.RequestURI(),
+				loggingResponseWriter.statusCode,
+				duration,
+				loggingResponseWriter.bytesWritten,
+				statusTextClass(loggingResponseWriter.statusCode),
+			)
+
+			if loggingResponseWriter.statusCode >= 500 {
+				logger.Error(msg)
+				return
+			}
+
+			logger.Info(msg)

Evidence

The middleware builds msg via fmt.Sprintf and then calls logger.Error(msg) / logger.Info(msg).
The logger contract and implementation show that Info/Error take (format string, args...) and
internally run fmt.Sprintf(format, args...), meaning msg is treated as a format string a second
time; any '%' coming from r.URL.RequestURI() can be re-parsed as a formatting verb and corrupt the
final output.

internal/http/middleware/logging.go[57-73]
internal/log/logging.go[9-47]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`LoggingMiddleware` constructs a string with `fmt.Sprintf` and then passes it as the logger’s *format string* (`logger.Info(msg)`). Since the logger implementation itself calls `fmt.Sprintf(format, args...)`, any `%` characters in the URI can be interpreted as format verbs and corrupt output.
### Issue Context
`r.URL.RequestURI()` can include percent-encoding (e.g. `%2F`, `%20`), so this can happen in normal traffic.
### Fix Focus Areas
- internal/http/middleware/logging.go[3-73]
- internal/log/logging.go[9-47]
### Suggested change
Remove the outer `fmt.Sprintf` and pass arguments directly:
- `logger.Error("%s %s %d %s %dB class=%s", ...)`
- `logger.Info("%s %s %d %s %dB class=%s", ...)`
(or if you keep `msg`, call `logger.Info("%s", msg)` / `logger.Error("%s", msg)` and drop `fmt` when unused).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. ResponseWriter interfaces dropped ☑ 🐞 Bug ⛯ Reliability

Description

The loggingResponseWriter wrapper does not forward optional http.ResponseWriter interfaces (e.g.,
http.Flusher/http.Hijacker/http.Pusher). Because the middleware is applied globally, any
handler/middleware that relies on type assertions for these interfaces will stop working once added
(common for streaming/SSE/websockets).

Code

internal/http/middleware/logging.go[R10-32]

+type loggingResponseWriter struct {
+	http.ResponseWriter
+	statusCode   int
+	bytesWritten int
+}
+
+func newLoggingResponseWriter(w http.ResponseWriter) *loggingResponseWriter {
+	return &loggingResponseWriter{
+		ResponseWriter: w,
+		statusCode:     http.StatusOK,
+	}
+}
+
+func (w *loggingResponseWriter) WriteHeader(statusCode int) {
+	w.statusCode = statusCode
+	w.ResponseWriter.WriteHeader(statusCode)
+}
+
+func (w *loggingResponseWriter) Write(b []byte) (int, error) {
+	n, err := w.ResponseWriter.Write(b)
+	w.bytesWritten += n
+	return n, err
+}

Evidence

loggingResponseWriter only embeds http.ResponseWriter and overrides WriteHeader/Write, with
no forwarding methods for other interfaces. The router applies LoggingMiddleware to the entire
mux, so all handlers receive this wrapped writer.

internal/http/middleware/logging.go[10-32]
internal/http/router.go[28-31]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`loggingResponseWriter` wraps `http.ResponseWriter` but does not preserve optional interfaces (e.g., `http.Flusher`, `http.Hijacker`, `http.Pusher`). Middleware that wraps writers should forward these when the underlying writer supports them; otherwise, future handlers that depend on these capabilities can break.
### Issue Context
This middleware is applied globally in `Router.Handler()`, so the wrapper affects all endpoints.
### Fix Focus Areas
- internal/http/middleware/logging.go[10-32]
- internal/http/router.go[28-31]
### Suggested change
Implement interface forwarding methods with conditional delegation, e.g.:
- If underlying implements `http.Flusher`, provide `Flush()`.
- If underlying implements `http.Hijacker`, provide `Hijack()`.
- If underlying implements `http.Pusher`, provide `Push()`.
Alternatively, replace the custom wrapper with a well-tested approach (e.g., a middleware utility that preserves optional interfaces) while still tracking status/bytes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more