Add verbose logging #804
Merged
Add verbose logging #804
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
d882fca to
919a694
Compare
There was a problem hiding this comment.
This works nicely as a CLI tool, but couples us tightly to the pterm.Logger type which will be less suitable when using pgroll as a dependency from other Go code. I think it's fine for now but we may want to revisit this.
A general suggestion would be to introduce an interface something like:
type OperationLogger interface {
// Operation lifecycle methods
LogOperationStart(op Operation)
LogOperationComplete(op Operation)
LogOperationRollback(op Operation)
// Specific logging events for common scenarios
LogBackfillStart(table string)
LogBackfillComplete(table string)
LogConstraintValidation(table, constraint string)
LogSchemaCreation(schemaName string)
// Generic methods for custom events
LogInfo(msg string, args ...any)
LogError(msg string, err error, args ...any)
}and implement it for pterm such that the different operations can do:
logger.LogOperationStart(o)rather than having to repeat themselves with "starting operation..." etc everywhere.
The loggerArgs methods could then move to the implementation of the OperationLogger interface:
func (l *ptermOperationLogger) extractOperationArgs(op Operation) []any {
// Extract common fields
var args []any
// Type-switch to extract operation-specific fields
switch o := op.(type) {
case *OpAddColumn:
args = append(args,
"operation", OpNameAddColumn,
"name", o.Column.Name,
"type", o.Column.Type,
"table", o.Table,
"nullable", o.Column.Nullable,
"unique", o.Column.Unique,
)
case *OpCreateTable:
args = append(args,
"operation", OpNameCreateTable,
"name", o.Name,
)
if len(o.Columns) > 0 {
columnNames := getColumnNames(o.Columns)
args = append(args, "columns", columnNames)
}
// Handle other operation types
}
return args
}this would keep the operations themselves relatively free of logging concerns. WDYT?
pkg/migrations/op_add_column.go
Outdated
| @@ -189,7 +194,9 @@ func (o *OpAddColumn) Complete(ctx context.Context, conn db.DB, s *schema.Schema | |||
| return nil | |||
| } | |||
|
|
|||
| func (o *OpAddColumn) Rollback(ctx context.Context, conn db.DB, s *schema.Schema) error { | |||
| func (o *OpAddColumn) Rollback(ctx context.Context, logger pterm.Logger, conn db.DB, s *schema.Schema) error { | |||
| logger.Info("completing operation", logger.Args(o.loggerArgs()...)) | |||
There was a problem hiding this comment.
should say rolling back operation
andrew-farries
approved these changes
Apr 29, 2025
|
As we agreed offline, I am implementing the requested changes in a follow-up PR. |
kvch
added a commit
that referenced
this pull request
Apr 30, 2025
As requested in #804, I added a `Logger` interface for verbose mode. I have kept the interface minimal. I only added an extra `Info` function. But the idea is that we can extend this interface when we need to emit more logs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new flag called
--verbose. If you set it,pgrollwill be more verbose.It logs the following events with contextual info to stdout:
Example outputs
Create multiple tables table
2025-04-29 12:12:32 INFO starting migration ├ name: 01_create_tables └ operation_count: 3 2025-04-29 12:12:32 INFO starting operation ├ operation: create_table ├ name: customers ├ columns: [id name credit_card] ├ comment: <nil> └ constraints: [] 2025-04-29 12:12:32 INFO starting operation ├ operation: create_table ├ name: bills ├ columns: [id date quantity] ├ comment: <nil> └ constraints: [] 2025-04-29 12:12:32 INFO starting operation ├ operation: create_table ├ name: sellers ├ columns: [name zip description] ├ comment: <nil> └ constraints: [] 2025-04-29 12:12:32 INFO created versioned schema for migration ├ migration: 01_create_tables └ schema_name: public_01_create_tables 2025-04-29 12:12:32 INFO completing migration ├ migration: 01_create_tables └ operation_count: 3 2025-04-29 12:12:32 INFO completing operation ├ operation: create_table ├ name: customers ├ columns: [id name credit_card] ├ comment: <nil> └ constraints: [] 2025-04-29 12:12:32 INFO completing operation ├ operation: create_table ├ name: bills ├ columns: [id date quantity] ├ comment: <nil> └ constraints: [] 2025-04-29 12:12:32 INFO completing operation ├ operation: create_table ├ name: sellers ├ columns: [name zip description] ├ comment: <nil> └ constraints: [] 2025-04-29 12:12:32 INFO completed migration ├ migration: 01_create_tables └ operation_count: 3 SUCCESS New version of the schema available under the postgres "public_01_create_tables" schemaAdd foreign key constraint
2025-04-29 12:12:34 INFO starting migration ├ name: 47_add_table_foreign_key_constraint └ operation_count: 1 2025-04-29 12:12:34 INFO starting operation ├ operation: create_constraint ├ table: tickets └ name: fk_sellers 2025-04-29 12:12:35 INFO created versioned schema for migration ├ migration: 47_add_table_foreign_key_constraint └ schema_name: public_47_add_table_foreign_key_constraint 2025-04-29 12:12:35 INFO backfilling started table: tickets 2025-04-29 12:12:35 INFO backfilling completed table: tickets 2025-04-29 12:12:35 INFO completing migration ├ migration: 47_add_table_foreign_key_constraint └ operation_count: 1 2025-04-29 12:12:35 INFO completing operation ├ operation: create_constraint ├ table: tickets └ name: fk_sellers 2025-04-29 12:12:35 INFO completing operation ├ operation: alter_column ├ table: tickets ├ column: └ references: fk_sellers 2025-04-29 12:12:35 INFO completed migration ├ migration: 47_add_table_foreign_key_constraint └ operation_count: 1 SUCCESS New version of the schema available under the postgres "public_47_add_table_foreign_key_constraint" schemaRollback
Closes #784