Skip to content

Commit

Permalink
[exporter/datadog] Avoid logging in chain provider goroutines to avoi…
Browse files Browse the repository at this point in the history
…d data race (#24899)

**Description:** 

Fixes data race on `TestChain` that was caused by logging after the test
was ended (see
uber-go/zap#687 (comment) for
details).
To do this, we move the logging outside of the goroutines in the Chain
provider.
Goroutines can still be running after the function has returned (this is
intentional, their return value will just be ignored), but they will no
longer log anything.

I also made the `Source` method honor cancellation of the context when
waiting on providers.

**Link to tracking Issue:** Reported on Slack, see
https://cloud-native.slack.com/archives/C01N6P7KR6W/p1691074033049049?thread_ts=1690992010.123699&cid=C01N6P7KR6W
  • Loading branch information
mx-psi committed Aug 4, 2023
1 parent 70946c9 commit cfb8f20
Showing 1 changed file with 15 additions and 19 deletions.
34 changes: 15 additions & 19 deletions exporter/datadogexporter/internal/hostmetadata/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,32 +37,28 @@ func (p *chainProvider) Source(ctx context.Context) (source.Source, error) {
for i, source := range p.priorityList {
provider := p.providers[source]
replies[i] = make(chan reply)

go func(i int, source string) {
zapProvider := zap.String("provider", source)
p.logger.Debug("Trying out source provider", zapProvider)

p.logger.Debug("Trying out source provider", zap.String("provider", source))
go func(i int) {
src, err := provider.Source(ctx)
if err != nil {
p.logger.Debug("Unavailable source provider", zapProvider, zap.Error(err))
}

replies[i] <- reply{src: src, err: err}
}(i, source)
}(i)
}

// Check provider responses in order to ensure priority
for i, ch := range replies {
reply := <-ch
if reply.err != nil {
// Provider was unavailable, error was logged on goroutine
continue
}
zapProvider := zap.String("provider", p.priorityList[i])
select {
case <-ctx.Done():
return source.Source{}, fmt.Errorf("context was cancelled: %w", ctx.Err())
case reply := <-ch:
if reply.err != nil {
p.logger.Debug("Unavailable source provider", zapProvider, zap.Error(reply.err))
continue
}

p.logger.Info("Resolved source",
zap.String("provider", p.priorityList[i]), zap.Any("source", reply.src),
)
return reply.src, nil
p.logger.Info("Resolved source", zapProvider, zap.Any("source", reply.src))
return reply.src, nil
}
}

return source.Source{}, fmt.Errorf("no source provider was available")
Expand Down

0 comments on commit cfb8f20

Please sign in to comment.