Skip to content

perf: cut request ingress and library logging overhead - #140

Merged
Suhaibinator merged 2 commits into
mainfrom
perf/request-hot-path
Sep 24, 2026
Merged

Suhaibinator merged 2 commits into
mainfrom
perf/request-hot-path

Conversation

@Suhaibinator

Copy link
Copy Markdown
Owner

Summary

Four localized hot-path optimizations. No public API changes.

Change Where Effect
Early level check + cached library logger internal/requestlog, pkg/scontext/logger.go, new internal/loghook requestlog.Check rejected disabled levels only after GetLogger derived the correlated logger (JSON core clone + field encoding). It now checks the source's base core first and caches the Named("SRouter").WithOptions(AddCallerSkip(1)) child against the logger version. The per-request Debug "Authentication successful" record no longer derives anything when Debug is off.
One request copy at ingress pkg/router/router.go withRuntimeIdentities and withRequestLogging each called req.WithContext; they now thread one context and copy the request once.
IP normalization fast path pkg/scontext/client_ip.go Skips net.SplitHostPort for inputs it always rejects (bare IPv4/IPv6, bracketed IPv6 without port), avoiding its error allocation, and slices [host] instead of concatenating. All five common shapes are now 0 allocs.
Nil flags on clone pkg/scontext/context.go cloneSRouterContext no longer allocates an empty flags map; WithFlag allocates lazily as for new contexts.

Why internal/loghook

requestlog imports scontext, so it can't reach the unexported cache. Exporting library-only helpers would widen the scontext API, so scontext sets a hook variable during init instead.

Semantics

  • The early check assumes a core's With does not enable levels its base core disables. This holds for Zap's cores and wrappers (sampler, IncreaseLevel, tee), and is documented in docs/logging.md. The derived core's own Check still runs, so narrowing cores behave as before.
  • The library logger cache is published only when it is a child of the currently cached request logger, so a snapshot from a raced derivation is used once and dropped.
  • cleanClientIP is fuzz-tested against the original implementation (FuzzCleanClientIPMatchesReference, 8.3M execs locally, no mismatches).
  • Rate-limit composite keys were considered and left alone: limiters is a sync.Map, so a struct key would be boxed per lookup and likely not beat the string concat.

Benchmark

New BenchmarkProductionAuthRoute: build/config IDs, X-Real-IP client IP, trace IDs, auth-required route, Info-level JSON logger.

                      before        after        delta
sec/op                1145n ±11%    660.7n ±5%   -42.30% (p=0.002 n=6)
B/op                  2766          824          -70.21%
allocs/op             18            10           -44.44%

Test plan

  • go test -race ./...
  • go vet ./..., golangci-lint run (0 issues), go build ./examples/..., go test ./internal/doccheck
  • New tests: disabled cold path does no derivation, enabled warm path makes 0 allocs (skipped under -race, where sync.Pool drops items), library logger follows correlation writes and ClearRequestLogger, cleanClientIP makes 0 allocs, nil/empty flags stay nil on clone and still accept later writes

🤖 Generated with Claude Code

- Check library log levels against the source's base core before deriving
  the correlated logger, and cache the "SRouter" child per logger version.
  Disabled records such as the per-request Debug "Authentication successful"
  no longer encode correlation fields.
- Install runtime identities, the log source, and client info with a single
  http.Request copy instead of two.
- Skip net.SplitHostPort for inputs it always rejects (portless IPv4/IPv6,
  bracketed IPv6 without a port), avoiding its error allocation, and slice
  bracketed IPv6 hosts instead of concatenating.
- Leave cloned flags nil when the source has none.

BenchmarkProductionAuthRoute: 1145ns -> 661ns, 2766B -> 824B, 18 -> 10 allocs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.92%. Comparing base (f384154) to head (e8ddd3e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #140   +/-   ##
=======================================
  Coverage   99.92%   99.92%           
=======================================
  Files          26       26           
  Lines        2604     2629   +25     
=======================================
+ Hits         2602     2627   +25     
  Misses          2        2           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Suhaibinator
Suhaibinator merged commit f3bdd70 into main Sep 24, 2026
11 checks passed
@Suhaibinator
Suhaibinator deleted the perf/request-hot-path branch September 24, 2026 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant