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.

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

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

Open
charlesoconor opened this issue Jan 19, 2023 · 2 comments

Comments

@charlesoconor
Copy link

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
@cleptric
Copy link
Member

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.

@090809
Copy link

090809 commented Jan 19, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants