sentry-go icon indicating copy to clipboard operation
sentry-go copied to clipboard

The root `sentry-go` packaged uses a different context value vs `sentry-go/gin`

Open charlesoconor opened this issue 2 years ago • 2 comments

Summary

The GetHubFromContext in the github.com/getsentry/sentry-go package isn't compatible with the GetHubFromContext from github.com/getsentry/sentry-go/gin since they both use a different value when putting the Hub into their respective contexts.

Steps To Reproduce


package main

import (
	"context"
	"fmt"

	"github.com/getsentry/sentry-go"
	sentrygin "github.com/getsentry/sentry-go/gin"
)

func check(err error) {
	if err != nil {
		panic(err)
	}
}

func main() {
	err := sentry.Init(sentrySdk.ClientOptions{
		Dsn:   "https://[email protected]/0000008",
		Debug: true,
	})
	check(err)

	ctx := context.Background()
	ctx = sentry.SetHubOnContext(ctx)

	fmt.Printf("I will be found %t", sentry.GetHubFromContext(ctx) != nil)
	fmt.Printf("I not will be found %t", sentrygin.GetHubFromContext(ctx) != nil)
}

The value used to set the hub in context in gin is "sentry", here. The value used to set Hub in the is HubContextKey here.

This is probably the case since gin.Context doesn't play well with the context.Context interface and has it's own way of setting values. The HubContextKey is exported from the github.com/getsentry/sentry-go package so it should be possible to switch it over to keep everything consistent.

The problem would be if you would have to set both for some time in case a user has hard-coded the key in their code instead of using the function. In that case, it could be a breaking change even if the value isn't exported out of the sentry-go/gin package.

Expected Behavior

Expected that using the sentry package would be compatible across. This leads to the application having to know what the higher context is doing instead of letting the context.Context interface get passed through. I was very confused when my gin servers weren't reporting any events.

SDK

  • sentry-go version: v0.17.0
  • Go version: 19
  • Using Go Modules? yes

Sentry

  • Using hosted Sentry in sentry.io? yes
  • Anything particular to your environment that could be related to this issue? no

charlesoconor avatar Jan 19 '23 08:01 charlesoconor

While I'm not having the historical context of why this was implemented like that, I noticed that gin's context relies on strings for Get() and Set(), while our HubContextKey is an int.

I'll backlog this for now, maybe we can improve this across our integrations in the future.

cleptric avatar Jan 19 '23 11:01 cleptric

Gin can work with context.Context, if we enable fallback to context in (*gin.Context).Request.Context(), like this:

engine := gin.New()
engine.ContextWithFallback = true

Here: https://github.com/gin-gonic/gin/blob/7cb151bb4c4cfc6018a00a125422ff38a041b9f8/context.go#L1214-L1217

But there is some problem with override this context, without breaking (*gin.Context) entity: (it's too big, for me, but in middlewares - there no other way for context propogation)

ctx.Request = ctx.Request.WithContext(span.Context())

Or U can just specify your http.Server with ConnContext, like this:

server := &http.Server{
	Handler: engine,
	ConnContext: func(ctx context.Context, c net.Conn) context.Context {
		return sentry.SetHubOnContext(
			ctx,
			sentry.CurrentHub().Clone(),
		)
	},
}

And any new request, if ContextWithFallback is enabled will have own Hub

P.S.: In own projects we don't use sentrygin.GetHubFromContext(), and all of sentrygin integration, 'cause it's not simple to catch.

090809 avatar Jan 19 '23 21:01 090809