Skip to content

feat(middleware): skip /readyz by default and add WithExcludedRoutes#10

Merged
brunognovaes merged 2 commits into
developfrom
fix/remove-readyz-logging
May 19, 2026
Merged

feat(middleware): skip /readyz by default and add WithExcludedRoutes#10
brunognovaes merged 2 commits into
developfrom
fix/remove-readyz-logging

Conversation

@brunognovaes
Copy link
Copy Markdown

@brunognovaes brunognovaes commented May 19, 2026

Lerian

Lib Observability


Description

Type of Change

  • feat: New feature or capability
  • fix: Bug fix
  • perf: Performance improvement
  • refactor: Internal restructuring with no behavior change
  • docs: Documentation only (README, docs/, inline comments)
  • style: Formatting, whitespace, naming (no logic change)
  • test: Adding or updating tests
  • ci: CI pipeline or workflow changes
  • build: Build system, Go module dependencies
  • chore: Maintenance, config, tooling
  • revert: Reverts a previous commit
  • BREAKING CHANGE: Consumers must update their integration

Breaking Changes

None.

Testing

  • make test passes
  • make test-int passes if integration paths are exercised
  • make lint passes
  • Coverage threshold respected (see Go Combined Analysis)

Test evidence / Actions run:

Architectural Checklist

  • No panic() in production paths — uses assert helpers or wrapped errors
  • Timestamps use time.Now().UTC()
  • Errors wrapped with %w
  • Public API additions documented (godoc + README if user-facing)
  • No leaking of context or goroutines (instrumentation must be safe under load)
  • Backwards-compatible by default; breaking changes flagged above
  • No lib-commons import (circular — see CLAUDE.md)

Related Issues

Closes #

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

Review Change Stack

Walkthrough

Adds configurable, prefix-based excluded routes to HTTP logging (defaults: /health, /readyz, /metrics), enforces segment-boundary matching, integrates an exported WithExcludedRoutes option, uses the exclusion list for early-return in the middleware, and adds unit tests validating behavior.

Changes

Access-log route exclusion feature

Layer / File(s) Summary
Route exclusion configuration and option
middleware/logging.go
Adds defaultLogExcludedRoutes (/health, /readyz, /metrics), a ExcludedRoutes []string field, and WithExcludedRoutes(...string) option; buildOpts initializes exclusions from defaults.
Segment-boundary matching helper
middleware/helpers.go, middleware/helpers_test.go
isRouteExcludedFromList now trims trailing slashes, ignores empty entries, and matches either exact route or route + "/" to enforce path-segment boundaries; new unit test TestIsRouteExcludedFromList.
Middleware early-exit integration
middleware/logging.go
WithHTTPLogging constructs middleware options once and uses isRouteExcludedFromList(c, mid.ExcludedRoutes) for early-return exclusion; redundant options rebuild removed.
Logging behavior tests
middleware/logging_test.go
Adds tests covering default probe-path suppression, custom excluded-prefix suppression, preservation of defaults when options overlap, and ignoring empty exclusion strings.

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


Comment @coderabbitai help to get the list of available commands and usage tips.

@lerian-studio lerian-studio added size/S PR changes 50–199 lines middleware HTTP/gRPC observability middleware tests Unit, integration and end-to-end tests labels May 19, 2026
@lerian-studio
Copy link
Copy Markdown
Contributor

lerian-studio commented May 19, 2026

🔒 Security Scan Results — lib-observability

✅ PR Mergeable — no blocking findings

Stage Status Blocking?
Filesystem Scan ✅ Clean
Docker Image Scan ➖ Skipped
Docker Hub Health Score ➖ Skipped
Pre-release Version Check ✅ Clean

Trivy

Filesystem Scan

✅ No vulnerabilities or secrets found.


Pre-release Version Check

✅ No unstable version pins found.


🔍 View full scan logs

@lerian-studio
Copy link
Copy Markdown
Contributor

lerian-studio commented May 19, 2026

🔍 PR Validation Summary

✅ PR Mergeable — no blocking failures

Check Status Blocking
Source Branch ✅ success yes
PR Title ✅ success yes
PR Description ✅ success yes
PR Size ✅ success no
Auto Labels ✅ success no
PR Metadata ✅ success no

🔍 View workflow run

@lerian-studio
Copy link
Copy Markdown
Contributor

lerian-studio commented May 19, 2026

📊 Unit Test Coverage Report: lib-observability

Metric Value
Overall Coverage 84.4% ✅ PASS
Threshold 80%

Coverage by Package

Package Coverage
github.com/LerianStudio/lib-observability/assert 97.9%
github.com/LerianStudio/lib-observability/constants 83.3%
github.com/LerianStudio/lib-observability/log 94.9%
github.com/LerianStudio/lib-observability/metrics 91.4%
github.com/LerianStudio/lib-observability/middleware 68.4%
github.com/LerianStudio/lib-observability/redaction 95.8%
github.com/LerianStudio/lib-observability/runtime 80.4%
github.com/LerianStudio/lib-observability/tracing 84.9%
github.com/LerianStudio/lib-observability/zap 96.0%
github.com/LerianStudio/lib-observability 91.1%

