diff --git a/plugins/inputs/win_perf_counters/win_perf_counters.go b/plugins/inputs/win_perf_counters/win_perf_counters.go index 06a1a333c..2bf50e5cc 100644 --- a/plugins/inputs/win_perf_counters/win_perf_counters.go +++ b/plugins/inputs/win_perf_counters/win_perf_counters.go @@ -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 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 81959ef8c..5052fb7a2 100644 --- a/plugins/inputs/win_perf_counters/win_perf_counters_test.go +++ b/plugins/inputs/win_perf_counters/win_perf_counters_test.go @@ -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 @@ -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) @@ -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) } @@ -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 @@ -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, }, @@ -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"}, }, @@ -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]}, @@ -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]}, @@ -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]}, @@ -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]}, @@ -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, }, @@ -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, }, @@ -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, }, @@ -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}, }, @@ -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() { @@ -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}, }, @@ -548,6 +579,7 @@ 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") } @@ -555,7 +587,7 @@ func TestGatherError(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{-2}), + counters: createCounterMap([]string{cp1}, []float64{-2}, []uint32{PDH_PLA_VALIDATION_WARNING}), expandPaths: map[string][]string{ cp1: {cp1}, }, @@ -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 @@ -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) { @@ -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]}, @@ -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", @@ -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, }, @@ -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, }, @@ -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]}, @@ -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]}, @@ -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]}, @@ -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]},