From 0e1382106ddf8e8538622af6a7c5f98cf4ba23ae Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 2 Nov 2023 17:07:24 +0100 Subject: [PATCH] [scraper/processscraper] Fix TestScrapeMetrics_MuteErrorFlags failures on windows and darwin (#28864) **Description:** There were some issues related to how `mock.On` works. With default mock and addition `On` which is already present it appends to a list and won't be called as one instance of a method is already there. So some expectations regarding return values were not met Metrics count for darwin is 3 because disk io is disabled [here](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/f509060a8d1ab5ca4b5827e0c60d1149e3059908/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper.go#L315) Tested locally on mac, windows 11 and ubuntu 22 **Link to tracking Issue:** #28828 --- .../processscraper/process_scraper_test.go | 112 ++++++++++++------ 1 file changed, 79 insertions(+), 33 deletions(-) diff --git a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go index ec0885646659..279bd9a6cdd6 100644 --- a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go +++ b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go @@ -475,23 +475,58 @@ func (p *processHandleMock) RlimitUsageWithContext(ctx context.Context, b bool) return args.Get(0).([]process.RlimitStat), args.Error(1) } -func newDefaultHandleMock() *processHandleMock { - handleMock := &processHandleMock{} - handleMock.On("UsernameWithContext", mock.Anything).Return("username", nil) - handleMock.On("CmdlineWithContext", mock.Anything).Return("cmdline", nil) - handleMock.On("CmdlineSliceWithContext", mock.Anything).Return([]string{"cmdline"}, nil) - handleMock.On("TimesWithContext", mock.Anything).Return(&cpu.TimesStat{}, nil) - handleMock.On("PercentWithContext", mock.Anything, mock.Anything).Return(float64(0), nil) - handleMock.On("MemoryInfoWithContext", mock.Anything).Return(&process.MemoryInfoStat{}, nil) - handleMock.On("MemoryPercentWithContext", mock.Anything).Return(float32(0), nil) - handleMock.On("IOCountersWithContext", mock.Anything).Return(&process.IOCountersStat{}, nil) - handleMock.On("PpidWithContext", mock.Anything).Return(int32(2), nil) - handleMock.On("NumThreadsWithContext", mock.Anything).Return(int32(0), nil) - handleMock.On("PageFaultsWithContext", mock.Anything).Return(&process.PageFaultsStat{}, nil) - handleMock.On("NumCtxSwitchesWithContext", mock.Anything).Return(&process.NumCtxSwitchesStat{}, nil) - handleMock.On("NumFDsWithContext", mock.Anything).Return(int32(0), nil) - handleMock.On("RlimitUsageWithContext", mock.Anything, mock.Anything).Return([]process.RlimitStat{}, nil) - return handleMock +func initDefaultsHandleMock(t mock.TestingT, handleMock *processHandleMock) { + if !handleMock.IsMethodCallable(t, "UsernameWithContext", mock.Anything) { + handleMock.On("UsernameWithContext", mock.Anything).Return("username", nil) + } + if !handleMock.IsMethodCallable(t, "NameWithContext", mock.Anything) { + handleMock.On("NameWithContext", mock.Anything).Return("processname", nil) + } + if !handleMock.IsMethodCallable(t, "CmdlineWithContext", mock.Anything) { + handleMock.On("CmdlineWithContext", mock.Anything).Return("cmdline", nil) + } + if !handleMock.IsMethodCallable(t, "CmdlineSliceWithContext", mock.Anything) { + handleMock.On("CmdlineSliceWithContext", mock.Anything).Return([]string{"cmdline"}, nil) + } + if !handleMock.IsMethodCallable(t, "TimesWithContext", mock.Anything) { + handleMock.On("TimesWithContext", mock.Anything).Return(&cpu.TimesStat{}, nil) + } + if !handleMock.IsMethodCallable(t, "PercentWithContext", mock.Anything, mock.Anything) { + handleMock.On("PercentWithContext", mock.Anything, mock.Anything).Return(float64(0), nil) + } + if !handleMock.IsMethodCallable(t, "MemoryInfoWithContext", mock.Anything) { + handleMock.On("MemoryInfoWithContext", mock.Anything).Return(&process.MemoryInfoStat{}, nil) + } + if !handleMock.IsMethodCallable(t, "MemoryPercentWithContext", mock.Anything) { + handleMock.On("MemoryPercentWithContext", mock.Anything).Return(float32(0), nil) + } + if !handleMock.IsMethodCallable(t, "IOCountersWithContext", mock.Anything) { + handleMock.On("IOCountersWithContext", mock.Anything).Return(&process.IOCountersStat{}, nil) + } + if !handleMock.IsMethodCallable(t, "PpidWithContext", mock.Anything) { + handleMock.On("PpidWithContext", mock.Anything).Return(int32(2), nil) + } + if !handleMock.IsMethodCallable(t, "NumThreadsWithContext", mock.Anything) { + handleMock.On("NumThreadsWithContext", mock.Anything).Return(int32(0), nil) + } + if !handleMock.IsMethodCallable(t, "PageFaultsWithContext", mock.Anything) { + handleMock.On("PageFaultsWithContext", mock.Anything).Return(&process.PageFaultsStat{}, nil) + } + if !handleMock.IsMethodCallable(t, "NumCtxSwitchesWithContext", mock.Anything) { + handleMock.On("NumCtxSwitchesWithContext", mock.Anything).Return(&process.NumCtxSwitchesStat{}, nil) + } + if !handleMock.IsMethodCallable(t, "NumFDsWithContext", mock.Anything) { + handleMock.On("NumFDsWithContext", mock.Anything).Return(int32(0), nil) + } + if !handleMock.IsMethodCallable(t, "RlimitUsageWithContext", mock.Anything, mock.Anything) { + handleMock.On("RlimitUsageWithContext", mock.Anything, mock.Anything).Return([]process.RlimitStat{}, nil) + } + if !handleMock.IsMethodCallable(t, "CreateTimeWithContext", mock.Anything) { + handleMock.On("CreateTimeWithContext", mock.Anything).Return(time.Now().UnixMilli(), nil) + } + if !handleMock.IsMethodCallable(t, "ExeWithContext", mock.Anything) { + handleMock.On("ExeWithContext", mock.Anything).Return("processname", nil) + } } func TestScrapeMetrics_Filtered(t *testing.T) { @@ -598,10 +633,12 @@ func TestScrapeMetrics_Filtered(t *testing.T) { handles := make([]*processHandleMock, 0, len(test.names)) for i, name := range test.names { - handleMock := newDefaultHandleMock() + handleMock := &processHandleMock{} handleMock.On("NameWithContext", mock.Anything).Return(name, nil) handleMock.On("ExeWithContext", mock.Anything).Return(name, nil) handleMock.On("CreateTimeWithContext", mock.Anything).Return(time.Now().UnixMilli()-test.upTimeMs[i], nil) + initDefaultsHandleMock(t, handleMock) + handles = append(handles, handleMock) } @@ -1001,22 +1038,34 @@ func TestScrapeMetrics_MuteErrorFlags(t *testing.T) { }, { name: "Process User Error Muted", - skipTestCase: runtime.GOOS != "linux", muteProcessUserError: true, skipProcessNameError: true, muteProcessExeError: true, muteProcessNameError: true, - expectedCount: 4, + expectedCount: func() int { + if runtime.GOOS == "darwin" { + // disk.io is not collected on darwin + return 3 + } + return 4 + }(), }, { name: "Process User Error Unmuted", - skipTestCase: runtime.GOOS != "linux", muteProcessUserError: false, skipProcessNameError: true, muteProcessExeError: true, muteProcessNameError: true, - expectedError: fmt.Sprintf("error reading username for process \"processname\" (pid 1): %v", processNameError), - expectedCount: 4, + expectedError: func() string { + return fmt.Sprintf("error reading username for process \"processname\" (pid 1): %v", processNameError) + }(), + expectedCount: func() int { + if runtime.GOOS == "darwin" { + // disk.io is not collected on darwin + return 3 + } + return 4 + }(), }, } @@ -1037,21 +1086,17 @@ func TestScrapeMetrics_MuteErrorFlags(t *testing.T) { err = scraper.start(context.Background(), componenttest.NewNopHost()) require.NoError(t, err, "Failed to initialize process scraper: %v", err) - handleMock := newDefaultHandleMock() + handleMock := &processHandleMock{} if !test.skipProcessNameError { handleMock.On("NameWithContext", mock.Anything).Return("test", processNameError) + handleMock.On("ExeWithContext", mock.Anything).Return("test", processNameError) + handleMock.On("CmdlineWithContext", mock.Anything).Return("test", processNameError) } else { - for _, c := range handleMock.ExpectedCalls { - if c.Method == "UsernameWithContext" { - c.ReturnArguments = []interface{}{"processname", processNameError} - break - } - } + handleMock.On("UsernameWithContext", mock.Anything).Return("processname", processNameError) handleMock.On("NameWithContext", mock.Anything).Return("processname", nil) handleMock.On("CreateTimeWithContext", mock.Anything).Return(time.Now().UnixMilli(), nil) } - handleMock.On("ExeWithContext", mock.Anything).Return("test", processNameError) - handleMock.On("CmdlineWithContext", mock.Anything).Return("test", processNameError) + initDefaultsHandleMock(t, handleMock) if config.MuteProcessIOError { handleMock.On("IOCountersWithContext", mock.Anything).Return("test", errors.New("permission denied")) @@ -1182,10 +1227,11 @@ func TestScrapeMetrics_CpuUtilizationWhenCpuTimesIsDisabled(t *testing.T) { err = scraper.start(context.Background(), componenttest.NewNopHost()) require.NoError(t, err, "Failed to initialize process scraper: %v", err) - handleMock := newDefaultHandleMock() + handleMock := &processHandleMock{} handleMock.On("NameWithContext", mock.Anything).Return("test", nil) handleMock.On("ExeWithContext", mock.Anything).Return("test", nil) handleMock.On("CreateTimeWithContext", mock.Anything).Return(time.Now().UnixMilli(), nil) + initDefaultsHandleMock(t, handleMock) scraper.getProcessHandles = func(context.Context) (processHandles, error) { return &processHandlesMock{handles: []*processHandleMock{handleMock}}, nil