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.

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

perfmon - remove negative counter value errors from the event output #18361

Merged
merged 4 commits into from
May 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Fix storage metricset to allow config without region/zone. {issue}17623[17623] {pull}17624[17624]
- Add a switch to the driver definition on SQL module to use pretty names. {pull}17378[17378]
- Fix overflow on Prometheus rates when new buckets are added on the go. {pull}17753[17753]
- Remove specific win32 api errors from events in perfmon. {issue}18292[18292] {pull}18361[18361]

*Packetbeat*

Expand Down
27 changes: 21 additions & 6 deletions metricbeat/helper/windows/pdh/pdh_query_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ type Query struct {
type CounterValue struct {
Instance string
Measurement interface{}
Err error
Err CounterValueError
}

// CounterValueError contains the performance counter error.
type CounterValueError struct {
narph marked this conversation as resolved.
Show resolved Hide resolved
Error error
CStatus uint32
}

// Open creates a new query.
Expand Down Expand Up @@ -235,31 +241,40 @@ func MatchInstanceName(counterPath string) (string, error) {

// getCounterValue will retrieve the counter value based on the format applied in the config options
func getCounterValue(counter *Counter) CounterValue {
counterValue := CounterValue{Instance: counter.instanceName}
counterValue := CounterValue{Instance: counter.instanceName, Err: CounterValueError{CStatus: 0}}
switch counter.format {
case PdhFmtLong:
_, value, err := PdhGetFormattedCounterValueLong(counter.handle)
if err != nil {
counterValue.Err = err
counterValue.Err.Error = err
narph marked this conversation as resolved.
Show resolved Hide resolved
if value != nil {
counterValue.Err.CStatus = value.CStatus
}
} else {
counterValue.Measurement = value.Value
}
case PdhFmtLarge:
_, value, err := PdhGetFormattedCounterValueLarge(counter.handle)
if err != nil {
counterValue.Err = err
counterValue.Err.Error = err
if value != nil {
counterValue.Err.CStatus = value.CStatus
}
} else {
counterValue.Measurement = value.Value
}
case PdhFmtDouble:
_, value, err := PdhGetFormattedCounterValueDouble(counter.handle)
if err != nil {
counterValue.Err = err
counterValue.Err.Error = err
if value != nil {
counterValue.Err.CStatus = value.CStatus
}
} else {
counterValue.Measurement = value.Value
}
default:
counterValue.Err = errors.Errorf("initialization failed: format '%#v' "+
counterValue.Err.Error = errors.Errorf("initialization failed: format '%#v' "+
"for instance '%s' is invalid (must be PdhFmtDouble, PdhFmtLarge or PdhFmtLong)",
counter.format, counter.instanceName)
}
Expand Down
6 changes: 3 additions & 3 deletions metricbeat/helper/windows/pdh/pdh_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func PdhGetFormattedCounterValueDouble(counter PdhCounterHandle) (uint32, *PdhCo
var counterType uint32
var value PdhCounterValueDouble
if err := _PdhGetFormattedCounterValueDouble(counter, PdhFmtDouble|PdhFmtNoCap100, &counterType, &value); err != nil {
return 0, nil, PdhErrno(err.(syscall.Errno))
return 0, &value, PdhErrno(err.(syscall.Errno))
}

return counterType, &value, nil
Expand All @@ -172,7 +172,7 @@ func PdhGetFormattedCounterValueLarge(counter PdhCounterHandle) (uint32, *PdhCou
var counterType uint32
var value PdhCounterValueLarge
if err := _PdhGetFormattedCounterValueLarge(counter, PdhFmtLarge|PdhFmtNoCap100, &counterType, &value); err != nil {
return 0, nil, PdhErrno(err.(syscall.Errno))
return 0, &value, PdhErrno(err.(syscall.Errno))
}

return counterType, &value, nil
Expand All @@ -183,7 +183,7 @@ func PdhGetFormattedCounterValueLong(counter PdhCounterHandle) (uint32, *PdhCoun
var counterType uint32
var value PdhCounterValueLong
if err := _PdhGetFormattedCounterValueLong(counter, PdhFmtLong|PdhFmtNoCap100, &counterType, &value); err != nil {
return 0, nil, PdhErrno(err.(syscall.Errno))
return 0, &value, PdhErrno(err.(syscall.Errno))
}

return counterType, &value, nil
Expand Down
3 changes: 2 additions & 1 deletion metricbeat/helper/windows/pdh/pdh_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ func TestPdhAddCounterInvalidCounter(t *testing.T) {
func TestPdhGetFormattedCounterValueInvalidCounter(t *testing.T) {
counterType, counterValue, err := PdhGetFormattedCounterValueDouble(InvalidCounterHandle)
assert.EqualValues(t, counterType, 0)
assert.EqualValues(t, counterValue, (*PdhCounterValueDouble)(nil))
assert.NotNil(t, counterValue)
assert.Equal(t, counterValue.Value, float64(0))
assert.EqualValues(t, err, PDH_INVALID_HANDLE)
}

Expand Down
37 changes: 27 additions & 10 deletions metricbeat/module/windows/perfmon/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,23 @@ func (re *Reader) groupToEvents(counters map[string][]pdh.CounterValue) []mb.Eve
for ind, val := range values {
// Some counters, such as rate counters, require two counter values in order to compute a displayable value. In this case we must call PdhCollectQueryData twice before calling PdhGetFormattedCounterValue.
// For more information, see Collecting Performance Data (https://docs.microsoft.com/en-us/windows/desktop/PerfCtrs/collecting-performance-data).
if val.Err != nil && !re.executed {
re.log.Debugw("Ignoring the first measurement because the data isn't ready",
"error", val.Err, logp.Namespace("perfmon"), "query", counterPath)
continue
if val.Err.Error != nil {
if !re.executed {
re.log.Debugw("Ignoring the first measurement because the data isn't ready",
"error", val.Err.Error, logp.Namespace("perfmon"), "query", counterPath)
continue
}
// The counter has a negative value or the counter was successfully found, but the data returned is not valid.
// This error can occur if the counter value is less than the previous value. (Because counter values always increment, the counter value rolls over to zero when it reaches its maximum value.)
// This is not an error that stops the application from running successfully and a positive counter value should be retrieved in the later calls.
if val.Err.Error == pdh.PDH_CALC_NEGATIVE_VALUE || val.Err.Error == pdh.PDH_INVALID_DATA {
re.log.Debugw("Counter value retrieval returned",
"error", val.Err.Error, "cstatus", pdh.PdhErrno(val.Err.CStatus), logp.Namespace("perfmon"), "query", counterPath)
continue
}
}
var eventKey string
if re.config.GroupMeasurements && val.Err == nil {
if re.config.GroupMeasurements && val.Err.Error == nil {
// Send measurements with the same instance label as part of the same event
eventKey = val.Instance
} else {
Expand All @@ -60,7 +70,7 @@ func (re *Reader) groupToEvents(counters map[string][]pdh.CounterValue) []mb.Eve
if _, ok := eventMap[eventKey]; !ok {
eventMap[eventKey] = &mb.Event{
MetricSetFields: common.MapStr{},
Error: errors.Wrapf(val.Err, "failed on query=%v", counterPath),
Error: errors.Wrapf(val.Err.Error, "failed on query=%v", counterPath),
}
if val.Instance != "" {
//will ignore instance counter
Expand Down Expand Up @@ -100,10 +110,17 @@ func (re *Reader) groupToSingleEvent(counters map[string][]pdh.CounterValue) mb.
for _, val := range values {
// Some counters, such as rate counters, require two counter values in order to compute a displayable value. In this case we must call PdhCollectQueryData twice before calling PdhGetFormattedCounterValue.
// For more information, see Collecting Performance Data (https://docs.microsoft.com/en-us/windows/desktop/PerfCtrs/collecting-performance-data).
if val.Err != nil && !re.executed {
re.log.Debugw("Ignoring the first measurement because the data isn't ready",
"error", val.Err, logp.Namespace("perfmon"), "query", counterPath)
continue
if val.Err.Error != nil {
if !re.executed {
re.log.Debugw("Ignoring the first measurement because the data isn't ready",
"error", val.Err, logp.Namespace("perfmon"), "query", counterPath)
continue
}
if val.Err.Error == pdh.PDH_CALC_NEGATIVE_VALUE || val.Err.Error == pdh.PDH_INVALID_DATA {
re.log.Debugw("Counter value retrieval returned",
"error", val.Err.Error, "cstatus", pdh.PdhErrno(val.Err.CStatus), logp.Namespace("perfmon"), "query", counterPath)
continue
}
}
if val.Measurement == nil {
continue
Expand Down
2 changes: 1 addition & 1 deletion metricbeat/module/windows/perfmon/data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestGroupToEvents(t *testing.T) {
{
Instance: "",
Measurement: 23,
Err: nil,
Err: pdh.CounterValueError{},
},
},
}
Expand Down
5 changes: 4 additions & 1 deletion metricbeat/module/windows/perfmon/perfmon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func TestQuery(t *testing.T) {
t.Fatal(path[0], "not found")
}

assert.NoError(t, value[0].Err)
assert.NoError(t, value[0].Err.Error)
assert.Equal(t, "TestInstanceName", value[0].Instance)
}

Expand Down Expand Up @@ -444,6 +444,7 @@ func TestWildcardQuery(t *testing.T) {
}
config.Queries[0].Name = "Processor Information"
config.Queries[0].Instance = []string{"*"}
config.Queries[0].Namespace = "metrics"
config.Queries[0].Counters = []QueryCounter{
{
Name: "% Processor Time",
Expand Down Expand Up @@ -478,6 +479,7 @@ func TestWildcardQueryNoInstanceName(t *testing.T) {
}
config.Queries[0].Name = "Process"
config.Queries[0].Instance = []string{"*"}
config.Queries[0].Namespace = "metrics"
config.Queries[0].Counters = []QueryCounter{
{
Name: "Private Bytes",
Expand Down Expand Up @@ -523,6 +525,7 @@ func TestGroupByInstance(t *testing.T) {
}
config.Queries[0].Name = "Processor Information"
config.Queries[0].Instance = []string{"_Total"}
config.Queries[0].Namespace = "metrics"
config.Queries[0].Counters = []QueryCounter{
{
Name: "% Processor Time",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestReadSuccessfully(t *testing.T) {
// For more information, see Collecting Performance Data (https://docs.microsoft.com/en-us/windows/desktop/PerfCtrs/collecting-performance-data).
events, err := reader.Read()
assert.Nil(t, err)
assert.NotNil(t, events)
assert.Nil(t, events)
assert.Zero(t, len(events))
events, err = reader.Read()
assert.Nil(t, err)
Expand Down