chore: make heartbeat interval configurable

Signed-off-by: Alex Castilio dos Santos <[email protected]>

cmd/standard/daemon.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"os"
 	"strings"
-	"time"
 
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
@@ -48,8 +47,7 @@ import (
 )
 
 const (
-	logFileName       = "retina.log"
-	heartbeatInterval = 15 * time.Minute
+	logFileName = "retina.log"
 
 	nodeNameEnvKey = "NODE_NAME"
 	nodeIPEnvKey   = "NODE_IP"
@@ -309,7 +307,7 @@ func (d *Daemon) Start() error {
 	defer controllerMgr.Stop(ctx)
 
 	// start heartbeat goroutine for application insights
-	go tel.Heartbeat(ctx, heartbeatInterval)
+	go tel.Heartbeat(ctx, daemonConfig.TelemetryInterval)
 
 	// Start controller manager, which will start http server and plugin manager.
 	go controllerMgr.Start(ctx)

deploy/standard/manifests/controller/helm/retina/templates/configmap.yaml

@@ -24,6 +24,7 @@ data:
     enableAnnotations: {{ .Values.enableAnnotations }}
     bypassLookupIPOfInterest: {{ .Values.bypassLookupIPOfInterest }}
     dataAggregationLevel: {{ .Values.dataAggregationLevel }}
+    telemetryInterval: {{ .Values.daemonset.telemetryInterval }}
 {{- end}}
 ---
 {{- if .Values.os.windows}}
@@ -48,6 +49,7 @@ data:
     enableTelemetry: {{ .Values.enableTelemetry }}
     enablePodLevel: {{ .Values.enablePodLevel }}
     remoteContext: {{ .Values.remoteContext }}
+    telemetryInterval: {{ .Values.daemonset.telemetryInterval }}
 {{- end}}
 
 

deploy/standard/manifests/controller/helm/retina/templates/operator.yaml

@@ -307,6 +307,7 @@ data:
     captureDebug: {{ .Values.capture.debug }}
     captureJobNumLimit: {{ .Values.capture.jobNumLimit }}
     enableManagedStorageAccount: {{ .Values.capture.enableManagedStorageAccount }}
+    telemetryInterval: {{ .Values.operator.telemetryInterval }}
 {{- if .Values.capture.enableManagedStorageAccount }}
     azureCredentialConfig: /etc/cloud-config/azure.json
 {{- end }}

deploy/standard/manifests/controller/helm/retina/values.yaml

@@ -27,6 +27,7 @@ operator:
     args:
       - "--config"
       - "/retina/operator-config.yaml"
+  telemetryInterval: "5m"
 
 image:
   repository: ghcr.io/microsoft/retina/retina-agent
@@ -87,6 +88,7 @@ daemonset:
       metricsBindAddress: ":18080"
       ports:
         containerPort: 10093
+  telemetryInterval: "15m"
 
 # volume mounts with name and mountPath
 volumeMounts:

operator/cmd/standard/deployment.go

@@ -9,7 +9,6 @@ import (
 	"net/http"
 	"net/http/pprof"
 	"os"
-	"time"
 
 	"go.uber.org/zap/zapcore"
 
@@ -55,8 +54,6 @@ var (
 	MaxFileSizeMB = 100
 	MaxBackups    = 3
 	MaxAgeDays    = 30
-
-	HeartbeatFrequency = 5 * time.Minute
 )
 
 func init() {
@@ -255,7 +252,7 @@ func (o *Operator) Start() {
 	}
 
 	// start heartbeat goroutine for application insights
-	go tel.Heartbeat(ctx, HeartbeatFrequency)
+	go tel.Heartbeat(ctx, oconfig.TelemetryInterval)
 }
 
 func EnablePProf() {

operator/config/config.go

@@ -2,20 +2,30 @@ package config
 
 import (
 	"fmt"
+	"log"
+	"time"
 
 	"github.com/microsoft/retina/pkg/config"
 	"github.com/spf13/viper"
 )
 
+const MinTelemetryInterval time.Duration = 2 * time.Minute
+
+var (
+	DefaultTelemetryInterval = 5 * time.Minute
+	ErrorTelemetryIntervalTooSmall = fmt.Errorf("telemetryInterval smaller than %v is not allowed", MinTelemetryInterval)
+)
+
 type OperatorConfig struct {
 	config.CaptureConfig `mapstructure:",squash"`
 
 	InstallCRDs     bool   `yaml:"installCRDs"`
 	EnableTelemetry bool   `yaml:"enableTelemetry"`
 	LogLevel        string `yaml:"logLevel"`
 	// EnableRetinaEndpoint indicates whether to enable RetinaEndpoint
-	EnableRetinaEndpoint bool `yaml:"enableRetinaEndpoint"`
-	RemoteContext        bool `yaml:"remoteContext"`
+	EnableRetinaEndpoint bool          `yaml:"enableRetinaEndpoint"`
+	RemoteContext        bool          `yaml:"remoteContext"`
+	TelemetryInterval    time.Duration `yaml:"telemetryInterval"`
 }
 
 func GetConfig(cfgFileName string) (*OperatorConfig, error) {
@@ -35,5 +45,13 @@ func GetConfig(cfgFileName string) (*OperatorConfig, error) {
 		return nil, fmt.Errorf("error unmarshalling config: %w", err)
 	}
 
+	// If unset, default telemetry interval to 5 minutes.
+	if cfg.TelemetryInterval == 0 {
+		log.Printf("telemetryInterval is not set, defaulting to %v", DefaultTelemetryInterval)
+		cfg.TelemetryInterval = DefaultTelemetryInterval
+	} else if cfg.TelemetryInterval < MinTelemetryInterval {
+		return nil, ErrorTelemetryIntervalTooSmall
+	}
+
 	return &cfg, nil
 }

operator/config/config_test.go

@@ -0,0 +1,43 @@
+package config_test
+
+import (
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/microsoft/retina/operator/config"
+)
+
+func TestGetConfig(t *testing.T) {
+	c, err := config.GetConfig("./testwith/config.yaml")
+	if err != nil {
+		t.Errorf("Expected no error, instead got %+v", err)
+	}
+
+	if !c.InstallCRDs ||
+		!c.EnableTelemetry ||
+		c.LogLevel != "info" ||
+		!c.EnableRetinaEndpoint ||
+		!c.RemoteContext ||
+		c.TelemetryInterval != 15*time.Minute {
+		t.Errorf("Expeted config should be same as ./testwith/config.yaml; instead got %+v", c)
+	}
+}
+
+func TestGetConfig_DefaultTelemetryInterval(t *testing.T) {
+	c, err := config.GetConfig("./testwith/config-without-telemetry-interval.yaml")
+	if err != nil {
+		t.Errorf("Expected no error, instead got %+v", err)
+	}
+
+	if c.TelemetryInterval != config.DefaultTelemetryInterval {
+		t.Errorf("Expected telemetry interval to be %v, instead got %v", config.DefaultTelemetryInterval, c.TelemetryInterval)
+	}
+}
+
+func TestGetConfig_SmallTelemetryInterval(t *testing.T) {
+	_, err := config.GetConfig("./testwith/config-small-telemetry-interval.yaml")
+	if !errors.Is(err, config.ErrorTelemetryIntervalTooSmall) {
+		t.Errorf("Expected error %s, instead got %s", config.ErrorTelemetryIntervalTooSmall, err)
+	}
+}

operator/config/testwith/config-small-telemetry-interval.yaml

@@ -0,0 +1,9 @@
+apiServer:
+  host: "0.0.0.0"
+  port: 10093
+installCRDs: true
+enableTelemetry: true
+logLevel: info
+enableRetinaEndpoint: true
+remoteContext: true
+telemetryInterval: "10s"

operator/config/testwith/config-without-telemetry-interval.yaml

@@ -0,0 +1,8 @@
+apiServer:
+  host: "0.0.0.0"
+  port: 10093
+installCRDs: true
+enableTelemetry: true
+logLevel: info
+enableRetinaEndpoint: true
+remoteContext: true

operator/config/testwith/config.yaml

@@ -0,0 +1,9 @@
+apiServer:
+  host: "0.0.0.0"
+  port: 10093
+installCRDs: true
+enableTelemetry: true
+logLevel: info
+enableRetinaEndpoint: true
+remoteContext: true
+telemetryInterval: "15m"

pkg/config/config.go

@@ -16,11 +16,18 @@ import (
 // Level defines the level of monitor aggregation.
 type Level int
 
+const MinTelemetryInterval time.Duration = 2 * time.Minute
+
 const (
 	Low Level = iota
 	High
 )
 
+var (
+	ErrorTelemetryIntervalTooSmall = fmt.Errorf("telemetryInterval smaller than %v is not allowed", MinTelemetryInterval)
+	DefaultTelemetryInterval = 15 * time.Minute
+)
+
 func (l *Level) UnmarshalText(text []byte) error {
 	s := strings.ToLower(string(text))
 	switch s {
@@ -67,6 +74,7 @@ type Config struct {
 	BypassLookupIPOfInterest bool          `yaml:"bypassLookupIPOfInterest"`
 	DataAggregationLevel     Level         `yaml:"dataAggregationLevel"`
 	MonitorSockPath          string        `yaml:"monitorSockPath"`
+	TelemetryInterval        time.Duration `yaml:"telemetryInterval"`
 }
 
 func GetConfig(cfgFilename string) (*Config, error) {
@@ -107,6 +115,14 @@ func GetConfig(cfgFilename string) (*Config, error) {
 		log.Print("metricsInterval is deprecated, please use metricsIntervalDuration instead")
 	}
 
+	// If unset, default telemetry interval to 15 minutes.
+	if config.TelemetryInterval == 0 {
+		log.Printf("telemetryInterval is not set, defaulting to %v", DefaultTelemetryInterval)
+		config.TelemetryInterval = DefaultTelemetryInterval
+	} else if config.TelemetryInterval < MinTelemetryInterval {
+		return nil, ErrorTelemetryIntervalTooSmall
+	}
+
 	return &config, nil
 }
 

pkg/config/config_test.go

@@ -4,6 +4,7 @@
 package config
 
 import (
+	"errors"
 	"reflect"
 	"testing"
 	"time"
@@ -26,8 +27,26 @@ func TestGetConfig(t *testing.T) {
 		!c.EnableRetinaEndpoint ||
 		c.RemoteContext ||
 		c.EnableAnnotations ||
+		c.TelemetryInterval != 15*time.Minute ||
 		c.DataAggregationLevel != Low {
-		t.Fatalf("Expeted config should be same as ./testwith/config.yaml; instead got %+v", c)
+		t.Errorf("Expeted config should be same as ./testwith/config.yaml; instead got %+v", c)
+	}
+}
+
+func TestGetConfig_SmallTelemetryInterval(t *testing.T) {
+	_, err := GetConfig("./testwith/config-small-telemetry-interval.yaml")
+	if !errors.Is(err, ErrorTelemetryIntervalTooSmall) {
+		t.Errorf("Expected error %s, instead got %s", ErrorTelemetryIntervalTooSmall, err)
+	}
+}
+
+func TestGetConfig_DefaultTelemetryInterval(t *testing.T) {
+	c, err := GetConfig("./testwith/config-without-telemetry-interval.yaml")
+	if err != nil {
+		t.Errorf("Expected no error, instead got %+v", err)
+	}
+	if c.TelemetryInterval != DefaultTelemetryInterval {
+		t.Errorf("Expected telemetry interval to be %v, instead got %v", DefaultTelemetryInterval, c.TelemetryInterval)
 	}
 }
 

pkg/config/testwith/config-small-telemetry-interval.yaml

@@ -0,0 +1,12 @@
+apiServer:
+  host: "0.0.0.0"
+  port: 10093
+# Supported - debug, info, error, warn, panic, fatal.
+logLevel: info
+enabledPlugin: ["dropreason", "packetforward", "linuxutil"]
+# Interval, in seconds, to scrape/publish metrics.
+metricsIntervalDuration: "10s"
+# used to export telemetry to AppInsights
+telemetryEnabled: true
+dataAggregationLevel: "low"
+telemetryInterval: "10s"

pkg/config/testwith/config-without-telemetry-interval.yaml

@@ -0,0 +1,11 @@
+apiServer:
+  host: "0.0.0.0"
+  port: 10093
+# Supported - debug, info, error, warn, panic, fatal.
+logLevel: info
+enabledPlugin: ["dropreason", "packetforward", "linuxutil"]
+# Interval, in seconds, to scrape/publish metrics.
+metricsIntervalDuration: "10s"
+# used to export telemetry to AppInsights
+telemetryEnabled: true
+dataAggregationLevel: "low"

pkg/config/testwith/config.yaml

@@ -9,3 +9,4 @@ metricsIntervalDuration: "10s"
 # used to export telemetry to AppInsights
 telemetryEnabled: true
 dataAggregationLevel: "low"
+telemetryInterval: "15m"