blackbox monitoring core #6
Conversation
| if err := run(); err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| os.Exit(0) |
| func (l *logResult) Report(resp <-chan Result) { | ||
| for r := range resp { | ||
| if r.err != nil { | ||
| l.l.Error("unable to process request", zap.Error(r.err)) |
There was a problem hiding this comment.
worth adding too
l := l.l.With(
zap.String("URL", URL),
zap.Int("status", code),
zap.String("job_name", r.jobName),
| return err | ||
| } | ||
|
|
||
| resultCh := s.RunReporter() |
There was a problem hiding this comment.
will be nicer if you add this to NewLogResult()
and close the channel on logReporter.Close()
| // RunReporter crates new report channel and spin up new go routine with Report method | ||
| // with this newly created channel. Every scraped job needs to report result into this | ||
| // newly created report channel. | ||
| func (s *Scraper) RunReporter() chan Result { |
There was a problem hiding this comment.
if this actually is need - it should be a method of logResult not Scraper
| }, err | ||
| } | ||
|
|
||
| func (l *logResult) Report(resp <-chan Result) { |
There was a problem hiding this comment.
if in the end - reporting is only required with the log and not something else - there is no need to gather it - log is concurrency free structure so it can be executed in multiple go routines at the same time
blackbox/blackbox/httpjob.go
Outdated
| for range ticker { | ||
| responses = make([]http.Response, 0, len(j.Targets)) | ||
| ctx, cancel := context.WithTimeout(context.Background(), j.Timeout) | ||
| g, ctx := errgroup.WithContext(ctx) |
There was a problem hiding this comment.
why do you need this context?
blackbox/blackbox/httpjob.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| mu.Lock() |
There was a problem hiding this comment.
this is quite unusual to have mutex together with chan for collecting results
imho - worth changing Result a bit to be able to collect the parts of job and combine it to one job if needed
| sig := make(chan os.Signal, 1) | ||
| signal.Notify(sig, os.Interrupt, syscall.SIGTERM) | ||
| <-sig | ||
|
|
There was a problem hiding this comment.
you may catch panic on while closing app - "publish to closed channel" as you are not actually stopping scrapers
No description provided.