-
Notifications
You must be signed in to change notification settings - Fork 135
feat(otelfiber): add fiber v3 support #1277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request updates several files to support the upgrade from Fiber v2 to Fiber v3. Function signatures and field types have been changed from using pointer receivers (e.g., *fiber.Ctx) to value receivers (fiber.Ctx). In addition, calls to context methods (e.g., switching from UserContext() to Context()) have been adjusted. New utility functions have been introduced to facilitate zero-allocation string and byte conversions, and test configurations were refined with a more explicit timeout setup. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant M as Middleware
participant H as Handler
participant U as Utils
C->>M: Send HTTP Request
M->>M: Create fiber.Ctx (value type) from request
M->>U: Optionally invoke CopyString / UnsafeBytes if needed
M->>H: Pass updated context to handler
H-->>M: Return handler response
M-->>C: Deliver HTTP Response
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
otelfiber/example/server.go (1)
37-39
: Fix commented-out code to match new API.The commented-out code for
WithSpanNameFormatter
still uses a pointer receiver (*fiber.Ctx
) which doesn't match the updated API.-//app.Use(otelfiber.Middleware(otelfiber.WithSpanNameFormatter(func(ctx *fiber.Ctx) string { +//app.Use(otelfiber.Middleware(otelfiber.WithSpanNameFormatter(func(ctx fiber.Ctx) string {otelfiber/semconv.go (1)
84-112
: Consider adding validation for empty credentials.The
HasBasicAuth
function correctly identifies and decodes basic authentication headers. However, it might be worth adding validation for empty usernames or passwords to prevent potential security issues.func HasBasicAuth(auth string) (string, bool) { if auth == "" { return "", false } // Check if the Authorization header is Basic if !strings.HasPrefix(auth, "Basic ") { return "", false } // Decode the header contents raw, err := base64.StdEncoding.DecodeString(auth[6:]) if err != nil { return "", false } // Get the credentials creds := UnsafeString(raw) // Check if the credentials are in the correct form // which is "username:password". index := strings.Index(creds, ":") if index == -1 { return "", false } // Get the username + username := creds[:index] + if username == "" { + return "", false + } + - return creds[:index], true + return username, true }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
otelfiber/example/go.mod
is excluded by!**/*.mod
otelfiber/example/go.sum
is excluded by!**/*.sum
,!**/*.sum
otelfiber/go.mod
is excluded by!**/*.mod
otelfiber/go.sum
is excluded by!**/*.sum
,!**/*.sum
📒 Files selected for processing (6)
otelfiber/config.go
(5 hunks)otelfiber/example/server.go
(2 hunks)otelfiber/fiber.go
(6 hunks)otelfiber/otelfiber_test/fiber_test.go
(17 hunks)otelfiber/semconv.go
(5 hunks)otelfiber/utils.go
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
otelfiber/fiber.go (1)
otelfiber/utils.go (1)
CopyString
(18-20)
otelfiber/semconv.go (1)
otelfiber/utils.go (3)
CopyString
(18-20)CopyBytes
(23-27)UnsafeString
(8-10)
🔇 Additional comments (22)
otelfiber/utils.go (1)
1-28
: Good implementation of utility functions for string and byte conversions.These utility functions are well-documented and provide a clean way to unbundle from Fiber v2, supporting the PR's goal of decoupling. Using
unsafe
for zero-allocation conversions is a common pattern for performance-critical code.A few observations:
UnsafeString
andUnsafeBytes
properly leverage Go's unsafe package for zero-allocation conversionsCopyString
andCopyBytes
provide safer alternatives when immutability is needed- All functions are properly documented with clear comments
otelfiber/fiber.go (6)
8-8
: Updated import to Fiber v3.The import has been correctly updated to use v3 of the Fiber framework.
82-82
: Updated middleware function signature to use value receiver instead of pointer.The function signature has been correctly updated to match Fiber v3's API change from pointer receivers to value receivers for context objects.
89-89
: Updated context access method.Changed from
c.UserContext()
toc.Context()
to match Fiber v3's API changes.
114-114
: Using newly created utility function.Switched from
utils.CopyString()
to the localCopyString()
implementation from the new utils.go file, helping to decouple from Fiber v2's utilities.
119-119
: Updated context manipulation methods.Changed from
SetUserContext()
toSetContext()
and fromUserContext()
toContext()
to align with Fiber v3's API changes.Also applies to: 151-151, 167-167
178-178
: Updated span name formatter function signature.The function signature of
defaultSpanNameFormatter
has been correctly updated to use a value receiver instead of a pointer receiver for the context object.otelfiber/config.go (3)
4-4
: Updated import to Fiber v3.The import has been correctly updated to use v3 of the Fiber framework.
13-13
: Updated config struct field types.All function type signatures in the config struct have been correctly updated to use value receivers instead of pointer receivers for the context object, aligning with Fiber v3's API changes.
Also applies to: 19-21
38-38
: Updated option function signatures.All option function signatures have been correctly updated to use value receivers instead of pointer receivers for the context object, maintaining consistency with the config struct changes.
Also applies to: 71-71, 96-96, 104-104
otelfiber/example/server.go (3)
10-10
: Updated import to Fiber v3.The import has been correctly updated to use v3 of the Fiber framework.
43-45
: Updated handler function signatures.Route handler function signatures have been correctly updated to use value receivers instead of pointer receivers for the context object, aligning with Fiber v3's API changes.
Also applies to: 47-51
49-49
: Updated context access method.Changed from
c.UserContext()
toc.Context()
to match Fiber v3's API changes.otelfiber/otelfiber_test/fiber_test.go (4)
10-13
: Successfully updated import paths for fiber v3.The imports have been appropriately updated to reflect the new versions:
- Added
time
package for the explicit timeout configurations- Changed
github.com/gofiber/contrib/otelfiber/v2
to/v3
- Changed
github.com/gofiber/fiber/v2
to/v3
This aligns with the PR objective to add fiber v3 support.
41-43
: Correctly updated handler signature from pointer to value receiver.The function signature has been updated from
func(ctx *fiber.Ctx) error
tofunc(ctx fiber.Ctx) error
, changing from a pointer receiver to a value receiver, which is consistent with Fiber v3's API changes.
176-179
: Context access method correctly updated.The change from the implicit context access to the explicit
ctx.Context()
method reflects Fiber v3's API for accessing the underlying context.
181-181
: Enhanced test configuration with explicit timeout.The test configuration has been improved by using
fiber.TestConfig{Timeout: 3 * time.Second}
instead of a hardcoded millisecond value, which is more readable and maintainable.otelfiber/semconv.go (5)
7-7
: Successfully updated import for fiber v3.The import has been updated from
github.com/gofiber/fiber/v2
github.com/gofiber/fiber/v3
.
12-33
: Function signature updated to use value receiver.The function signature has been changed from pointer to value receiver (
*fiber.Ctx
tofiber.Ctx
), which matches Fiber v3's API changes. The function body has also been updated to use direct utility functions without theutils.
prefix.
35-74
: Comprehensive update to value receivers and utility functions.This function has been correctly updated to:
- Use value receivers (
fiber.Ctx
instead of*fiber.Ctx
)- Remove
utils.
prefix from utility functions (CopyString
,CopyBytes
)- Maintain comments explaining the need for string copying (lines 38-40)
The changes maintain the original functionality while adapting to the new API.
76-82
: Updated helper function to use value receiver.The
httpFlavorAttribute
function has been properly updated to use a value receiver for consistency with the rest of the changes.
101-101
: Removed utils prefix from UnsafeString.The function call to
utils.UnsafeString
has been updated toUnsafeString
, consistent with the approach in the rest of the file where utility functions are now directly called without theutils.
prefix.
@drev74 The team is currently managing all these migrations. Fiber v3 is still under development. |
What was done
Issues:
Summary by CodeRabbit