Skip to content
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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+21 −1
Open

contrib/gofiber/fiber.v2: use UserContext in Middleware for span creation #3035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8a2662b
contrib/gofiber/fiber.v2: use UserContext in Middleware for span crea…
tonyduburque Dec 12, 2024
d590841
Merge branch 'main' into main
tonyduburque Dec 22, 2024
95ea83f
Merge branch 'main' into main
tonyduburque Dec 27, 2024
8a61a98
Merge branch 'main' into main
tonyduburque Jan 9, 2025
2ed3f12
Merge branch 'main' into main
tonyduburque Jan 13, 2025
385eab9
Merge branch 'main' into main
tonyduburque Jan 14, 2025
61cfd11
Merge branch 'DataDog:main' into main
tonyduburque Jan 15, 2025
7ae80cf
contrib/gofiber/fiber.v2: fix UserContext test in Middleware for span…
tonyduburque Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion contrib/gofiber/fiber.v2/fiber.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Copy link
Contributor

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? 🙏

Copy link
Author

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.

Copy link
Author

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

span, ctx := tracer.StartSpanFromContext(c.UserContext(), cfg.spanName, opts...)

defer span.Finish()

Expand Down
20 changes: 20 additions & 0 deletions contrib/gofiber/fiber.v2/fiber_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package fiber

import (
"context"
"fmt"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -178,11 +179,30 @@ func TestUserContext(t *testing.T) {

// setup
router := fiber.New()

// define a custom context key
type contextKey string
const fooKey contextKey = "foo"

// add a middleware that adds a value to the context
router.Use(func(c *fiber.Ctx) error {
ctx := context.WithValue(c.UserContext(), fooKey, "bar")
c.SetUserContext(ctx)
return c.Next()
})

// add the middleware
router.Use(Middleware(WithServiceName("foobar")))

router.Get("/", func(c *fiber.Ctx) error {
// check if not default empty context
assert.NotEmpty(c.UserContext())

// checks that the user context still has the information provided before using the middleware
foo, ok := c.UserContext().Value(fooKey).(string)
assert.True(ok)
assert.Equal(foo, "bar")

span, _ := tracer.StartSpanFromContext(c.UserContext(), "http.request")
defer span.Finish()
return c.SendString("test")
Expand Down