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

StartTransaction returns transaction with outdated context, when the transaction already exists #854

Closed
far4599 opened this issue Jul 9, 2024 · 0 comments

Comments

@far4599
Copy link
Contributor

far4599 commented Jul 9, 2024

Summary

StartTransaction function returns currentTransaction with outdated context.

Steps To Reproduce

  1. Create transaction with StartTransaction: tr := sentry.StartTransaction(context.TODO(),...)
  2. Add a new value to its context: ctx := context.WithValue(tc.Context(), "key", "value")
  3. Receive transaction from the context like it is done somewhere down the stack: tr := sentry.StartTransaction(ctx,...)
  4. Check if "key" exists in the transaction context: _, exist := tc.Context().Value("key").(string) // exist == false

Expected Behavior

StartTransaction may create a new transaction or return the one from the context, it is expected that tc.Context() returns the latest provided context, but not the outdated one.

Additional context

I found a tricky bug here.

func StartTransaction(ctx context.Context, name string, options ...SpanOption) *Span {
	currentTransaction, exists := ctx.Value(spanContextKey{}).(*Span)
	if exists {
		return currentTransaction
	}
	...

StartTransaction function returns currentTransaction when it exists in the provided context. And currentTransaction contains a pointer to the context inside, but the older version of the context. It is expected that a newly started transaction will provide access to the modified context through transaction.Context() method.

To resolve the issue we need to update the pointer to the context, stored in currentTransaction

func StartTransaction(ctx context.Context, name string, options ...SpanOption) *Span {
	currentTransaction, exists := ctx.Value(spanContextKey{}).(*Span)
	if exists {
		currentTransaction.ctx = ctx
		return currentTransaction
	}
	...

I found it when I used the grpc gateway with implemented interceptors. As the service supports HTTP and grpc the code reuses interceptors for both protocols. Sentry tracing is added in both HTTP middleware and grpc interceptor layers, and when the StartTransaction function is called second type the content for transaction.Context() contains the previous version of the content which was saved at the moment of object creation.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jul 9, 2024
far4599 added a commit to far4599/sentry-go that referenced this issue Jul 9, 2024
@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Jul 9, 2024
@far4599 far4599 changed the title StartTransaction bring new context when transaction already exist in the context StartTransaction returns transaction with outdated context, when the transaction already exist Jul 10, 2024
@far4599 far4599 changed the title StartTransaction returns transaction with outdated context, when the transaction already exist StartTransaction returns transaction with outdated context, when the transaction already exists Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants