From cc69afda27c9c97abdc48f33b85dcde4cbdfa08c Mon Sep 17 00:00:00 2001 From: Sven Rebhan Date: Thu, 29 Aug 2024 15:12:36 +0200 Subject: [PATCH 1/2] chore(inputs.win_perf_counter): Extend logging --- .../win_perf_counters/performance_query.go | 72 +++++++++++-------- .../win_perf_counters/win_perf_counters.go | 2 + .../win_perf_counters_test.go | 5 +- 3 files changed, 48 insertions(+), 31 deletions(-) diff --git a/plugins/inputs/win_perf_counters/performance_query.go b/plugins/inputs/win_perf_counters/performance_query.go index b548f63909b96..eddd317c78ee6 100644 --- a/plugins/inputs/win_perf_counters/performance_query.go +++ b/plugins/inputs/win_perf_counters/performance_query.go @@ -5,9 +5,12 @@ package win_perf_counters import ( "errors" + "fmt" "syscall" "time" "unsafe" + + "github.com/influxdata/telegraf" ) // Initial buffer size for return buffers @@ -42,6 +45,7 @@ type PerformanceQuery interface { type PerformanceQueryCreator interface { NewPerformanceQuery(string, uint32) PerformanceQuery + SetLogger(telegraf.Logger) } // PdhError represents error returned from Performance Counters API @@ -65,12 +69,19 @@ func NewPdhError(code uint32) error { type PerformanceQueryImpl struct { maxBufferSize uint32 query pdhQueryHandle + log telegraf.Logger +} + +type PerformanceQueryCreatorImpl struct { + log telegraf.Logger } -type PerformanceQueryCreatorImpl struct{} +func (m *PerformanceQueryCreatorImpl) NewPerformanceQuery(_ string, maxBufferSize uint32) PerformanceQuery { + return &PerformanceQueryImpl{maxBufferSize: maxBufferSize, log: m.log} +} -func (m PerformanceQueryCreatorImpl) NewPerformanceQuery(_ string, maxBufferSize uint32) PerformanceQuery { - return &PerformanceQueryImpl{maxBufferSize: maxBufferSize} +func (m *PerformanceQueryCreatorImpl) SetLogger(l telegraf.Logger) { + m.log = l } // Open creates a new counterPath that is used to manage the collection of performance data. @@ -195,38 +206,37 @@ func (m *PerformanceQueryImpl) GetFormattedCounterValueDouble(hCounter pdhCounte } func (m *PerformanceQueryImpl) GetFormattedCounterArrayDouble(hCounter pdhCounterHandle) ([]CounterValue, error) { - for buflen := initialBufferSize; buflen <= m.maxBufferSize; buflen *= 2 { - buf := make([]byte, buflen) - - // Get the info with the current buffer size - var itemCount uint32 - size := buflen - ret := PdhGetFormattedCounterArrayDouble(hCounter, &size, &itemCount, &buf[0]) - if ret == ErrorSuccess { - //nolint:gosec // G103: Valid use of unsafe call to create PDH_FMT_COUNTERVALUE_ITEM_DOUBLE - items := (*[1 << 20]PdhFmtCountervalueItemDouble)(unsafe.Pointer(&buf[0]))[:itemCount] - values := make([]CounterValue, 0, itemCount) - for _, item := range items { - if item.FmtValue.CStatus == PdhCstatusValidData || item.FmtValue.CStatus == PdhCstatusNewData { - val := CounterValue{UTF16PtrToString(item.SzName), item.FmtValue.DoubleValue} - values = append(values, val) - } - } - return values, nil - } + // Determine the required buffer size + var itemCount uint32 + var buflen uint32 + if ret := PdhGetFormattedCounterArrayDouble(hCounter, &buflen, &itemCount, nil); ret != PdhMoreData { + return nil, fmt.Errorf("getting buffer size failed: %w", NewPdhError(ret)) + } + if buflen > m.maxBufferSize { + return nil, errBufferLimitReached + } - // Use the size as a hint if it exceeds the current buffer size - if size > buflen { - buflen = size - } + // Do the actual formatting + buf := make([]byte, buflen) + size := buflen + ret := PdhGetFormattedCounterArrayDouble(hCounter, &size, &itemCount, &buf[0]) + if ret != ErrorSuccess { + return nil, NewPdhError(ret) + } - // We got a non-recoverable error so exit here - if ret != PdhMoreData { - return nil, NewPdhError(ret) + //nolint:gosec // G103: Valid use of unsafe call to create PDH_FMT_COUNTERVALUE_ITEM_DOUBLE + items := (*[1 << 20]PdhFmtCountervalueItemDouble)(unsafe.Pointer(&buf[0]))[:itemCount] + values := make([]CounterValue, 0, itemCount) + for _, item := range items { + if item.FmtValue.CStatus == PdhCstatusValidData || item.FmtValue.CStatus == PdhCstatusNewData { + v := CounterValue{UTF16PtrToString(item.SzName), item.FmtValue.DoubleValue} + values = append(values, v) + } else { + m.log.Debugf("got status %d for item %+v", item.FmtValue.CStatus, item) } } - return nil, errBufferLimitReached + return values, nil } func (m *PerformanceQueryImpl) GetRawCounterArray(hCounter pdhCounterHandle) ([]CounterValue, error) { @@ -245,6 +255,8 @@ func (m *PerformanceQueryImpl) GetRawCounterArray(hCounter pdhCounterHandle) ([] if item.RawValue.CStatus == PdhCstatusValidData || item.RawValue.CStatus == PdhCstatusNewData { val := CounterValue{UTF16PtrToString(item.SzName), item.RawValue.FirstValue} values = append(values, val) + } else { + m.log.Debugf("got status %d for item %+v", item.RawValue.CStatus, item) } } return values, nil diff --git a/plugins/inputs/win_perf_counters/win_perf_counters.go b/plugins/inputs/win_perf_counters/win_perf_counters.go index 2b2992a330194..503bfc3bee88d 100644 --- a/plugins/inputs/win_perf_counters/win_perf_counters.go +++ b/plugins/inputs/win_perf_counters/win_perf_counters.go @@ -584,6 +584,8 @@ func isKnownCounterDataError(err error) bool { } func (m *WinPerfCounters) Init() error { + m.queryCreator.SetLogger(m.Log) + // Check the buffer size if m.MaxBufferSize < config.Size(initialBufferSize) { return fmt.Errorf("maximum buffer size should at least be %d", 2*initialBufferSize) diff --git a/plugins/inputs/win_perf_counters/win_perf_counters_test.go b/plugins/inputs/win_perf_counters/win_perf_counters_test.go index dfa51db2aec07..c19cb18e4c119 100644 --- a/plugins/inputs/win_perf_counters/win_perf_counters_test.go +++ b/plugins/inputs/win_perf_counters/win_perf_counters_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/influxdata/telegraf" "github.com/influxdata/telegraf/config" "github.com/influxdata/telegraf/testutil" ) @@ -215,7 +216,7 @@ type FakePerformanceQueryCreator struct { fakeQueries map[string]*FakePerformanceQuery } -func (m FakePerformanceQueryCreator) NewPerformanceQuery(computer string, _ uint32) PerformanceQuery { +func (m *FakePerformanceQueryCreator) NewPerformanceQuery(computer string, _ uint32) PerformanceQuery { var ret PerformanceQuery var ok bool if ret, ok = m.fakeQueries[computer]; !ok { @@ -224,6 +225,8 @@ func (m FakePerformanceQueryCreator) NewPerformanceQuery(computer string, _ uint return ret } +func (m *FakePerformanceQueryCreator) SetLogger(telegraf.Logger) {} + //nolint:revive //argument-limit allowed for helper function func createPerfObject( computer string, From 74d6fea0a6e25d5f2e4539b584947e4c9aa36c54 Mon Sep 17 00:00:00 2001 From: Sven Rebhan Date: Thu, 29 Aug 2024 18:12:50 +0200 Subject: [PATCH 2/2] Avoid panic in tests --- plugins/inputs/win_perf_counters/win_perf_counters_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/inputs/win_perf_counters/win_perf_counters_test.go b/plugins/inputs/win_perf_counters/win_perf_counters_test.go index c19cb18e4c119..bd7c98026e922 100644 --- a/plugins/inputs/win_perf_counters/win_perf_counters_test.go +++ b/plugins/inputs/win_perf_counters/win_perf_counters_test.go @@ -2022,6 +2022,7 @@ func TestNoWildcards(t *testing.T) { UseWildcardsExpansion: true, LocalizeWildcardsExpansion: false, Log: testutil.Logger{}, + queryCreator: &PerformanceQueryCreatorImpl{}, } require.Error(t, m.Init()) m = WinPerfCounters{ @@ -2029,6 +2030,7 @@ func TestNoWildcards(t *testing.T) { UseWildcardsExpansion: true, LocalizeWildcardsExpansion: false, Log: testutil.Logger{}, + queryCreator: &PerformanceQueryCreatorImpl{}, } require.Error(t, m.Init()) }