feature: instrumentation for gofiber #356
Conversation
| package xray | ||
|
|
||
| import ( | ||
| "context" |
There was a problem hiding this comment.
Can we run gofmt -w -s ./... to format the files?
| "github.com/aws/aws-xray-sdk-go/header" | ||
| "github.com/gofiber/fiber/v2" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Can we add a description here?
| } | ||
|
|
||
| // Handler wraps the provided fiber.Handler. | ||
| func (h *fiberHandler) Handler(sn SegmentNamer, handler fiber.Handler) fiber.Handler { |
There was a problem hiding this comment.
I think probably a good idea to provide context as an argument of this method to provide abilities to cancel the context or pass the context with a deadline.
| @@ -0,0 +1,37 @@ | |||
| package xray | |||
|
|
|||
| import ( | |||
There was a problem hiding this comment.
We should add a benchmark here.
| return | ||
| } | ||
|
|
||
| assert.Equal(t, fiber.StatusOK, rc.Response().StatusCode()) |
There was a problem hiding this comment.
Probably a good idea to validate all the three error flags fault, error and throttle.
| "testing" | ||
| ) | ||
|
|
||
| func TestFiberHandler(t *testing.T) { |
There was a problem hiding this comment.
comment explaining what this test case is testing would be good for readability.
| return func(ctx *fiber.Ctx) error { | ||
| auxCtx := context.Background() | ||
| if h.cfg != nil { | ||
| auxCtx = context.WithValue(context.Background(), fasthttpContextConfigKey, h.cfg) |
There was a problem hiding this comment.
not super familiar with gofiber but what I can tell is it's built on fasthttp and looks like we use the same key named as fasthttpContextConfigKey even in fasthttp instrumentation. Do you think that would cause any issue when both the instrumentation are used in a single application?
Instrumentation for fiber, a very popular framework built on fasthttp.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.