sa: refactor how metrics and logging are set up (#7031)

This eliminates the need for a pair of accessors on `db.WrappedMap` that
expose the underlying `*sql.DB` and `*borp.DbMap`.

Fixes #6991
This commit is contained in:
Jacob Hoffman-Andrews 2023-08-08 09:51:23 -07:00 committed by GitHub
parent 9a4f0ca678
commit 38fc840184
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 27 additions and 52 deletions

View File

@ -9,7 +9,6 @@ import (
"os"
"strings"
"github.com/go-sql-driver/mysql"
"github.com/prometheus/client_golang/prometheus"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
"google.golang.org/grpc/resolver"
@ -88,20 +87,6 @@ func (d *DBConfig) URL() (string, error) {
return strings.TrimSpace(string(url)), err
}
// DSNAddressAndUser returns the Address and User of the DBConnect DSN from
// this object.
func (d *DBConfig) DSNAddressAndUser() (string, string, error) {
dsnStr, err := d.URL()
if err != nil {
return "", "", fmt.Errorf("failed to load DBConnect URL: %s", err)
}
config, err := mysql.ParseDSN(dsnStr)
if err != nil {
return "", "", fmt.Errorf("failed to parse DSN from the DBConnect URL: %s", err)
}
return config.Addr, config.User, nil
}
type SMTPConfig struct {
PasswordConfig
Server string `validate:"required"`

View File

@ -937,15 +937,16 @@ type testCtx struct {
}
func setup(t *testing.T, nagTimes []time.Duration) *testCtx {
log := blog.NewMock()
// We use the test_setup user (which has full permissions to everything)
// because the SA we return is used for inserting data to set up the test.
dbMap, err := sa.DBMapForTest(vars.DBConnSAFullPerms)
dbMap, err := sa.DBMapForTestWithLog(vars.DBConnSAFullPerms, log)
if err != nil {
t.Fatalf("Couldn't connect the database: %s", err)
}
fc := clock.NewFake()
log := blog.NewMock()
ssa, err := sa.NewSQLStorageAuthority(dbMap, dbMap, nil, 1, 0, fc, log, metrics.NoopRegisterer)
if err != nil {
t.Fatalf("unable to create SQLStorageAuthority: %s", err)

View File

@ -56,7 +56,6 @@ func TestGetStartingID(t *testing.T) {
dbMap, err := sa.DBMapForTest(vars.DBConnSAFullPerms)
test.AssertNotError(t, err, "failed setting up db client")
defer test.ResetBoulderTestDatabase(t)()
sa.SetSQLDebug(dbMap, blog.Get())
cs := core.CertificateStatus{
Serial: "1337",

View File

@ -68,14 +68,6 @@ func NewWrappedMap(dbMap *borp.DbMap) *WrappedMap {
return &WrappedMap{dbMap: dbMap}
}
func (m *WrappedMap) SQLDb() *sql.DB {
return m.dbMap.Db
}
func (m *WrappedMap) BorpDB() *borp.DbMap {
return m.dbMap
}
func (m *WrappedMap) TableFor(t reflect.Type, checkPK bool) (*borp.TableMap, error) {
return m.dbMap.TableFor(t, checkPK)
}

View File

@ -67,26 +67,11 @@ func InitWrappedDb(config cmd.DBConfig, scope prometheus.Registerer, logger blog
return nil, err
}
dbMap, err := newDbMapFromMySQLConfig(mysqlConfig, settings)
dbMap, err := newDbMapFromMySQLConfig(mysqlConfig, settings, scope, logger)
if err != nil {
return nil, err
}
if logger != nil {
SetSQLDebug(dbMap, logger)
}
addr, user, err := config.DSNAddressAndUser()
if err != nil {
return nil, fmt.Errorf("while parsing DSN: %w", err)
}
if scope != nil {
err = initDBMetrics(dbMap.SQLDb(), scope, settings, addr, user)
if err != nil {
return nil, fmt.Errorf("while initializing metrics: %w", err)
}
}
return dbMap, nil
}
@ -95,6 +80,12 @@ func InitWrappedDb(config cmd.DBConfig, scope prometheus.Registerer, logger blog
// tables. It automatically maps the tables for the primary parts of Boulder
// around the Storage Authority.
func DBMapForTest(dbConnect string) (*boulderDB.WrappedMap, error) {
return DBMapForTestWithLog(dbConnect, nil)
}
// DBMapForTestWithLog does the same as DBMapForTest but also routes the debug logs
// from the database driver to the given log (usually a `blog.NewMock`).
func DBMapForTestWithLog(dbConnect string, log blog.Logger) (*boulderDB.WrappedMap, error) {
var err error
var config *mysql.Config
@ -103,7 +94,7 @@ func DBMapForTest(dbConnect string) (*boulderDB.WrappedMap, error) {
return nil, err
}
return newDbMapFromMySQLConfig(config, DbSettings{})
return newDbMapFromMySQLConfig(config, DbSettings{}, nil, log)
}
// sqlOpen is used in the tests to check that the arguments are properly
@ -148,7 +139,10 @@ var setConnMaxIdleTime = func(db *sql.DB, connMaxIdleTime time.Duration) {
// - pings the database (and errors if it's unreachable)
// - wraps the connection in a borp.DbMap so we can use the handy Get/Insert methods borp provides
// - wraps that in a db.WrappedMap to get more useful error messages
func newDbMapFromMySQLConfig(config *mysql.Config, settings DbSettings) (*boulderDB.WrappedMap, error) {
//
// If logger is non-nil, it will receive debug log messages from borp.
// If scope is non-nil, it will be used to register Prometheus metrics.
func newDbMapFromMySQLConfig(config *mysql.Config, settings DbSettings, scope prometheus.Registerer, logger blog.Logger) (*boulderDB.WrappedMap, error) {
err := adjustMySQLConfig(config)
if err != nil {
return nil, err
@ -166,9 +160,20 @@ func newDbMapFromMySQLConfig(config *mysql.Config, settings DbSettings) (*boulde
setConnMaxLifetime(db, settings.ConnMaxLifetime)
setConnMaxIdleTime(db, settings.ConnMaxIdleTime)
if scope != nil {
err = initDBMetrics(db, scope, settings, config.Addr, config.User)
if err != nil {
return nil, fmt.Errorf("while initializing metrics: %w", err)
}
}
dialect := borp.MySQLDialect{Engine: "InnoDB", Encoding: "UTF8"}
dbmap := &borp.DbMap{Db: db, Dialect: dialect, TypeConverter: BoulderTypeConverter{}}
if logger != nil {
dbmap.TraceOn("SQL: ", &SQLLogger{logger})
}
initTables(dbmap)
return boulderDB.NewWrappedMap(dbmap), nil
}
@ -239,17 +244,12 @@ func adjustMySQLConfig(conf *mysql.Config) error {
return nil
}
// SetSQLDebug enables borp SQL-level Debugging
func SetSQLDebug(dbMap *boulderDB.WrappedMap, log blog.Logger) {
dbMap.BorpDB().TraceOn("SQL: ", &SQLLogger{log})
}
// SQLLogger adapts the Boulder Logger to a format borp can use.
type SQLLogger struct {
blog.Logger
}
// Printf adapts the AuditLogger to borp's interface
// Printf adapts the Logger to borp's interface
func (log *SQLLogger) Printf(format string, v ...interface{}) {
log.Debugf(format, v...)
}

View File

@ -56,8 +56,6 @@ func NewSQLStorageAuthorityWrapping(
dbMap *db.WrappedMap,
stats prometheus.Registerer,
) (*SQLStorageAuthority, error) {
SetSQLDebug(dbMap, ssaro.log)
rateLimitWriteErrors := prometheus.NewCounter(prometheus.CounterOpts{
Name: "rate_limit_write_errors",
Help: "number of failed ratelimit update transactions during AddCertificate",