fix(inputs.win_perf_counter): Do not rely on returned buffer size (#14241)

This commit is contained in:
Sven Rebhan 2023-11-07 15:37:24 +01:00 committed by GitHub
parent 247a808769
commit 0e2203d7a4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 187 additions and 107 deletions

View File

@ -345,6 +345,10 @@ See the [CONFIGURATION.md][CONFIGURATION.md] for more details.
## e.g. IgnoredErrors = ["PDH_NO_DATA"]
# IgnoredErrors = []
## Maximum size of the buffer for values returned by the API
## Increase this value if you experience "buffer limit reached" errors.
# MaxBufferSize = "4MiB"
## NOTE: Due to the way TOML is parsed, tables must be at the END of the
## plugin definition, otherwise additional config options are read as part of
## the table

View File

@ -10,6 +10,11 @@ import (
"unsafe"
)
// Initial buffer size for return buffers
const initialBufferSize = uint32(1024) // 1kB
var errBufferLimitReached = errors.New("buffer limit reached")
// CounterValue is abstraction for PdhFmtCountervalueItemDouble
type CounterValue struct {
InstanceName string
@ -36,7 +41,7 @@ type PerformanceQuery interface {
}
type PerformanceQueryCreator interface {
NewPerformanceQuery(string) PerformanceQuery
NewPerformanceQuery(string, uint32) PerformanceQuery
}
// PdhError represents error returned from Performance Counters API
@ -58,14 +63,14 @@ func NewPdhError(code uint32) error {
// PerformanceQueryImpl is implementation of PerformanceQuery interface, which calls phd.dll functions
type PerformanceQueryImpl struct {
query pdhQueryHandle
maxBufferSize uint32
query pdhQueryHandle
}
type PerformanceQueryCreatorImpl struct {
}
type PerformanceQueryCreatorImpl struct{}
func (m PerformanceQueryCreatorImpl) NewPerformanceQuery(string) PerformanceQuery {
return &PerformanceQueryImpl{}
func (m PerformanceQueryCreatorImpl) NewPerformanceQuery(_ string, maxBufferSize uint32) PerformanceQuery {
return &PerformanceQueryImpl{maxBufferSize: maxBufferSize}
}
// Open creates a new counterPath that is used to manage the collection of performance data.
@ -124,64 +129,82 @@ func (m *PerformanceQueryImpl) AddEnglishCounterToQuery(counterPath string) (pdh
// GetCounterPath return counter information for given handle
func (m *PerformanceQueryImpl) GetCounterPath(counterHandle pdhCounterHandle) (string, error) {
var bufSize uint32
var buff []byte
var ret uint32
if ret = PdhGetCounterInfo(counterHandle, 0, &bufSize, nil); ret == PdhMoreData {
buff = make([]byte, bufSize)
bufSize = uint32(len(buff))
if ret = PdhGetCounterInfo(counterHandle, 0, &bufSize, &buff[0]); ret == ErrorSuccess {
ci := (*PdhCounterInfo)(unsafe.Pointer(&buff[0])) //nolint:gosec // G103: Valid use of unsafe call to create PDH_COUNTER_INFO
for buflen := initialBufferSize; buflen <= m.maxBufferSize; buflen *= 2 {
buf := make([]byte, buflen)
// Get the info with the current buffer size
size := buflen
ret := PdhGetCounterInfo(counterHandle, 0, &size, &buf[0])
if ret == ErrorSuccess {
ci := (*PdhCounterInfo)(unsafe.Pointer(&buf[0])) //nolint:gosec // G103: Valid use of unsafe call to create PDH_COUNTER_INFO
return UTF16PtrToString(ci.SzFullPath), nil
}
// Use the size as a hint if it exceeds the current buffer size
if size > buflen {
buflen = size
}
// We got a non-recoverable error so exit here
if ret != PdhMoreData {
return "", NewPdhError(ret)
}
}
return "", NewPdhError(ret)
return "", errBufferLimitReached
}
// ExpandWildCardPath examines local computer and returns those counter paths that match the given counter path which contains wildcard characters.
func (m *PerformanceQueryImpl) ExpandWildCardPath(counterPath string) ([]string, error) {
var bufSize uint32
var buff []uint16
var ret uint32
for buflen := initialBufferSize; buflen <= m.maxBufferSize; buflen *= 2 {
buf := make([]uint16, buflen)
if ret = PdhExpandWildCardPath(counterPath, nil, &bufSize); ret == PdhMoreData {
buff = make([]uint16, bufSize)
bufSize = uint32(len(buff))
ret = PdhExpandWildCardPath(counterPath, &buff[0], &bufSize)
// Get the info with the current buffer size
size := buflen
ret := PdhExpandWildCardPath(counterPath, &buf[0], &size)
if ret == ErrorSuccess {
list := UTF16ToStringArray(buff)
return list, nil
return UTF16ToStringArray(buf), nil
}
// Use the size as a hint if it exceeds the current buffer size
if size > buflen {
buflen = size
}
// We got a non-recoverable error so exit here
if ret != PdhMoreData {
return nil, NewPdhError(ret)
}
}
return nil, NewPdhError(ret)
return nil, errBufferLimitReached
}
// GetFormattedCounterValueDouble computes a displayable value for the specified counter
func (m *PerformanceQueryImpl) GetFormattedCounterValueDouble(hCounter pdhCounterHandle) (float64, error) {
var counterType uint32
var value PdhFmtCountervalueDouble
var ret uint32
if ret = PdhGetFormattedCounterValueDouble(hCounter, &counterType, &value); ret == ErrorSuccess {
if value.CStatus == PdhCstatusValidData || value.CStatus == PdhCstatusNewData {
return value.DoubleValue, nil
}
return 0, NewPdhError(value.CStatus)
if ret := PdhGetFormattedCounterValueDouble(hCounter, &counterType, &value); ret != ErrorSuccess {
return 0, NewPdhError(ret)
}
return 0, NewPdhError(ret)
if value.CStatus == PdhCstatusValidData || value.CStatus == PdhCstatusNewData {
return value.DoubleValue, nil
}
return 0, NewPdhError(value.CStatus)
}
func (m *PerformanceQueryImpl) GetFormattedCounterArrayDouble(hCounter pdhCounterHandle) ([]CounterValue, error) {
var buffSize uint32
var itemCount uint32
var ret uint32
for buflen := initialBufferSize; buflen <= m.maxBufferSize; buflen *= 2 {
buf := make([]byte, buflen)
if ret = PdhGetFormattedCounterArrayDouble(hCounter, &buffSize, &itemCount, nil); ret == PdhMoreData {
buff := make([]byte, buffSize)
if ret = PdhGetFormattedCounterArrayDouble(hCounter, &buffSize, &itemCount, &buff[0]); ret == ErrorSuccess {
// 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(&buff[0]))[:itemCount]
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 {
@ -191,21 +214,32 @@ func (m *PerformanceQueryImpl) GetFormattedCounterArrayDouble(hCounter pdhCounte
}
return values, nil
}
// Use the size as a hint if it exceeds the current buffer size
if size > buflen {
buflen = size
}
// We got a non-recoverable error so exit here
if ret != PdhMoreData {
return nil, NewPdhError(ret)
}
}
return nil, NewPdhError(ret)
return nil, errBufferLimitReached
}
func (m *PerformanceQueryImpl) GetRawCounterArray(hCounter pdhCounterHandle) ([]CounterValue, error) {
var buffSize uint32
var itemCount uint32
var ret uint32
for buflen := initialBufferSize; buflen <= m.maxBufferSize; buflen *= 2 {
buf := make([]byte, buflen)
if ret = PdhGetRawCounterArray(hCounter, &buffSize, &itemCount, nil); ret == PdhMoreData {
buff := make([]byte, buffSize)
if ret = PdhGetRawCounterArray(hCounter, &buffSize, &itemCount, &buff[0]); ret == ErrorSuccess {
// Get the info with the current buffer size
var itemCount uint32
size := buflen
ret := PdhGetRawCounterArray(hCounter, &size, &itemCount, &buf[0])
if ret == ErrorSuccess {
//nolint:gosec // G103: Valid use of unsafe call to create PDH_RAW_COUNTER_ITEM
items := (*[1 << 20]PdhRawCounterItem)(unsafe.Pointer(&buff[0]))[:itemCount]
items := (*[1 << 20]PdhRawCounterItem)(unsafe.Pointer(&buf[0]))[:itemCount]
values := make([]CounterValue, 0, itemCount)
for _, item := range items {
if item.RawValue.CStatus == PdhCstatusValidData || item.RawValue.CStatus == PdhCstatusNewData {
@ -215,8 +249,19 @@ func (m *PerformanceQueryImpl) GetRawCounterArray(hCounter pdhCounterHandle) ([]
}
return values, nil
}
// Use the size as a hint if it exceeds the current buffer size
if size > buflen {
buflen = size
}
// We got a non-recoverable error so exit here
if ret != PdhMoreData {
return nil, NewPdhError(ret)
}
}
return nil, NewPdhError(ret)
return nil, errBufferLimitReached
}
func (m *PerformanceQueryImpl) CollectData() error {

View File

@ -41,6 +41,10 @@
## e.g. IgnoredErrors = ["PDH_NO_DATA"]
# IgnoredErrors = []
## Maximum size of the buffer for values returned by the API
## Increase this value if you experience "buffer limit reached" errors.
# MaxBufferSize = "4MiB"
## NOTE: Due to the way TOML is parsed, tables must be at the END of the
## plugin definition, otherwise additional config options are read as part of
## the table

View File

@ -7,6 +7,7 @@ import (
_ "embed"
"errors"
"fmt"
"math"
"os"
"strings"
"sync"
@ -20,6 +21,8 @@ import (
//go:embed sample.conf
var sampleConfig string
var defaultMaxBufferSize = config.Size(100 * 1024 * 1024)
type WinPerfCounters struct {
PrintValid bool `toml:"PrintValid"`
PreVistaSupport bool `toml:"PreVistaSupport" deprecated:"1.7.0;determined dynamically"`
@ -29,6 +32,7 @@ type WinPerfCounters struct {
UseWildcardsExpansion bool
LocalizeWildcardsExpansion bool
IgnoredErrors []string `toml:"IgnoredErrors"`
MaxBufferSize config.Size
Sources []string
Log telegraf.Logger
@ -207,7 +211,7 @@ func (m *WinPerfCounters) AddItem(counterPath, computer, objectName, instance, c
if !ok {
hostCounter = &hostCountersInfo{computer: computer, tag: sourceTag}
m.hostCounters[computer] = hostCounter
hostCounter.query = m.queryCreator.NewPerformanceQuery(computer)
hostCounter.query = m.queryCreator.NewPerformanceQuery(computer, uint32(m.MaxBufferSize))
if err := hostCounter.query.Open(); err != nil {
return err
}
@ -579,9 +583,16 @@ func isKnownCounterDataError(err error) bool {
}
func (m *WinPerfCounters) Init() error {
// Check the buffer size
if m.MaxBufferSize < config.Size(initialBufferSize) {
return fmt.Errorf("maximum buffer size should at least be %d", 2*initialBufferSize)
}
if m.MaxBufferSize > math.MaxUint32 {
return fmt.Errorf("maximum buffer size should be smaller than %d", uint32(math.MaxUint32))
}
if m.UseWildcardsExpansion && !m.LocalizeWildcardsExpansion {
// Counters must not have wildcards with this option
found := false
wildcards := []string{"*", "?"}
@ -614,6 +625,7 @@ func init() {
return &WinPerfCounters{
CountersRefreshInterval: config.Duration(time.Second * 60),
LocalizeWildcardsExpansion: true,
MaxBufferSize: defaultMaxBufferSize,
queryCreator: &PerformanceQueryCreatorImpl{},
}
})

View File

@ -18,7 +18,7 @@ func TestWinPerformanceQueryImplIntegration(t *testing.T) {
if testing.Short() {
t.Skip("Skipping integration test in short mode")
}
query := &PerformanceQueryImpl{}
query := &PerformanceQueryImpl{maxBufferSize: uint32(defaultMaxBufferSize)}
err := query.Close()
require.Error(t, err, "uninitialized query must return errors")
@ -62,11 +62,11 @@ func TestWinPerformanceQueryImplIntegration(t *testing.T) {
fcounter, err := query.GetFormattedCounterValueDouble(hCounter)
require.NoError(t, err)
require.Greater(t, fcounter, 0)
require.Greater(t, fcounter, float64(0))
rcounter, err := query.GetRawCounterValue(hCounter)
require.NoError(t, err)
require.Greater(t, rcounter, 10000000)
require.Greater(t, rcounter, int64(10000000))
now := time.Now()
mtime, err := query.CollectDataWithTime()
@ -131,10 +131,11 @@ func TestWinPerfCountersConfigGet1Integration(t *testing.T) {
}}
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
Log: testutil.Logger{},
PrintValid: false,
Object: perfObjects,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
require.NoError(t, m.ParseConfig())
@ -158,10 +159,11 @@ func TestWinPerfCountersConfigGet2Integration(t *testing.T) {
}}
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
Log: testutil.Logger{},
PrintValid: false,
Object: perfObjects,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
require.NoError(t, m.ParseConfig())
@ -200,10 +202,11 @@ func TestWinPerfCountersConfigGet3Integration(t *testing.T) {
}}
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
Log: testutil.Logger{},
PrintValid: false,
Object: perfObjects,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
require.NoError(t, m.ParseConfig())
@ -240,10 +243,11 @@ func TestWinPerfCountersConfigGet4Integration(t *testing.T) {
}}
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
Log: testutil.Logger{},
PrintValid: false,
Object: perfObjects,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
require.NoError(t, m.ParseConfig())
@ -280,10 +284,11 @@ func TestWinPerfCountersConfigGet5Integration(t *testing.T) {
}}
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
Log: testutil.Logger{},
PrintValid: false,
Object: perfObjects,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
require.NoError(t, m.ParseConfig())
@ -320,10 +325,11 @@ func TestWinPerfCountersConfigGet6Integration(t *testing.T) {
}}
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
Log: testutil.Logger{},
PrintValid: false,
Object: perfObjects,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
require.NoError(t, m.ParseConfig())
@ -347,10 +353,11 @@ func TestWinPerfCountersConfigGet7Integration(t *testing.T) {
}}
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
Log: testutil.Logger{},
PrintValid: false,
Object: perfObjects,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
require.NoError(t, m.ParseConfig())
@ -387,10 +394,11 @@ func TestWinPerfCountersConfigError1Integration(t *testing.T) {
}}
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
Log: testutil.Logger{},
PrintValid: false,
Object: perfObjects,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
require.Error(t, m.ParseConfig())
@ -414,10 +422,11 @@ func TestWinPerfCountersConfigError2Integration(t *testing.T) {
}}
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
Log: testutil.Logger{},
PrintValid: false,
Object: perfObjects,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
require.NoError(t, m.ParseConfig())
@ -443,10 +452,11 @@ func TestWinPerfCountersConfigError3Integration(t *testing.T) {
}}
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
Log: testutil.Logger{},
PrintValid: false,
Object: perfObjects,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
require.Error(t, m.ParseConfig())
@ -470,10 +480,11 @@ func TestWinPerfCountersCollect1Integration(t *testing.T) {
}}
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
Log: testutil.Logger{},
PrintValid: false,
Object: perfObjects,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
var acc testutil.Accumulator
@ -510,9 +521,10 @@ func TestWinPerfCountersCollect2Integration(t *testing.T) {
PrintValid: false,
UsePerfCounterTime: true,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
UseWildcardsExpansion: true,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
var acc testutil.Accumulator
@ -550,9 +562,10 @@ func TestWinPerfCountersCollectRawIntegration(t *testing.T) {
m := WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
UseWildcardsExpansion: true,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
var acc testutil.Accumulator
require.NoError(t, m.Gather(&acc))
@ -566,17 +579,18 @@ func TestWinPerfCountersCollectRawIntegration(t *testing.T) {
val, ok := metric.Fields[expectedCounter]
require.True(t, ok, "Expected presence of %s field", expectedCounter)
valInt64, ok := val.(int64)
require.True(t, ok, fmt.Sprintf("Expected int64, got %T", val))
require.Greater(t, valInt64, 0, fmt.Sprintf("Expected > 0, got %d, for %#v", valInt64, metric))
require.Truef(t, ok, "Expected int64, got %T", val)
require.Greaterf(t, valInt64, int64(0), "Expected > 0, got %d, for %#v", valInt64, metric)
}
// Test *Array way
m = WinPerfCounters{
PrintValid: false,
Object: perfObjects,
queryCreator: &PerformanceQueryCreatorImpl{},
UseWildcardsExpansion: false,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
queryCreator: &PerformanceQueryCreatorImpl{},
}
var acc2 testutil.Accumulator
require.NoError(t, m.Gather(&acc))
@ -589,7 +603,7 @@ func TestWinPerfCountersCollectRawIntegration(t *testing.T) {
val, ok := metric.Fields[expectedCounter]
require.True(t, ok, "Expected presence of %s field", expectedCounter)
valInt64, ok := val.(int64)
require.True(t, ok, fmt.Sprintf("Expected int64, got %T", val))
require.Greater(t, valInt64, 0, fmt.Sprintf("Expected > 0, got %d, for %#v", valInt64, metric))
require.Truef(t, ok, "Expected int64, got %T", val)
require.Greaterf(t, valInt64, int64(0), "Expected > 0, got %d, for %#v", valInt64, metric)
}
}

View File

@ -214,7 +214,7 @@ type FakePerformanceQueryCreator struct {
fakeQueries map[string]*FakePerformanceQuery
}
func (m FakePerformanceQueryCreator) NewPerformanceQuery(computer string) PerformanceQuery {
func (m FakePerformanceQueryCreator) NewPerformanceQuery(computer string, _ uint32) PerformanceQuery {
var ret PerformanceQuery
var ok bool
if ret, ok = m.fakeQueries[computer]; !ok {
@ -2043,6 +2043,7 @@ func TestLocalizeWildcardsExpansion(t *testing.T) {
[]string{"_Total"}, []string{counter}, true, false, false),
LocalizeWildcardsExpansion: false,
UseWildcardsExpansion: true,
MaxBufferSize: defaultMaxBufferSize,
Log: testutil.Logger{},
}