From 52347c0a2698c0b2a21bcacaefe7512c640e7253 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Thu, 23 Mar 2023 12:58:23 +0100 Subject: [PATCH] Also remove the unused SQL trace driver --- docs/development/sytest.md | 1 - go.mod | 1 - go.sum | 2 - internal/sqlutil/sqlite_cgo.go | 5 -- internal/sqlutil/sqlite_native.go | 5 -- internal/sqlutil/sqlutil.go | 7 +- internal/sqlutil/trace.go | 109 -------------------------- internal/sqlutil/trace_driver.go | 35 --------- internal/sqlutil/trace_driver_wasm.go | 34 -------- internal/sqlutil/writer_exclusive.go | 5 -- 10 files changed, 1 insertion(+), 203 deletions(-) delete mode 100644 internal/sqlutil/trace.go delete mode 100644 internal/sqlutil/trace_driver.go delete mode 100644 internal/sqlutil/trace_driver_wasm.go diff --git a/docs/development/sytest.md b/docs/development/sytest.md index 3cfb99e60..4fae2ea3d 100644 --- a/docs/development/sytest.md +++ b/docs/development/sytest.md @@ -61,7 +61,6 @@ When debugging, the following Docker `run` options may also be useful: * `-e "DENDRITE_TRACE_HTTP=1"`: Adds HTTP tracing to server logs. * `-e "DENDRITE_TRACE_INTERNAL=1"`: Adds roomserver internal API tracing to server logs. -* `-e "DENDRITE_TRACE_SQL=1"`: Adds tracing to all SQL statements to server logs. The docker command also supports a single positional argument for the test file to run, so you can run a single `.pl` file rather than the whole test suite. For example: diff --git a/go.mod b/go.mod index cd9e0838f..7ac294f2a 100644 --- a/go.mod +++ b/go.mod @@ -30,7 +30,6 @@ require ( github.com/nats-io/nats.go v1.24.0 github.com/neilalexander/utp v0.1.1-0.20210727203401-54ae7b1cd5f9 github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646 - github.com/ngrok/sqlmw v0.0.0-20220520173518-97c9c04efc79 github.com/opentracing/opentracing-go v1.2.0 github.com/patrickmn/go-cache v2.1.0+incompatible github.com/pkg/errors v0.9.1 diff --git a/go.sum b/go.sum index 1484f8aef..bf8fbdf3d 100644 --- a/go.sum +++ b/go.sum @@ -368,8 +368,6 @@ github.com/neilalexander/utp v0.1.1-0.20210727203401-54ae7b1cd5f9 h1:lrVQzBtkeQE github.com/neilalexander/utp v0.1.1-0.20210727203401-54ae7b1cd5f9/go.mod h1:NPHGhPc0/wudcaCqL/H5AOddkRf8GPRhzOujuUKGQu8= github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646 h1:zYyBkD/k9seD2A7fsi6Oo2LfFZAehjjQMERAvZLEDnQ= github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646/go.mod h1:jpp1/29i3P1S/RLdc7JQKbRpFeM1dOBd8T9ki5s+AY8= -github.com/ngrok/sqlmw v0.0.0-20220520173518-97c9c04efc79 h1:Dmx8g2747UTVPzSkmohk84S3g/uWqd6+f4SSLPhLcfA= -github.com/ngrok/sqlmw v0.0.0-20220520173518-97c9c04efc79/go.mod h1:E26fwEtRNigBfFfHDWsklmo0T7Ixbg0XXgck+Hq4O9k= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/onsi/ginkgo/v2 v2.3.0 h1:kUMoxMoQG3ogk/QWyKh3zibV7BKZ+xBpWil1cTylVqc= github.com/onsi/ginkgo/v2 v2.3.0/go.mod h1:Eew0uilEqZmIEZr8JrvYlvOM7Rr6xzTmMV8AyFNU9d0= diff --git a/internal/sqlutil/sqlite_cgo.go b/internal/sqlutil/sqlite_cgo.go index efb743fc7..2fe2396ff 100644 --- a/internal/sqlutil/sqlite_cgo.go +++ b/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{} -} diff --git a/internal/sqlutil/sqlite_native.go b/internal/sqlutil/sqlite_native.go index ed500afc6..3c545b659 100644 --- a/internal/sqlutil/sqlite_native.go +++ b/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{} -} diff --git a/internal/sqlutil/sqlutil.go b/internal/sqlutil/sqlutil.go index 39a067e52..b2d0d2186 100644 --- a/internal/sqlutil/sqlutil.go +++ b/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 diff --git a/internal/sqlutil/trace.go b/internal/sqlutil/trace.go deleted file mode 100644 index 7b637106b..000000000 --- a/internal/sqlutil/trace.go +++ /dev/null @@ -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 -} diff --git a/internal/sqlutil/trace_driver.go b/internal/sqlutil/trace_driver.go deleted file mode 100644 index a2e0d12e2..000000000 --- a/internal/sqlutil/trace_driver.go +++ /dev/null @@ -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))) - -} diff --git a/internal/sqlutil/trace_driver_wasm.go b/internal/sqlutil/trace_driver_wasm.go deleted file mode 100644 index 51b60c3c8..000000000 --- a/internal/sqlutil/trace_driver_wasm.go +++ /dev/null @@ -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))) - -} diff --git a/internal/sqlutil/writer_exclusive.go b/internal/sqlutil/writer_exclusive.go index 8eff3ce55..c6a271c1c 100644 --- a/internal/sqlutil/writer_exclusive.go +++ b/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 {