Consider using `diesel::connection::Instrumentation` instead of bringing your own connection type

[weiznich]

Diesel introduced a way to instrument existing connection implementations with diesel 2.2. The entry point for this is Connection::set_instrumentation. You can easily define your own instrumentation handler by implementing the Instrumentation trait or you could just provide a handler function. I think this could greatly simplify diesel-dtrace as it only would need to provide that single type and handle the events it's actually interested in.

[bnaecker]

Hi @weiznich, thanks for filing this. I think using the new instrumentation trait would be a reasonable addition to the crate! After looking through the documentation, I do think we should add that implementation rather than completely replace the existing connection type we currently expose.

The main reason is that the API appears to use a process-wide global default, which one manipulates through the diesel::connection::{set,get}_default_instrumentation() functions. One would need to use that in order to accurately instrument the creation / establishment of connections, because the Connection::establish() constructor uses the current value of that global to instrument that code path. (That's at least true for the PgConnection type, though I've not looked at the others in detail.) Creating the connection through Connection::establish() and then setting the per-Connection instrumentation type would preclude tracing the establishment path.

The global also means that users cannot instrument connection-establishment in different ways in the same process. For most folks, that's probably not a big deal, but I think it'd be nice if this crate didn't preclude that use-case as well.

If that all makes sense, then I definitely think adding a implementor of the Instrumentation trait in this crate makes a lot of sense. It would be useful for a wide range of cases, leaving the existing connection type available for those other use cases described above.

[weiznich]

You are correct in so far that diesel::connection::{set,get}_default_instrumentation() allow to set a global instrumentation method that is used by default. In addition you can also use Connection::set_instrumentation to override this per connection with a different implementation. Which method to use mostly depends on your usecase.

Keeping the existing connection implementation can still be desirable from a compatibility point of view. The implementation might be as simple as just calling Connection::set_instrumentation from the establish function and otherwise forward all other functions to the inner connection type.

Edit: I just read your comment again and yes you are right that using the Connection::set_instrumentation method won't allow to instrumentation the establish call. For that the custom connection is still required if you don't use the global provider.