Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add PDH_NO_DATA to known counter error codes #5182

Merged
merged 2 commits into from
Dec 26, 2018
Merged

Add PDH_NO_DATA to known counter error codes #5182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
149895a
Add PDH_NO_DATA to known error codes
Dec 22, 2018
d41b1e2
fix pointless string copy
nicgrobler Dec 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion plugins/inputs/win_perf_counters/win_perf_counters.go
View file
Open in desktop
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
Open in desktop
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