routine performance checks part 1 #2184

zackattack01 · 2025-03-28T17:00:36Z

I'm breaking off this bit from the larger effort to routinely collect cpu/mem profiles so that we can get some discussion going about criteria for triggering these and ensure we're gathering all of the statistics we want correctly.

This does a few things:

upgrades our gopsutil package to v4. I didn't see any significant changes for our purposes but there were some bug fixes and memory leaks noted that I thought would make it worth upgrading now while we're looking at all of this closely. there were two small backwards compatibility breaks (updated in osquerylogs/logs.go and desktop/runner/runner_linux.go)
adds a new performance package for us to collect some documentation and helper functionality for handling these statistics. we do a lot of similar things in several places but if we're going to start taking actions based on this data we will likely need some stricter patterns/expectations/types to work off of. Let me know if there is more you'd like to see added here, it was just meant as a starting point
- this is very intentional in the data it collects for the PerformanceStats it exposes. All fields are expected to be supported on all platforms, and it does not include noisier things like the runtime GC stats, to allow us to add this to a log checkpoint without too much noise
Adds a new checkup which reports this data. I had originally only meant it to be a logCheckup since a lot of this information is covered in our existing runtime checkup, but it seemed helpful as a quicker reference while looking at flares/doctor so I included it there too. happy to remove if people disagree, I could have gone either way

In it's current form, this will emit checkup logs like this once an hour:

checkup log

{
  "time": "2025-03-28T16:21:22.834035Z",
  "level": "DEBUG",
  "source": {
    "function": "github.com/kolide/launcher/ee/debug/checkups.(*logCheckPointer).Once",
    "file": "/Users/zackolson/go/src/launcher/ee/debug/checkups/checkpoint.go",
    "line": 68
  },
  "msg": "ran checkup",
  "run_id": "01JQERXN2V9FF2ZVJ7GMS0G1QE",
  "component": "log_checkpoint",
  "checkup": "Performance",
  "summary": "process 52226 is using 13.84% CPU, RSS: 53.34 MB (0.08% memory). Note CPU will be higher while running flare.",
  "data": {
    "stats": {
      "pid": 52226,
      "exe": "/Users/zackolson/go/src/launcher/build/launcher",
      "mem_info": {
        "rss_bytes": 55934976,
        "vms_bytes": 422118621184,
        "heap_total_bytes": 11067392,
        "go_mem_bytes": 21505288,
        "non_go_mem_bytes": 34429688,
        "mem_percent": 0.0813961
      },
      "cpu_percent": 13.839578399624044
    }
  },
  "status": "Informational"
}

An informational line will also be added to doctor like so:
Performance: process 50853 is using 16.88% CPU, RSS: 40.66 MB (0.06% memory). Note CPU will be higher while running flare.

zackattack01 · 2025-03-28T18:30:23Z

there seems to be a data race introduced from the gopsutil update, it looks related to their shift from cgo to purego and difficult to avoid. I am going to try upgrading us to the release just before that happened instead

…ata races

RebeccaMahany · 2025-03-28T20:10:59Z

ee/debug/checkups/checkups.go

@@ -109,6 +109,7 @@ func checkupsFor(k types.Knapsack, target targetBits) []checkupInt {
 		{&desktopMenu{k: k}, flareSupported},
 		{&coredumpCheckup{}, doctorSupported | flareSupported},
 		{&downloadDirectory{}, flareSupported},
+		{&perfCheckup{}, doctorSupported | flareSupported | logSupported},


I think we maybe either don't want this in doctor, or would only want a modified version in doctor? Since doctor is always run via the command line, the runtime stats will never be accurate, right?

was thinking it would be nice in the doctor output in flares and didn't even consider that 🤦 you're absolutely right, will pull it

RebeccaMahany · 2025-03-28T20:12:42Z

ee/performance/stats.go

+- process.MemoryInfoStat (e.g. RSS, VMS). these help to give a picture of the usage from the OS perspective
+- runtime.MemStats (e.g. heap stats). these help to give a picture of the go runtime usage
+Used together, we are able to estimate things like allocations outside of our golang runtime (see nonGoMemUsage).
+The runtime MemStats can be confusing - here is an attempt at outlining some of the fields we're interested in:


Thank you for this, really helpful

directionless

nice

directionless · 2025-03-28T21:19:41Z

ee/performance/stats.go

+Used together, we are able to estimate things like allocations outside of our golang runtime (see nonGoMemUsage).
+The runtime MemStats can be confusing - here is an attempt at outlining some of the fields we're interested in:
+
+	Sys is total bytes of memory obtained from the OS. The virtual address space reserved by the Go runtime for:


love the comment, the whitespace is a little fishy...

directionless · 2025-03-28T21:21:08Z

ee/performance/stats.go

+	CPUPercent float64  `json:"cpu_percent"`
+}
+
+func CurrentProcessStats(ctx context.Context) (*PerformanceStats, error) {


One caveat here, is that this gathers stats for the current process. Which omits any desktop processes, or any osquery processes.

zackattack01 added 8 commits March 26, 2025 14:39

add template for performance checkup and upgrade gopsutil

2241575

get basic stats working with log checkups

42c73be

add docs for calculations, perf checkup updates

9432478

make work with doctor/flare

b37c4e2

remove double writes for perf data

18861d0

remove extra file name

4aa9cf4

finish comment for nonGoMemUsage

888869f

coerce uid to uint32 for gopsutil updates

a18d011

downgrade gopsutil to 4.24.8 to avoid purego shift and accompanying d…

92169ef

…ata races

RebeccaMahany reviewed Mar 28, 2025

View reviewed changes

zackattack01 and others added 2 commits March 28, 2025 16:27

pull perfCheckup from doctor and remove note for doctor output

7129066

Merge branch 'main' into zack/periodic_mem_checks

032f582

RebeccaMahany approved these changes Mar 28, 2025

View reviewed changes

zackattack01 added this pull request to the merge queue Mar 28, 2025

Merged via the queue into main with commit 5f4ce67 Mar 28, 2025
36 checks passed

zackattack01 deleted the zack/periodic_mem_checks branch March 28, 2025 21:03

directionless reviewed Mar 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

routine performance checks part 1 #2184

routine performance checks part 1 #2184

Uh oh!

zackattack01 commented Mar 28, 2025 •

edited

Loading

Uh oh!

zackattack01 commented Mar 28, 2025

Uh oh!

RebeccaMahany Mar 28, 2025

Uh oh!

zackattack01 Mar 28, 2025

Uh oh!

RebeccaMahany Mar 28, 2025

Uh oh!

Uh oh!

directionless left a comment

Uh oh!

directionless Mar 28, 2025

Uh oh!

directionless Mar 28, 2025

Uh oh!

Uh oh!

routine performance checks part 1 #2184

routine performance checks part 1 #2184

Uh oh!

Conversation

zackattack01 commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zackattack01 commented Mar 28, 2025

Uh oh!

RebeccaMahany Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

zackattack01 Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

RebeccaMahany Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

directionless left a comment

Choose a reason for hiding this comment

Uh oh!

directionless Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

directionless Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zackattack01 commented Mar 28, 2025 •

edited

Loading