Skip to content

Commit

Permalink
Add PDH_NO_DATA to known counter error codes in win_perf_counters (#5182
Browse files Browse the repository at this point in the history 
)



(cherry picked from commit 10a067a)
  • Loading branch information
@nicgrobler @danielnelson
nicgrobler authored and danielnelson committed Dec 26, 2018
1 parent 734032c commit dbd515a
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 43 deletions.
3 changes: 2 additions & 1 deletion plugins/inputs/win_perf_counters/win_perf_counters.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,8 @@ func addCounterMeasurement(metric *counter, instanceName string, value float64,
func isKnownCounterDataError(err error) bool {
if pdhErr, ok := err.(*PdhError); ok && (pdhErr.ErrorCode == PDH_INVALID_DATA ||
pdhErr.ErrorCode == PDH_CALC_NEGATIVE_VALUE ||
pdhErr.ErrorCode == PDH_CSTATUS_INVALID_DATA) {
pdhErr.ErrorCode == PDH_CSTATUS_INVALID_DATA ||
pdhErr.ErrorCode == PDH_NO_DATA) {
return true
}
return false
Expand Down
119 changes: 77 additions & 42 deletions plugins/inputs/win_perf_counters/win_perf_counters_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,20 @@ package win_perf_counters
import (
"errors"
"fmt"
"testing"
"time"

"github.com/influxdata/telegraf/internal"
"github.com/influxdata/telegraf/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"testing"
"time"
)

type testCounter struct {
handle PDH_HCOUNTER
path string
value float64
status uint32 // allows for tests against specific pdh_error codes, rather than assuming all cases of "value == 0" to indicate error conditions
}
type FakePerformanceQuery struct {
counters map[string]testCounter
Expand Down Expand Up @@ -99,17 +101,10 @@ func (m *FakePerformanceQuery) GetFormattedCounterValueDouble(counterHandle PDH_
}
for _, counter := range m.counters {
if counter.handle == counterHandle {
if counter.value > 0 {
return counter.value, nil
} else {
if counter.value == 0 {
return 0, NewPdhError(PDH_INVALID_DATA)
} else if counter.value == -1 {
return 0, NewPdhError(PDH_CALC_NEGATIVE_VALUE)
} else {
return 0, NewPdhError(PDH_ACCESS_DENIED)
}
if counter.status > 0 {
return 0, NewPdhError(counter.status)
}
return counter.value, nil
}
}
return 0, fmt.Errorf("GetFormattedCounterValueDouble: invalid handle: %d", counterHandle)
Expand Down Expand Up @@ -143,17 +138,10 @@ func (m *FakePerformanceQuery) GetFormattedCounterArrayDouble(hCounter PDH_HCOUN
for _, p := range e {
counter := m.findCounterByPath(p)
if counter != nil {
if counter.value > 0 {
counters = append(counters, *counter.ToCounterValue())
} else {
if counter.value == 0 {
return nil, NewPdhError(PDH_INVALID_DATA)
} else if counter.value == -1 {
return nil, NewPdhError(PDH_CALC_NEGATIVE_VALUE)
} else {
return nil, NewPdhError(PDH_ACCESS_DENIED)
}
if counter.status > 0 {
return nil, NewPdhError(counter.status)
}
counters = append(counters, *counter.ToCounterValue())
} else {
return nil, fmt.Errorf("GetFormattedCounterArrayDouble: invalid counter : %s", p)
}
Expand Down Expand Up @@ -199,13 +187,14 @@ func createPerfObject(measurement string, object string, instances []string, cou
return perfobjects
}

func createCounterMap(counterPaths []string, values []float64) map[string]testCounter {
func createCounterMap(counterPaths []string, values []float64, status []uint32) map[string]testCounter {
counters := make(map[string]testCounter)
for i, cp := range counterPaths {
counters[cp] = testCounter{
PDH_HCOUNTER(i),
cp,
values[i],
status[i],
}
}
return counters
Expand Down Expand Up @@ -259,7 +248,7 @@ func TestAddItemSimple(t *testing.T) {
var err error
cps1 := []string{"\\O(I)\\C"}
m := Win_PerfCounters{PrintValid: false, Object: nil, query: &FakePerformanceQuery{
counters: createCounterMap(cps1, []float64{1.1}),
counters: createCounterMap(cps1, []float64{1.1}, []uint32{0}),
expandPaths: map[string][]string{
cps1[0]: cps1,
},
Expand All @@ -277,7 +266,7 @@ func TestAddItemInvalidCountPath(t *testing.T) {
var err error
cps1 := []string{"\\O\\C"}
m := Win_PerfCounters{PrintValid: false, Object: nil, UseWildcardsExpansion: true, query: &FakePerformanceQuery{
counters: createCounterMap(cps1, []float64{1.1}),
counters: createCounterMap(cps1, []float64{1.1}, []uint32{0}),
expandPaths: map[string][]string{
cps1[0]: {"\\O/C"},
},
Expand All @@ -296,7 +285,7 @@ func TestParseConfigBasic(t *testing.T) {
perfObjects := createPerfObject("m", "O", []string{"I1", "I2"}, []string{"C1", "C2"}, false, false)
cps1 := []string{"\\O(I1)\\C1", "\\O(I1)\\C2", "\\O(I2)\\C1", "\\O(I2)\\C2"}
m := Win_PerfCounters{PrintValid: false, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap(cps1, []float64{1.1, 1.2, 1.3, 1.4}),
counters: createCounterMap(cps1, []float64{1.1, 1.2, 1.3, 1.4}, []uint32{0, 0, 0, 0}),
expandPaths: map[string][]string{
cps1[0]: {cps1[0]},
cps1[1]: {cps1[1]},
Expand Down Expand Up @@ -330,7 +319,7 @@ func TestParseConfigNoInstance(t *testing.T) {
perfObjects := createPerfObject("m", "O", []string{"------"}, []string{"C1", "C2"}, false, false)
cps1 := []string{"\\O\\C1", "\\O\\C2"}
m := Win_PerfCounters{PrintValid: false, Object: perfObjects, UseWildcardsExpansion: false, query: &FakePerformanceQuery{
counters: createCounterMap(cps1, []float64{1.1, 1.2}),
counters: createCounterMap(cps1, []float64{1.1, 1.2}, []uint32{0, 0}),
expandPaths: map[string][]string{
cps1[0]: {cps1[0]},
cps1[1]: {cps1[1]},
Expand Down Expand Up @@ -362,7 +351,7 @@ func TestParseConfigInvalidCounterError(t *testing.T) {
perfObjects := createPerfObject("m", "O", []string{"I1", "I2"}, []string{"C1", "C2"}, true, false)
cps1 := []string{"\\O(I1)\\C2", "\\O(I2)\\C1", "\\O(I2)\\C2"}
m := Win_PerfCounters{PrintValid: false, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap(cps1, []float64{1.1, 1.2, 1.3}),
counters: createCounterMap(cps1, []float64{1.1, 1.2, 1.3}, []uint32{0, 0, 0}),
expandPaths: map[string][]string{
cps1[0]: {cps1[0]},
cps1[1]: {cps1[1]},
Expand Down Expand Up @@ -393,7 +382,7 @@ func TestParseConfigInvalidCounterNoError(t *testing.T) {
perfObjects := createPerfObject("m", "O", []string{"I1", "I2"}, []string{"C1", "C2"}, false, false)
cps1 := []string{"\\O(I1)\\C2", "\\O(I2)\\C1", "\\O(I2)\\C2"}
m := Win_PerfCounters{PrintValid: false, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap(cps1, []float64{1.1, 1.2, 1.3}),
counters: createCounterMap(cps1, []float64{1.1, 1.2, 1.3}, []uint32{0, 0, 0}),
expandPaths: map[string][]string{
cps1[0]: {cps1[0]},
cps1[1]: {cps1[1]},
Expand Down Expand Up @@ -425,7 +414,7 @@ func TestParseConfigTotalExpansion(t *testing.T) {
perfObjects := createPerfObject("m", "O", []string{"*"}, []string{"*"}, true, true)
cps1 := []string{"\\O(I1)\\C1", "\\O(I1)\\C2", "\\O(_Total)\\C1", "\\O(_Total)\\C2"}
m := Win_PerfCounters{PrintValid: false, UseWildcardsExpansion: true, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap(append(cps1, "\\O(*)\\*"), []float64{1.1, 1.2, 1.3, 1.4, 0}),
counters: createCounterMap(append(cps1, "\\O(*)\\*"), []float64{1.1, 1.2, 1.3, 1.4, 0}, []uint32{0, 0, 0, 0, 0}),
expandPaths: map[string][]string{
"\\O(*)\\*": cps1,
},
Expand All @@ -442,7 +431,7 @@ func TestParseConfigTotalExpansion(t *testing.T) {
perfObjects[0].IncludeTotal = false

m = Win_PerfCounters{PrintValid: false, UseWildcardsExpansion: true, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap(append(cps1, "\\O(*)\\*"), []float64{1.1, 1.2, 1.3, 1.4, 0}),
counters: createCounterMap(append(cps1, "\\O(*)\\*"), []float64{1.1, 1.2, 1.3, 1.4, 0}, []uint32{0, 0, 0, 0, 0}),
expandPaths: map[string][]string{
"\\O(*)\\*": cps1,
},
Expand All @@ -462,7 +451,7 @@ func TestParseConfigExpand(t *testing.T) {
perfObjects := createPerfObject("m", "O", []string{"*"}, []string{"*"}, false, false)
cps1 := []string{"\\O(I1)\\C1", "\\O(I1)\\C2", "\\O(I2)\\C1", "\\O(I2)\\C2"}
m := Win_PerfCounters{PrintValid: false, UseWildcardsExpansion: true, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap(append(cps1, "\\O(*)\\*"), []float64{1.1, 1.2, 1.3, 1.4, 0}),
counters: createCounterMap(append(cps1, "\\O(*)\\*"), []float64{1.1, 1.2, 1.3, 1.4, 0}, []uint32{0, 0, 0, 0, 0}),
expandPaths: map[string][]string{
"\\O(*)\\*": cps1,
},
Expand All @@ -486,7 +475,7 @@ func TestSimpleGather(t *testing.T) {
perfObjects := createPerfObject(measurement, "O", []string{"I"}, []string{"C"}, false, false)
cp1 := "\\O(I)\\C"
m := Win_PerfCounters{PrintValid: false, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap([]string{cp1}, []float64{1.2}),
counters: createCounterMap([]string{cp1}, []float64{1.2}, []uint32{0}),
expandPaths: map[string][]string{
cp1: {cp1},
},
Expand Down Expand Up @@ -516,6 +505,48 @@ func TestSimpleGather(t *testing.T) {
acc1.AssertContainsTaggedFields(t, measurement, fields1, tags1)
}

func TestSimpleGatherNoData(t *testing.T) {
var err error
if testing.Short() {
t.Skip("Skipping long taking test in short mode")
}
measurement := "test"
perfObjects := createPerfObject(measurement, "O", []string{"I"}, []string{"C"}, false, false)
cp1 := "\\O(I)\\C"
m := Win_PerfCounters{PrintValid: false, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap([]string{cp1}, []float64{1.2}, []uint32{PDH_NO_DATA}),
expandPaths: map[string][]string{
cp1: {cp1},
},
vistaAndNewer: false,
}}
var acc1 testutil.Accumulator
err = m.Gather(&acc1)
// this "PDH_NO_DATA" error should not be returned to caller, but checked, and handled
require.NoError(t, err)

// fields would contain if the error was ignored, and we simply added garbage
fields1 := map[string]interface{}{
"C": float32(1.2),
}
// tags would contain if the error was ignored, and we simply added garbage
tags1 := map[string]string{
"instance": "I",
"objectname": "O",
}
acc1.AssertDoesNotContainsTaggedFields(t, measurement, fields1, tags1)

m.UseWildcardsExpansion = true
m.counters = nil
m.lastRefreshed = time.Time{}

var acc2 testutil.Accumulator

err = m.Gather(&acc2)
require.NoError(t, err)
acc1.AssertDoesNotContainsTaggedFields(t, measurement, fields1, tags1)
}

func TestSimpleGatherWithTimestamp(t *testing.T) {
var err error
if testing.Short() {
Expand All @@ -525,7 +556,7 @@ func TestSimpleGatherWithTimestamp(t *testing.T) {
perfObjects := createPerfObject(measurement, "O", []string{"I"}, []string{"C"}, false, false)
cp1 := "\\O(I)\\C"
m := Win_PerfCounters{PrintValid: false, UsePerfCounterTime: true, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap([]string{cp1}, []float64{1.2}),
counters: createCounterMap([]string{cp1}, []float64{1.2}, []uint32{0}),
expandPaths: map[string][]string{
cp1: {cp1},
},
Expand All @@ -548,14 +579,15 @@ func TestSimpleGatherWithTimestamp(t *testing.T) {

func TestGatherError(t *testing.T) {
var err error
expected_error := "error while getting value for counter \\O(I)\\C: The information passed is not valid.\r\n"
if testing.Short() {
t.Skip("Skipping long taking test in short mode")
}
measurement := "test"
perfObjects := createPerfObject(measurement, "O", []string{"I"}, []string{"C"}, false, false)
cp1 := "\\O(I)\\C"
m := Win_PerfCounters{PrintValid: false, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap([]string{cp1}, []float64{-2}),
counters: createCounterMap([]string{cp1}, []float64{-2}, []uint32{PDH_PLA_VALIDATION_WARNING}),
expandPaths: map[string][]string{
cp1: {cp1},
},
Expand All @@ -564,6 +596,7 @@ func TestGatherError(t *testing.T) {
var acc1 testutil.Accumulator
err = m.Gather(&acc1)
require.Error(t, err)
require.Equal(t, expected_error, err.Error())

m.UseWildcardsExpansion = true
m.counters = nil
Expand All @@ -573,6 +606,7 @@ func TestGatherError(t *testing.T) {

err = m.Gather(&acc2)
require.Error(t, err)
require.Equal(t, expected_error, err.Error())
}

func TestGatherInvalidDataIgnore(t *testing.T) {
Expand All @@ -584,7 +618,7 @@ func TestGatherInvalidDataIgnore(t *testing.T) {
perfObjects := createPerfObject(measurement, "O", []string{"I"}, []string{"C1", "C2", "C3"}, false, false)
cps1 := []string{"\\O(I)\\C1", "\\O(I)\\C2", "\\O(I)\\C3"}
m := Win_PerfCounters{PrintValid: false, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap(cps1, []float64{1.2, -1, 0}),
counters: createCounterMap(cps1, []float64{1.2, 1, 0}, []uint32{0, PDH_INVALID_DATA, 0}),
expandPaths: map[string][]string{
cps1[0]: {cps1[0]},
cps1[1]: {cps1[1]},
Expand All @@ -598,6 +632,7 @@ func TestGatherInvalidDataIgnore(t *testing.T) {

fields1 := map[string]interface{}{
"C1": float32(1.2),
"C3": float32(0),
}
tags1 := map[string]string{
"instance": "I",
Expand Down Expand Up @@ -625,7 +660,7 @@ func TestGatherRefreshingWithExpansion(t *testing.T) {
perfObjects := createPerfObject(measurement, "O", []string{"*"}, []string{"*"}, true, false)
cps1 := []string{"\\O(I1)\\C1", "\\O(I1)\\C2", "\\O(I2)\\C1", "\\O(I2)\\C2"}
fpm := &FakePerformanceQuery{
counters: createCounterMap(append(cps1, "\\O(*)\\*"), []float64{1.1, 1.2, 1.3, 1.4, 0}),
counters: createCounterMap(append(cps1, "\\O(*)\\*"), []float64{1.1, 1.2, 1.3, 1.4, 0}, []uint32{0, 0, 0, 0, 0}),
expandPaths: map[string][]string{
"\\O(*)\\*": cps1,
},
Expand Down Expand Up @@ -659,7 +694,7 @@ func TestGatherRefreshingWithExpansion(t *testing.T) {
acc1.AssertContainsTaggedFields(t, measurement, fields2, tags2)
cps2 := []string{"\\O(I1)\\C1", "\\O(I1)\\C2", "\\O(I2)\\C1", "\\O(I2)\\C2", "\\O(I3)\\C1", "\\O(I3)\\C2"}
fpm = &FakePerformanceQuery{
counters: createCounterMap(append(cps2, "\\O(*)\\*"), []float64{1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 0}),
counters: createCounterMap(append(cps2, "\\O(*)\\*"), []float64{1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 0}, []uint32{0, 0, 0, 0, 0, 0, 0}),
expandPaths: map[string][]string{
"\\O(*)\\*": cps2,
},
Expand Down Expand Up @@ -710,7 +745,7 @@ func TestGatherRefreshingWithoutExpansion(t *testing.T) {
perfObjects := createPerfObject(measurement, "O", []string{"*"}, []string{"C1", "C2"}, true, false)
cps1 := []string{"\\O(I1)\\C1", "\\O(I1)\\C2", "\\O(I2)\\C1", "\\O(I2)\\C2"}
fpm := &FakePerformanceQuery{
counters: createCounterMap(append([]string{"\\O(*)\\C1", "\\O(*)\\C2"}, cps1...), []float64{0, 0, 1.1, 1.2, 1.3, 1.4}),
counters: createCounterMap(append([]string{"\\O(*)\\C1", "\\O(*)\\C2"}, cps1...), []float64{0, 0, 1.1, 1.2, 1.3, 1.4}, []uint32{0, 0, 0, 0, 0, 0}),
expandPaths: map[string][]string{
"\\O(*)\\C1": {cps1[0], cps1[2]},
"\\O(*)\\C2": {cps1[1], cps1[3]},
Expand Down Expand Up @@ -746,7 +781,7 @@ func TestGatherRefreshingWithoutExpansion(t *testing.T) {
//test finding new instance
cps2 := []string{"\\O(I1)\\C1", "\\O(I1)\\C2", "\\O(I2)\\C1", "\\O(I2)\\C2", "\\O(I3)\\C1", "\\O(I3)\\C2"}
fpm = &FakePerformanceQuery{
counters: createCounterMap(append([]string{"\\O(*)\\C1", "\\O(*)\\C2"}, cps2...), []float64{0, 0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6}),
counters: createCounterMap(append([]string{"\\O(*)\\C1", "\\O(*)\\C2"}, cps2...), []float64{0, 0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6}, []uint32{0, 0, 0, 0, 0, 0, 0, 0}),
expandPaths: map[string][]string{
"\\O(*)\\C1": {cps2[0], cps2[2], cps2[4]},
"\\O(*)\\C2": {cps2[1], cps2[3], cps2[5]},
Expand Down Expand Up @@ -779,7 +814,7 @@ func TestGatherRefreshingWithoutExpansion(t *testing.T) {
perfObjects = createPerfObject(measurement, "O", []string{"*"}, []string{"C1", "C2", "C3"}, true, false)
cps3 := []string{"\\O(I1)\\C1", "\\O(I1)\\C2", "\\O(I1)\\C3", "\\O(I2)\\C1", "\\O(I2)\\C2", "\\O(I2)\\C3"}
fpm = &FakePerformanceQuery{
counters: createCounterMap(append([]string{"\\O(*)\\C1", "\\O(*)\\C2", "\\O(*)\\C3"}, cps3...), []float64{0, 0, 0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6}),
counters: createCounterMap(append([]string{"\\O(*)\\C1", "\\O(*)\\C2", "\\O(*)\\C3"}, cps3...), []float64{0, 0, 0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6}, []uint32{0, 0, 0, 0, 0, 0, 0, 0, 0}),
expandPaths: map[string][]string{
"\\O(*)\\C1": {cps3[0], cps3[3]},
"\\O(*)\\C2": {cps3[1], cps3[4]},
Expand Down Expand Up @@ -828,7 +863,7 @@ func TestGatherTotalNoExpansion(t *testing.T) {
perfObjects := createPerfObject(measurement, "O", []string{"*"}, []string{"C1", "C2"}, true, true)
cps1 := []string{"\\O(I1)\\C1", "\\O(I1)\\C2", "\\O(_Total)\\C1", "\\O(_Total)\\C2"}
m := Win_PerfCounters{PrintValid: false, UseWildcardsExpansion: false, Object: perfObjects, query: &FakePerformanceQuery{
counters: createCounterMap(append([]string{"\\O(*)\\C1", "\\O(*)\\C2"}, cps1...), []float64{0, 0, 1.1, 1.2, 1.3, 1.4}),
counters: createCounterMap(append([]string{"\\O(*)\\C1", "\\O(*)\\C2"}, cps1...), []float64{0, 0, 1.1, 1.2, 1.3, 1.4}, []uint32{0, 0, 0, 0, 0, 0}),
expandPaths: map[string][]string{
"\\O(*)\\C1": {cps1[0], cps1[2]},
"\\O(*)\\C2": {cps1[1], cps1[3]},
Expand Down

0 comments on commit dbd515a

Please sign in to comment.