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

PdhExpandWildCardPathW will not expand counter paths in 32 bit windows systems #12622

Merged
merged 4 commits into from
Jun 28, 2019
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 @@ -133,6 +133,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- The `elasticsearch/index_summary` metricset gracefully handles an empty Elasticsearch cluster when `xpack.enabled: true` is set. {pull}12489[12489] {issue}12487[12487]
- When TLS is configured for the http metricset and a `certificate_authorities` is configured we now default to `required` for the `client_authentication`. {pull}12584[12584]
- Reuse connections in PostgreSQL metricsets. {issue}12504[12504] {pull}12603[12603]
- PdhExpandWildCardPathW will not expand counter paths in 32 bit windows systems, workaround will use a different function.{issue}12590[12590]{pull}12622[12622]

*Packetbeat*

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ func TestData(t *testing.T) {
"query": `\Processor Information(_Total)\% Processor Time`,
},
{
"instance_label": "disk.bytes.name",
"measurement_label": "disk.bytes.read.total",
"query": `\FileSystem Disk Activity(_Total)\FileSystem Bytes Read`,
"instance_label": "process.name",
"measurement_label": "process.ID",
"query": `\Process(_Total)\ID Process`,
},
{
"instance_label": "processor.name",
"measurement_label": "processor.time.idle.average.ns",
"query": `\Processor Information(_Total)\Average Idle Time`,
"measurement_label": "processor.time.user.ns",
"query": `\Processor Information(_Total)\% User Time`,
},
},
}
Expand Down Expand Up @@ -389,8 +389,8 @@ func TestGroupByInstance(t *testing.T) {
config.CounterConfig[1].Format = "float"

config.CounterConfig[2].InstanceLabel = "processor.name"
config.CounterConfig[2].MeasurementLabel = "processor.time.idle.average.ns"
config.CounterConfig[2].Query = `\Processor Information(_Total)\Average Idle Time`
config.CounterConfig[2].MeasurementLabel = "processor.time.privileged.ns"
config.CounterConfig[2].Query = `\Processor Information(_Total)\% Privileged Time`
config.CounterConfig[2].Format = "float"

handle, err := NewReader(config)
Expand Down Expand Up @@ -423,7 +423,7 @@ func TestGroupByInstance(t *testing.T) {
}
assert.True(t, pctKey)

pctKey, err = values[0].MetricSetFields.HasKey("processor.time.idle.average.ns")
pctKey, err = values[0].MetricSetFields.HasKey("processor.time.privileged.ns")
if err != nil {
t.Fatal(err)
}
Expand Down
13 changes: 12 additions & 1 deletion metricbeat/module/windows/perfmon/pdh_query_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package perfmon

import (
"regexp"
"runtime"
"syscall"
"unsafe"

Expand Down Expand Up @@ -114,8 +115,18 @@ func (q *Query) ExpandWildCardPath(wildCardPath string) ([]string, error) {
if wildCardPath == "" {
return nil, errors.New("no query path given")
}
utfPath, err := syscall.UTF16PtrFromString(wildCardPath)
if err != nil {
return nil, err
}
var expdPaths []uint16

expdPaths, err := PdhExpandWildCardPath(wildCardPath)
// PdhExpandWildCardPath will not return the counter paths for windows 32 bit systems but PdhExpandCounterPath will.
if runtime.GOARCH == "386" {
expdPaths, err = PdhExpandCounterPath(utfPath)
} else {
expdPaths, err = PdhExpandWildCardPath(utfPath)
}
if err != nil {
return nil, err
}
Expand Down
29 changes: 20 additions & 9 deletions metricbeat/module/windows/perfmon/pdh_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
//sys _PdhGetFormattedCounterValueLong(counter PdhCounterHandle, format PdhCounterFormat, counterType *uint32, value *PdhCounterValueLong) (errcode error) [failretval!=0]= pdh.PdhGetFormattedCounterValue
//sys _PdhCloseQuery(query PdhQueryHandle) (errcode error) [failretval!=0] = pdh.PdhCloseQuery
//sys _PdhExpandWildCardPath(dataSource *uint16, wildcardPath *uint16, expandedPathList *uint16, pathListLength *uint32) (errcode error) [failretval!=0] = pdh.PdhExpandWildCardPathW
//sys _PdhExpandCounterPath(wildcardPath *uint16, expandedPathList *uint16, pathListLength *uint32) (errcode error) [failretval!=0] = pdh.PdhExpandCounterPathW

type PdhQueryHandle uintptr

Expand Down Expand Up @@ -140,23 +141,33 @@ func PdhGetFormattedCounterValueLong(counter PdhCounterHandle) (uint32, *PdhCoun
}

// PdhExpandWildCardPath returns counter paths that match the given counter path.
func PdhExpandWildCardPath(wildCardPath string) ([]uint16, error) {
utfPath, err := syscall.UTF16PtrFromString(wildCardPath)
if err != nil {
return nil, err
}
func PdhExpandWildCardPath(utfPath *uint16) ([]uint16, error) {
var bufferSize uint32
if err := _PdhExpandWildCardPath(nil, utfPath, nil, &bufferSize); err != nil {
if PdhErrno(err.(syscall.Errno)) != PDH_MORE_DATA {
return nil, PdhErrno(err.(syscall.Errno))
}
expandPaths := make([]uint16, bufferSize)
if err := _PdhExpandWildCardPath(nil, utfPath, &expandPaths[0], &bufferSize); err != nil {
return nil, PdhErrno(err.(syscall.Errno))
}
return expandPaths, nil
}
return nil, nil
}

expdPaths := make([]uint16, bufferSize)
bufferSize = uint32(len(expdPaths))
if err := _PdhExpandWildCardPath(nil, utfPath, &expdPaths[0], &bufferSize); err != nil {
// PdhExpandCounterPath returns counter paths that match the given counter path, for 32 bit windows.
func PdhExpandCounterPath(utfPath *uint16) ([]uint16, error) {
var bufferSize uint32
if err := _PdhExpandCounterPath(utfPath, nil, &bufferSize); err != nil {
if PdhErrno(err.(syscall.Errno)) != PDH_MORE_DATA {
return nil, PdhErrno(err.(syscall.Errno))
}
expandPaths := make([]uint16, bufferSize)
if err := _PdhExpandCounterPath(utfPath, &expandPaths[0], &bufferSize); err != nil {
return nil, PdhErrno(err.(syscall.Errno))
}
return expdPaths, nil
return expandPaths, nil
}
return nil, nil
}
Expand Down
11 changes: 9 additions & 2 deletions metricbeat/module/windows/perfmon/pdh_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package perfmon

import (
"syscall"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -56,7 +57,9 @@ func TestPdhGetFormattedCounterValueInvalidCounter(t *testing.T) {

// TestPdhExpandWildCardPathInvalidPath will test for invalid query path.
func TestPdhExpandWildCardPathInvalidPath(t *testing.T) {
queryList, err := PdhExpandWildCardPath("sdfhsdhfd")
utfPath, err := syscall.UTF16PtrFromString("sdfhsdhfd")
assert.Nil(t, err)
queryList, err := PdhExpandWildCardPath(utfPath)
assert.Nil(t, queryList)
assert.EqualValues(t, err, PDH_INVALID_PATH)
}
Expand All @@ -80,7 +83,11 @@ func TestPdhSuccessfulCounterRetrieval(t *testing.T) {
t.Fatal(err)
}
defer PdhCloseQuery(queryHandle)
queryList, err := PdhExpandWildCardPath(validQuery)
utfPath, err := syscall.UTF16PtrFromString(validQuery)
if err != nil {
t.Fatal(err)
}
queryList, err := PdhExpandWildCardPath(utfPath)
if err != nil {
t.Fatal(err)
}
Expand Down
8 changes: 5 additions & 3 deletions metricbeat/module/windows/perfmon/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package perfmon
import (
"regexp"
"strconv"
"strings"

"github.com/pkg/errors"

Expand Down Expand Up @@ -74,9 +75,10 @@ func NewReader(config Config) (*Reader, error) {
return nil, errors.Wrapf(err, `failed to expand counter (query="%v")`, counter.Query)
}
}
if childQueries == nil || len(childQueries) == 0 {
// check if the pdhexpandcounterpath/pdhexpandwildcardpath functions have expanded the counter successfully.
if len(childQueries) == 0 || (len(childQueries) == 1 && strings.Contains(childQueries[0], "*")) {
query.Close()
return nil, errors.Wrapf(err, `failed to expand counter (query="%v")`, counter.Query)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch here btw 🙂 I guess it was failing silently before? (errors.Wrap(nil) returns nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsoriano , yes, found it while running the tests.

return nil, errors.Errorf(`failed to expand counter (query="%v")`, counter.Query)
}
for _, v := range childQueries {
if err := query.AddCounter(v, counter, len(childQueries) > 1); err != nil {
Expand Down Expand Up @@ -108,7 +110,7 @@ func (r *Reader) Read() ([]mb.Event, error) {

for counterPath, values := range values {
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.
// 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 && !r.executed {
r.log.Debugw("Ignoring the first measurement because the data isn't ready",
Expand Down
9 changes: 9 additions & 0 deletions metricbeat/module/windows/perfmon/zpdh_windows.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.