probe: Stats gathering can be started twice

[ekimekim]

In probe/docker/container.go, StartGatheringStats(), we check if c.statsConn is set and only start a new stats connection if it is not. However, we do not immediately then set this value (that happens only after some other setup inside a goroutine we then launch), meaning it might not be set by the time we get called again.
In this case, we would have two connections reporting stats for the same probe.

The simplest fix would be to add a statsStarted bool to the struct, and do:

c.Lock()
if c.statsStarted {
  c.Unlock()
  return nil
}
c.statsStarted = true
c.Unlock()

perhaps modulo some nicer handling of the Unlock

[rade]

I was wondering whether we could simply do everything up to setting c.statsConn before spawning the go-routine.

[ekimekim]

That might have further impact, since it seems to be doing potentially long things like making http requests. I don't know the full context of this code.

[rade]

That might have further impact

Hence "I was wondering whether we could" rather than "We should" :)

[ekimekim]

Ah, on closer inspection, it sets up and makes a new connection, but then pushes data over it long-term. So you'd still block for the duration of connect(), which may not be acceptable.

[2opremio]

I think we should try using the Stats() function from the client library. We discarded it in the past due to some problems I don't recall right now, but those problems (cancellation?) may be gone now.