Refactored Windows perfmon metricset: replaced method to retrieve counter paths with PdhExpandWildCardPathW, separated code by responsibility, removed unused functions

CHANGELOG.next.asciidoc

@@ -123,8 +123,9 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Avoid generating hints-based configuration with empty hosts when no exposed port is suitable for the hosts hint. {issue}8264[8264] {pull}12086[12086]
 - Fixed a socket leak in the postgresql module under Windows when SSL is disabled on the server. {pull}11393[11393]
 - Change some field type from scaled_float to long in aws module. {pull}11982[11982]
-- Fixed RabbitMQ `queue` metricset gathering when `consumer_utilisation` is set empty at the metrics source {pull}12089[12089] 
+- Fixed RabbitMQ `queue` metricset gathering when `consumer_utilisation` is set empty at the metrics source {pull}12089[12089]
 - Fix direction of incoming IPv6 sockets. {pull}12248[12248]
+- Small refactoring of perfom metricset, removal of unused functions, separation of responsibility {pull}12212[12212]
 
 *Packetbeat*
 

metricbeat/module/windows/perfmon/defs_pdh_windows.go

@@ -233,9 +233,6 @@ const (
 	PdhFmtMultiply1000 PdhCounterFormat = C.PDH_FMT_1000
 )
 
-// PdhCounterValue is the structure that receives the counter value.
-type PdhCounterValue C.PDH_FMT_COUNTERVALUE
-
 // PdhRawCounter is the structure that receives the raw counter.
 type PdhRawCounter C.PDH_RAW_COUNTER
 

metricbeat/module/windows/perfmon/mkpdh_defs.go

@@ -95,9 +95,6 @@ const (
 	PdhFmtMultiply1000 PdhCounterFormat = C.PDH_FMT_1000
 )
 
-// PdhCounterValue is the structure that receives the counter value.
-type PdhCounterValue C.PDH_FMT_COUNTERVALUE
-
 // PdhRawCounter is the structure that receives the raw counter.
 type PdhRawCounter C.PDH_RAW_COUNTER
 

metricbeat/module/windows/perfmon/pdh_integration_windows_test.go

