JMXFetch cycle management on Windows #5309
Conversation
albertvaka
force-pushed
the
albertvaka/jmx-windows-lifecycle
branch
from
3ab1338 to
6e72a8d
Compare
There was a problem hiding this comment.
While going over the jmxfetch.go file I noticed the Up() function here https://github.com/DataDog/datadog-agent/blob/master/pkg/jmxfetch/jmxfetch.go#L306-L318, I'm not sure it's used anymore, as there is an internal heartbeat logic.
I guess we can remove it if it's useless (and if that's not the case it should probably be updated).
| @@ -30,10 +29,6 @@ func (r *runner) initRunner() { | |||
| func (r *runner) startRunner() error { | |||
|
|
|||
| lifecycleMgmt := true | |||
There was a problem hiding this comment.
Is there any reason to keep the start(bool) vs just start() method ?
There was a problem hiding this comment.
This is a good question. I guess the only reason would be to keep it around is if we want to make it configurable via a user-facing (maybe hidden) config option.
I'm fine with removing the argument.
There was a problem hiding this comment.
There is a reason actually: when the restart logic triggers, it calls the same start function but passing false [1]. That's because the thread running the restart logic will continue running and we don't want to start another one.
[1] https://github.com/DataDog/datadog-agent/pull/5309/files#diff-7389c2caae3e69d2d88fd7cf12dc8792R152
What does this PR do?
Ports the lifecycle management code to Windows. No code changes were needed, since the Linux/Mac code seems to work fine on Windows too.
Motivation
If JMXFetch crashed on Windows it was not restarted.