JaegerReceiver: Do not try to stop if failed to start. Collector service will do that #1434
Conversation
bogdandrutu
requested review from
dmitryax,
james-bebbington,
nilebox,
owais,
pjanotti and
tigrannajaryan
as code owners
Codecov Report
@@ Coverage Diff @@
## master #1434 +/- ##
==========================================
- Coverage 90.31% 90.29% -0.03%
==========================================
Files 232 232
Lines 16392 16385 -7
==========================================
- Hits 14805 14795 -10
- Misses 1134 1136 +2
- Partials 453 454 +1
Continue to review full report at Codecov.
|
| return jr.stopTraceReceptionLocked() | ||
| } | ||
|
|
||
| func (jr *jReceiver) stopTraceReceptionLocked() error { | ||
| var err = componenterror.ErrAlreadyStopped | ||
| jr.stopOnce.Do(func() { |
There was a problem hiding this comment.
Doesn't need to block this PR but Is this stopOnce needed? I believe we decided that collector guarantees Shutdown is only called once. Right now there are a mixture of components with stopOnce so we should probably get rid of all of them if not needed.
There was a problem hiding this comment.
Would address in a separate PR.
There was a problem hiding this comment.
Given you removed mu from Shutdown in this PR, I would recommend removing this from Start as well (and may as well remove startOnce / stopOnce as well while you're at it 😄 )
There was a problem hiding this comment.
Did I? It is just moved the next line.
| return jr.stopTraceReceptionLocked() | ||
| } | ||
|
|
||
| func (jr *jReceiver) stopTraceReceptionLocked() error { | ||
| var err = componenterror.ErrAlreadyStopped | ||
| jr.stopOnce.Do(func() { |
There was a problem hiding this comment.
Given you removed mu from Shutdown in this PR, I would recommend removing this from Start as well (and may as well remove startOnce / stopOnce as well while you're at it 😄 )
…ice will do that Signed-off-by: Bogdan Drutu <[email protected]>
* Add a Config/Option for histogram * Just one option here * Test fixes * Support and test int64 histograms * Changelog * Lint * Un-export three things.
Fixes #437