@@ -22,7 +22,6 @@ package perfmon
 import (
 	"testing"
 	"time"
-	"unsafe"
 
 	"github.com/elastic/beats/libbeat/common"
 
@@ -105,25 +104,26 @@ func TestCounterWithNoInstanceName(t *testing.T) {
 }
 
 func TestQuery(t *testing.T) {
-	q, err := NewQuery("")
+	var q Query
+	err := q.Open()
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer q.Close()
-
-	err = q.AddCounter(processorTimeCounter, FloatFormat, "")
+	counter := CounterConfig{Format: "float", InstanceName: "TestInstanceName"}
+	err = q.AddCounter(processorTimeCounter, counter, false)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	for i := 0; i < 2; i++ {
-		err = q.Execute()
+		err = q.CollectData()
 		if err != nil {
 			t.Fatal(err)
 		}
 	}
 
-	values, err := q.Values()
+	values, err := q.GetFormattedCounterValues()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -136,6 +136,7 @@ func TestQuery(t *testing.T) {
 	}
 
 	assert.NoError(t, value[0].Err)
+	assert.Equal(t, "TestInstanceName", value[0].Instance)
 }
 
 func TestExistingCounter(t *testing.T) {
@@ -146,7 +147,7 @@ func TestExistingCounter(t *testing.T) {
 	config.CounterConfig[0].MeasurementLabel = "processor.time.total.pct"
 	config.CounterConfig[0].Query = processorTimeCounter
 	config.CounterConfig[0].Format = "float"
-	handle, err := NewPerfmonReader(config)
+	handle, err := NewReader(config)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -168,7 +169,7 @@ func TestNonExistingCounter(t *testing.T) {
 	config.CounterConfig[0].MeasurementLabel = "processor.time.total.pct"
 	config.CounterConfig[0].Query = "\\Processor Information(_Total)\\not existing counter"
 	config.CounterConfig[0].Format = "float"
-	handle, err := NewPerfmonReader(config)
+	handle, err := NewReader(config)
 	if assert.Error(t, err) {
 		assert.EqualValues(t, PDH_CSTATUS_NO_COUNTER, errors.Cause(err))
 	}
@@ -188,7 +189,7 @@ func TestIgnoreNonExistentCounter(t *testing.T) {
 	config.CounterConfig[0].MeasurementLabel = "processor.time.total.pct"
 	config.CounterConfig[0].Query = "\\Processor Information(_Total)\\not existing counter"
 	config.CounterConfig[0].Format = "float"
-	handle, err := NewPerfmonReader(config)
+	handle, err := NewReader(config)
 
 	values, err := handle.Read()
 
@@ -212,7 +213,7 @@ func TestNonExistingObject(t *testing.T) {
 	config.CounterConfig[0].MeasurementLabel = "processor.time.total.pct"
 	config.CounterConfig[0].Query = "\\non existing object\\% Processor Performance"
 	config.CounterConfig[0].Format = "float"
-	handle, err := NewPerfmonReader(config)
+	handle, err := NewReader(config)
 	if assert.Error(t, err) {
 		assert.EqualValues(t, PDH_CSTATUS_NO_OBJECT, errors.Cause(err))
 	}
@@ -224,64 +225,66 @@ func TestNonExistingObject(t *testing.T) {
 }
 
 func TestLongOutputFormat(t *testing.T) {
-	query, err := NewQuery("")
+	var query Query
+	err := query.Open()
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer query.Close()
-
-	err = query.AddCounter(processorTimeCounter, LongFormat, "")
+	counter := CounterConfig{Format: "long"}
+	err = query.AddCounter(processorTimeCounter, counter, false)
 	if err != nil && err != PDH_NO_MORE_DATA {
 		t.Fatal(err)
 	}
 
-	err = query.Execute()
+	err = query.CollectData()
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	time.Sleep(time.Millisecond * 1000)
 
-	err = query.Execute()
+	err = query.CollectData()
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	values, err := query.Values()
+	values, err := query.GetFormattedCounterValues()
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	_, okLong := values[processorTimeCounter][0].Measurement.(int64)
+	_, okLong := values[processorTimeCounter][0].Measurement.(int32)
 
 	assert.True(t, okLong)
 }
 
 func TestFloatOutputFormat(t *testing.T) {
-	query, err := NewQuery("")
+	var query Query
+	err := query.Open()
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer query.Close()
-
-	err = query.AddCounter(processorTimeCounter, FloatFormat, "")
+	counter := CounterConfig{Format: "float"}
+	err = query.AddCounter(processorTimeCounter, counter, false)
 	if err != nil && err != PDH_NO_MORE_DATA {
 		t.Fatal(err)
 	}
 
-	err = query.Execute()
+	err = query.CollectData()
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	time.Sleep(time.Millisecond * 1000)
 
-	err = query.Execute()
+	err = query.CollectData()
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	values, err := query.Values()
+	values, err := query.GetFormattedCounterValues()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -291,61 +294,16 @@ func TestFloatOutputFormat(t *testing.T) {
 	assert.True(t, okFloat)
 }
 
-func TestRawValues(t *testing.T) {
-	query, err := NewQuery("")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer query.Close()
-
-	err = query.AddCounter(processorTimeCounter, FloatFormat, "")
-	if err != nil && err != PDH_NO_MORE_DATA {
-		t.Fatal(err)
-	}
-
-	var values []float64
-
-	for i := 0; i < 2; i++ {
-		if err = query.Execute(); err != nil {
-			t.Fatal(err)
-		}
-
-		_, rawvalue1, err := PdhGetRawCounterValue(query.counters[processorTimeCounter].handle)
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		time.Sleep(time.Millisecond * 1000)
-
-		if err = query.Execute(); err != nil {
-			t.Fatal(err)
-		}
-
-		_, rawvalue2, err := PdhGetRawCounterValue(query.counters[processorTimeCounter].handle)
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		value, err := PdhCalculateCounterFromRawValue(query.counters[processorTimeCounter].handle, PdhFmtDouble|PdhFmtNoCap100, rawvalue2, rawvalue1)
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		values = append(values, *(*float64)(unsafe.Pointer(&value.LongValue)))
-	}
-
-	t.Log(values)
-}
-
 func TestWildcardQuery(t *testing.T) {
 	config := Config{
 		CounterConfig: make([]CounterConfig, 1),
 	}
 	config.CounterConfig[0].InstanceLabel = "processor.name"
+	config.CounterConfig[0].InstanceName = "TestInstanceName"
 	config.CounterConfig[0].MeasurementLabel = "processor.time.pct"
 	config.CounterConfig[0].Query = `\Processor Information(*)\% Processor Time`
 	config.CounterConfig[0].Format = "float"
-	handle, err := NewPerfmonReader(config)
+	handle, err := NewReader(config)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -366,6 +324,12 @@ func TestWildcardQuery(t *testing.T) {
 	}
 	assert.True(t, pctKey)
 
+	pct, err := values[0].MetricSetFields.GetValue("processor.name")
+	if err != nil {
+		t.Fatal(err)
+	}
+	assert.NotEqual(t, "TestInstanceName", pct)
+
 	t.Log(values)
 }
 
@@ -389,7 +353,7 @@ func TestGroupByInstance(t *testing.T) {
 	config.CounterConfig[2].Query = `\Processor Information(_Total)\Average Idle Time`
 	config.CounterConfig[2].Format = "float"
 
-	handle, err := NewPerfmonReader(config)
+	handle, err := NewReader(config)
 	if err != nil {
 		t.Fatal(err)
 	}