Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nrpostgres driver doesn't work with Sqlx named queries. #116

Closed
Jwonsever opened this issue Nov 9, 2019 · 3 comments
Closed

Nrpostgres driver doesn't work with Sqlx named queries. #116

Jwonsever opened this issue Nov 9, 2019 · 3 comments

Comments

@Jwonsever
Copy link

Hi, I've been investigating a hookup of newRelic to sqlx, using the example at https://github.com/newrelic/go-agent/blob/master/_integrations/nrpq/example/sqlx/main.go as a starting point.

I use sqlx named queries. They would go something (psuedocode) like:

		stmt, err := db.PrepareNamed(query)
		if err != nil { return err}
		_, err = stmt.ExecContext(ctx, model)

However, sqlx uses the driver name to determine how to bind parameters to the query, and the 'nrpostgres' driver is not supported. You can see this at https://github.com/jmoiron/sqlx/blob/master/bind.go

it looks like the easiest way for me to get around this is to fork sqlx to allow the newrelic driver at this point, which I would ideally like to avoid. Are there any other options available to me?

@Jwonsever Jwonsever changed the title Nrpostgres driver doesn't work with Sqlx Named Queries. Nrpostgres driver doesn't work with Sqlx named queries. Nov 9, 2019
@purple4reina
Copy link
Contributor

Hey @Jwonsever !

Thanks for opening this issue! I was able to test this out using your example and confirm that no Datastore metrics are created for NamedStmt's in sqlx.

I've got both good and bad news though. I'll start with the good. I think there's a way to get around the need to fork sqlx. Instead of using sqlx.Open, you can do it in stages and pass in a custom value for the driverName:

     sqldb, err := sql.Open("nrpostgres", "user=foo dbname=bar sslmode=disable")
     if err != nil {
         log.Fatalln(err)
     }
     db := sqlx.NewDb(sqldb, "postgres")

This will allow your underlying sql.DB to use the nrpostgres driver but will tell the sqlx.DB that you're actually just using postgres.

Now, sadly, it seems even with this change that metrics will still not be created. That is because sql.Stmt exec and query calls are not supported with the lib/pq driver since pq.Stmt does not have ExecContext and QueryContext methods. Take a look at this pull request: lib/pq#768.

Though it may not work for use case, I believe our mysql instrumentation will record metrics for sql.Stmt exec and query calls.

Thanks again and keep us up to date with what you discover. We always love hearing from our users!

@Jwonsever
Copy link
Author

Thanks, that appears to work, I'll let you know if I run into any side effects of the change.

On the second topic, I ran into that and was able to switch to tx.NamedExecContext() which started getting results. I think that will work for me, at least for now.

@Jwonsever
Copy link
Author

This PR is relevant. jmoiron/sqlx#401 . It would let us bind a custom driver, nrpostgres. Its been open since 2018 though, so it's kind of a lost cause I suppose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants