Update echo middleware to get Hub instance from context

[j--wong]

Modified to support getting sentry.Hub instance from parent Context to include extra fields defined in the parent context.

Background -- we use sentry with echo web framework in a AWS lambda environment behind AWS API Gateway. We have top-level sentry instance defined with extra fields.

The purpose of this change is to use the existing hub on context by default.

[rhcarvalho]

@j--wong would you have a short example of how you're using this?

I would like to understand what use case this is solving, because if we change the echo middleware we may want to consider the same change for other frameworks.

[j--wong]

Hi @rhcarvalho, thanks for the reply.

We are building rest apis in AWS using AWS API Gateway > Lambda > Echo.

We configure logging, tracing (honeycomb) and error handling/monitoring (sentry) at the Lambda layer with extra information.

The key use case for us is we want that same set of extra information to flow to sentry, honeycomb and logs without having to reconfigure them with every error/log/trace.

We configure a hub instance at Lambda layer and attach it to the context. We'd like Echo layer (ie., sentry-echo middleware) to use the same hub instance.

---

Without this PR, those extra information is not preserved in sentry-echo middleware as it creates a new Hub instance internally instead of getting existing one from the parent context -- which is what I am trying to address with this PR.

We configure honeycomb the same way. Honeycomb version of echo middleware looks for existing trace from parent context first, and create a new one only if there is no existing trace in parent context. As seen here: https://github.com/honeycombio/beeline-go/blob/d24142d664/wrappers/common/common.go#L45-L57

I believe this should be applicable to other web frameworks as well. I think it's sensible to use existing hub from context as default.

Maybe you could introduce this behind an option like UseHubFromContext bool or something along those lines if you want to provide a toggle for this behaviour.

Sorry it was a bit long, hope this all makes sense 😄

[rhcarvalho]

We configure [...] error handling/monitoring (sentry) at the Lambda layer with extra information.

@j--wong thanks for the description. Do you have a short piece of code to demonstrate how you're putting things together? Particularly interested in the part related to sentry-go.

I'd like to see precisely how you're inserting a hub in the request context before the sentry middleware.

In this example code there is a demonstration of how to add a tag to all Sentry events:

sentry-go/example/echo/main.go

Lines 35 to 46 in af3076c

	app.Use(sentryecho.New(sentryecho.Options{
	Repanic: true,
	}))
	app.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
	return func(ctx echo.Context) error {
	if hub := sentryecho.GetHubFromContext(ctx); hub != nil {
	hub.Scope().SetTag("someRandomTag", "maybeYouNeedIt")
	}
	return next(ctx)
	}
	})

With the same pattern you can configure the request's hub/scope arbitrarily.

---

I am imagining you have a middleware that is running before Sentry's middleware, that's why you are trying to add a hub to the Echo context manually?!

[j--wong]

I am imagining you have a middleware that is running before Sentry's middleware, that's why you are trying to add a hub to the Echo context manually?!

Hi @rhcarvalho, in my case, I already have a Hub instance on context with all extra fields set. I just wish the sentry-echo middleware could use that instance.

Yes, I got a middleware running before sentry-echo middleware (it's lambda middlware, not echo middleware). The code looks like this:

return func(next lambda.Handler) lambda.Handler {
	...
	...
	return func(ctx context.Context, payload []byte) ([]byte, error) {
		lc, _ := lambdacontext.FromContext(ctx)

		hub := sentry.CurrentHub().Clone()
		hub.Scope().SetExtras(cfg.Fields) // extra information
		hub.Scope().SetExtra("aws_request_id", lc.AwsRequestID)
		hub.Scope().SetExtra("...",...) // more lines omitted
		ctx = sentry.SetHubOnContext(ctx, hub)
		...
		...
		// invoke next lambda middleware; from this point forward, we have hub instance on context
		return next.Invoke(ctx, payload) 
	}
}

The reason I configured sentry this way in Lambda layer is so that Sentry can catch lambda errors too, not just errors from Echo.

The other approach I could take is, like in the example you shared, duplicate above code in another echo middleware func. So I add whatever fields I want to add after sentry middleware.

I guess the key use-case question I'd like to ask is -- do you believe this is a valid/reasonable scenario for sentry/echo users to have Hub instance configured higher up in the chain before echo middleware run?

To me, that's exactly my use case 😄. Maybe not many people are doing Lambda > Echo + Sentry combo for their apis. So this may not be common use case?

[rhcarvalho]

This is a reasonable use case. I'll follow up with adding similar behavior to the other integrations. Thanks, @j--wong!

This will be part of the upcoming next release.

[j--wong]

Great stuff @rhcarvalho! Thanks for that 👍