Repository: kolide/launcher

Issue:
Enable performance monitoring flare uploads for high CPU usage
The [performance monitor](https://github.com/kolide/launcher/blob/main/ee/debug/performance_monitor.go) currently only triggers a flare upload if it notices high Golang memory usage. However, we should also trigger a flare if we notice CPU usage above 10%.

- [x] The performance monitor should also check CPU usage, and trigger a flare upload if it's above 10%
- [x] The performance monitor should use `shipper.WithNote(p.uploadNote("high CPU usage"))` to set the note appropriately when it's a high CPU flare, `shipper.WithNote(p.uploadNote("high Golang memory usage"))` when it's a high Golang memory usage flare, and `shipper.WithNote(p.uploadNote("high CPU and Golang memory usage"))` when both values are in violation
- [x] All log messages in the performance monitor should be updated to refer high CPU usage or high memory usage, not just high memory usage, and to include "cpu" in the log fields

Discussion:

Solution patch:
diff --git a/ee/debug/performance_monitor.go b/ee/debug/performance_monitor.go
index 8862339a..ef91eb9c 100644
--- a/ee/debug/performance_monitor.go
+++ b/ee/debug/performance_monitor.go
@@ -17,7 +17,8 @@ const (
 	performanceCheckInitialDelay      = 5 * time.Minute
 	performanceCheckInterval          = 15 * time.Minute
 	golangMemUsageThresholdForFlare   = 1000 * 1024 * 1024 // 1 GB in bytes
-	minimumFlareResendIntervalSeconds = 24 * 60 * 60       // 1 day
+	cpuUsageThresholdForFlare         = 10
+	minimumFlareResendIntervalSeconds = 24 * 60 * 60 // 1 day
 	flareUploadRequestUrl             = "https://api.kolide.com/api/agent/flare"
 	notePrefix                        = "automated flare"
 )
@@ -83,37 +84,26 @@ func (p *performanceMonitor) checkPerformance() {
 		return
 	}
 
-	// Check if we should trigger a flare
-	if stats.MemInfo.GoMemUsage < golangMemUsageThresholdForFlare {
+	doUpload, uploadReason := p.shouldTriggerFlareUpload(ctx, stats)
+	if !doUpload {
 		return
 	}
 
-	// Make sure we haven't sent a flare too recently
-	nextAllowableFlareTimestamp := p.lastFlareSent.Load() + minimumFlareResendIntervalSeconds
-	if nextAllowableFlareTimestamp > time.Now().Unix() {
-		p.slogger.Log(ctx, slog.LevelWarn,
-			"noticed abnormally high Golang memory usage while monitoring, but triggered flare too recently",
-			"golang_memory_usage", stats.MemInfo.GoMemUsage,
-			"non_golang_memory_usage", stats.MemInfo.NonGoMemUsage,
-			"rss", stats.MemInfo.RSS,
-		)
-		return
-	}
-
-	// If we've noticed extraordinarily high Golang memory usage, trigger a flare.
-	// Since we're looking at Golang memory here, the flare's memprofile will be useful
-	// in understanding what's using memory.
+	// If we've noticed extraordinarily high Golang memory or CPU usage, trigger a flare.
+	// Since we're looking at Golang memory and CPU here, the flare's memprofile and cpuprofile
+	// will be useful in understanding what's using memory.
 	p.slogger.Log(ctx, slog.LevelWarn,
-		"noticed abnormally high Golang memory usage while monitoring, triggering flare",
+		"noticed abnormally high Golang memory and/or CPU usage while monitoring, triggering flare",
 		"golang_memory_usage", stats.MemInfo.GoMemUsage,
 		"non_golang_memory_usage", stats.MemInfo.NonGoMemUsage,
 		"rss", stats.MemInfo.RSS,
+		"cpu", stats.CPUPercent,
 	)
 
-	flareShipper, err := shipper.New(p.knapsack, shipper.WithUploadRequestURL(flareUploadRequestUrl), shipper.WithNote(p.uploadNote("high Golang memory usage")))
+	flareShipper, err := shipper.New(p.knapsack, shipper.WithUploadRequestURL(flareUploadRequestUrl), shipper.WithNote(p.uploadNote(uploadReason)))
 	if err != nil {
 		p.slogger.Log(ctx, slog.LevelError,
-			"could not create flare shipper to capture high Golang memory usage",
+			"could not create flare shipper to capture high Golang memory and/or CPU usage",
 			"err", err,
 		)
 		return
@@ -121,20 +111,54 @@ func (p *performanceMonitor) checkPerformance() {
 
 	if err := checkups.RunFlare(ctx, p.knapsack, flareShipper, checkups.InSituEnvironment); err != nil {
 		p.slogger.Log(ctx, slog.LevelError,
-			"could not run and ship flare to capture high Golang memory usage",
+			"could not run and ship flare to capture high Golang memory and/or CPU usage",
 			"err", err,
 		)
 		return
 	}
 
 	p.slogger.Log(ctx, slog.LevelInfo,
-		"successfully triggered flare to capture abnormally high Golang memory usage",
+		"successfully triggered flare to capture abnormally high Golang memory and/or CPU usage",
 		"flare_id", flareShipper.Name(),
 	)
 
 	p.lastFlareSent.Store(time.Now().Unix())
 }
 
+// shouldTriggerFlareUpload checks stats and last sent flare timestamp to determine if it should
+// trigger an automatic flare upload. If a flare should be uploaded, it also returns the reason
+// for the flare, to include in the upload note.
+func (p *performanceMonitor) shouldTriggerFlareUpload(ctx context.Context, stats *performance.PerformanceStats) (bool, string) {
+	// Check if performance stats warrant a flare upload
+	excessiveMemUsage := stats.MemInfo.GoMemUsage >= golangMemUsageThresholdForFlare
+	excessiveCpuUsage := stats.CPUPercent >= cpuUsageThresholdForFlare
+	if !excessiveMemUsage && !excessiveCpuUsage {
+		return false, ""
+	}
+
+	// Make sure we haven't sent a flare too recently
+	nextAllowableFlareTimestamp := p.lastFlareSent.Load() + minimumFlareResendIntervalSeconds
+	if nextAllowableFlareTimestamp > time.Now().Unix() {
+		p.slogger.Log(ctx, slog.LevelWarn,
+			"noticed abnormally high Golang memory and/or CPU usage while monitoring, but triggered flare too recently",
+			"golang_memory_usage", stats.MemInfo.GoMemUsage,
+			"non_golang_memory_usage", stats.MemInfo.NonGoMemUsage,
+			"rss", stats.MemInfo.RSS,
+			"cpu", stats.CPUPercent,
+		)
+		return false, ""
+	}
+
+	uploadReason := "high Golang memory usage"
+	if excessiveMemUsage && excessiveCpuUsage {
+		uploadReason = "high Golang memory and CPU usage"
+	} else if excessiveCpuUsage {
+		uploadReason = "high CPU usage"
+	}
+
+	return true, uploadReason
+}
+
 // uploadNote creates a flare note, identifying that this is a) an automated flare, b) the reason
 // the flare is being created, and c) the device via the serial, if available.
 func (p *performanceMonitor) uploadNote(reason string) string {



Test patch:
diff --git a/ee/debug/performance_monitor_test.go b/ee/debug/performance_monitor_test.go
index 78977559..53822cda 100644
--- a/ee/debug/performance_monitor_test.go
+++ b/ee/debug/performance_monitor_test.go
@@ -1,12 +1,15 @@
 package debug
 
 import (
+	"context"
 	"errors"
 	"log/slog"
 	"testing"
 	"time"
 
 	typesmocks "github.com/kolide/launcher/ee/agent/types/mocks"
+	"github.com/kolide/launcher/ee/performance"
+	"github.com/kolide/launcher/pkg/log/multislogger"
 	"github.com/kolide/launcher/pkg/threadsafebuffer"
 	"github.com/stretchr/testify/require"
 )
@@ -57,3 +60,114 @@ func TestInterrupt_Multiple(t *testing.T) {
 
 	require.Equal(t, expectedInterrupts, receivedInterrupts)
 }
+
+func Test_checkPerformance_notEnabled(t *testing.T) {
+	t.Parallel()
+
+	testKnapsack := typesmocks.NewKnapsack(t)
+	testKnapsack.On("Slogger").Return(multislogger.NewNopLogger())
+	testKnapsack.On("PerformanceMonitoringEnabled").Return(false)
+
+	p := NewPerformanceMonitor(testKnapsack)
+
+	p.checkPerformance()
+
+	// Expect that we did not trigger a flare
+	require.Equal(t, int64(0), p.lastFlareSent.Load())
+}
+
+func Test_shouldTriggerFlareUpload(t *testing.T) {
+	t.Parallel()
+
+	for _, tt := range []struct {
+		testCaseName      string
+		cpuPercent        float64
+		goMemUsage        uint64
+		lastSentTimestamp int64
+		expectFlareUpload bool
+		expectedReason    string
+	}{
+		{
+			testCaseName:      "CPU too high, flare not triggered recently",
+			cpuPercent:        cpuUsageThresholdForFlare + 5,
+			goMemUsage:        golangMemUsageThresholdForFlare - 800,
+			lastSentTimestamp: time.Now().Unix() - minimumFlareResendIntervalSeconds - 5000,
+			expectFlareUpload: true,
+			expectedReason:    "high CPU usage",
+		},
+		{
+			testCaseName:      "memory too high, flare not triggered recently",
+			cpuPercent:        cpuUsageThresholdForFlare - 3,
+			goMemUsage:        golangMemUsageThresholdForFlare + 1500,
+			lastSentTimestamp: time.Now().Unix() - minimumFlareResendIntervalSeconds - 1000,
+			expectFlareUpload: true,
+			expectedReason:    "high Golang memory usage",
+		},
+		{
+			testCaseName:      "CPU and memory too high, flare not triggered recently",
+			cpuPercent:        cpuUsageThresholdForFlare + 4,
+			goMemUsage:        golangMemUsageThresholdForFlare + 6000,
+			lastSentTimestamp: time.Now().Unix() - minimumFlareResendIntervalSeconds - 600,
+			expectFlareUpload: true,
+			expectedReason:    "high Golang memory and CPU usage",
+		},
+		{
+			testCaseName:      "CPU and memory within allowable range, flare not triggered recently",
+			cpuPercent:        cpuUsageThresholdForFlare - 2,
+			goMemUsage:        golangMemUsageThresholdForFlare - 4000,
+			lastSentTimestamp: time.Now().Unix() - minimumFlareResendIntervalSeconds - 300,
+			expectFlareUpload: false,
+		},
+		{
+			testCaseName:      "CPU too high, flare triggered recently",
+			cpuPercent:        cpuUsageThresholdForFlare + 6,
+			goMemUsage:        golangMemUsageThresholdForFlare - 1000,
+			lastSentTimestamp: time.Now().Unix() - (minimumFlareResendIntervalSeconds - 5000),
+			expectFlareUpload: false,
+		},
+		{
+			testCaseName:      "memory too high, flare triggered recently",
+			cpuPercent:        cpuUsageThresholdForFlare - 1,
+			goMemUsage:        golangMemUsageThresholdForFlare + 2000,
+			lastSentTimestamp: time.Now().Unix() - (minimumFlareResendIntervalSeconds - 1000),
+			expectFlareUpload: false,
+		},
+		{
+			testCaseName:      "CPU and memory too high, flare triggered recently",
+			cpuPercent:        cpuUsageThresholdForFlare + 7,
+			goMemUsage:        golangMemUsageThresholdForFlare + 3000,
+			lastSentTimestamp: time.Now().Unix() - (minimumFlareResendIntervalSeconds - 600),
+			expectFlareUpload: false,
+		},
+		{
+			testCaseName:      "CPU and memory within allowable range, flare triggered recently",
+			cpuPercent:        cpuUsageThresholdForFlare - 2,
+			goMemUsage:        golangMemUsageThresholdForFlare - 900,
+			lastSentTimestamp: time.Now().Unix() - (minimumFlareResendIntervalSeconds - 300),
+			expectFlareUpload: false,
+		},
+	} {
+		tt := tt
+		t.Run(tt.testCaseName, func(t *testing.T) {
+			t.Parallel()
+
+			testKnapsack := typesmocks.NewKnapsack(t)
+			testKnapsack.On("Slogger").Return(multislogger.NewNopLogger())
+
+			p := NewPerformanceMonitor(testKnapsack)
+			p.lastFlareSent.Store(tt.lastSentTimestamp)
+
+			testStats := &performance.PerformanceStats{
+				CPUPercent: tt.cpuPercent,
+				MemInfo: &performance.MemInfo{
+					GoMemUsage: tt.goMemUsage,
+				},
+			}
+
+			performUpload, uploadReason := p.shouldTriggerFlareUpload(context.TODO(), testStats)
+
+			require.Equal(t, tt.expectFlareUpload, performUpload)
+			require.Equal(t, tt.expectedReason, uploadReason)
+		})
+	}
+}