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

Load SQLite instrumentation only when build tag present #601

Merged
merged 1 commit into from
Oct 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions connection_instrumented.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ import (
pgx "github.com/jackc/pgx/v4/stdlib"
"github.com/jmoiron/sqlx"
"github.com/luna-duclos/instrumentedsql"
"github.com/mattn/go-sqlite3"
"github.com/pkg/errors"
)

const instrumentedDriverName = "instrumented-sql-driver"

func instrumentDriver(deets *ConnectionDetails, defaultDriverName string) (driverName, dialect string) {
func instrumentDriver(deets *ConnectionDetails, defaultDriverName string) (driverName, dialect string, err error) {
driverName = defaultDriverName
if deets.Driver != "" {
driverName = deets.Driver
Expand All @@ -27,7 +26,7 @@ func instrumentDriver(deets *ConnectionDetails, defaultDriverName string) (drive
}

// If instrumentation is disabled, we just return the driver name we got (e.g. "pgx").
return driverName, dialect
return driverName, dialect, nil
}

if len(deets.InstrumentedDriverOptions) == 0 {
Expand All @@ -48,7 +47,11 @@ func instrumentDriver(deets *ConnectionDetails, defaultDriverName string) (drive
dr = mysqld.MySQLDriver{}
newDriverName = instrumentedDriverName + "-" + nameMySQL
case nameSQLite3:
dr = new(sqlite3.SQLiteDriver)
var err error
dr, err = newSQLiteDriver()
if err != nil {
return "", "", err
}
newDriverName = instrumentedDriverName + "-" + nameSQLite3
}

Expand All @@ -64,7 +67,7 @@ func instrumentDriver(deets *ConnectionDetails, defaultDriverName string) (drive
sql.Register(newDriverName, instrumentedsql.WrapDriver(dr, deets.InstrumentedDriverOptions...))
}

return newDriverName, dialect
return newDriverName, dialect, nil
}

// openPotentiallyInstrumentedConnection first opens a raw SQL connection and then wraps it with `sqlx`.
Expand All @@ -74,7 +77,11 @@ func instrumentDriver(deets *ConnectionDetails, defaultDriverName string) (drive
// a custom driver name when using instrumentation, this detection would fail
// otherwise.
func openPotentiallyInstrumentedConnection(c dialect, dsn string) (*sqlx.DB, error) {
driverName, dialect := instrumentDriver(c.Details(), c.DefaultDriver())
driverName, dialect, err := instrumentDriver(c.Details(), c.DefaultDriver())
if err != nil {
return nil, err
}

con, err := sql.Open(driverName, dsn)
if err != nil {
return nil, errors.Wrap(err, "could not open database connection")
Expand Down
15 changes: 15 additions & 0 deletions connection_instrumented_nosqlite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// +build !sqlite

package pop

import (
"github.com/stretchr/testify/require"
"testing"
)

func TestInstrumentation_WithoutSqlite(t *testing.T) {
_, _, err := instrumentDriver(&ConnectionDetails{
URL: "sqlite://:memory:",
}, "sqlite")
require.NoError(t, err)
}
14 changes: 14 additions & 0 deletions dialect_nosqlite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// +build !sqlite

package pop

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestSqlite_NewDriver(t *testing.T) {
_, err := newSQLiteDriver()
require.Error(t, err)
}
6 changes: 6 additions & 0 deletions dialect_sqlite.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
package pop

import (
"database/sql/driver"
"fmt"
"github.com/mattn/go-sqlite3"
"io"
"net/url"
"os"
Expand Down Expand Up @@ -290,3 +292,7 @@ func finalizerSQLite(cd *ConnectionDetails) {
} // or fix user specified url?
}
}

func newSQLiteDriver() (driver.Driver, error) {
return new(sqlite3.SQLiteDriver), nil
}
5 changes: 5 additions & 0 deletions dialect_sqlite_shim.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package pop

import (
"database/sql/driver"
"errors"
)

Expand All @@ -16,3 +17,7 @@ func init() {
func newSQLite(deets *ConnectionDetails) (dialect, error) {
return nil, errors.New("sqlite3 support was not compiled into the binary")
}

func newSQLiteDriver() (driver.Driver, error) {
return nil, errors.New("sqlite3 support was not compiled into the binary")
}
5 changes: 5 additions & 0 deletions dialect_sqlite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,8 @@ func TestSqlite_CreateDB(t *testing.T) {
// Creating DB twice should produce an error
r.EqualError(dialect.CreateDB(), fmt.Sprintf("could not create SQLite database '%s'; database exists", p))
}

func TestSqlite_NewDriver(t *testing.T) {
_, err := newSQLiteDriver()
require.NoError(t, err)
}