upgrade otelfiber to fiber v3 #1163
Conversation
heisGarvit
requested review from
gaby,
sixcolors,
ReneWerner87 and
efectn
and removed request for
a team
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Middleware
Client->>Server: Request
Server->>Middleware: Pass context
Middleware-->>Server: Process context
Server-->>Client: Response
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
otelfiber/example/server.go (1)
37-37: Update middleware span name formatter usage.The commented-out code shows an example of customizing span names with the new
fiber.Ctxtype. Ensure any custom middleware logic aligns with Fiber v3.- //app.Use(otelfiber.Middleware(otelfiber.WithSpanNameFormatter(func(ctx fiber.Ctx) string { - // return fmt.Sprintf("%s - %s", ctx.Method(), ctx.Route().Path) - //}))) + // Example: Customize span name with Fiber v3 + // app.Use(otelfiber.Middleware(otelfiber.WithSpanNameFormatter(func(ctx fiber.Ctx) string { + // return fmt.Sprintf("%s - %s", ctx.Method(), ctx.Route().Path) + // })))otelfiber/README.md (3)
17-17: Use a heading instead of emphasis for the note.The note about the Go version requirement should use a heading for better visibility and structure.
- **Note: Requires Go 1.21 and above** + ### Note + Requires Go 1.21 and aboveTools
Markdownlint
17-17: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
23-23: Specify the language for the code block.The installation command should specify the language for syntax highlighting.
- ``` + ```shellTools
Markdownlint
23-23: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
39-39: Clarify the description forWithNext.The description for the
WithNextfunction should clarify the context of "returned true."- Define a function to skip this middleware when returned true . + Define a function to skip this middleware when it returns true.Tools
LanguageTool
[uncategorized] ~39-~39: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...a function to skip this middleware when returned true .| nil ...(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (4)
otelfiber/example/go.modis excluded by!**/*.modotelfiber/example/go.sumis excluded by!**/*.sum,!**/*.sumotelfiber/go.modis excluded by!**/*.modotelfiber/go.sumis excluded by!**/*.sum,!**/*.sum
Files selected for processing (7)
- otelfiber/README.md (4 hunks)
- otelfiber/config.go (5 hunks)
- otelfiber/doc.go (1 hunks)
- otelfiber/example/server.go (3 hunks)
- otelfiber/fiber.go (4 hunks)
- otelfiber/otelfiber_test/fiber_test.go (18 hunks)
- otelfiber/semconv.go (3 hunks)
Files skipped from review due to trivial changes (1)
- otelfiber/doc.go
Additional context used
Markdownlint
otelfiber/README.md
17-17: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
23-23: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
66-66: Column: 1
Hard tabs(MD010, no-hard-tabs)
68-68: Column: 1
Hard tabs(MD010, no-hard-tabs)
69-69: Column: 1
Hard tabs(MD010, no-hard-tabs)
70-70: Column: 1
Hard tabs(MD010, no-hard-tabs)
71-71: Column: 1
Hard tabs(MD010, no-hard-tabs)
73-73: Column: 1
Hard tabs(MD010, no-hard-tabs)
74-74: Column: 1
Hard tabs(MD010, no-hard-tabs)
75-75: Column: 1
Hard tabs(MD010, no-hard-tabs)
76-76: Column: 1
Hard tabs(MD010, no-hard-tabs)
94-94: Column: 1
Hard tabs(MD010, no-hard-tabs)
95-95: Column: 1
Hard tabs(MD010, no-hard-tabs)
96-96: Column: 1
Hard tabs(MD010, no-hard-tabs)
98-98: Column: 1
Hard tabs(MD010, no-hard-tabs)
LanguageTool
otelfiber/README.md
[uncategorized] ~39-~39: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...a function to skip this middleware when returned true .| nil ...(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
Additional comments not posted (24)
otelfiber/example/server.go (5)
10-12: Update import paths for Fiber v3.The import paths have been updated to use Fiber v3 and its associated otelfiber middleware. Ensure that all references in the code align with these new import paths.
20-20: Update OpenTelemetry semantic conventions.The semantic version of OpenTelemetry conventions has been updated to
v1.12.0. Ensure compatibility with this version throughout the codebase.
43-43: Update handler function signature.The handler function now uses
fiber.Ctxas a value receiver. Ensure all handler functions are updated accordingly.
47-47: Update handler function signature and JSON response.The handler function now uses
fiber.Ctxas a value receiver. Also, ensure that the JSON response correctly maps keys to values.
68-68: Update service name in tracer provider.The service name in the tracer provider has been updated to
"my-service-fiber-v3". Ensure this change is consistent with the application's service naming conventions.otelfiber/semconv.go (6)
7-7: Update import path for Fiber v3.The import path for Fiber has been updated to version 3. Ensure all references in the code align with this new import path.
13-13: Transition to value receiver forfiber.Ctx.The function
httpServerMetricAttributesFromRequestnow usesfiber.Ctxas a value receiver. Verify that this change does not affect the intended behavior of the function.
17-17: Update method call toc.Scheme().The method call has been updated from
c.Protocol()toc.Scheme(). Ensure this change aligns with the desired functionality and the new Fiber v3 API.
36-36: Transition to value receiver forfiber.Ctx.The function
httpServerTraceAttributesFromRequestnow usesfiber.Ctxas a value receiver. Verify that this change does not affect the intended behavior of the function.
44-44: Update method call toc.Scheme().The method call has been updated from
c.Protocol()toc.Scheme(). Ensure this change aligns with the desired functionality and the new Fiber v3 API.
77-77: Transition to value receiver forfiber.Ctx.The function
httpFlavorAttributenow usesfiber.Ctxas a value receiver. Verify that this change does not affect the intended behavior of the function.otelfiber/config.go (6)
4-4: Update import path for Fiber v3.The import path for Fiber has been updated to version 3. Ensure all references in the code align with this new import path.
13-21: Transition to value receivers forfiber.Ctxin config struct.The
configstruct now usesfiber.Ctxas a value receiver for several functions. Verify that these changes do not affect the intended behavior of the middleware.
38-38: Transition to value receiver forfiber.Ctx.The function
WithNextnow usesfiber.Ctxas a value receiver. Verify that this change does not affect the intended behavior of the function.
71-71: Transition to value receiver forfiber.Ctx.The function
WithSpanNameFormatternow usesfiber.Ctxas a value receiver. Verify that this change does not affect the intended behavior of the function.
96-96: Transition to value receiver forfiber.Ctx.The function
WithCustomAttributesnow usesfiber.Ctxas a value receiver. Verify that this change does not affect the intended behavior of the function.
104-104: Transition to value receiver forfiber.Ctx.The function
WithCustomMetricAttributesnow usesfiber.Ctxas a value receiver. Verify that this change does not affect the intended behavior of the function.otelfiber/fiber.go (4)
8-9: Update import paths for Fiber v3 and utils.The import paths have been correctly updated to reflect the new versions of Fiber and utils. Ensure that all usages of these packages are compatible with the new versions.
21-21: Update instrumentation name constant.The
instrumentationNameconstant has been updated to include the version. This change is correct and aligns with the new versioning scheme.
179-179: Update default span name formatter function signature.The
defaultSpanNameFormatterfunction now usesfiber.Ctxinstead of*fiber.Ctx. This change aligns with the updated middleware signature.
83-83: Shift from pointer to value receiver forfiber.Ctx.The middleware function signature now uses
fiber.Ctxinstead of*fiber.Ctx. This change is significant and may impact performance or memory usage. Ensure that all middleware logic is compatible with this change.otelfiber/otelfiber_test/fiber_test.go (3)
11-12: Update import paths for Fiber v3 and otelfiber.The import paths have been correctly updated to reflect the new versions of Fiber and otelfiber. Ensure that all test cases are compatible with the new versions.
31-31: Update instrumentation name constant.The
instrumentationNameconstant has been updated to include the version. This change is correct and aligns with the new versioning scheme.
40-40: Shift from pointer to value receiver forfiber.Ctxin test cases.The test cases now use
fiber.Ctxinstead of*fiber.Ctx. This change is significant and may impact how context is handled in tests. Ensure that all test logic is compatible with this change.Also applies to: 57-57, 73-73, 77-77, 100-100, 135-135, 157-157, 163-163, 175-175, 201-201, 230-230, 295-295, 416-416, 424-424, 429-429, 464-464, 472-472, 513-513, 517-517, 537-537, 547-547, 571-571
|
@heisGarvit v3 is still in beta, middlewares will be upgraded once we are closer to release. |
|
@gaby is there any plan for beta release of v3 middleware? So that users of fiber v3 can use opentelemetry to track performance of upcoming release? |
| "github.com/gofiber/fiber/v2" | ||
| "github.com/gofiber/fiber/v2/utils" | ||
| "github.com/gofiber/fiber/v3" | ||
| "github.com/gofiber/utils/v2" | ||
| "go.opentelemetry.io/otel/attribute" | ||
| semconv "go.opentelemetry.io/otel/semconv/v1.12.0" |
There was a problem hiding this comment.
Do you plan to ugprade semconv version? One notable change from 1.21 is that http.server.request.duration now has unit of seconds.
vs
(I also wonder if this upgrade should be done for fiber 2)
PR contains upgrade of otelfiber to v3 for compatibility with fiber v3
Summary by CodeRabbit
New Features
Bug Fixes
fiber.Ctx.Documentation