🐛 [Bug]: logger locals:requestid not working #3441
Conversation
WalkthroughThe change updates the logger middleware to support the extraction of the request ID when using the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FiberApp
participant LoggerMiddleware
participant RequestIDMiddleware
Client->>FiberApp: Sends HTTP request
FiberApp->>RequestIDMiddleware: Process request
RequestIDMiddleware->>FiberApp: Attach request ID to context
FiberApp->>LoggerMiddleware: Pass context
LoggerMiddleware->>LoggerMiddleware: Check for locals:requestid tag
LoggerMiddleware->>RequestIDMiddleware: Retrieve request ID from context
LoggerMiddleware->>FiberApp: Log request with request ID
FiberApp->>Client: Send response
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3441 +/- ##
==========================================
+ Coverage 83.85% 83.94% +0.09%
==========================================
Files 119 119
Lines 11897 11900 +3
==========================================
+ Hits 9976 9990 +14
+ Misses 1490 1482 -8
+ Partials 431 428 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
brunodmartins
force-pushed
the
gh-3436
branch
2 times, most recently
from
499ddcc to
8d18b42
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🛑 Comments failed to post (1)
middleware/logger/tags.go (1)
5-5:
⚠️ Potential issueFix the import formatting to resolve the golangci-lint error.
The import statement for the requestid middleware is not properly formatted according to gofumpt, as indicated by the static analysis and pipeline failures. Go imports should be organized into groups (standard library first, then third-party packages).
Fix the import formatting by running:
go fmt ./... # or preferably gofumpt -l -w .🧰 Tools
🪛 GitHub Check: lint
[failure] 5-5:
File is not properly formatted (gofumpt)🪛 GitHub Actions: golangci-lint
[error] 5-5: File is not properly formatted (gofumpt)
|
I made a first version of the PR changing the I would prefer to the first version which could cause collision. But I am opened to suggestions and reviews :) |
|
@sixcolors we use the pattern with the inaccessible context keys with some middlewares since we only have string possibilities in the logger pattern, the question is how we solve this cleanly |
|
Hm, did CustomTags is always an option too :) |
There was a problem hiding this comment.
Closes #3436
This is expected behavior and doesn’t require any changes to the logger.
As noted in the referenced issue #2684, the reason Locals("requestid") no longer works in v3 is because Fiber middlewares—like requestid—follow Go’s context best practices, which state:
“The provided key must be comparable and should not be of type string or any other built-in type to avoid collisions between packages using context. Users of WithValue should define their own types for keys.”
To comply, middleware values are stored in the context using unexported keys of custom types rather than strings. As a result, values like the request ID aren’t accessible directly via Locals with a string key.
Recommended: use CustomTags
For most users, the cleanest and most maintainable way to include middleware values in your logs is via the CustomTags option:
app.Use(logger.New(logger.Config{
CustomTags: map[string]logger.LogFunc{
"requestid": func(out logger.Buffer, c fiber.Ctx, _ *logger.Data, _ string) (int, error) {
return out.WriteString(requestid.FromContext(c))
},
},
Format: "[${time}] ${ip} ${method} ${path} ${status} - requestid=${requestid}\n",
}))- Keeps logger decoupled from other middleware
- No need to touch
Localsat all - Fully configurable naming, formatting, and behaviour
General case: use Locals
If you prefer or already have a pattern that accesses values from Locals, you can manually copy the context value into Locals:
app.Use(func(c fiber.Ctx) error {
c.Locals("requestid", requestid.FromContext(c))
return c.Next()
})- Works with existing
Locals-based logging - Gives you control over when and how values get set
Both approaches follow Go’s best practices and keep your logger free of unsafe assumptions or hidden dependencies. Feel free to choose whichever fits your project style!
|
Let's close this in favor of #3446 |
|
@brunodmartins Thanks! We are closing this in favor of #3446 which improves the documentation with the correct way of showing a |
5 participants
Description
Fixes #3436.
Changing the log format tags to read the
requestidfrom the context, since the key values fromLocalsAPI was changed due to the keys collisions. See more on #2684Type of change
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/directory for Fiber's documentation.Commit formatting
Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md