Propagate context to the SQL layer

[sagikazarmark]

Description

It would be nice if a context could be passed down to the underlying SQL layer.

Apart from the usual use cases (cancelling, deadlines) it would help a lot with providing trace information for a request.

[vilterp]

Could introduce this in a backwards-compatible way by adding a context-taking variant for each method that runs a query ({Exec, Find, Create, ...}Ctx(ctx context.Context, ...)), and always use the underlying sqlx methods that take contexts.

My motivation is specifically to use it with Honeycomb, via this: https://github.com/honeycombio/beeline-go/blob/master/wrappers/hnypop/pop.go

[duckbrain]

I know it's discouraged to put context.Context in a field vs. the first argument of a function, but I'd be pretty happy with it being optionally attached to a pop.Connection (maybe when you create a transaction?) and implicitly used when you do database operations.

We've got a lot of HTTP requests that get canceled, but we don't have a way to pass that cancellation to the database layer. Doing this would let us add the context in the middleware, without a lot of additional changes.

[aeneasr]

Any updates on this? Adding tracing and metrics to database calls is such an important thing to do in production using, for example, https://github.com/luna-duclos/instrumentedsql .

Sorry for the ping, but this hasn't seen a note from maintainers in over a year, but is an essential feature for production (metrics, tracing). @stanislas-m @markbates

I'm also happy to help of course.

I'd be pretty happy with it being optionally attached to a pop.Connection (maybe when you create a transaction?) and implicitly used when you do database operations.

That wouldn't be thread safe unless you use transactions everywhere.

[duckbrain]

@aeneasr I think that should be fine with thread-saftey as long as attaching the context returns a modified clone. There is some precedent for doing something like that in the standard library http package. It makes it so it doesn't require a huge API change on the library.

I started playing around with a possible implementation, but I didn't really get to test it out, and haven't been able to work on it for a little while. If someone wants to pick it up and turn it into a MR, they are more than welcome. https://github.com/ec2-software/pop/tree/context

[aeneasr]

@aeneasr I think that should be fine with thread-saftey as long as attaching the context returns a modified clone.

Right, that was what I wanted to get at :)

I started playing around with a possible implementation, but I didn't really get to test it out, and haven't been able to work on it for a little while. If someone wants to pick it up and turn it into a MR, they are more than welcome. https://github.com/ec2-software/pop/tree/context

Awesome! I've also started to to a very basic PoC ( #468 ) just to see how difficult/easy it would be. I think it's doable, but I also have very little time to work on that at the moment. I guess that we'll contribute it when we start migrating other projects that need to be reliable in production.

[stanislas-m]

Fixed with #497 (rebased from #472).