diff --git a/internal/logging/discard.go b/internal/logging/discard.go index 0373c08..da02ca6 100644 --- a/internal/logging/discard.go +++ b/internal/logging/discard.go @@ -12,4 +12,4 @@ func (noop) Warn(msg string, args ...any) {} func (noop) Error(msg string, args ...any) {} -func (noop) AddEnricher(enricher Enricher) {} +func (noop) AddArgsUpdater(updater ArgsUpdater) {} diff --git a/internal/logging/enricher.go b/internal/logging/enricher.go index 3cafd3c..2d53335 100644 --- a/internal/logging/enricher.go +++ b/internal/logging/enricher.go @@ -1,23 +1,83 @@ package logging +import ( + "reflect" + + "github.com/leg100/pug/internal/resource" +) + // enricher enriches a log record with further meaningful attributes that aren't // readily available to the caller. type enricher struct { - enrichers []Enricher + updaters []ArgsUpdater } -// Enricher implementations update a log record with further info -type Enricher interface { - EnrichLogRecord(args ...any) []any +func (e *enricher) AddArgsUpdater(updater ArgsUpdater) { + e.updaters = append(e.updaters, updater) } -func (e *enricher) AddEnricher(enricher Enricher) { - e.enrichers = append(e.enrichers, enricher) +func (e *enricher) enrich(args ...any) []any { + for _, en := range e.updaters { + args = en.UpdateArgs(args...) + } + return args } -func (e *enricher) enrich(args ...any) []any { - for _, en := range e.enrichers { - args = en.EnrichLogRecord(args...) +// ArgsUpdater updates a log message's arguments. +type ArgsUpdater interface { + UpdateArgs(args ...any) []any +} + +// ReferenceUpdater checks log arguments for references to T via its ID, either directly +// or via a struct field, and updates or adds T to the log arguments +// accordingly. +type ReferenceUpdater[T any] struct { + Getter[T] + + Name string + Field string +} + +type Getter[T any] interface { + Get(resource.ID) (T, error) +} + +func (e *ReferenceUpdater[T]) UpdateArgs(args ...any) []any { + for i, arg := range args { + // Where an argument is of type resource.ID, try and retrieve the + // resource corresponding to the ID and replace argument with the + // resource. + if id, ok := arg.(resource.ID); ok { + t, err := e.Get(id) + if err != nil { + continue + } + // replace id with T + args[i] = t + return args + } + // Where an argument is a struct (or a pointer to a struct), check if it + // has a field matching the expect field name, with a corresponding + // value of type resource.ID, and if so, try and retrieve resource with + // that resource.ID and add it as a log argument preceded with e.Name. + v := reflect.Indirect(reflect.ValueOf(arg)) + if v.Kind() != reflect.Struct { + continue + } + f := reflect.Indirect(v.FieldByName(e.Field)) + if !f.IsValid() { + continue + } + id, ok := f.Interface().(resource.ID) + if !ok { + continue + } + t, err := e.Get(id) + if err != nil { + // T with id does not exist + continue + } + return append(args, e.Name, t) } return args } diff --git a/internal/logging/enricher_test.go b/internal/logging/enricher_test.go new file mode 100644 index 0000000..5881d5e --- /dev/null +++ b/internal/logging/enricher_test.go @@ -0,0 +1,72 @@ +package logging + +import ( + "testing" + + "github.com/leg100/pug/internal/resource" + "github.com/stretchr/testify/assert" +) + +func TestReferenceUpdater(t *testing.T) { + res := &fakeResource{ID: resource.NewID(resource.Module)} + updater := &ReferenceUpdater[*fakeResource]{ + Getter: &fakeResourceGetter{res: res}, + Name: "fake", + Field: "FakeResourceID", + } + + t.Run("replace resource id with resource", func(t *testing.T) { + args := []any{"fake", res.ID} + got := updater.UpdateArgs(args...) + + want := []any{"fake", res} + assert.Equal(t, want, got) + }) + + t.Run("add resource when referenced from struct with pointer field", func(t *testing.T) { + type logMsgArg struct { + FakeResourceID *resource.ID + } + + args := []any{"arg1", logMsgArg{FakeResourceID: &res.ID}} + got := updater.UpdateArgs(args...) + + want := append(args, "fake", res) + assert.Equal(t, want, got) + }) + + t.Run("add resource when referenced from struct with non-pointer field", func(t *testing.T) { + type logMsgArg struct { + FakeResourceID resource.ID + } + + args := []any{"arg1", logMsgArg{FakeResourceID: res.ID}} + got := updater.UpdateArgs(args...) + + want := append(args, "fake", res) + assert.Equal(t, want, got) + }) + + t.Run("handle nil pointer from struct", func(t *testing.T) { + type logMsgArg struct { + FakeResourceID *resource.ID + } + + args := []any{"arg1", logMsgArg{FakeResourceID: nil}} + got := updater.UpdateArgs(args...) + + assert.Equal(t, got, got) + }) +} + +type fakeResource struct { + resource.ID +} + +type fakeResourceGetter struct { + res *fakeResource +} + +func (f *fakeResourceGetter) Get(resource.ID) (*fakeResource, error) { + return f.res, nil +} diff --git a/internal/logging/interface.go b/internal/logging/interface.go index 2524a0e..391c830 100644 --- a/internal/logging/interface.go +++ b/internal/logging/interface.go @@ -5,5 +5,5 @@ type Interface interface { Info(msg string, args ...any) Warn(msg string, args ...any) Error(msg string, args ...any) - AddEnricher(enricher Enricher) + AddArgsUpdater(enricher ArgsUpdater) } diff --git a/internal/module/log_enricher.go b/internal/module/log_enricher.go deleted file mode 100644 index 86a7243..0000000 --- a/internal/module/log_enricher.go +++ /dev/null @@ -1,66 +0,0 @@ -package module - -import ( - "reflect" - - "github.com/leg100/pug/internal/resource" -) - -// logEnricher adds module related attributes to log records where pertinent. -type logEnricher struct { - table *resource.Table[*Module] -} - -func (e *logEnricher) EnrichLogRecord(args ...any) []any { - args = e.addModulePath(args...) - args = e.replaceIDWithModule(args...) - return args -} - -// addModulePath checks if one of the log message args is a struct with a ModuleID -// field, and if so, looks up the corresponding module and adds it to the -// message. -func (e *logEnricher) addModulePath(args ...any) []any { - for _, arg := range args { - v := reflect.Indirect(reflect.ValueOf(arg)) - if v.Kind() != reflect.Struct { - continue - } - f := v.FieldByName("ModuleID") - if !f.IsValid() { - continue - } - id, ok := f.Interface().(resource.ID) - if !ok { - continue - } - mod, err := e.table.Get(id) - if err != nil { - // module with id does not exist - continue - } - return append(args, "module", mod) - } - return args -} - -// replaceIDWithModule checks if one of the arguments is a module ID, and -// if so, replaces it with a module instance. -func (e *logEnricher) replaceIDWithModule(args ...any) []any { - for i, arg := range args { - id, ok := arg.(resource.ID) - if !ok { - // Not an id - continue - } - mod, err := e.table.Get(id) - if err != nil { - // Not a module id - continue - } - // replace id with module - args[i] = mod - return args - } - return args -} diff --git a/internal/module/service.go b/internal/module/service.go index 5ae8f48..aacbc23 100644 --- a/internal/module/service.go +++ b/internal/module/service.go @@ -50,7 +50,11 @@ func NewService(opts ServiceOptions) *Service { broker := pubsub.NewBroker[*Module](opts.Logger) table := resource.NewTable(broker) - opts.Logger.AddEnricher(&logEnricher{table: table}) + opts.Logger.AddArgsUpdater(&logging.ReferenceUpdater[*Module]{ + Getter: table, + Name: "module", + Field: "ModuleID", + }) return &Service{ table: table, diff --git a/internal/workspace/log_enricher.go b/internal/workspace/log_enricher.go deleted file mode 100644 index 3c1abd4..0000000 --- a/internal/workspace/log_enricher.go +++ /dev/null @@ -1,66 +0,0 @@ -package workspace - -import ( - "reflect" - - "github.com/leg100/pug/internal/resource" -) - -// logEnricher adds module related attributes to log records where pertinent. -type logEnricher struct { - table *resource.Table[*Workspace] -} - -func (e *logEnricher) EnrichLogRecord(args ...any) []any { - args = e.addWorkspaceName(args...) - args = e.replaceIDWithWorkspace(args...) - return args -} - -// addWorkspacePath checks if one of the log message args is a struct with a WorkspaceID -// field, and if so, looks up the corresponding workspace and adds it to the -// message. -func (e *logEnricher) addWorkspaceName(args ...any) []any { - for _, arg := range args { - v := reflect.Indirect(reflect.ValueOf(arg)) - if v.Kind() != reflect.Struct { - continue - } - f := v.FieldByName("WorkspaceID") - if !f.IsValid() { - continue - } - id, ok := f.Interface().(resource.ID) - if !ok { - continue - } - ws, err := e.table.Get(id) - if err != nil { - // workspace with id does not exist - continue - } - return append(args, "workspace", ws) - } - return args -} - -// replaceIDWithWorkspace checks if one of the arguments is a workspace ID, and -// if so, replaces it with a workspace instance. -func (e *logEnricher) replaceIDWithWorkspace(args ...any) []any { - for i, arg := range args { - id, ok := arg.(resource.ID) - if !ok { - // Not an id - continue - } - ws, err := e.table.Get(id) - if err != nil { - // Not a workspace id - continue - } - // replace id with workspace - args[i] = ws - return args - } - return args -} diff --git a/internal/workspace/service.go b/internal/workspace/service.go index 1603f1f..97a74a6 100644 --- a/internal/workspace/service.go +++ b/internal/workspace/service.go @@ -53,7 +53,11 @@ func NewService(opts ServiceOptions) *Service { broker := pubsub.NewBroker[*Workspace](opts.Logger) table := resource.NewTable(broker) - opts.Logger.AddEnricher(&logEnricher{table: table}) + opts.Logger.AddArgsUpdater(&logging.ReferenceUpdater[*Workspace]{ + Getter: table, + Name: "workspace", + Field: "WorkspaceID", + }) s := &Service{ Broker: broker,