Added graceful shutdown timeout #1825
Conversation
pkg/beyla/config.go
Outdated
| @@ -45,6 +45,7 @@ const ( | |||
| var DefaultConfig = Config{ | |||
| ChannelBufferLen: 10, | |||
| LogLevel: "INFO", | |||
| ShutdownTimeout: 5 * time.Minute, | |||
There was a problem hiding this comment.
I'm not sure what is the best default time to set, left it pretty big imho
There was a problem hiding this comment.
It should be few seconds (maybe 10?). Actually Kubernetes would kill Beyla before that timeout.
There was a problem hiding this comment.
okay, changed it to 10 seconds, thanks
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1825 +/- ##
==========================================
+ Coverage 70.23% 74.28% +4.04%
==========================================
Files 173 177 +4
Lines 18435 19222 +787
==========================================
+ Hits 12948 14279 +1331
+ Misses 4646 4214 -432
+ Partials 841 729 -112
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pkg/beyla/config.go
Outdated
| @@ -45,6 +45,7 @@ const ( | |||
| var DefaultConfig = Config{ | |||
| ChannelBufferLen: 10, | |||
| LogLevel: "INFO", | |||
| ShutdownTimeout: 5 * time.Minute, | |||
There was a problem hiding this comment.
It should be few seconds (maybe 10?). Actually Kubernetes would kill Beyla before that timeout.
|
|
||
| // of one of both nodes fail, the other should stop | ||
| ctx, cancel := context.WithCancel(ctx) | ||
| errs := make(chan error, 2) | ||
| g, ctx := errgroup.WithContext(ctx) |
There was a problem hiding this comment.
Nice!!! Didn't know this tool.
| go func() { | ||
| log := log() | ||
| for { | ||
| select { | ||
| case <-i.ctx.Done(): | ||
| log.Debug("stopped searching for new processes to instrument. Waiting for the eBPF tracers to be unloaded") |
There was a problem hiding this comment.
I'd anyway log the messages, for debugging purposes.
There was a problem hiding this comment.
This log is also still there, just moved to Instrumenter.stop() :)
pkg/internal/appolly/appolly.go
Outdated
|
|
||
| return nil | ||
| func (i *Instrumenter) Stop() error { |
There was a problem hiding this comment.
What's the reason to move the stop/wait code to a function that needs to be explicitly invoked?
Maybe it's just a personal taste, but the previous version looked more "self-contained" as it was a single function invocation.
There was a problem hiding this comment.
yeah, a matter of taste, but no problem, I returned a single call to the public run function and made the stop function private
|
|
d3a5df9 to
9bfd941
Compare
|
@grcevski thanks, fixed the pipeline :) |
|
Thanks so much again for your contribution @almostinf ! |
Refactored of starting beyla components and added a graceful shutdown timeout
Closes #820