From d52efdce94eb95ecef066ea3b81e35ca09c6edc3 Mon Sep 17 00:00:00 2001 From: William Perron Date: Fri, 23 Feb 2024 17:56:35 -0500 Subject: [PATCH] fix lints and update logger --- absences.go | 9 ++------- absences_test.go | 5 +++-- audit_log.go | 1 - cmd/themis-server/main.go | 7 +++++-- conflicts_test.go | 3 ++- correlation/opentelemetry.go | 2 +- fmt_test.go | 7 ++++--- store.go | 30 ++++++++++++++++-------------- store_test.go | 13 +++++++------ 9 files changed, 40 insertions(+), 37 deletions(-) diff --git a/absences.go b/absences.go index 51ba5b2..0e6f61b 100644 --- a/absences.go +++ b/absences.go @@ -2,7 +2,6 @@ package themis import ( "context" - "errors" "fmt" "time" @@ -25,7 +24,7 @@ func (s *Store) AddAbsence(ctx context.Context, session time.Time, userId string if session.Weekday() != time.Monday { log.Debug().Ctx(ctx).Msg(fmt.Sprintf("%s is not a monday", session)) - span.RecordError(errors.New(fmt.Sprintf("%s is not a monday", session))) + span.RecordError(fmt.Errorf("%s is not a monday", session)) return fmt.Errorf("not a monday") } @@ -154,11 +153,7 @@ func (s *Store) GetSchedule(ctx context.Context, from, to time.Time) (Schedule, return nil, fmt.Errorf("failed to scan row: %w", err) } - if _, ok := schedule[date]; ok { - schedule[date] = append(schedule[date], user) - } else { - schedule[date] = []string{user} - } + schedule[date] = append(schedule[date], user) } return schedule, nil diff --git a/absences_test.go b/absences_test.go index 7d762be..24d5dff 100644 --- a/absences_test.go +++ b/absences_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -14,7 +15,7 @@ import ( func TestAddAbsence(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "TestAddAbsence")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) require.NoError(t, err) now := NextMonday() @@ -36,7 +37,7 @@ func TestAddAbsence(t *testing.T) { func TestGetSchedule(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "TestGetSchedule")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) require.NoError(t, err) now := NextMonday() diff --git a/audit_log.go b/audit_log.go index 7645258..5144c54 100644 --- a/audit_log.go +++ b/audit_log.go @@ -55,7 +55,6 @@ func EventTypeFromString(ev string) (EventType, error) { type AuditableEvent struct { userId string eventType EventType - timestamp time.Time err error } diff --git a/cmd/themis-server/main.go b/cmd/themis-server/main.go index 51bd6a7..c686c83 100644 --- a/cmd/themis-server/main.go +++ b/cmd/themis-server/main.go @@ -86,7 +86,7 @@ func main() { log.Fatal().Err(err).Msg("failed to open database") } - store, err = themis.NewStore(db) + store, err = themis.NewStore(db, *zerolog.DefaultContextLogger) if err != nil { log.Fatal().Err(err).Msg("failed to initialize database") } @@ -785,7 +785,7 @@ func registerHandlers(sess *discordgo.Session, handlers map[string]Handler) { defer cancel() if h, ok := handlers[i.ApplicationCommandData().Name]; ok { - inSpan(i.ApplicationCommandData().Name, withLogging(i.ApplicationCommandData().Name, h))(ctx, s, i) + _ = inSpan(i.ApplicationCommandData().Name, withLogging(i.ApplicationCommandData().Name, h))(ctx, s, i) } case discordgo.InteractionModalSubmit: ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) @@ -1033,6 +1033,9 @@ func initTracing(ctx context.Context, db *sql.DB) error { resource.WithTelemetrySDK(), resource.WithAttributes(semconv.ServiceName("themis")), ) + if err != nil { + return fmt.Errorf("failed to create resource: %w", err) + } ex, err := sqliteexporter.NewSqliteSDKTraceExporterWithDB(db) if err != nil { diff --git a/conflicts_test.go b/conflicts_test.go index 7469f97..0d4e553 100644 --- a/conflicts_test.go +++ b/conflicts_test.go @@ -7,6 +7,7 @@ import ( "reflect" "testing" + "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -14,7 +15,7 @@ import ( func TestStore_FindConflicts(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "TestStore_FindConflicts")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) assert.NoError(t, err) id, err := store.Claim(context.Background(), "000000000000000001", "foo", "Bordeaux", CLAIM_TYPE_TRADE) diff --git a/correlation/opentelemetry.go b/correlation/opentelemetry.go index 0829a65..da0bd9c 100644 --- a/correlation/opentelemetry.go +++ b/correlation/opentelemetry.go @@ -32,7 +32,7 @@ func (u UrlValuesCarrier) Get(key string) string { func (u UrlValuesCarrier) Keys() []string { raw := map[string][]string(u) ks := make([]string, 0, len(raw)) - for k, _ := range raw { + for k := range raw { ks = append(ks, k) } return ks diff --git a/fmt_test.go b/fmt_test.go index 75d8d33..b0fb9da 100644 --- a/fmt_test.go +++ b/fmt_test.go @@ -6,6 +6,7 @@ import ( "fmt" "testing" + "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -13,7 +14,7 @@ import ( func TestFormatRows(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "format-rows")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) assert.NoError(t, err) rows, err := store.db.Query("SELECT provinces.name, provinces.region, provinces.area, provinces.trade_node FROM provinces WHERE area = 'Gascony'") @@ -33,7 +34,7 @@ func TestFormatRows(t *testing.T) { func TestFormatRowsAggregated(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "format-rows")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) assert.NoError(t, err) rows, err := store.db.Query("SELECT count(1) as total, trade_node from provinces where region = 'France' group by trade_node") @@ -54,7 +55,7 @@ func TestFormatRowsAggregated(t *testing.T) { func TestFormatRowsInvalidQuery(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "format-rows")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) assert.NoError(t, err) _, err = store.db.Query("SELECT count(name), distinct(trade_node) from provinces where region = 'France'") diff --git a/store.go b/store.go index 30ccb45..a725e61 100644 --- a/store.go +++ b/store.go @@ -13,7 +13,7 @@ import ( "github.com/golang-migrate/migrate/v4/database/sqlite3" "github.com/golang-migrate/migrate/v4/source/iofs" _ "github.com/mattn/go-sqlite3" - "github.com/rs/zerolog/log" + "github.com/rs/zerolog" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" @@ -32,10 +32,11 @@ func init() { } type Store struct { - db *sql.DB + db *sql.DB + logger zerolog.Logger } -func NewStore(db *sql.DB) (*Store, error) { +func NewStore(db *sql.DB, logger zerolog.Logger) (*Store, error) { d, err := iofs.New(migrations, "migrations") if err != nil { return nil, fmt.Errorf("failed to open iofs migration source: %w", err) @@ -61,15 +62,16 @@ func NewStore(db *sql.DB) (*Store, error) { return nil, fmt.Errorf("failed to get database migration version: %w", err) } - log.Debug().Uint("current_version", ver).Bool("dirty", dirty).Msg("running database migrations") + logger.Debug().Uint("current_version", ver).Bool("dirty", dirty).Msg("running database migrations") return &Store{ - db: db, + logger: logger, + db: db, }, nil } func (s *Store) Close() error { - log.Debug().Msg("closing database") + s.logger.Debug().Msg("closing database") return s.db.Close() } @@ -84,7 +86,7 @@ func (s *Store) Claim(ctx context.Context, userId, player, province string, clai )) defer span.End() - log.Debug(). + s.logger.Debug(). Ctx(ctx). Str("userid", userId). Str("player", player). @@ -115,7 +117,7 @@ func (s *Store) Claim(ctx context.Context, userId, player, province string, clai } if len(conflicts) > 0 { - log.Debug().Ctx(ctx).Int("len", len(conflicts)).Msg("found conflicts") + s.logger.Debug().Ctx(ctx).Int("len", len(conflicts)).Msg("found conflicts") audit.err = errors.New("found conflicts") return 0, ErrConflict{Conflicts: conflicts} } @@ -185,7 +187,7 @@ func (s *Store) ListAvailability(ctx context.Context, claimType ClaimType, searc )) defer span.End() - log.Debug().Ctx(ctx).Stringer("claim_type", claimType).Strs("search_terms", search).Msg("listing available entries") + s.logger.Debug().Ctx(ctx).Stringer("claim_type", claimType).Strs("search_terms", search).Msg("listing available entries") queryParams := []any{string(claimType)} queryPattern := `SELECT distinct name @@ -237,7 +239,7 @@ func (s *Store) ListClaims(ctx context.Context) ([]Claim, error) { )) defer span.End() - log.Debug().Ctx(ctx).Msg("listing all claims currently in database") + s.logger.Debug().Ctx(ctx).Msg("listing all claims currently in database") stmt, err := s.db.PrepareContext(ctx, `SELECT id, player, claim_type, val FROM claims`) if err != nil { span.RecordError(err) @@ -300,7 +302,7 @@ func (s *Store) DescribeClaim(ctx context.Context, ID int) (ClaimDetail, error) )) defer span.End() - log.Debug().Ctx(ctx).Int("id", ID).Msg("describing claim") + s.logger.Debug().Ctx(ctx).Int("id", ID).Msg("describing claim") stmt, err := s.db.PrepareContext(ctx, `SELECT id, player, claim_type, val FROM claims WHERE id = ?`) if err != nil { span.RecordError(err) @@ -374,7 +376,7 @@ func (s *Store) DeleteClaim(ctx context.Context, ID int, userId string) error { )) defer span.End() - log.Debug().Ctx(ctx).Str("userid", userId).Int("id", ID).Msg("deleting claim") + s.logger.Debug().Ctx(ctx).Str("userid", userId).Int("id", ID).Msg("deleting claim") audit := &AuditableEvent{ userId: userId, eventType: EventUnclaim, @@ -421,7 +423,7 @@ func (s *Store) CountClaims(ctx context.Context) (total, uniquePlayers int, err )) defer span.End() - log.Debug().Ctx(ctx).Msg("counting all claims and unique users") + s.logger.Debug().Ctx(ctx).Msg("counting all claims and unique users") stmt, err := s.db.PrepareContext(ctx, "SELECT COUNT(1), COUNT(DISTINCT(userid)) FROM claims") if err != nil { span.RecordError(err) @@ -449,7 +451,7 @@ func (s *Store) Flush(ctx context.Context, userId string) error { )) defer span.End() - log.Debug().Ctx(ctx).Str("initiated_by", userId).Msg("flushing all currently help claims") + s.logger.Debug().Ctx(ctx).Str("initiated_by", userId).Msg("flushing all currently help claims") audit := &AuditableEvent{ userId: userId, eventType: EventFlush, diff --git a/store_test.go b/store_test.go index 79ead17..1269b89 100644 --- a/store_test.go +++ b/store_test.go @@ -9,6 +9,7 @@ import ( "testing" _ "github.com/mattn/go-sqlite3" + "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -18,7 +19,7 @@ const TEST_CONN_STRING_PATTERN = "file:%s?mode=memory&cache=shared" func TestStore_Claim(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "TestStore_Claim")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) require.NoError(t, err) type args struct { @@ -111,7 +112,7 @@ func TestStore_Claim(t *testing.T) { func TestAvailability(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "TestStore_Availability")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) assert.NoError(t, err) store.Claim(context.TODO(), "000000000000000001", "foo", "Genoa", CLAIM_TYPE_TRADE) @@ -160,7 +161,7 @@ func TestAvailability(t *testing.T) { func TestDeleteClaim(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "TestStore_DeleteClaim")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) assert.NoError(t, err) // make sure all claims are gone, this is due to how the in-memory database // with a shared cache interacts with other tests running in parallel @@ -194,7 +195,7 @@ func TestDeleteClaim(t *testing.T) { func TestDescribeClaim(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "TestStore_DescribeClaim")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) assert.NoError(t, err) id, err := store.Claim(context.TODO(), "000000000000000001", "foo", "Genoa", CLAIM_TYPE_TRADE) @@ -212,7 +213,7 @@ func TestDescribeClaim(t *testing.T) { func TestCountClaims(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "TestStore_CountClaim")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) assert.NoError(t, err) store.Claim(context.TODO(), "000000000000000001", "foo", "Genoa", CLAIM_TYPE_TRADE) @@ -230,7 +231,7 @@ func TestCountClaims(t *testing.T) { func TestFlush(t *testing.T) { db, err := sql.Open("sqlite3", fmt.Sprintf(TEST_CONN_STRING_PATTERN, "TestStore_Flush")) require.NoError(t, err) - store, err := NewStore(db) + store, err := NewStore(db, zerolog.Nop()) assert.NoError(t, err) store.Claim(context.TODO(), "000000000000000001", "foo", "Genoa", CLAIM_TYPE_TRADE)