Simplify process FindAndInstrument synchronization #1579
Conversation
| instr := appolly.New(ctx, ctxInfo, config) | ||
| if err := instr.FindAndInstrument(&wg); err != nil { |
There was a problem hiding this comment.
This change avoids sharing the ownership of a critical synchronization mechanism between different components.
The usual convention in Go is expose a simpler synchronization mechanism, such as returning a close channel.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1579 +/- ##
==========================================
+ Coverage 71.20% 71.23% +0.02%
==========================================
Files 197 197
Lines 19754 19750 -4
==========================================
+ Hits 14066 14068 +2
+ Misses 5006 5002 -4
+ Partials 682 680 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| ctx context.Context | ||
| cancel func() | ||
| } | ||
| contexts := map[uint64]cancelCtx{} |
There was a problem hiding this comment.
Didn't saw the utility of storing all the contexts and explicitly cancelling them after the parent context is cancelled.
If the parent context its Done, all the child contexts are Done in cascade.
| @@ -95,7 +87,7 @@ func (i *Instrumenter) FindAndInstrument(wg *sync.WaitGroup) error { | |||
| } | |||
| }() | |||
| // TODO: wait until all the resources have been freed/unmounted | |||
There was a problem hiding this comment.
Not sure if this already happens. I will try to check/fix the RingbufForwarder in a future PR.
To be merged after 2.0 is released