Make SetContext atomic

[joshuap]

I think this should fix #31:

This allows concurrent goroutines to call honeybadger.SetContext. It does not make context.Update safe/atomic because for ease of use, Context must function as a normal map. This means any client code which calls context.Update directly should perform its own locking if necessary; this may be a case for making context.Update a private method, since it's not really meant to be used directly anyway.

Closes #31

cc @evt @pauldub @odarriba

[odarriba]

Thanks for the update @joshuap

I think this solves the concurrent map issue, but the underlying problem (several goroutines sharing same context) is there, right?

For our particular use case, several HTTP requests can come nearly-same time and a timeline like this can happen (and in fact already happened): 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

I know that's a difficult thing to solve, as we cannot attach parameters or values to goroutines themselves, like in Elixir's processes (I come from Elixir, and the way this is solved on the HB's elixir library this is the only way i can think of at the moment 😅 )

At the moment we avoid setting the context and just attach it in any controlled notification to HB (we have controlled a lot of them), so i think that would work as a solution for us (and hopefully for any other reading this comment with the same issue!)

The PR itself looks amazing and for sure will solve some concurrency problems (specially the one on the #31), so thank you for the effort and the work!

[joshuap]

@odarriba thanks for the well-thought-out comment, you raise an excellent point. I think our context implementation in honeybadger-go is a bit naive. :)

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. :)

Anyway, I'll merge this PR just to prevent errors, but I think moving forward we'll have to include the caveat that honeybadger.setContext will not work reliably across goroutines, and head back to the drawing board for a solution for goroutine/request context.