diff --git a/internal/signalfx-agent/pkg/monitors/processlist/processlist_windows.go b/internal/signalfx-agent/pkg/monitors/processlist/processlist_windows.go index c2eb604be6..8a21bc85d8 100644 --- a/internal/signalfx-agent/pkg/monitors/processlist/processlist_windows.go +++ b/internal/signalfx-agent/pkg/monitors/processlist/processlist_windows.go @@ -15,9 +15,6 @@ import ( ) const ( - // represents the thread is in waiting state - // ref: https://docs.microsoft.com/en-us/windows/win32/cimwin32prov/win32-thread - threadWaitingState = 5 processQueryLimitedInformation = 0x00001000 ) @@ -42,7 +39,6 @@ type Win32Process struct { // Win32Thread is a WMI struct used for WMI calls // https://docs.microsoft.com/en-us/windows/win32/cimwin32prov/win32-thread type Win32Thread struct { - ThreadState uint32 ProcessHandle string } @@ -60,20 +56,37 @@ func initOSCache() *osCache { return &osCache{} } -// getAllProcesses retrieves all processes. It is set as a package variable so we can mock it during testing -var getAllProcesses = func() (ps []Win32Process, err error) { +// getAllProcesses retrieves all processes. +func getAllProcesses() (ps []Win32Process, err error) { err = wmi.Query("select Name, ExecutablePath, CommandLine, CreationDate, Priority, ProcessID, Status, ExecutionState, KernelModeTime, PageFileUsage, UserModeTime, WorkingSetSize, VirtualSize from Win32_Process", &ps) return ps, err } -// getAllThreads retrieves all the threads. It is set as a package variable so we can mock it during testing -var getAllThreads = func() (threads []Win32Thread, err error) { - err = wmi.Query("select ThreadState, ProcessHandle from Win32_Thread", &threads) - return threads, err +// getProcessesWithNonWaitingThreads retrieves all processes with non-waiting threads. +func getProcessesWithNonWaitingThreads() (map[uint32]struct{}, error) { + var nonWaitingThreads []Win32Thread + // ThreadState equals 5 means the thread is in a waiting state. + // ref: https://docs.microsoft.com/en-us/windows/win32/cimwin32prov/win32-thread + err := wmi.Query("select ProcessHandle from Win32_Thread where ThreadState<>5", &nonWaitingThreads) + if err != nil { + return make(map[uint32]struct{}), err + } + + processWithNonWaitingThreads := make(map[uint32]struct{}, len(nonWaitingThreads)) + for _, nonWaitingThread := range nonWaitingThreads { + val, err := strconv.ParseUint(nonWaitingThread.ProcessHandle, 10, 32) + if err == nil { + pid := uint32(val) + if _, ok := processWithNonWaitingThreads[pid]; !ok { + processWithNonWaitingThreads[pid] = struct{}{} + } + } + } + return processWithNonWaitingThreads, nil } -// getUsername - retrieves a username from an open process handle it is set as a package variable so we can mock it during testing -var getUsername = func(id uint32) (username string, err error) { +// getUsername - retrieves a username from an open process handle. +func getUsername(id uint32) (username string, err error) { // open the process handle and collect any information that requires it var h windows.Handle if h, err = windows.OpenProcess(processQueryLimitedInformation, false, id); err != nil { @@ -123,13 +136,12 @@ func ProcessList(conf *Config, cache *osCache, logger logrus.FieldLogger) ([]*To return nil, err } - // Get all threads - threads, err := getAllThreads() - if err != nil { - return nil, err + // Get a map of processes with running threads + processWithNonWaitingThreads, err := getProcessesWithNonWaitingThreads() + if err != nil && logger != nil { + logger.Debugf("Unable to collect non waiting threads. %v", err) } - processMap := mapThreadsToProcess(threads) // iterate over each process and build an entry for the process list for _, p := range ps { username, err := getUsername(p.ProcessID) @@ -152,7 +164,13 @@ func ProcessList(conf *Config, cache *osCache, logger logrus.FieldLogger) ([]*To if command == "" { command = p.Name } - status := statusMapping(processMap[strconv.Itoa(int(p.ProcessID))]) + + // update process status + status := "S" + if _, ok := processWithNonWaitingThreads[p.ProcessID]; ok { + status = "R" + } + //example process "3":["root",20,"0",0,0,0,"S",0.0,0.0,"01:28.31","[ksoftirqd/0]"] procs = append(procs, &TopProcess{ ProcessID: int(p.ProcessID), @@ -171,31 +189,3 @@ func ProcessList(conf *Config, cache *osCache, logger logrus.FieldLogger) ([]*To } return procs, nil } - -// Mapping each thread's state to its respective process. -// for example, threadList = []Win32Thread{{ProcessHandle: "1", ThreadState: 3}, -// {ProcessHandle: "2", ThreadState: 3},{ProcessHandle: "1", ThreadState: 5},{ProcessHandle: "1", ThreadState: 5},} -// it returns map[string][]uint32{"1": []uint32{3, 5, 5}, "2": []uint32{3},}, -func mapThreadsToProcess(threadList []Win32Thread) map[string][]uint32 { - var processes = make(map[string][]uint32) - for _, thread := range threadList { - processes[thread.ProcessHandle] = append(processes[thread.ProcessHandle], thread.ThreadState) - } - return processes -} - -// Returns the process status depending upon all thread's state. -// if all the threads of a process are in waiting state then it returns "S"(sleeping) -// else it returns "R"(running) -func statusMapping(threadStates []uint32) string { - if len(threadStates) == 0 { - return "" - } - - for _, state := range threadStates { - if state != threadWaitingState { - return "R" - } - } - return "S" -} diff --git a/internal/signalfx-agent/pkg/monitors/processlist/processlist_windows_test.go b/internal/signalfx-agent/pkg/monitors/processlist/processlist_windows_test.go index 8d211f8198..cde6a7ae7b 100644 --- a/internal/signalfx-agent/pkg/monitors/processlist/processlist_windows_test.go +++ b/internal/signalfx-agent/pkg/monitors/processlist/processlist_windows_test.go @@ -4,252 +4,53 @@ package processlist import ( - "reflect" "testing" - "time" - "github.com/signalfx/golib/v3/event" - "github.com/signalfx/golib/v3/pointer" - "github.com/signalfx/signalfx-agent/pkg/core/config" - "github.com/signalfx/signalfx-agent/pkg/neotest" + "github.com/stretchr/testify/assert" ) -func TestMonitor_Configure(t *testing.T) { - tests := []struct { - name string - m *Monitor - processes []Win32Process - threads []Win32Thread - cpuPercent map[uint32]uint64 - usernames map[uint32]string - want *event.Event - wantErr bool - }{ - { - name: "test1", - m: &Monitor{Output: neotest.NewTestOutput()}, - processes: []Win32Process{ - { - Name: "testProcess1", - ExecutablePath: pointer.String("C:\\HelloWorld.exe"), - CommandLine: pointer.String("HelloWorld.exe"), - Priority: 8, - ProcessID: 0, - Status: pointer.String(""), - ExecutionState: pointer.Uint16(0), - KernelModeTime: 1500, - PageFileUsage: 1600, - UserModeTime: 1700, - WorkingSetSize: 1800, - VirtualSize: 1900, - }, - { - Name: "testProcess2", - ExecutablePath: pointer.String("C:\\HelloWorld2.exe"), - CommandLine: pointer.String("HelloWorld2.exe"), - Priority: 8, - ProcessID: 1, - Status: pointer.String(""), - ExecutionState: pointer.Uint16(0), - KernelModeTime: 1500, - PageFileUsage: 1600, - UserModeTime: 1700, - WorkingSetSize: 1800, - VirtualSize: 1900, - }, - }, - threads: []Win32Thread{ - { - ThreadState: 3, - ProcessHandle: "1", - }, - { - ThreadState: 5, - ProcessHandle: "0", - }, - }, - usernames: map[uint32]string{ - 0: "tedMosby", - 1: "barneyStinson", - }, - want: &event.Event{ - EventType: "objects.top-info", - Category: event.AGENT, - Dimensions: map[string]string{}, - Properties: map[string]interface{}{ - "message": "{\"t\":\"eJyqVjJQsopWKklN8c0vTqpU0rHQUSrNy87LL89T0jHUMdQx0FEKVtIx0DMwgBBKBgZWBiCWko6Ss1VMjEdqTk5+eH5RTopeakWqUqyOkiHIwKTEorzUyuCSzLzi/DyspgYRZ6oRxNhaQAAAAP//UTMulQ==\",\"v\":\"0.0.30\"}", - }, - }, - }, - { - name: "handles nested quotes", - m: &Monitor{Output: neotest.NewTestOutput()}, - processes: []Win32Process{ - { - Name: "test-proc", - ExecutablePath: pointer.String("C:\\HelloWorld2\"quoted\".exe"), - CommandLine: pointer.String("HelloWorld2.exe"), - Priority: 8, - ProcessID: 0, - Status: pointer.String(""), - ExecutionState: pointer.Uint16(0), - KernelModeTime: 1500, - PageFileUsage: 1600, - UserModeTime: 1700, - WorkingSetSize: 1800, - VirtualSize: 1900, - }, - }, - threads: []Win32Thread{ - { - ThreadState: 5, - ProcessHandle: "0", - }, - }, - usernames: map[uint32]string{ - 0: "ted\"bud\"Mosby", - }, - want: &event.Event{ - EventType: "objects.top-info", - Category: event.AGENT, - Dimensions: map[string]string{}, - Properties: map[string]interface{}{ - "message": "{\"t\":\"eJyqVjJQsopWKklNUU8qTVH3zS9OqlTSsdBRKs3Lzssvz1PSMdQx1DHQUQpW0jHQMzCAEEoGBlYGIJaSjpKzVUyMR2pOTn54flFOipF6YWk+yDS91IpUpdhaQAAAAP///QsbHw==\",\"v\":\"0.0.30\"}", - }, - }, - }, - } - for i := range tests { - origGetAllProcesses := getAllProcesses - origGetUsername := getUsername - origGetAllThreads := getAllThreads - - tt := tests[i] - - t.Run(tt.name, func(t *testing.T) { - getAllProcesses = func() ([]Win32Process, error) { - return tt.processes, nil - } - getUsername = func(id uint32) (string, error) { - username, ok := tt.usernames[id] - if !ok { - t.Error("unable to find username") - } - return username, nil - } - getAllThreads = func() ([]Win32Thread, error) { - return tt.threads, nil - } - if err := tt.m.Configure(&Config{config.MonitorConfig{IntervalSeconds: 10}}); (err != nil) != tt.wantErr { - t.Errorf("Monitor.Configure() error = %v, wantErr %v", err, tt.wantErr) - } - time.Sleep(3 * time.Second) - events := tt.m.Output.(*neotest.TestOutput).FlushEvents() - if len(events) == 0 { - t.Errorf("events %v != %v", events, tt.want) - return - } - - lastEvent := events[len(events)-1] - - w := tt.want - if lastEvent.EventType != w.EventType || - lastEvent.Category != w.Category || - !reflect.DeepEqual(lastEvent.Dimensions, w.Dimensions) || - !reflect.DeepEqual(lastEvent.Properties, w.Properties) { - t.Errorf("events %v != %v", lastEvent, tt.want) - return - } - }) - getAllProcesses = origGetAllProcesses - getUsername = origGetUsername - getAllThreads = origGetAllThreads +func TestProcessList(t *testing.T) { + // On Windows all parameters are ignored, pass nil, so the benchmark is re-checked in + // case of changes in the implementation. + processList, err := ProcessList(nil, nil, nil) + if err != nil { + t.Fatal(err) + } + assert.NotEmpty(t, processList) + + runningProcesses := []*TopProcess{} + waitingProcesses := []*TopProcess{} + unknownStatusProcesses := []*TopProcess{} + for _, p := range processList { + if p.Status == "R" { + runningProcesses = append(runningProcesses, p) + } else if p.Status == "S" { + waitingProcesses = append(waitingProcesses, p) + } else { + unknownStatusProcesses = append(unknownStatusProcesses, p) + } + } + assert.NotEmpty(t, runningProcesses) + assert.NotEmpty(t, waitingProcesses) + assert.Empty(t, unknownStatusProcesses) + + t.Logf("Running processes:") + for _, p := range runningProcesses { + t.Logf("%d\t\t%q", p.ProcessID, p.Command) } } -func TestMapThreadsToProcess(t *testing.T) { - type args struct { - threadList []Win32Thread - } - tests := []struct { - name string - args args - want map[string][]uint32 - }{ - { - name: "check correct mapping", - args: args{ - threadList: []Win32Thread{ - {ProcessHandle: "2", ThreadState: 3}, - }, - }, - want: map[string][]uint32{ - "2": {3}, - }, - }, - { - name: "check correct mapping 2", - args: args{ - threadList: []Win32Thread{ - {ProcessHandle: "1", ThreadState: 3}, - {ProcessHandle: "2", ThreadState: 3}, - {ProcessHandle: "1", ThreadState: 5}, - {ProcessHandle: "1", ThreadState: 5}, - }, - }, - want: map[string][]uint32{ - "1": {3, 5, 5}, - "2": {3}, - }, - }, - } - for i := range tests { - tt := tests[i] - t.Run(tt.name, func(t *testing.T) { - if got := mapThreadsToProcess(tt.args.threadList); !reflect.DeepEqual(got, tt.want) { - t.Errorf("mapThreadsToProcess() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestStatusMapping(t *testing.T) { - type args struct { - threadStates []uint32 - } - tests := []struct { - name string - args args - want string - }{ - { - name: "running process", - args: args{ - threadStates: []uint32{5, 5, 5, 5, 2, 5}, - }, - want: "R", - }, - { - name: "waiting process", - args: args{ - threadStates: []uint32{5, 5, 5, 5, 5, 5}, - }, - want: "S", - }, - { - name: "empty list", - args: args{ - threadStates: []uint32{}, - }, - want: "", - }, - } - for i := range tests { - tt := tests[i] - t.Run(tt.name, func(t *testing.T) { - if got := statusMapping(tt.args.threadStates); got != tt.want { - t.Errorf("statusMapping() = %v, want %v", got, tt.want) - } - }) - } +var topProcesses []*TopProcess // A global variable to prevent the compiler from optimizing the benchmark away. +func BenchmarkProcessList(b *testing.B) { + var tp []*TopProcess + for i := 0; i < b.N; i++ { + // On Windows all parameters are ignored, pass nil, so the benchmark is re-checked in + // case of changes in the implementation. + processList, err := ProcessList(nil, nil, nil) + if err != nil { + b.Fatal(err) + } + tp = processList + } + topProcesses = tp }