JMXFetch cycle management on Windows (#5309)

Ports the lifecycle management code to Windows. No code changes were needed, since the Linux/Mac code seems to work fine on Windows too.

pkg/collector/corechecks/embed/jmx/runner.go

@@ -8,7 +8,6 @@
 package jmx
 
 import (
-	"runtime"
 	"time"
 
 	"github.com/DataDog/datadog-agent/pkg/autodiscovery/integration"
@@ -30,10 +29,6 @@ func (r *runner) initRunner() {
 func (r *runner) startRunner() error {
 
 	lifecycleMgmt := true
-	if runtime.GOOS == "windows" {
-		lifecycleMgmt = false
-	}
-
 	err := r.jmxfetch.Start(lifecycleMgmt)
 	if err != nil {
 		s := status.JMXStartupError{LastError: err.Error(), Timestamp: time.Now().Unix()}

pkg/jmxfetch/jmxfetch.go

@@ -21,6 +21,7 @@ import (
 	api "github.com/DataDog/datadog-agent/pkg/api/util"
 	"github.com/DataDog/datadog-agent/pkg/autodiscovery/integration"
 	"github.com/DataDog/datadog-agent/pkg/config"
+	"github.com/DataDog/datadog-agent/pkg/status"
 	"github.com/DataDog/datadog-agent/pkg/status/health"
 	"github.com/DataDog/datadog-agent/pkg/util/log"
 	"gopkg.in/yaml.v2"
@@ -105,6 +106,56 @@ type checkInitCfg struct {
 	JavaOptions    string   `yaml:"java_options,omitempty"`
 }
 
+func (j *JMXFetch) Monitor() {
+	idx := 0
+	maxRestarts := config.Datadog.GetInt("jmx_max_restarts")
+	ival := float64(config.Datadog.GetInt("jmx_restart_interval"))
+	stopTimes := make([]time.Time, maxRestarts)
+	ticker := time.NewTicker(500 * time.Millisecond)
+
+	defer ticker.Stop()
+	defer close(j.stopped)
+
+	go j.heartbeat(ticker)
+
+	for {
+		err := j.Wait()
+		if err == nil {
+			log.Infof("JMXFetch stopped and exited sanely.")
+			break
+		}
+
+		stopTimes[idx] = time.Now()
+		oldestIdx := (idx + maxRestarts + 1) % maxRestarts
+
+		// Please note that the zero value for `time.Time` is `0001-01-01 00:00:00 +0000 UTC`
+		// therefore for the first iteration (the initial launch attempt), the interval will
+		// always be biger than ival (jmx_restart_interval). In fact, this sub operation with
+		// stopTimes here will only start yielding values potentially <= ival _after_ the first
+		// maxRestarts attempts, which is fine and consistent.
+		if stopTimes[idx].Sub(stopTimes[oldestIdx]).Seconds() <= ival {
+			msg := fmt.Sprintf("Too many JMXFetch restarts (%v) in time interval (%vs) - giving up", maxRestarts, ival)
+			log.Errorf(msg)
+			s := status.JMXStartupError{LastError: msg, Timestamp: time.Now().Unix()}
+			status.SetJMXStartupError(s)
+			return
+		}
+
+		idx = (idx + 1) % maxRestarts
+
+		select {
+		case <-j.shutdown:
+			return
+		default:
+			// restart
+			log.Warnf("JMXFetch process had to be restarted.")
+			j.Start(false)
+		}
+	}
+
+	<-j.shutdown
+}
+
 func (j *JMXFetch) setDefaults() {
 	if j.JavaBinPath == "" {
 		j.JavaBinPath = defaultJavaBinPath

pkg/jmxfetch/jmxfetch_nix.go

@@ -9,66 +9,13 @@
 package jmxfetch
 
 import (
-	"fmt"
 	"os"
 	"syscall"
 	"time"
 
-	"github.com/DataDog/datadog-agent/pkg/config"
-	"github.com/DataDog/datadog-agent/pkg/status"
 	"github.com/DataDog/datadog-agent/pkg/util/log"
 )
 
-func (j *JMXFetch) Monitor() {
-	idx := 0
-	maxRestarts := config.Datadog.GetInt("jmx_max_restarts")
-	ival := float64(config.Datadog.GetInt("jmx_restart_interval"))
-	stopTimes := make([]time.Time, maxRestarts)
-	ticker := time.NewTicker(500 * time.Millisecond)
-
-	defer ticker.Stop()
-	defer close(j.stopped)
-
-	go j.heartbeat(ticker)
-
-	for {
-		err := j.Wait()
-		if err == nil {
-			log.Infof("JMXFetch stopped and exited sanely.")
-			break
-		}
-
-		stopTimes[idx] = time.Now()
-		oldestIdx := (idx + maxRestarts + 1) % maxRestarts
-
-		// Please note that the zero value for `time.Time` is `0001-01-01 00:00:00 +0000 UTC`
-		// therefore for the first iteration (the initial launch attempt), the interval will
-		// always be biger than ival (jmx_restart_interval). In fact, this sub operation with
-		// stopTimes here will only start yielding values potentially <= ival _after_ the first
-		// maxRestarts attempts, which is fine and consistent.
-		if stopTimes[idx].Sub(stopTimes[oldestIdx]).Seconds() <= ival {
-			msg := fmt.Sprintf("Too many JMXFetch restarts (%v) in time interval (%vs) - giving up", maxRestarts, ival)
-			log.Errorf(msg)
-			s := status.JMXStartupError{LastError: msg, Timestamp: time.Now().Unix()}
-			status.SetJMXStartupError(s)
-			return
-		}
-
-		idx = (idx + 1) % maxRestarts
-
-		select {
-		case <-j.shutdown:
-			return
-		default:
-			// restart
-			log.Warnf("JMXFetch process had to be restarted.")
-			j.Start(false)
-		}
-	}
-
-	<-j.shutdown
-}
-
 // Stop stops the JMXFetch process
 func (j *JMXFetch) Stop() error {
 	var stopChan chan struct{}

pkg/jmxfetch/jmxfetch_windows.go

@@ -13,20 +13,26 @@ import (
 	"github.com/DataDog/datadog-agent/pkg/util/log"
 )
 
-func (j *JMXFetch) Monitor() {}
-
 // Stop stops the JMXFetch process
 func (j *JMXFetch) Stop() error {
+	var stopChan chan struct{}
+
 	err := j.cmd.Process.Kill()
 	if err != nil {
 		return err
 	}
 
-	stopChan := make(chan struct{})
-	go func() {
-		j.Wait()
-		close(stopChan)
-	}()
+	if j.managed {
+		stopChan = j.stopped
+		close(j.shutdown)
+	} else {
+		stopChan = make(chan struct{})
+
+		go func() {
+			j.Wait()
+			close(stopChan)
+		}()
+	}
 
 	select {
 	case <-time.After(time.Millisecond * 1000):

releasenotes/notes/jmx-windows-lifecycle-a3bea187d613c003.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    JMXFetch (helper for JMX checks) is now restarted if it crashes on Windows.