Skip to content

fix: do not allocate log options for excluded urls#1220

Merged
ferhatelmas merged 1 commit into
masterfrom
ferhat/less-alloc-plugins
Jul 9, 2026
Merged

fix: do not allocate log options for excluded urls#1220
ferhatelmas merged 1 commit into
masterfrom
ferhat/less-alloc-plugins

Conversation

@ferhatelmas

@ferhatelmas ferhatelmas commented Jul 9, 2026

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

Refactor for performance

What is the current behavior?

  • logRequest options is allocated to be ignored for excluded urls
  • headerValidator lowercases url and allocates unnecessarily
  • httpMetrics allocates exclude prefix unnecessarily

What is the new behavior?

  • hoist exclude check in logRequest to prevent allocation
  • remove lowercasing in headerValidator
  • precompile prefix exclude array in httpMetrics

Additional context

Use performance.now (since we checking diff, not absolute) and fix unit for failed requests.
It will help minor GC.

* don't allocate in header validator, no need for lowercasing
* precompile excluded prefix for http metrics plugin

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas requested a review from a team as a code owner July 9, 2026 10:23
Copilot AI review requested due to automatic review settings July 9, 2026 10:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors request logging and metrics collection to reduce per-request allocations on excluded routes and to standardize timing measurements using monotonic clocks.

Changes:

  • Switch excluded route/url configuration from string[] to Set<string> to enable O(1) membership checks and reduce repeated allocations.
  • Hoist excluded-url checks earlier in logRequest and remove per-request option allocation for excluded endpoints.
  • Precompute excluded-route prefix checks in httpMetrics and remove lowercasing/allocation in headerValidator.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/http/plugins/metrics.ts Uses Set-based route exclusion and precomputes excluded prefixes to avoid per-request allocations.
src/http/plugins/log-request.ts Uses performance.now() and early excluded-url checks to avoid building log payloads for excluded URLs; fixes response time unit consistency.
src/http/plugins/header-validator.ts Uses Set exclusion and removes toLowerCase() allocation for URL checks.
src/app.ts Updates monitoring exclusions to a shared Set and passes it into metrics/logging/header validation plugins.
src/admin-app.ts Updates admin app log-request exclusions to use Set.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ferhatelmas ferhatelmas merged commit 007ec1c into master Jul 9, 2026
37 of 38 checks passed
@ferhatelmas ferhatelmas deleted the ferhat/less-alloc-plugins branch July 9, 2026 10:29
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 29011440956

Coverage decreased (-0.02%) to 79.237%

Details

  • Coverage decreased (-0.02%) from the base build.
  • Patch coverage: 6 uncovered changes across 2 files (14 of 20 lines covered, 70.0%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
src/http/plugins/log-request.ts 9 5 55.56%
src/http/plugins/metrics.ts 7 5 71.43%
Total (5 files) 20 14 70.0%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 12592
Covered Lines: 10423
Line Coverage: 82.77%
Relevant Branches: 7299
Covered Branches: 5338
Branch Coverage: 73.13%
Branches in Coverage %: Yes
Coverage Strength: 423.35 hits per line

💛 - Coveralls

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.

4 participants