Context is shared across goroutines

[joshuap]

As described in #33, our context implementation in honeybadger-go is a bit naive; because it's global, it is prone to collisions when multiple goroutines update it (which is especially common in server implementations). Consider this scenario from @odarriba:

1. request A starts (one goroutine) -> set context A
2. request B starts (another goroutine) -> set context B
3. request A fails -> sends to HB with context B

The way we solve this issue in Ruby is using a thread local variable which solves the concurrent access problem, and then we reset the context after each request which solves the local request context issue. We do something similar in Elixir--the context is stored in the process dictionary.

I've been reading up on how Go handles passing request context across goroutines, specifically:

https://blog.golang.org/context
http://www.gorillatoolkit.org/pkg/context

From what understand, Go does not provide anything similar to thread local variables or a process dictionary, preferring to pass a context value as described in the blog post.

There may be a way we can integrate with this, but I'm still wrapping my head around it, so I'm not sure what that might be yet. I'm open to suggestions!

In the meantime, we have to live with the caveat that honeybadger.SetContext will not work reliably across goroutines.

Front conversations

[seanhagen]

Instead of a global context, could the library instead create a notification or event struct that's tied to a specific context.Context?

For example, in the Honeycomb go library, you can get the event for the current context by calling ev := beeline.ContextEvent(ctx). You can then add more data to the event. If you're using one of the wrappers ( like the ones that wrap net/http Handler or HandlerFuncs here ), then you don't even have to worry about sending it after the request is done.

That way, a wrapper in Honeybadger would just have to create an unsent notification and tie it to a context ( probably by injecting that event into the context using context.WithValue ). Then if an error occurs within a request, the user can grab the notification work with it using something like:

n := honeybadger.ContextNotification(ctx)
// Add(...interface{}), to add context to the error notification 
//   without triggering it to be sent
n.Add(aUserStruct, someDataInAMap)
// Error(error, ...interface{}) gives honeybadger the error to log, 
//  along with optional additional context
n.Error(err)

Then the data tied to an error notification is tied to a specific context ( whether that context is part of a request or not, it's just got to be a context.Context ).

I can't dive into trying to create an implementation like this right now, but I might be able to in a few days if nobody else gives it a shot.

[pkieltyka]

this seems like an important one to solve for the library to actually be production-level + usable for Go projects?