-
Notifications
You must be signed in to change notification settings - Fork 440
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
contrib/gofiber/fiber.v2: use UserContext in Middleware for span creation #3035
base: main
Are you sure you want to change the base?
Conversation
…tion The Middleware was incorrectly using `c.Context()` instead of `c.UserContext()` to start spans. This could break the propagation of the context defined by other middlewares earlier in the chain. This change ensures that `tracer.StartSpanFromContext` uses the correct context provided by Fiber, preserving context propagation and ensuring proper tracing behavior. Fixes DataDog#2199
@@ -73,7 +73,7 @@ func Middleware(opts ...Option) func(c *fiber.Ctx) error { | |||
tracer.Tag(ext.Component, componentName), | |||
tracer.Tag(ext.SpanKind, ext.SpanKindServer), | |||
) | |||
span, ctx := tracer.StartSpanFromContext(c.Context(), cfg.spanName, opts...) |
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.
could you please write a test reproducing the issue? 🙏
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.
There is an exemple in the linked issue.
But here is a simpler example :
package main
import (
"github.com/gofiber/fiber/v2"
"golang.org/x/net/context"
fibertrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/gofiber/fiber.v2"
)
func initUserContext() fiber.Handler {
return func(c *fiber.Ctx) error {
ctx := context.WithValue(c.UserContext(), "key", "value")
c.SetUserContext(ctx)
return c.Next()
}
}
func checkUserContext() fiber.Handler {
return func(c *fiber.Ctx) error {
_, ok := c.UserContext().Value("key").(string)
if !ok {
return fiber.ErrInternalServerError
}
return c.Next()
}
}
func defaultHandler(c *fiber.Ctx) error {
return c.SendStatus(fiber.StatusOK)
}
func main() {
app := fiber.New()
app.Use(initUserContext()) // Init UserContext with some key value
app.Use(fibertrace.Middleware()) // Add Datadog middleware
app.Use(checkUserContext()) // Check if UserContext is correctly set
app.Get("/", defaultHandler)
if err := app.Listen(":3000"); err != nil {
panic(err)
}
}
The expected outcome is a response with a status code of 200, but instead, a response with a status code of 500 is returned.
If the fibertrace middleware is removed, a response with a status code of 200 is obtained as expected.
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.
I modified the UserContext test to include the necessary checks
… creation The Middleware was incorrectly using `c.Context()` instead of `c.UserContext()` to start spans. This could break the propagation of the context defined by other middlewares earlier in the chain. This change ensures that `tracer.StartSpanFromContext` uses the correct context provided by Fiber, preserving context propagation and ensuring proper tracing behavior. Fixes DataDog#2199
What does this PR do?
The Middleware was incorrectly using
c.Context()
instead ofc.UserContext()
to start spans. This could break the propagation of the context defined by other middlewares earlier in the chain.Motivation
This change ensures that
tracer.StartSpanFromContext
uses the correct context provided by Fiber, preserving context propagation and ensuring proper tracing behavior.Fixes #2199
Reviewer's Checklist
v2-dev
branch and reviewed by @DataDog/apm-go.