From 68ba050c49ccc91776c248bafd12a12c38251aff Mon Sep 17 00:00:00 2001 From: Christopher Schleiden Date: Thu, 7 Sep 2023 17:03:17 -0700 Subject: [PATCH 1/8] Adopt slog.Logger This removes the custom logger interface and adopts the new go 1.21 structured logger --- activity/logger.go | 4 +- activitytester/activitytester.go | 7 +- backend/backend.go | 4 +- backend/mock_Backend.go | 23 +++--- backend/mysql/mysql.go | 4 +- backend/options.go | 11 ++- backend/redis/redis.go | 4 +- backend/sqlite/sqlite.go | 4 +- bench/main.go | 3 +- bench/nooplog.go | 33 ++++---- client/client_test.go | 6 +- go.mod | 2 +- internal/activity/activitystate.go | 5 +- internal/activity/executor.go | 5 +- internal/activity/executor_test.go | 4 +- internal/logger/logger.go | 62 --------------- internal/worker/activity.go | 7 +- internal/worker/workflow.go | 12 +-- internal/workflow/cache/cache_test.go | 11 ++- internal/workflow/executor.go | 11 ++- internal/workflow/executor_test.go | 4 +- internal/workflowstate/replaylogger.go | 51 ++++++------- internal/workflowstate/workflowstate.go | 7 +- internal/workflowstate/workflowstate_test.go | 4 +- log/logger.go | 12 --- samples/errors/errors.go | 4 +- tester/options.go | 6 +- tester/tester.go | 6 +- tester/tester_logger_test.go | 79 +++----------------- tester/tester_timers_test.go | 4 +- workflow/activity_test.go | 6 +- workflow/logger.go | 5 +- workflow/subworkflow_test.go | 6 +- workflow/timer_test.go | 4 +- 34 files changed, 151 insertions(+), 269 deletions(-) delete mode 100644 internal/logger/logger.go delete mode 100644 log/logger.go diff --git a/activity/logger.go b/activity/logger.go index f39835c0..3974dc33 100644 --- a/activity/logger.go +++ b/activity/logger.go @@ -2,12 +2,12 @@ package activity import ( "context" + "log/slog" "github.com/cschleiden/go-workflows/internal/activity" - "github.com/cschleiden/go-workflows/log" ) // Logger returns a logger with the workflow instance this activity is executed for set as default fields -func Logger(ctx context.Context) log.Logger { +func Logger(ctx context.Context) *slog.Logger { return activity.GetActivityState(ctx).Logger } diff --git a/activitytester/activitytester.go b/activitytester/activitytester.go index e8cfda91..b84b309a 100644 --- a/activitytester/activitytester.go +++ b/activitytester/activitytester.go @@ -2,16 +2,15 @@ package activitytester import ( "context" + "log/slog" "github.com/cschleiden/go-workflows/internal/activity" "github.com/cschleiden/go-workflows/internal/core" - dlogger "github.com/cschleiden/go-workflows/internal/logger" - "github.com/cschleiden/go-workflows/log" ) -func WithActivityTestState(ctx context.Context, activityID, instanceID string, logger log.Logger) context.Context { +func WithActivityTestState(ctx context.Context, activityID, instanceID string, logger *slog.Logger) context.Context { if logger == nil { - logger = dlogger.NewDefaultLogger() + logger = slog.Default() } return activity.WithActivityState(ctx, activity.NewActivityState(activityID, core.NewWorkflowInstance(instanceID, ""), logger)) diff --git a/backend/backend.go b/backend/backend.go index afd40756..20e099fc 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -3,13 +3,13 @@ package backend import ( "context" "errors" + "log/slog" "github.com/cschleiden/go-workflows/internal/contextpropagation" "github.com/cschleiden/go-workflows/internal/converter" core "github.com/cschleiden/go-workflows/internal/core" "github.com/cschleiden/go-workflows/internal/history" "github.com/cschleiden/go-workflows/internal/task" - "github.com/cschleiden/go-workflows/log" "github.com/cschleiden/go-workflows/metrics" "github.com/cschleiden/go-workflows/workflow" "go.opentelemetry.io/otel/trace" @@ -72,7 +72,7 @@ type Backend interface { GetStats(ctx context.Context) (*Stats, error) // Logger returns the configured logger for the backend - Logger() log.Logger + Logger() *slog.Logger // Tracer returns the configured trace provider for the backend Tracer() trace.Tracer diff --git a/backend/mock_Backend.go b/backend/mock_Backend.go index dc441c4b..52f3cf7b 100644 --- a/backend/mock_Backend.go +++ b/backend/mock_Backend.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.30.1. DO NOT EDIT. +// Code generated by mockery v2.20.0. DO NOT EDIT. package backend @@ -12,12 +12,12 @@ import ( history "github.com/cschleiden/go-workflows/internal/history" - log "github.com/cschleiden/go-workflows/log" - metrics "github.com/cschleiden/go-workflows/metrics" mock "github.com/stretchr/testify/mock" + slog "log/slog" + task "github.com/cschleiden/go-workflows/internal/task" trace "go.opentelemetry.io/otel/trace" @@ -273,15 +273,15 @@ func (_m *MockBackend) GetWorkflowTask(ctx context.Context) (*task.Workflow, err } // Logger provides a mock function with given fields: -func (_m *MockBackend) Logger() log.Logger { +func (_m *MockBackend) Logger() *slog.Logger { ret := _m.Called() - var r0 log.Logger - if rf, ok := ret.Get(0).(func() log.Logger); ok { + var r0 *slog.Logger + if rf, ok := ret.Get(0).(func() *slog.Logger); ok { r0 = rf() } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(log.Logger) + r0 = ret.Get(0).(*slog.Logger) } } @@ -348,12 +348,13 @@ func (_m *MockBackend) Tracer() trace.Tracer { return r0 } -// NewMockBackend creates a new instance of MockBackend. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. -func NewMockBackend(t interface { +type mockConstructorTestingTNewMockBackend interface { mock.TestingT Cleanup(func()) -}) *MockBackend { +} + +// NewMockBackend creates a new instance of MockBackend. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewMockBackend(t mockConstructorTestingTNewMockBackend) *MockBackend { mock := &MockBackend{} mock.Mock.Test(t) diff --git a/backend/mysql/mysql.go b/backend/mysql/mysql.go index fd63e0b4..7f6d420a 100644 --- a/backend/mysql/mysql.go +++ b/backend/mysql/mysql.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "strings" "time" @@ -17,7 +18,6 @@ import ( "github.com/cschleiden/go-workflows/internal/history" "github.com/cschleiden/go-workflows/internal/metrickeys" "github.com/cschleiden/go-workflows/internal/task" - "github.com/cschleiden/go-workflows/log" "github.com/cschleiden/go-workflows/metrics" "github.com/cschleiden/go-workflows/workflow" _ "github.com/go-sql-driver/mysql" @@ -63,7 +63,7 @@ type mysqlBackend struct { options backend.Options } -func (b *mysqlBackend) Logger() log.Logger { +func (b *mysqlBackend) Logger() *slog.Logger { return b.options.Logger } diff --git a/backend/options.go b/backend/options.go index 155f1186..e1040de1 100644 --- a/backend/options.go +++ b/backend/options.go @@ -1,21 +1,20 @@ package backend import ( + "log/slog" "time" "github.com/cschleiden/go-workflows/internal/contextpropagation" "github.com/cschleiden/go-workflows/internal/converter" - "github.com/cschleiden/go-workflows/internal/logger" mi "github.com/cschleiden/go-workflows/internal/metrics" "github.com/cschleiden/go-workflows/internal/tracing" - "github.com/cschleiden/go-workflows/log" "github.com/cschleiden/go-workflows/metrics" "github.com/cschleiden/go-workflows/workflow" "go.opentelemetry.io/otel/trace" ) type Options struct { - Logger log.Logger + Logger *slog.Logger Metrics metrics.Client @@ -46,7 +45,7 @@ var DefaultOptions Options = Options{ WorkflowLockTimeout: time.Minute, ActivityLockTimeout: time.Minute * 2, - Logger: logger.NewDefaultLogger(), + Logger: slog.Default(), Metrics: mi.NewNoopMetricsClient(), TracerProvider: trace.NewNoopTracerProvider(), Converter: converter.DefaultConverter, @@ -62,7 +61,7 @@ func WithStickyTimeout(timeout time.Duration) BackendOption { } } -func WithLogger(logger log.Logger) BackendOption { +func WithLogger(logger *slog.Logger) BackendOption { return func(o *Options) { o.Logger = logger } @@ -100,7 +99,7 @@ func ApplyOptions(opts ...BackendOption) Options { } if options.Logger == nil { - options.Logger = logger.NewDefaultLogger() + options.Logger = slog.Default() } return options diff --git a/backend/redis/redis.go b/backend/redis/redis.go index afa12a0e..25ccc407 100644 --- a/backend/redis/redis.go +++ b/backend/redis/redis.go @@ -3,6 +3,7 @@ package redis import ( "context" "fmt" + "log/slog" "time" "github.com/cschleiden/go-workflows/backend" @@ -11,7 +12,6 @@ import ( "github.com/cschleiden/go-workflows/internal/core" "github.com/cschleiden/go-workflows/internal/history" "github.com/cschleiden/go-workflows/internal/metrickeys" - "github.com/cschleiden/go-workflows/log" "github.com/cschleiden/go-workflows/metrics" "github.com/redis/go-redis/v9" "go.opentelemetry.io/otel/trace" @@ -86,7 +86,7 @@ type activityData struct { Event *history.Event `json:"event,omitempty"` } -func (rb *redisBackend) Logger() log.Logger { +func (rb *redisBackend) Logger() *slog.Logger { return rb.options.Logger } diff --git a/backend/sqlite/sqlite.go b/backend/sqlite/sqlite.go index ced1c3fc..b5f9ee62 100644 --- a/backend/sqlite/sqlite.go +++ b/backend/sqlite/sqlite.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "strings" "time" @@ -17,7 +18,6 @@ import ( "github.com/cschleiden/go-workflows/internal/history" "github.com/cschleiden/go-workflows/internal/metrickeys" "github.com/cschleiden/go-workflows/internal/task" - "github.com/cschleiden/go-workflows/log" "github.com/cschleiden/go-workflows/metrics" "github.com/cschleiden/go-workflows/workflow" "github.com/google/uuid" @@ -67,7 +67,7 @@ type sqliteBackend struct { var _ backend.Backend = (*sqliteBackend)(nil) -func (sb *sqliteBackend) Logger() log.Logger { +func (sb *sqliteBackend) Logger() *slog.Logger { return sb.options.Logger } diff --git a/bench/main.go b/bench/main.go index 872c2622..985dd58c 100644 --- a/bench/main.go +++ b/bench/main.go @@ -6,6 +6,7 @@ import ( "flag" "fmt" "log" + "log/slog" "os" "sync" "time" @@ -38,7 +39,7 @@ func main() { defer cancel() mm := newMemMetrics() - ba := getBackend(*b, backend.WithLogger(&nullLogger{}), backend.WithMetrics(mm)) + ba := getBackend(*b, backend.WithLogger(slog.New(&nullHandler{})), backend.WithMetrics(mm)) wo := worker.DefaultWorkerOptions wo.WorkflowExecutorCacheSize = *cacheSize diff --git a/bench/nooplog.go b/bench/nooplog.go index a0dd1086..74081eb0 100644 --- a/bench/nooplog.go +++ b/bench/nooplog.go @@ -1,30 +1,31 @@ package main -import "github.com/cschleiden/go-workflows/log" +import ( + "context" + "log/slog" +) -type nullLogger struct { - defaultFields []interface{} +type nullHandler struct { } -// Debug implements log.Logger -func (*nullLogger) Debug(msg string, fields ...interface{}) { +// Enabled implements slog.Handler. +func (*nullHandler) Enabled(context.Context, slog.Level) bool { + return false } -// Error implements log.Logger -func (*nullLogger) Error(msg string, fields ...interface{}) { +// Handle implements slog.Handler. +func (*nullHandler) Handle(context.Context, slog.Record) error { + return nil } -// Panic implements log.Logger -func (*nullLogger) Panic(msg string, fields ...interface{}) { -} - -// Warn implements log.Logger -func (*nullLogger) Warn(msg string, fields ...interface{}) { +// WithAttrs implements slog.Handler. +func (nl *nullHandler) WithAttrs(attrs []slog.Attr) slog.Handler { + return nl } -// With implements log.Logger -func (nl *nullLogger) With(fields ...interface{}) log.Logger { +// WithGroup implements slog.Handler. +func (nl *nullHandler) WithGroup(name string) slog.Handler { return nl } -var _ log.Logger = (*nullLogger)(nil) +var _ slog.Handler = (*nullHandler)(nil) diff --git a/client/client_test.go b/client/client_test.go index fda0fee8..f9e47e4a 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -3,6 +3,7 @@ package client import ( "bytes" "context" + "log/slog" "testing" "time" @@ -11,7 +12,6 @@ import ( "github.com/cschleiden/go-workflows/internal/converter" "github.com/cschleiden/go-workflows/internal/core" "github.com/cschleiden/go-workflows/internal/history" - "github.com/cschleiden/go-workflows/internal/logger" "github.com/cschleiden/go-workflows/workflow" "github.com/google/uuid" "github.com/stretchr/testify/mock" @@ -103,7 +103,7 @@ func Test_Client_SignalWorkflow(t *testing.T) { b := &backend.MockBackend{} b.On("Tracer").Return(trace.NewNoopTracerProvider().Tracer("test")) - b.On("Logger").Return(logger.NewDefaultLogger()) + b.On("Logger").Return(slog.Default()) b.On("Converter").Return(converter.DefaultConverter) b.On("SignalWorkflow", mock.Anything, instanceID, mock.MatchedBy(func(event *history.Event) bool { return event.Type == history.EventType_SignalReceived && @@ -132,7 +132,7 @@ func Test_Client_SignalWorkflow_WithArgs(t *testing.T) { b := &backend.MockBackend{} b.On("Tracer").Return(trace.NewNoopTracerProvider().Tracer("test")) - b.On("Logger").Return(logger.NewDefaultLogger()) + b.On("Logger").Return(slog.Default()) b.On("Converter").Return(converter.DefaultConverter) b.On("SignalWorkflow", mock.Anything, instanceID, mock.MatchedBy(func(event *history.Event) bool { return event.Type == history.EventType_SignalReceived && diff --git a/go.mod b/go.mod index 5668d8b5..a9f1837c 100644 --- a/go.mod +++ b/go.mod @@ -76,7 +76,7 @@ require ( github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect github.com/esimonov/ifshort v1.0.4 // indirect github.com/ettle/strcase v0.1.1 // indirect - github.com/fatih/color v1.14.1 + github.com/fatih/color v1.14.1 // indirect github.com/fatih/structtag v1.2.0 // indirect github.com/fsnotify/fsnotify v1.5.4 // indirect github.com/fzipp/gocyclo v0.6.0 // indirect diff --git a/internal/activity/activitystate.go b/internal/activity/activitystate.go index 53794ef8..6200767f 100644 --- a/internal/activity/activitystate.go +++ b/internal/activity/activitystate.go @@ -2,6 +2,7 @@ package activity import ( "context" + "log/slog" "github.com/cschleiden/go-workflows/log" "github.com/cschleiden/go-workflows/workflow" @@ -10,10 +11,10 @@ import ( type ActivityState struct { ActivityID string Instance *workflow.Instance - Logger log.Logger + Logger *slog.Logger } -func NewActivityState(activityID string, instance *workflow.Instance, logger log.Logger) *ActivityState { +func NewActivityState(activityID string, instance *workflow.Instance, logger *slog.Logger) *ActivityState { return &ActivityState{ activityID, instance, diff --git a/internal/activity/executor.go b/internal/activity/executor.go index 60f4751c..b0e97c8b 100644 --- a/internal/activity/executor.go +++ b/internal/activity/executor.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "reflect" "github.com/cschleiden/go-workflows/internal/args" @@ -20,14 +21,14 @@ import ( ) type Executor struct { - logger log.Logger + logger *slog.Logger tracer trace.Tracer converter converter.Converter propagators []contextpropagation.ContextPropagator r *workflow.Registry } -func NewExecutor(logger log.Logger, tracer trace.Tracer, converter converter.Converter, propagators []contextpropagation.ContextPropagator, r *workflow.Registry) *Executor { +func NewExecutor(logger *slog.Logger, tracer trace.Tracer, converter converter.Converter, propagators []contextpropagation.ContextPropagator, r *workflow.Registry) *Executor { return &Executor{ logger: logger, tracer: tracer, diff --git a/internal/activity/executor_test.go b/internal/activity/executor_test.go index 293cf829..17972272 100644 --- a/internal/activity/executor_test.go +++ b/internal/activity/executor_test.go @@ -3,6 +3,7 @@ package activity import ( "context" "errors" + "log/slog" "testing" "time" @@ -11,7 +12,6 @@ import ( "github.com/cschleiden/go-workflows/internal/core" "github.com/cschleiden/go-workflows/internal/fn" "github.com/cschleiden/go-workflows/internal/history" - "github.com/cschleiden/go-workflows/internal/logger" "github.com/cschleiden/go-workflows/internal/payload" "github.com/cschleiden/go-workflows/internal/task" "github.com/cschleiden/go-workflows/internal/workflow" @@ -111,7 +111,7 @@ func TestExecutor_ExecuteActivity(t *testing.T) { attr := tt.setup(t, r) e := &Executor{ - logger: logger.NewDefaultLogger(), + logger: slog.Default(), r: r, converter: converter.DefaultConverter, tracer: trace.NewNoopTracerProvider().Tracer(""), diff --git a/internal/logger/logger.go b/internal/logger/logger.go deleted file mode 100644 index b25110c0..00000000 --- a/internal/logger/logger.go +++ /dev/null @@ -1,62 +0,0 @@ -package logger - -import ( - "fmt" - "log" - - lg "github.com/cschleiden/go-workflows/log" - "github.com/fatih/color" -) - -type defaultLogger struct { - defaultFields []interface{} -} - -var _ lg.Logger = (*defaultLogger)(nil) - -func NewDefaultLogger() lg.Logger { - return &defaultLogger{} -} - -func (dl *defaultLogger) Debug(msg string, fields ...interface{}) { - log.Println(dl.formatFields("DEBUG", msg, fields...)...) -} - -func (dl *defaultLogger) Warn(msg string, fields ...interface{}) { - log.Println(dl.formatFields("WARN", msg, fields...)...) -} - -func (dl *defaultLogger) Error(msg string, fields ...interface{}) { - log.Println(dl.formatFields("ERROR", msg, fields...)...) -} - -func (dl *defaultLogger) Panic(msg string, fields ...interface{}) { - log.Panicln(dl.formatFields("PANIC", msg, fields...)...) -} - -func (dl *defaultLogger) With(fields ...interface{}) lg.Logger { - return &defaultLogger{ - defaultFields: append(dl.defaultFields, fields...), - } -} - -func (dl *defaultLogger) formatFields(level, msg string, fields ...interface{}) []any { - var result []any - - result = append(result, color.GreenString("|%s|", level)) - result = append(result, color.New(color.Bold, color.FgWhite).Sprintf("%-30s", msg)) - - for i := 0; i < len(dl.defaultFields)/2; i++ { - name := color.New(color.FgHiBlue).Sprintf("%v", dl.defaultFields[i*2]) - value := color.New(color.Faint).Sprintf("%v", dl.defaultFields[i*2+1]) - result = append(result, fmt.Sprintf("%v=%v", name, value)) - } - - for i := 0; i < len(fields)/2; i++ { - name := color.New(color.FgHiBlue).Sprintf("%v", fields[i*2]) - value := color.New(color.Faint).Sprintf("%v", fields[i*2+1]) - result = append(result, fmt.Sprintf("%v=%v", name, value)) - } - - return result -} diff --git a/internal/worker/activity.go b/internal/worker/activity.go index 6d83843b..8abb45c2 100644 --- a/internal/worker/activity.go +++ b/internal/worker/activity.go @@ -142,7 +142,8 @@ func (aw *ActivityWorker) handleTask(ctx context.Context, task *task.Activity) { return case <-t.C: if err := aw.backend.ExtendActivityTask(ctx, task.ID); err != nil { - aw.backend.Logger().Panic("extending activity task", "error", err) + aw.backend.Logger().Error("extending activity task", "error", err) + panic("extending activity task") } } } @@ -156,7 +157,9 @@ func (aw *ActivityWorker) handleTask(ctx context.Context, task *task.Activity) { event := aw.resultToEvent(task.Event.ScheduleEventID, result, err) if err := aw.backend.CompleteActivityTask(ctx, task.WorkflowInstance, task.ID, event); err != nil { - aw.backend.Logger().Panic("completing activity task", "error", err) + aw.backend.Logger().Error("completing activity task", "error", err) + panic("completing activity task") + } } diff --git a/internal/worker/workflow.go b/internal/worker/workflow.go index 428508a8..c3aa5805 100644 --- a/internal/worker/workflow.go +++ b/internal/worker/workflow.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "sync" "time" @@ -15,7 +16,6 @@ import ( "github.com/cschleiden/go-workflows/internal/task" "github.com/cschleiden/go-workflows/internal/workflow" "github.com/cschleiden/go-workflows/internal/workflow/cache" - "github.com/cschleiden/go-workflows/log" "github.com/cschleiden/go-workflows/metrics" ) @@ -30,7 +30,7 @@ type WorkflowWorker struct { workflowTaskQueue chan *task.Workflow - logger log.Logger + logger *slog.Logger pollersWg sync.WaitGroup wg sync.WaitGroup @@ -156,7 +156,7 @@ func (ww *WorkflowWorker) handle(ctx context.Context, t *task.Workflow) { result, err := ww.handleTask(ctx, t) if err != nil { - ww.logger.Panic("could not handle workflow task", "error", err) + ww.logger.ErrorContext(ctx, "could not handle workflow task", "error", err) } // Only record the time spent in the workflow code @@ -177,7 +177,8 @@ func (ww *WorkflowWorker) handle(ctx context.Context, t *task.Workflow) { if err := ww.backend.CompleteWorkflowTask( ctx, t, t.WorkflowInstance, state, result.Executed, result.ActivityEvents, result.TimerEvents, result.WorkflowEvents); err != nil { - ww.logger.Panic("could not complete workflow task", "error", err) + ww.logger.Error("could not complete workflow task", "error", err) + panic("could not complete workflow task") } } @@ -239,7 +240,8 @@ func (ww *WorkflowWorker) heartbeatTask(ctx context.Context, task *task.Workflow return case <-t.C: if err := ww.backend.ExtendWorkflowTask(ctx, task.ID, task.WorkflowInstance); err != nil { - ww.logger.Panic("could not heartbeat workflow task", "error", err) + ww.logger.Error("could not heartbeat workflow task", "error", err) + panic("could not heartbeat workflow task") } } } diff --git a/internal/workflow/cache/cache_test.go b/internal/workflow/cache/cache_test.go index 15f42c57..1104f71e 100644 --- a/internal/workflow/cache/cache_test.go +++ b/internal/workflow/cache/cache_test.go @@ -2,6 +2,7 @@ package cache import ( "context" + "log/slog" "runtime" "testing" "time" @@ -12,7 +13,6 @@ import ( "github.com/cschleiden/go-workflows/internal/converter" "github.com/cschleiden/go-workflows/internal/core" "github.com/cschleiden/go-workflows/internal/history" - "github.com/cschleiden/go-workflows/internal/logger" "github.com/cschleiden/go-workflows/internal/metrics" wf "github.com/cschleiden/go-workflows/internal/workflow" "github.com/cschleiden/go-workflows/workflow" @@ -28,14 +28,14 @@ func Test_Cache_StoreAndGet(t *testing.T) { i := core.NewWorkflowInstance("instanceID", "executionID") e, err := wf.NewExecutor( - logger.NewDefaultLogger(), trace.NewNoopTracerProvider().Tracer(backend.TracerName), r, converter.DefaultConverter, + slog.Default(), trace.NewNoopTracerProvider().Tracer(backend.TracerName), r, converter.DefaultConverter, []contextpropagation.ContextPropagator{}, &testHistoryProvider{}, i, &core.WorkflowMetadata{}, clock.New(), ) require.NoError(t, err) i2 := core.NewWorkflowInstance("instanceID2", "executionID2") e2, err := wf.NewExecutor( - logger.NewDefaultLogger(), trace.NewNoopTracerProvider().Tracer(backend.TracerName), r, converter.DefaultConverter, + slog.Default(), trace.NewNoopTracerProvider().Tracer(backend.TracerName), r, converter.DefaultConverter, []contextpropagation.ContextPropagator{}, &testHistoryProvider{}, i, &core.WorkflowMetadata{}, clock.New(), ) require.NoError(t, err) @@ -68,7 +68,10 @@ func Test_Cache_Evict(t *testing.T) { r := wf.NewRegistry() r.RegisterWorkflow(workflowWithActivity) e, err := wf.NewExecutor( - logger.NewDefaultLogger(), trace.NewNoopTracerProvider().Tracer(backend.TracerName), r, converter.DefaultConverter, []contextpropagation.ContextPropagator{}, &testHistoryProvider{}, i, &core.WorkflowMetadata{}, clock.New()) + slog.Default(), trace.NewNoopTracerProvider().Tracer(backend.TracerName), r, + converter.DefaultConverter, []contextpropagation.ContextPropagator{}, &testHistoryProvider{}, i, + &core.WorkflowMetadata{}, clock.New(), + ) require.NoError(t, err) err = c.Store(context.Background(), i, e) diff --git a/internal/workflow/executor.go b/internal/workflow/executor.go index a58fd423..4c560f7c 100644 --- a/internal/workflow/executor.go +++ b/internal/workflow/executor.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "reflect" "github.com/benbjohnson/clock" @@ -52,14 +53,14 @@ type executor struct { workflowCtxCancel sync.CancelFunc cv converter.Converter clock clock.Clock - logger log.Logger + logger *slog.Logger tracer trace.Tracer lastSequenceID int64 parentSpan trace.Span } func NewExecutor( - logger log.Logger, + logger *slog.Logger, tracer trace.Tracer, registry *Registry, cv converter.Converter, @@ -234,7 +235,8 @@ func (e *executor) replayHistory(h []*history.Event) error { e.workflowState.SetReplaying(true) for _, event := range h { if event.SequenceID < e.lastSequenceID { - e.logger.Panic("history has older events than current state") + e.logger.Error("history has older events than current state") + panic("history has older events than current state") } if err := e.executeEvent(event); err != nil { @@ -259,7 +261,8 @@ func (e *executor) executeNewEvents(newEvents []*history.Event) ([]*history.Even if e.workflow.Completed() { // TODO: Is this too early? We haven't committed some of the commands if e.workflowState.HasPendingFutures() { - e.logger.Panic("workflow completed, but there are still pending futures") + e.logger.Error("workflow completed, but there are still pending futures") + panic("workflow completed, but there are still pending futures") } if canErr, ok := e.workflow.Error().(*continueasnew.Error); ok { diff --git a/internal/workflow/executor_test.go b/internal/workflow/executor_test.go index 9c83e59b..ed07e8e1 100644 --- a/internal/workflow/executor_test.go +++ b/internal/workflow/executor_test.go @@ -3,6 +3,7 @@ package workflow import ( "context" "log" + "log/slog" "testing" "time" @@ -14,7 +15,6 @@ import ( "github.com/cschleiden/go-workflows/internal/core" "github.com/cschleiden/go-workflows/internal/fn" "github.com/cschleiden/go-workflows/internal/history" - "github.com/cschleiden/go-workflows/internal/logger" "github.com/cschleiden/go-workflows/internal/payload" "github.com/cschleiden/go-workflows/internal/sync" "github.com/cschleiden/go-workflows/internal/task" @@ -33,7 +33,7 @@ func (t *testHistoryProvider) GetWorkflowInstanceHistory(ctx context.Context, in } func newExecutor(r *Registry, i *core.WorkflowInstance, historyProvider WorkflowHistoryProvider) (*executor, error) { - logger := logger.NewDefaultLogger() + logger := slog.Default() tracer := trace.NewNoopTracerProvider().Tracer("test") e, err := NewExecutor(logger, tracer, r, converter.DefaultConverter, []contextpropagation.ContextPropagator{}, historyProvider, i, &core.WorkflowMetadata{}, clock.New()) diff --git a/internal/workflowstate/replaylogger.go b/internal/workflowstate/replaylogger.go index e23b9512..4843d8e7 100644 --- a/internal/workflowstate/replaylogger.go +++ b/internal/workflowstate/replaylogger.go @@ -1,46 +1,43 @@ package workflowstate import ( - "github.com/cschleiden/go-workflows/log" + "context" + "log/slog" ) -type replayLogger struct { +type replayHandler struct { state *WfState - logger log.Logger + hander slog.Handler } -func NewReplayLogger(state *WfState, logger log.Logger) log.Logger { - return &replayLogger{state, logger} +// Enabled implements slog.Handler. +func (rh *replayHandler) Enabled(ctx context.Context, level slog.Level) bool { + return rh.Enabled(ctx, level) } -func (r *replayLogger) Debug(msg string, fields ...interface{}) { - if !r.state.replaying { - r.logger.Debug(msg, fields...) +// Handle implements slog.Handler. +func (rh *replayHandler) Handle(ctx context.Context, r slog.Record) error { + if rh.state.Replaying() { + return nil } -} -// Error implements log.Logger -func (r *replayLogger) Error(msg string, fields ...interface{}) { - if !r.state.replaying { - r.logger.Error(msg, fields...) - } + return rh.Handle(ctx, r) } -// Panic implements log.Logger -func (r *replayLogger) Panic(msg string, fields ...interface{}) { - if !r.state.replaying { - r.logger.Panic(msg, fields...) - } +// WithAttrs implements slog.Handler. +func (rh *replayHandler) WithAttrs(attrs []slog.Attr) slog.Handler { + return rh.WithAttrs(attrs) } -// Warn implements log.Logger -func (r *replayLogger) Warn(msg string, fields ...interface{}) { - if !r.state.replaying { - r.logger.Warn(msg, fields...) - } +// WithGroup implements slog.Handler. +func (rh *replayHandler) WithGroup(name string) slog.Handler { + return rh.WithGroup(name) } -// With implements log.Logger -func (r *replayLogger) With(fields ...interface{}) log.Logger { - return NewReplayLogger(r.state, r.logger.With(fields...)) +var _ slog.Handler = (*replayHandler)(nil) + +func NewReplayLogger(state *WfState, logger *slog.Logger) *slog.Logger { + h := logger.Handler() + + return slog.New(&replayHandler{state, h}) } diff --git a/internal/workflowstate/workflowstate.go b/internal/workflowstate/workflowstate.go index 5fc426eb..b65cd767 100644 --- a/internal/workflowstate/workflowstate.go +++ b/internal/workflowstate/workflowstate.go @@ -2,6 +2,7 @@ package workflowstate import ( "fmt" + "log/slog" "time" "github.com/benbjohnson/clock" @@ -55,13 +56,13 @@ type WfState struct { pendingSignals map[string][]payload.Payload signalChannels map[string]*signalChannel - logger log.Logger + logger *slog.Logger clock clock.Clock time time.Time } -func NewWorkflowState(instance *core.WorkflowInstance, logger log.Logger, clock clock.Clock) *WfState { +func NewWorkflowState(instance *core.WorkflowInstance, logger *slog.Logger, clock clock.Clock) *WfState { state := &WfState{ instance: instance, commands: []command.Command{}, @@ -150,6 +151,6 @@ func (wf *WfState) Instance() *core.WorkflowInstance { return wf.instance } -func (wf *WfState) Logger() log.Logger { +func (wf *WfState) Logger() *slog.Logger { return wf.logger } diff --git a/internal/workflowstate/workflowstate_test.go b/internal/workflowstate/workflowstate_test.go index 645d8c53..d100442e 100644 --- a/internal/workflowstate/workflowstate_test.go +++ b/internal/workflowstate/workflowstate_test.go @@ -1,12 +1,12 @@ package workflowstate import ( + "log/slog" "testing" "github.com/benbjohnson/clock" "github.com/cschleiden/go-workflows/internal/converter" "github.com/cschleiden/go-workflows/internal/core" - "github.com/cschleiden/go-workflows/internal/logger" "github.com/cschleiden/go-workflows/internal/payload" "github.com/cschleiden/go-workflows/internal/sync" "github.com/google/uuid" @@ -16,7 +16,7 @@ import ( func Test_PendingFutures(t *testing.T) { i := core.NewWorkflowInstance(uuid.NewString(), "") - wfState := NewWorkflowState(i, logger.NewDefaultLogger(), clock.New()) + wfState := NewWorkflowState(i, slog.Default(), clock.New()) require.False(t, wfState.HasPendingFutures()) diff --git a/log/logger.go b/log/logger.go deleted file mode 100644 index 92091518..00000000 --- a/log/logger.go +++ /dev/null @@ -1,12 +0,0 @@ -package log - -// Logger is a basic logger interface. Fields have to be passed in pairs as "key", "value" -type Logger interface { - Debug(msg string, fields ...interface{}) - Warn(msg string, fields ...interface{}) - Error(msg string, fields ...interface{}) - Panic(msg string, fields ...interface{}) - - // With returns a logger instance that adds the given fields to every logged message - With(fields ...interface{}) Logger -} diff --git a/samples/errors/errors.go b/samples/errors/errors.go index 7733c1f8..df387575 100644 --- a/samples/errors/errors.go +++ b/samples/errors/errors.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "net/http" "os" "os/signal" @@ -11,7 +12,6 @@ import ( "github.com/cschleiden/go-workflows/backend" "github.com/cschleiden/go-workflows/client" "github.com/cschleiden/go-workflows/diag" - "github.com/cschleiden/go-workflows/log" "github.com/cschleiden/go-workflows/samples" "github.com/cschleiden/go-workflows/worker" "github.com/cschleiden/go-workflows/workflow" @@ -113,7 +113,7 @@ func Workflow1(ctx workflow.Context, msg string) error { return nil } -func handleError(ctx workflow.Context, name string, logger log.Logger, err error) { +func handleError(ctx workflow.Context, name string, logger *slog.Logger, err error) { logger = logger.With("activity", name) var werr *workflow.Error diff --git a/tester/options.go b/tester/options.go index 3b3ecc4e..78bcda9c 100644 --- a/tester/options.go +++ b/tester/options.go @@ -1,23 +1,23 @@ package tester import ( + "log/slog" "time" "github.com/cschleiden/go-workflows/internal/contextpropagation" "github.com/cschleiden/go-workflows/internal/converter" - "github.com/cschleiden/go-workflows/log" ) type options struct { TestTimeout time.Duration - Logger log.Logger + Logger *slog.Logger Converter converter.Converter Propagators []contextpropagation.ContextPropagator } type WorkflowTesterOption func(*options) -func WithLogger(logger log.Logger) WorkflowTesterOption { +func WithLogger(logger *slog.Logger) WorkflowTesterOption { return func(o *options) { o.Logger = logger } diff --git a/tester/tester.go b/tester/tester.go index 6d80146f..f65129c3 100644 --- a/tester/tester.go +++ b/tester/tester.go @@ -3,6 +3,7 @@ package tester import ( "context" "fmt" + "log/slog" "reflect" "sort" "sync" @@ -20,7 +21,6 @@ import ( "github.com/cschleiden/go-workflows/internal/core" "github.com/cschleiden/go-workflows/internal/fn" "github.com/cschleiden/go-workflows/internal/history" - "github.com/cschleiden/go-workflows/internal/logger" "github.com/cschleiden/go-workflows/internal/payload" "github.com/cschleiden/go-workflows/internal/signals" "github.com/cschleiden/go-workflows/internal/task" @@ -152,7 +152,7 @@ type workflowTester[TResult any] struct { runningActivities int32 - logger log.Logger + logger *slog.Logger tracer trace.Tracer @@ -177,7 +177,7 @@ func NewWorkflowTester[TResult any](wf interface{}, opts ...WorkflowTesterOption options := &options{ TestTimeout: time.Second * 10, - Logger: logger.NewDefaultLogger(), + Logger: slog.Default(), Converter: converter.DefaultConverter, } diff --git a/tester/tester_logger_test.go b/tester/tester_logger_test.go index e04dc1bf..fc669bbc 100644 --- a/tester/tester_logger_test.go +++ b/tester/tester_logger_test.go @@ -1,85 +1,28 @@ package tester import ( - "fmt" + "log/slog" "strings" - - "github.com/cschleiden/go-workflows/internal/logger" - "github.com/cschleiden/go-workflows/log" ) type debugLogger struct { - defaultFields []interface{} - lines *[]string - l log.Logger + b *strings.Builder + logger *slog.Logger } func newDebugLogger() *debugLogger { - lines := []string{} - return &debugLogger{ - lines: &lines, - defaultFields: []interface{}{}, - l: logger.NewDefaultLogger(), - } -} - -func (dl *debugLogger) hasLine(msg string) bool { - for _, line := range *dl.lines { - if strings.Contains(line, msg) { - return true - } - } + b := &strings.Builder{} - return false -} - -func (dl *debugLogger) formatFields(level, msg string, fields ...interface{}) string { - var result []string + logger := slog.New(slog.NewTextHandler(b, nil)) - result = append(result, fmt.Sprintf("|%s| %s", level, msg)) - - for i := 0; i < len(dl.defaultFields)/2; i++ { - result = append(result, fmt.Sprintf("%v=%v", dl.defaultFields[i*2], dl.defaultFields[i*2+1])) - } - - for i := 0; i < len(fields)/2; i++ { - result = append(result, fmt.Sprintf("%v=%v", fields[i*2], fields[i*2+1])) + return &debugLogger{ + b: b, + logger: logger, } - - return strings.Join(result, " ") } -func (dl *debugLogger) addLine(level, msg string, fields ...interface{}) { - // Persist for debugging - *dl.lines = append(*dl.lines, dl.formatFields(level, msg, fields...)) -} - -func (dl *debugLogger) Debug(msg string, fields ...interface{}) { - dl.addLine("DEBUG", msg, fields...) - dl.l.Debug(msg, fields...) -} - -func (dl *debugLogger) Error(msg string, fields ...interface{}) { - dl.addLine("ERROR", msg, fields...) - dl.l.Error(msg, fields...) -} - -func (dl *debugLogger) Panic(msg string, fields ...interface{}) { - dl.addLine("PANIC", msg, fields...) - dl.l.Panic(msg, fields...) -} - -func (dl *debugLogger) Warn(msg string, fields ...interface{}) { - dl.addLine("WARN", msg, fields...) - dl.l.Warn(msg, fields...) -} +func (dl *debugLogger) hasLine(msg string) bool { + s := dl.b.String() -func (dl *debugLogger) With(fields ...interface{}) log.Logger { - return &debugLogger{ - lines: dl.lines, // Keep this here - defaultFields: append(dl.defaultFields, fields...), - l: dl.l.With(fields...), - } + return strings.Contains(s, msg) } - -var _ log.Logger = (*debugLogger)(nil) diff --git a/tester/tester_timers_test.go b/tester/tester_timers_test.go index c900ca67..70278303 100644 --- a/tester/tester_timers_test.go +++ b/tester/tester_timers_test.go @@ -197,7 +197,7 @@ func Test_Timers_SetsTimeModeCorrectly(t *testing.T) { dl := newDebugLogger() - tester := NewWorkflowTester[string](wf, WithTestTimeout(time.Second*10), WithLogger(dl)) + tester := NewWorkflowTester[string](wf, WithTestTimeout(time.Second*10), WithLogger(dl.logger)) tester.OnActivity(activity1).Return("activity", nil) @@ -232,7 +232,7 @@ func Test_Timers_MultipleTimers(t *testing.T) { dl := newDebugLogger() - tester := NewWorkflowTester[string](wf, WithTestTimeout(time.Second*3), WithLogger(dl)) + tester := NewWorkflowTester[string](wf, WithTestTimeout(time.Second*3), WithLogger(dl.logger)) tester.OnActivity(activity1).Return("activity", nil) diff --git a/workflow/activity_test.go b/workflow/activity_test.go index 2a5ee784..098eb0d2 100644 --- a/workflow/activity_test.go +++ b/workflow/activity_test.go @@ -1,12 +1,12 @@ package workflow import ( + "log/slog" "testing" "github.com/benbjohnson/clock" "github.com/cschleiden/go-workflows/internal/converter" "github.com/cschleiden/go-workflows/internal/core" - "github.com/cschleiden/go-workflows/internal/logger" "github.com/cschleiden/go-workflows/internal/sync" "github.com/cschleiden/go-workflows/internal/workflowstate" "github.com/cschleiden/go-workflows/internal/workflowtracer" @@ -23,7 +23,7 @@ func Test_executeActivity_ResultMismatch(t *testing.T) { ctx = converter.WithConverter(ctx, converter.DefaultConverter) ctx = workflowstate.WithWorkflowState( ctx, - workflowstate.NewWorkflowState(core.NewWorkflowInstance("a", ""), logger.NewDefaultLogger(), clock.New()), + workflowstate.NewWorkflowState(core.NewWorkflowInstance("a", ""), slog.Default(), clock.New()), ) ctx = workflowtracer.WithWorkflowTracer(ctx, workflowtracer.New(trace.NewNoopTracerProvider().Tracer("test"))) @@ -47,7 +47,7 @@ func Test_executeActivity_ParamMismatch(t *testing.T) { ctx = converter.WithConverter(ctx, converter.DefaultConverter) ctx = workflowstate.WithWorkflowState( ctx, - workflowstate.NewWorkflowState(core.NewWorkflowInstance("a", ""), logger.NewDefaultLogger(), clock.New()), + workflowstate.NewWorkflowState(core.NewWorkflowInstance("a", ""), slog.Default(), clock.New()), ) ctx = workflowtracer.WithWorkflowTracer(ctx, workflowtracer.New(trace.NewNoopTracerProvider().Tracer("test"))) diff --git a/workflow/logger.go b/workflow/logger.go index 14395285..ea5fba50 100644 --- a/workflow/logger.go +++ b/workflow/logger.go @@ -1,11 +1,12 @@ package workflow import ( + "log/slog" + "github.com/cschleiden/go-workflows/internal/workflowstate" - "github.com/cschleiden/go-workflows/log" ) -func Logger(ctx Context) log.Logger { +func Logger(ctx Context) *slog.Logger { wfState := workflowstate.WorkflowState(ctx) return wfState.Logger() } diff --git a/workflow/subworkflow_test.go b/workflow/subworkflow_test.go index 706deb1f..19c12abc 100644 --- a/workflow/subworkflow_test.go +++ b/workflow/subworkflow_test.go @@ -1,12 +1,12 @@ package workflow import ( + "log/slog" "testing" "github.com/benbjohnson/clock" "github.com/cschleiden/go-workflows/internal/converter" "github.com/cschleiden/go-workflows/internal/core" - "github.com/cschleiden/go-workflows/internal/logger" "github.com/cschleiden/go-workflows/internal/sync" "github.com/cschleiden/go-workflows/internal/workflowstate" "github.com/cschleiden/go-workflows/internal/workflowtracer" @@ -23,7 +23,7 @@ func Test_createSubWorkflowInstance_ParamMismatch(t *testing.T) { ctx = converter.WithConverter(ctx, converter.DefaultConverter) ctx = workflowstate.WithWorkflowState( ctx, - workflowstate.NewWorkflowState(core.NewWorkflowInstance("a", ""), logger.NewDefaultLogger(), clock.New()), + workflowstate.NewWorkflowState(core.NewWorkflowInstance("a", ""), slog.Default(), clock.New()), ) ctx = workflowtracer.WithWorkflowTracer(ctx, workflowtracer.New(trace.NewNoopTracerProvider().Tracer("test"))) @@ -48,7 +48,7 @@ func Test_createSubWorkflowInstance_ReturnMismatch(t *testing.T) { ctx = converter.WithConverter(ctx, converter.DefaultConverter) ctx = workflowstate.WithWorkflowState( ctx, - workflowstate.NewWorkflowState(core.NewWorkflowInstance("a", ""), logger.NewDefaultLogger(), clock.New()), + workflowstate.NewWorkflowState(core.NewWorkflowInstance("a", ""), slog.Default(), clock.New()), ) ctx = workflowtracer.WithWorkflowTracer(ctx, workflowtracer.New(trace.NewNoopTracerProvider().Tracer("test"))) diff --git a/workflow/timer_test.go b/workflow/timer_test.go index 6bb84d28..7c5e8aa0 100644 --- a/workflow/timer_test.go +++ b/workflow/timer_test.go @@ -1,13 +1,13 @@ package workflow import ( + "log/slog" "testing" "time" "github.com/benbjohnson/clock" "github.com/cschleiden/go-workflows/internal/converter" "github.com/cschleiden/go-workflows/internal/core" - "github.com/cschleiden/go-workflows/internal/logger" "github.com/cschleiden/go-workflows/internal/sync" "github.com/cschleiden/go-workflows/internal/workflowstate" "github.com/cschleiden/go-workflows/internal/workflowtracer" @@ -16,7 +16,7 @@ import ( ) func Test_Timer_Cancellation(t *testing.T) { - state := workflowstate.NewWorkflowState(core.NewWorkflowInstance("a", ""), logger.NewDefaultLogger(), clock.New()) + state := workflowstate.NewWorkflowState(core.NewWorkflowInstance("a", ""), slog.Default(), clock.New()) ctx, cancel := sync.WithCancel(sync.Background()) ctx = converter.WithConverter(ctx, converter.DefaultConverter) From 8d7656e89a01a297b85e6988732681477a34f695 Mon Sep 17 00:00:00 2001 From: Christopher Schleiden Date: Thu, 7 Sep 2023 17:24:46 -0700 Subject: [PATCH 2/8] Upgrade workflows --- .github/workflows/bench.yml | 2 +- .github/workflows/go.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 174a7a76..60dcb13b 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -33,7 +33,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v3 with: - go-version: 1.19 + go-version: 1.21 check-latest: true cache: true diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 03168c24..03d2cfa4 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -21,7 +21,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v3 with: - go-version: 1.19 + go-version: 1.21 check-latest: true cache: true From 38ac4f5355cd9cfab7dd0400d58f45f7bd143c44 Mon Sep 17 00:00:00 2001 From: Christopher Schleiden Date: Thu, 7 Sep 2023 20:09:22 -0700 Subject: [PATCH 3/8] Fix debug logger --- tester/tester_logger_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tester/tester_logger_test.go b/tester/tester_logger_test.go index fc669bbc..c43b127d 100644 --- a/tester/tester_logger_test.go +++ b/tester/tester_logger_test.go @@ -13,7 +13,9 @@ type debugLogger struct { func newDebugLogger() *debugLogger { b := &strings.Builder{} - logger := slog.New(slog.NewTextHandler(b, nil)) + logger := slog.New(slog.NewTextHandler(b, &slog.HandlerOptions{ + Level: slog.LevelDebug, + })) return &debugLogger{ b: b, From 0df24496bb0f00ae96209d7945246b6346b857f2 Mon Sep 17 00:00:00 2001 From: Christopher Schleiden Date: Thu, 7 Sep 2023 20:09:34 -0700 Subject: [PATCH 4/8] Prevent infinite loop in replay logger --- internal/workflowstate/replaylogger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/workflowstate/replaylogger.go b/internal/workflowstate/replaylogger.go index 4843d8e7..cc7048c0 100644 --- a/internal/workflowstate/replaylogger.go +++ b/internal/workflowstate/replaylogger.go @@ -12,7 +12,7 @@ type replayHandler struct { // Enabled implements slog.Handler. func (rh *replayHandler) Enabled(ctx context.Context, level slog.Level) bool { - return rh.Enabled(ctx, level) + return rh.hander.Enabled(ctx, level) } // Handle implements slog.Handler. From adc591d4b4c28d80354cc296d7d201fd7d025bfc Mon Sep 17 00:00:00 2001 From: Christopher Schleiden Date: Thu, 7 Sep 2023 20:21:54 -0700 Subject: [PATCH 5/8] FIx missing logging field names --- samples/retries/retries.go | 6 +++--- samples/subworkflow/subworkflow.go | 4 ++-- samples/timer/timer.go | 10 ++++++---- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/samples/retries/retries.go b/samples/retries/retries.go index 5bdd0680..76b0314b 100644 --- a/samples/retries/retries.go +++ b/samples/retries/retries.go @@ -61,7 +61,7 @@ func RunWorker(ctx context.Context, mb backend.Backend) { func Workflow1(ctx workflow.Context, msg string) error { logger := workflow.Logger(ctx) - logger.Debug("Entering Workflow1", msg) + logger.Debug("Entering Workflow1", "msg", msg) defer logger.Debug("Leaving Workflow1") // Illustrate sub workflow retries. The called workflow will fail a few times, and its execution will be retried. @@ -84,7 +84,7 @@ var workflowCalls = 0 func WorkflowWithFailures(ctx workflow.Context, msg string) error { logger := workflow.Logger(ctx) - logger.Debug("Entering WorkflowWithFailures", msg) + logger.Debug("Entering WorkflowWithFailures", "msg", msg) defer logger.Debug("Leaving WorkflowWithFailures") workflowCalls++ @@ -101,7 +101,7 @@ func WorkflowWithFailures(ctx workflow.Context, msg string) error { }, }, Activity1, 35).Get(ctx) if err != nil { - logger.Debug("Error from Activity 1", err) + logger.Error("Error from Activity 1", "err", err) return fmt.Errorf("getting result from activity 1: %w", err) } diff --git a/samples/subworkflow/subworkflow.go b/samples/subworkflow/subworkflow.go index 3dede44d..991bec44 100644 --- a/samples/subworkflow/subworkflow.go +++ b/samples/subworkflow/subworkflow.go @@ -95,13 +95,13 @@ func SubWorkflow(ctx workflow.Context, msg string) (string, error) { if err != nil { logger.Error("error getting activity 1 result", "err", err) } - logger.Debug("R1 result:", r1) + logger.Debug("R1 result:", "r1", r1) r2, err := workflow.ExecuteActivity[int](ctx, workflow.DefaultActivityOptions, Activity2).Get(ctx) if err != nil { logger.Error("error getting activity 2 result", "err", err) } - logger.Debug("R2 result:", r2) + logger.Debug("R2 result:", "r2", r2) return "W2 Result", nil } diff --git a/samples/timer/timer.go b/samples/timer/timer.go index d091c292..c192161a 100644 --- a/samples/timer/timer.go +++ b/samples/timer/timer.go @@ -5,6 +5,7 @@ import ( "log" "time" + "github.com/cschleiden/go-workflows/activity" "github.com/cschleiden/go-workflows/backend" "github.com/cschleiden/go-workflows/client" "github.com/cschleiden/go-workflows/samples" @@ -63,7 +64,7 @@ func RunWorker(ctx context.Context, mb backend.Backend) worker.Worker { func Workflow1(ctx workflow.Context, msg string) (string, error) { logger := workflow.Logger(ctx) - logger.Debug("Entering Workflow1, input: ", msg) + logger.Debug("Entering Workflow1, input: ", "msg", msg) defer logger.Debug("Leaving Workflow1") a1 := workflow.ExecuteActivity[int](ctx, workflow.DefaultActivityOptions, Activity1, 35, 12) @@ -85,7 +86,7 @@ func Workflow1(ctx workflow.Context, msg string) (string, error) { panic(err) } - logger.Debug("Activity result", r) + logger.Debug("Activity result", "r", r) // Cancel timer cancel() @@ -96,12 +97,13 @@ func Workflow1(ctx workflow.Context, msg string) (string, error) { } func Activity1(ctx context.Context, a, b int) (int, error) { - log.Println("Entering Activity1") + logger := activity.Logger(ctx) + logger.Debug("Entering Activity1") time.Sleep(10 * time.Second) defer func() { - log.Println("Leaving Activity1") + logger.Debug("Leaving Activity1") }() return a + b, nil From 9d80d14ed83f0d7ce932e94a48de0d4a0076a7a3 Mon Sep 17 00:00:00 2001 From: Christopher Schleiden Date: Thu, 7 Sep 2023 20:33:22 -0700 Subject: [PATCH 6/8] Use go 1.21 for all workflow jbos --- .github/workflows/go.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 03d2cfa4..ddeb2afa 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -51,7 +51,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v3 with: - go-version: 1.19 + go-version: 1.21 check-latest: true cache: true @@ -85,7 +85,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v3 with: - go-version: 1.19 + go-version: 1.21 check-latest: true cache: true @@ -111,7 +111,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v3 with: - go-version: 1.19 + go-version: 1.21 check-latest: true cache: true From daf2807803c2d72b4a7b8a68c02e4e326a6aea7a Mon Sep 17 00:00:00 2001 From: Christopher Schleiden Date: Thu, 7 Sep 2023 20:37:44 -0700 Subject: [PATCH 7/8] Correctly pass through replay logger --- internal/workflowstate/replaylogger.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/workflowstate/replaylogger.go b/internal/workflowstate/replaylogger.go index cc7048c0..e69956b7 100644 --- a/internal/workflowstate/replaylogger.go +++ b/internal/workflowstate/replaylogger.go @@ -6,13 +6,13 @@ import ( ) type replayHandler struct { - state *WfState - hander slog.Handler + state *WfState + handler slog.Handler } // Enabled implements slog.Handler. func (rh *replayHandler) Enabled(ctx context.Context, level slog.Level) bool { - return rh.hander.Enabled(ctx, level) + return rh.handler.Enabled(ctx, level) } // Handle implements slog.Handler. @@ -21,17 +21,17 @@ func (rh *replayHandler) Handle(ctx context.Context, r slog.Record) error { return nil } - return rh.Handle(ctx, r) + return rh.handler.Handle(ctx, r) } // WithAttrs implements slog.Handler. func (rh *replayHandler) WithAttrs(attrs []slog.Attr) slog.Handler { - return rh.WithAttrs(attrs) + return rh.handler.WithAttrs(attrs) } // WithGroup implements slog.Handler. func (rh *replayHandler) WithGroup(name string) slog.Handler { - return rh.WithGroup(name) + return rh.handler.WithGroup(name) } var _ slog.Handler = (*replayHandler)(nil) From a5c269a3e2fe3cf7311069324c74c1b0271b3d6d Mon Sep 17 00:00:00 2001 From: Christopher Schleiden Date: Fri, 8 Sep 2023 08:10:21 -0700 Subject: [PATCH 8/8] Increase timeout --- backend/test/e2e.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/test/e2e.go b/backend/test/e2e.go index 306c5620..5c7abec6 100644 --- a/backend/test/e2e.go +++ b/backend/test/e2e.go @@ -654,7 +654,7 @@ func EndToEndBackendTest(t *testing.T, setup func(options ...backend.BackendOpti instance := runWorkflow(t, ctx, c, wf, 0) - r, err := client.GetWorkflowResult[int](ctx, c, instance, time.Second*10) + r, err := client.GetWorkflowResult[int](ctx, c, instance, time.Second*20) require.NoError(t, err) require.Equal(t, 3, r) },