Monorepo for Tangled tangled.org

appview/notify: pass logger with mergedLogger #693

merged opened by boltless.me targeting master from boltless.me/core: feat/search
Labels

None yet.

assignee

None yet.

Participants 2
AT URI
at://did:plc:xasnlahkri4ewmbuzly2rlc5/sh.tangled.repo.pull/3m3q47rjacq22
+14 -9
Diff #6
+3 -3
appview/indexer/notifier.go
··· 11 11 var _ notify.Notifier = &Indexer{} 12 12 13 13 func (ix *Indexer) NewIssue(ctx context.Context, issue *models.Issue) { 14 - l := log.FromContext(ctx).With("notifier", "indexer.NewIssue", "issue", issue) 14 + l := log.FromContext(ctx).With("notifier", "indexer", "issue", issue) 15 15 l.Debug("indexing new issue") 16 16 err := ix.Issues.Index(ctx, *issue) 17 17 if err != nil { ··· 20 20 } 21 21 22 22 func (ix *Indexer) NewIssueClosed(ctx context.Context, issue *models.Issue) { 23 - l := log.FromContext(ctx).With("notifier", "indexer.NewIssueClosed", "issue", issue) 23 + l := log.FromContext(ctx).With("notifier", "indexer", "issue", issue) 24 24 l.Debug("updating an issue") 25 25 err := ix.Issues.Index(ctx, *issue) 26 26 if err != nil { ··· 29 29 } 30 30 31 31 func (ix *Indexer) DeleteIssue(ctx context.Context, issue *models.Issue) { 32 - l := log.FromContext(ctx).With("notifier", "indexer.DeleteIssue", "issue", issue) 32 + l := log.FromContext(ctx).With("notifier", "indexer", "issue", issue) 33 33 l.Debug("deleting an issue") 34 34 err := ix.Issues.Delete(ctx, issue.Id) 35 35 if err != nil {
+10 -5
appview/notify/merged_notifier.go
··· 2 2 3 3 import ( 4 4 "context" 5 + "log/slog" 5 6 "reflect" 6 7 "sync" 7 8 8 9 "tangled.org/core/appview/models" 10 + "tangled.org/core/log" 9 11 ) 10 12 11 13 type mergedNotifier struct { 12 14 notifiers []Notifier 15 + logger *slog.Logger 13 16 } 14 17 15 - func NewMergedNotifier(notifiers ...Notifier) Notifier { 16 - return &mergedNotifier{notifiers} 18 + func NewMergedNotifier(notifiers []Notifier, logger *slog.Logger) Notifier { 19 + return &mergedNotifier{notifiers, logger} 17 20 } 18 21 19 22 var _ Notifier = &mergedNotifier{} 20 23 21 24 // fanout calls the same method on all notifiers concurrently 22 - func (m *mergedNotifier) fanout(method string, args ...any) { 25 + func (m *mergedNotifier) fanout(method string, ctx context.Context, args ...any) { 26 + ctx = log.IntoContext(ctx, m.logger.With("method", method)) 23 27 var wg sync.WaitGroup 24 28 for _, n := range m.notifiers { 25 29 wg.Add(1) 26 30 go func(notifier Notifier) { 27 31 defer wg.Done() 28 32 v := reflect.ValueOf(notifier).MethodByName(method) 29 - in := make([]reflect.Value, len(args)) 33 + in := make([]reflect.Value, len(args)+1) 34 + in[0] = reflect.ValueOf(ctx) 30 35 for i, arg := range args { 31 - in[i] = reflect.ValueOf(arg) 36 + in[i+1] = reflect.ValueOf(arg) 32 37 } 33 38 v.Call(in) 34 39 }(n)
+1 -1
appview/state/state.go
··· 168 168 notifiers = append(notifiers, phnotify.NewPosthogNotifier(posthog)) 169 169 } 170 170 notifiers = append(notifiers, indexer) 171 - notifier := notify.NewMergedNotifier(notifiers...) 171 + notifier := notify.NewMergedNotifier(notifiers, tlog.SubLogger(logger, "notify")) 172 172 173 173 state := &State{ 174 174 d,

History

10 rounds 2 comments
sign up or login to add to the discussion
1 commit
expand
appview/notify: pass logger with mergedLogger
expand 0 comments
pull request successfully merged
1 commit
expand
appview/notify: pass logger with mergedLogger
expand 0 comments
1 commit
expand
appview/notify: pass logger with mergedLogger
expand 0 comments
1 commit
expand
appview/notify: pass logger with mergedLogger
expand 0 comments
1 commit
expand
appview/notify: pass logger with mergedLogger
expand 0 comments
1 commit
expand
appview/notify: pass logger with mergedLogger
expand 0 comments
1 commit
expand
appview/notify: pass logger with mergedLogger
expand 0 comments
1 commit
expand
appview/notify: pass logger with mergedLogger
expand 0 comments
1 commit
expand
appview/notify: pass logger with mergedLogger
expand 2 comments

nice lgtm, i was considering add a "logger" notifier, separate from every other notifier.

Each notifiers will need their own separate logs, so I think it would be better to let them call logger individually. For example, in same NewIssue event, one notifier might need to log the failure while all other notifiers worked fine.

1 commit
expand
appview/notify: pass logger with mergedLogger
expand 0 comments