sqlmw icon indicating copy to clipboard operation
sqlmw copied to clipboard

Store user-defined data in Rows, Stmt and Tx objects

Open fho opened this issue 4 years ago • 2 comments

Hello,

I'm implementing an interceptor with sqlmw that records traces for database operations (instrumentedsql->sqlmw->tracing with sqlmw :-)).

When a Rows, Stmt or Tx object is created I create a parent tracing span. Operations on the Rows, Stmt and Tx structs should be created as child spans. When the Rows, Stmt, Tx operations is finished (Close(), Commit(), Rollback(), etc), the parent span is also finished.

To be able to create a child span, the parent span must be available in the methods of the Rows, Stmt and Tx objects. One way to to achieve this would be to wrap the Rows, Stmt and Tx objects again in my interceptor implementation and store the additional information in the struct.

I would like to avoid having to wrap those structs again in my interceptor. Wrapping it also requires to implement the fallback logic for the Stmt.QueryContext() and Stmt.ExecContext() methods to their non context aware variants again. This means maintaining another copy of namedValueToValue.

It would be great if sqlmw would support the usecase to store user-defined data when creating Rows, Stmt and Tx structs and access it in their methods instead. sqlmw is doing something similiar already. When for example QueryContext() is called, it stores the cx in the wrappedRows objects and passes it as parameters to methods like RowsNext() as argument, which do not have a ctx argument in the stdlib implementation.

What do you think about the idea? Does somebody have an suggestion for how to implement it? (I'll extend this issue or create a PR, when I have an idea.)

fho avatar Aug 17 '21 08:08 fho

The stated goal of the proposal seems reasonable to me. I'm afraid I don't have the time or the context in my head to evaluate the proposed solution though, although it principle it sounds like a reasonable approach.

inconshreveable avatar Aug 17 '21 14:08 inconshreveable

ok, thanks for the fast response nonetheless. Maybe something else sees it and has an opinion or even a better idea.

I experimented a bit more in the meantime: The Rows and Tx objects currently can be simply wrapped by the middleware and the parent Rows, Tx embedded. In the related interceptor methods (RowsNext, RowsClose, TxCommit, etc), a type check can be done if the passed rows or Tx is the own wrapped type. This works fine and the wrapping overhead is also negligible.

For statements the approach does not work. sqlmw wraps the driver.Stmt that is returned by the interceptors ConnPrepareContext method again into a sqlmw.wrappedStmt. Because that type is private the user can not convert it to it's own wrapped stmt type in the middleware anymore. Another difficulty with Stmts is that ConnPrepareContext returns a driver.Stmt interface. The interceptor methods operating on more specific interfaces like driver.QueryerContext, driver.ExecerContext. To be able to handle that, the user has to check in ConnPrepareContext which interfaces the passed driver.ConnPrepareContext actually implement and then wraps it in objects that have the same interfaces. Currently there are 3 interfaces that extends the Stmt in the stdlib, all combinations of those must be handled. Alternatively the same then sqlmw.WrappedParentStatement must be implemented in the middleware again.

Possible Solutions:

  • make sqlmw.WrappedParentStmt public and change the parameter type of stmt in StmtQueryContext and StmtExecContext to sqlmw.WrappedParentStmt. A middleware can then extract it's own wrapped statement. This would be a simple change in sqlmw, the user would still have to cope with all the different Stmt interfaces though.
    (Independent of this issue I think this change would still make sense to clarify the interface. Currently it is hidden, that the statement interceptor methods do not get passed the same stmt that the user returned in PrepareStatement.)
  • Support to store user-defined data in Stmts, Tx, Rows in the following way: A type UserData struct { atomic.Value } is introduced. Interceptor methods that return Rows, Tx and Stmts are changed to have an additional *UserData parameter. In the interceptor methods Userdata.Store() can be called to store custom data for Rows, Tx or Stmts. sqlmw stores the UserData in the wrapped Rows, Tx, Stmts. The data can be in RowsNext(), StmtQueryContext(), TxCommit(), etc retrieved via helper functions like: func GetUserDataRows(driver.Row) interface{} {}, func GetUserDataStmts(driver.Stmt) interface{} {}, func GetUserDataTx(driver.Tx) interface{} {}.

fho avatar Aug 17 '21 16:08 fho