Skip to content

refactor: move trace middleware addition to wrapHandler and update mi…#92

Merged
Suhaibinator merged 1 commit intomainfrom
middlewareOrderFix
Jul 17, 2025
Merged

refactor: move trace middleware addition to wrapHandler and update mi…#92
Suhaibinator merged 1 commit intomainfrom
middlewareOrderFix

Conversation

@Suhaibinator
Copy link
Copy Markdown
Owner

…ddleware order

@Suhaibinator Suhaibinator requested a review from Copilot July 17, 2025 06:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 the trace middleware initialization by moving it from the router constructor to the wrapHandler method and updates the middleware execution order accordingly.

  • Removes trace middleware addition from NewRouter constructor
  • Adds trace middleware creation and positioning in wrapHandler method early in the chain
  • Updates middleware order comments to reflect the new sequence

Comment thread pkg/router/router.go
// 2. Authentication (Runs early)
// 2. Trace middleware (if enabled) - positioned early so all middlewares have access to trace ID
if r.traceIDGenerator != nil {
traceMW := middleware.CreateTraceMiddleware[T, U](r.traceIDGenerator)
Copy link

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

Creating trace middleware on every request in wrapHandler could impact performance. Consider creating the middleware once during router initialization and reusing it, or document why per-request creation is necessary.

Copilot uses AI. Check for mistakes.
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.17%. Comparing base (261621e) to head (22d9c74).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #92   +/-   ##
=======================================
  Coverage   97.17%   97.17%           
=======================================
  Files          18       18           
  Lines        2369     2370    +1     
=======================================
+ Hits         2302     2303    +1     
  Misses         55       55           
  Partials       12       12           

☔ View full report in Codecov by Sentry.
📢 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.

@Suhaibinator Suhaibinator merged commit 141fbf1 into main Jul 17, 2025
11 checks passed
@Suhaibinator Suhaibinator deleted the middlewareOrderFix branch July 17, 2025 06:28
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.

2 participants