Move every db.Prepare to sqlutil.Statementlist, remove trace driver (#3026)

Doesn't buy us much, but makes everything a bit more consistent.

Also removes the SQL trace driver, as it is unused and the output is
hard to read anyway.

internal/sqlutil/sqlite_cgo.go

@@ -4,7 +4,6 @@
 package sqlutil
 
 import (
-	"github.com/mattn/go-sqlite3"
 	_ "github.com/mattn/go-sqlite3"
 )
 
@@ -13,7 +12,3 @@ const SQLITE_DRIVER_NAME = "sqlite3"
 func sqliteDSNExtension(dsn string) string {
 	return dsn
 }
-
-func sqliteDriver() *sqlite3.SQLiteDriver {
-	return &sqlite3.SQLiteDriver{}
-}

internal/sqlutil/sqlite_native.go

@@ -4,7 +4,6 @@
 package sqlutil
 
 import (
-	"modernc.org/sqlite"
 	"strings"
 )
 
@@ -23,7 +22,3 @@ func sqliteDSNExtension(dsn string) string {
 	dsn += "_pragma=busy_timeout%3d10000"
 	return dsn
 }
-
-func sqliteDriver() *sqlite.Driver {
-	return &sqlite.Driver{}
-}

internal/sqlutil/sqlutil.go

@@ -13,8 +13,7 @@ import (
 var skipSanityChecks = flag.Bool("skip-db-sanity", false, "Ignore sanity checks on the database connections (NOT RECOMMENDED!)")
 
 // Open opens a database specified by its database driver name and a driver-specific data source name,
-// usually consisting of at least a database name and connection information. Includes tracing driver
-// if DENDRITE_TRACE_SQL=1
+// usually consisting of at least a database name and connection information.
 func Open(dbProperties *config.DatabaseOptions, writer Writer) (*sql.DB, error) {
 	var err error
 	var driverName, dsn string
@@ -32,10 +31,6 @@ func Open(dbProperties *config.DatabaseOptions, writer Writer) (*sql.DB, error)
 	default:
 		return nil, fmt.Errorf("invalid database connection string %q", dbProperties.ConnectionString)
 	}
-	if tracingEnabled {
-		// install the wrapped driver
-		driverName += "-trace"
-	}
 	db, err := sql.Open(driverName, dsn)
 	if err != nil {
 		return nil, err

internal/sqlutil/trace.go

@@ -1,109 +0,0 @@
-// Copyright 2020 The Matrix.org Foundation C.I.C.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package sqlutil
-
-import (
-	"context"
-	"database/sql/driver"
-	"fmt"
-	"io"
-	"os"
-	"runtime"
-	"strconv"
-	"strings"
-	"sync"
-	"time"
-
-	"github.com/ngrok/sqlmw"
-	"github.com/sirupsen/logrus"
-)
-
-var tracingEnabled = os.Getenv("DENDRITE_TRACE_SQL") == "1"
-var goidToWriter sync.Map
-
-type traceInterceptor struct {
-	sqlmw.NullInterceptor
-}
-
-func (in *traceInterceptor) StmtQueryContext(ctx context.Context, stmt driver.StmtQueryContext, query string, args []driver.NamedValue) (context.Context, driver.Rows, error) {
-	startedAt := time.Now()
-	rows, err := stmt.QueryContext(ctx, args)
-
-	trackGoID(query)
-
-	logrus.WithField("duration", time.Since(startedAt)).WithField(logrus.ErrorKey, err).Debug("executed sql query ", query, " args: ", args)
-
-	return ctx, rows, err
-}
-
-func (in *traceInterceptor) StmtExecContext(ctx context.Context, stmt driver.StmtExecContext, query string, args []driver.NamedValue) (driver.Result, error) {
-	startedAt := time.Now()
-	result, err := stmt.ExecContext(ctx, args)
-
-	trackGoID(query)
-
-	logrus.WithField("duration", time.Since(startedAt)).WithField(logrus.ErrorKey, err).Debug("executed sql query ", query, " args: ", args)
-
-	return result, err
-}
-
-func (in *traceInterceptor) RowsNext(c context.Context, rows driver.Rows, dest []driver.Value) error {
-	err := rows.Next(dest)
-	if err == io.EOF {
-		// For all cases, we call Next() n+1 times, the first to populate the initial dest, then eventually
-		// it will io.EOF. If we log on each Next() call we log the last element twice, so don't.
-		return err
-	}
-	cols := rows.Columns()
-	logrus.Debug(strings.Join(cols, " | "))
-
-	b := strings.Builder{}
-	for i, val := range dest {
-		b.WriteString(fmt.Sprintf("%q", val))
-		if i+1 <= len(dest)-1 {
-			b.WriteString(" | ")
-		}
-	}
-	logrus.Debug(b.String())
-	return err
-}
-
-func trackGoID(query string) {
-	thisGoID := goid()
-	if _, ok := goidToWriter.Load(thisGoID); ok {
-		return // we're on a writer goroutine
-	}
-
-	q := strings.TrimSpace(query)
-	if strings.HasPrefix(q, "SELECT") {
-		return // SELECTs can go on other goroutines
-	}
-	logrus.Warnf("unsafe goid %d: SQL executed not on an ExclusiveWriter: %s", thisGoID, q)
-}
-
-func init() {
-	registerDrivers()
-}
-
-func goid() int {
-	var buf [64]byte
-	n := runtime.Stack(buf[:], false)
-	idField := strings.Fields(strings.TrimPrefix(string(buf[:n]), "goroutine "))[0]
-	id, err := strconv.Atoi(idField)
-	if err != nil {
-		panic(fmt.Sprintf("cannot get goroutine id: %v", err))
-	}
-	return id
-}

internal/sqlutil/trace_driver.go

@@ -1,35 +0,0 @@
-// Copyright 2020 The Matrix.org Foundation C.I.C.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-//go:build !wasm
-// +build !wasm
-
-package sqlutil
-
-import (
-	"database/sql"
-
-	"github.com/lib/pq"
-	"github.com/ngrok/sqlmw"
-)
-
-func registerDrivers() {
-	if !tracingEnabled {
-		return
-	}
-	// install the wrapped drivers
-	sql.Register("postgres-trace", sqlmw.Driver(&pq.Driver{}, new(traceInterceptor)))
-	sql.Register("sqlite3-trace", sqlmw.Driver(sqliteDriver(), new(traceInterceptor)))
-
-}

internal/sqlutil/trace_driver_wasm.go

@@ -1,34 +0,0 @@
-// Copyright 2020 The Matrix.org Foundation C.I.C.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-//go:build wasm
-// +build wasm
-
-package sqlutil
-
-import (
-	"database/sql"
-
-	sqlitejs "github.com/matrix-org/go-sqlite3-js"
-	"github.com/ngrok/sqlmw"
-)
-
-func registerDrivers() {
-	if !tracingEnabled {
-		return
-	}
-	// install the wrapped drivers
-	sql.Register("sqlite3_js-trace", sqlmw.Driver(&sqlitejs.SqliteJsDriver{}, new(traceInterceptor)))
-
-}

internal/sqlutil/writer_exclusive.go

@@ -60,11 +60,6 @@ func (w *ExclusiveWriter) run() {
 	if !w.running.CompareAndSwap(false, true) {
 		return
 	}
-	if tracingEnabled {
-		gid := goid()
-		goidToWriter.Store(gid, w)
-		defer goidToWriter.Delete(gid)
-	}
 
 	defer w.running.Store(false)
 	for task := range w.todo {