add logs into git service #120
Conversation
Signed-off-by: Maxim Sukharev <maxim@sourced.tech>
service/git/syncer.go
Outdated
| @@ -53,6 +54,8 @@ func (s *Syncer) Sync(ctx context.Context, | |||
| } | |||
|
|
|||
| func (s *Syncer) fetch(ctx context.Context, repoURL string, r *git.Repository, refspecs []config.RefSpec) error { | |||
| log.Infof("fetching references for repository %s: %v", repoURL, refspecs) | |||
There was a problem hiding this comment.
As this process might take some time, do you think it might make sense to add another log to this function at the end of fetching?
There was a problem hiding this comment.
example of the logs
$ make dry-run
go run cmd/lookout/*.go serve --dry-run github.com/src-d/lookout
[2018-08-09T16:08:24.187459705+02:00] INFO Starting watcher urls=[github.com/src-d/lookout]
[2018-08-09T16:08:24.190722352+02:00] INFO connection state changed to 'READY' addr=ipv4://localhost:10302 analyzer=Dummy
[2018-08-09T16:08:27.589335009+02:00] INFO event successfully processed, skipping... event-id=a29ea85b92358e61e3a56281c5f6e885fa170ab7 event-type=2
[2018-08-09T16:08:27.7994484+02:00] INFO processing push app=lookout head=refs/heads/master provider=github repository=git://github.com/src-d/lookout.git
[2018-08-09T16:08:27.799639533+02:00] INFO creating local repository for: git://github.com/src-d/lookout.git app=lookout
[2018-08-09T16:08:27.801553032+02:00] INFO fetching references for repository git://github.com/src-d/lookout.git: [refs/heads/master:refs/heads/master] app=lookout
[2018-08-09T16:08:39.005883394+02:00] INFO repository config is not found app=lookout head=refs/heads/master provider=github repository=git://github.com/src-d/lookout.git
[2018-08-09T16:08:39.005989076+02:00] INFO status: pending app=lookout
[2018-08-09T16:08:39.006093794+02:00] INFO unary client call started app=lookout grpc.method=NotifyPushEvent grpc.service=pb.Analyzer span.kind=client system=grpc
you can clearly see when it finished fetching. Imo that is more than enough for logs.
Or do you want to know how much time did it take? Should it be in the logs or in tracing?
There was a problem hiding this comment.
Or do you want to know how much time did it take?
yes, was thinking that could be easy way to see elapsed time in debug, until we have a tracing
service/git/syncer.go
Outdated
| start := time.Now() | ||
| defer func() { | ||
| if err == nil { | ||
| log.Debugf("references %v fetched for repository %s in %v", repoURL, refspecs, time.Now().Sub(start)) |
There was a problem hiding this comment.
should we use log with field duration, to follow the guide conventions more closely?
There was a problem hiding this comment.
I think for debug logs it doesn't make much sense. Debug level shouldn't be enabled in production. Though I'm changing it just for the sake of consistency with other log messages.
Signed-off-by: Maxim Sukharev <maxim@sourced.tech>
Fix: #112
I have inspected the source code and found only a few places where logs (at least without context) make sense.
Looks like anything else even with debug level would just pollute output.
More verbose debug information will be helpful only if the log is scoped to request/event.