Generated by Go PR Analysis workflow

@lerian-studio lerian-studio added size/M PR changes 200–499 lines and removed size/S PR changes 50–199 lines labels May 19, 2026
@brunognovaes brunognovaes force-pushed the fix/remove-readyz-logging branch from 4eab6b2 to 5f3e048 Compare May 19, 2026 12:13
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@middleware/logging.go`:
- Around line 218-220: The early return when isRouteExcludedFromList(c,
mid.ExcludedRoutes) causes excluded routes to skip request-context setup
(setRequestHeaderID and logger context), so change the flow in the middleware to
always run setRequestHeaderID(c) and wire the request-scoped logger before
checking exclusion; then use isRouteExcludedFromList only to skip emitting
access logs (i.e., call c.Next() after context setup but avoid the access-log
path for excluded routes). Ensure WithExcludedRoutes still references
mid.ExcludedRoutes and the exclusion check happens after request ID/header and
logger are installed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d4eb1489-08a5-4024-aa32-40e338f5071c

📥 Commits

Reviewing files that changed from the base of the PR and between f43c51d and 4eab6b2.

📒 Files selected for processing (4)
  • middleware/helpers.go
  • middleware/helpers_test.go
  • middleware/logging.go
  • middleware/logging_test.go

Comment thread middleware/logging.go
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
middleware/logging.go (1)

218-220: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Excluded-route early return skips request context setup.

At Line 218, return c.Next() runs before setRequestHeaderID and logger context wiring, so WithExcludedRoutes(...) suppresses more than access logs (excluded paths lose request ID/header + request-scoped logger).

Proposed fix
 func WithHTTPLogging(opts ...LogMiddlewareOption) fiber.Handler {
 	return func(c *fiber.Ctx) error {
 		mid := buildOpts(opts...)
 
+		setRequestHeaderID(c)
+		requestID := c.Get(headerID)
+		logger := mid.Logger.
+			With(obslog.String(headerID, requestID)).
+			With(obslog.String("message_prefix", requestID+constant.LoggerDefaultSeparator))
+		c.SetUserContext(observability.ContextWithLogger(c.UserContext(), logger))
+
 		if isRouteExcludedFromList(c, mid.ExcludedRoutes) {
 			return c.Next()
 		}
 
 		if strings.Contains(c.Path(), "swagger") && c.Path() != "/swagger/index.html" {
 			return c.Next()
 		}
-
-		setRequestHeaderID(c)
 
 		info := NewRequestInfo(c, mid.ObfuscationDisabled)
-
-		requestID := c.Get(headerID)
-		logger := mid.Logger.
-			With(obslog.String(headerID, info.TraceID)).
-			With(obslog.String("message_prefix", requestID+constant.LoggerDefaultSeparator))
-
-		ctx := observability.ContextWithLogger(c.UserContext(), logger)
-		c.SetUserContext(ctx)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@middleware/logging.go` around lines 218 - 220, The early return when an
excluded route is detected (call to isRouteExcludedFromList with
mid.ExcludedRoutes) bypasses request setup — move the request-scoped setup
(call(s) to setRequestHeaderID and the logger context wiring) so they always run
before returning to c.Next(), or alternatively, perform setRequestHeaderID and
attach the request-scoped logger even for excluded routes before executing the
early return; update the logic in the middleware handling excluded routes to
ensure setRequestHeaderID and the request logger are invoked for all requests
while preserving suppression of access logs for excluded paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@middleware/logging.go`:
- Around line 218-220: The early return when an excluded route is detected (call
to isRouteExcludedFromList with mid.ExcludedRoutes) bypasses request setup —
move the request-scoped setup (call(s) to setRequestHeaderID and the logger
context wiring) so they always run before returning to c.Next(), or
alternatively, perform setRequestHeaderID and attach the request-scoped logger
even for excluded routes before executing the early return; update the logic in
the middleware handling excluded routes to ensure setRequestHeaderID and the
request logger are invoked for all requests while preserving suppression of
access logs for excluded paths.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: bebc4ed3-ecbd-451a-9711-2534b8112a1b

📥 Commits

Reviewing files that changed from the base of the PR and between 4eab6b2 and 5f3e048.

📒 Files selected for processing (4)
  • middleware/helpers.go
  • middleware/helpers_test.go
  • middleware/logging.go
  • middleware/logging_test.go

@brunognovaes brunognovaes merged commit bfa4277 into develop May 19, 2026
21 checks passed
@brunognovaes brunognovaes deleted the fix/remove-readyz-logging branch May 19, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

middleware HTTP/gRPC observability middleware size/M PR changes 200–499 lines tests Unit, integration and end-to-end tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants