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

[receiver/windowsperfcounters] fix: Drop metrics with empty datapoints. #32384

Merged
Merged
Show file tree
Hide file tree
Changes from 13 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
6 changes: 6 additions & 0 deletions .chloggen/windowsperfcounters-empty-datapoints.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
change_type: bug_fix
component: windowsperfcountersreceiver
note: Metric definitions with no matching performance counter are no longer included as metrics with zero datapoints in the scrape output.
issues: [4972]
subtext:
change_logs: [user]
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
resourceMetrics:
- resource: {}
scopeMetrics:
- metrics:
- description: percentage of time CPU is idle.
gauge:
dataPoints:
- asDouble: 0
timeUnixNano: "1646857199239674900"
name: cpu.idle
unit: '%'
# Should not be present in the scrape output.
# - name: no.counter
scope: {}
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ resourceMetrics:
- asInt: "25089622016"
timeUnixNano: "1647459021285009300"
name: \Memory\Committed Bytes
unit: '1'
scope: {}
Original file line number Diff line number Diff line change
Expand Up @@ -11,48 +11,6 @@ resourceMetrics:
value:
stringValue: "0"
timeUnixNano: "1646857199239674900"
- asDouble: 0
attributes:
- key: instance
value:
stringValue: "1"
timeUnixNano: "1646857199239674900"
- asDouble: 0
attributes:
- key: instance
value:
stringValue: "2"
timeUnixNano: "1646857199239674900"
- asDouble: 0
attributes:
- key: instance
value:
stringValue: "3"
timeUnixNano: "1646857199239674900"
- asDouble: 0
attributes:
- key: instance
value:
stringValue: "4"
timeUnixNano: "1646857199239674900"
- asDouble: 0
attributes:
- key: instance
value:
stringValue: "5"
timeUnixNano: "1646857199239674900"
- asDouble: 0
attributes:
- key: instance
value:
stringValue: "6"
timeUnixNano: "1646857199239674900"
- asDouble: 0
attributes:
- key: instance
value:
stringValue: "7"
timeUnixNano: "1646857199239674900"
name: cpu.idle
unit: '%'
- description: number of bytes committed to memory
Expand All @@ -71,12 +29,6 @@ resourceMetrics:
value:
stringValue: "1"
timeUnixNano: "1646857199239674900"
- asDouble: 0
attributes:
- key: instance
value:
stringValue: "2"
timeUnixNano: "1646857199239674900"
name: processor.time
unit: '%'
scope: {}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ resourceMetrics:
name: bytes.committed
sum:
aggregationTemporality: 2
isMonotonic: true
dataPoints:
- asDouble: 1.94461696e+10
timeUnixNano: "1646862225775600200"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,21 @@ func (s *scraper) scrape(context.Context) (pmetric.Metrics, error) {
initializeMetricDps(metric, now, val, watcher.MetricRep.Attributes)
}
}

// Drop metrics with no datapoints. This happens when configured counters don't exist on the host.
// This may result in a Metrics message with no metrics if all counters are missing.
metricSlice.RemoveIf(func(m pmetric.Metric) bool {
alxbl marked this conversation as resolved.
Show resolved Hide resolved
if m.Type() == pmetric.MetricTypeGauge {
return m.Gauge().DataPoints().Len() == 0
} else {
return m.Sum().DataPoints().Len() == 0
}
alxbl marked this conversation as resolved.
Show resolved Hide resolved
})

if scrapeFailures != 0 && scrapeFailures != len(s.watchers) {
errs = scrapererror.NewPartialScrapeError(errs, scrapeFailures)
}

return md, errs
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func Test_WindowsPerfCounterScraper(t *testing.T) {
"bytes.committed": {
Description: "number of bytes committed to memory",
Unit: "By",
Sum: SumMetric{},
Sum: SumMetric{Aggregation: "cumulative", Monotonic: true},
},
},
PerfCounters: []ObjectConfig{
Expand Down Expand Up @@ -156,6 +156,28 @@ func Test_WindowsPerfCounterScraper(t *testing.T) {
startMessage: "some performance counters could not be initialized",
startErr: "failed to create perf counter with path \\Invalid Object\\Invalid Counter: The specified object was not found on the computer.\r\n",
},
{
name: "MetricDefinedButNoScrapedValue",
alxbl marked this conversation as resolved.
Show resolved Hide resolved
cfg: &Config{
MetricMetaData: map[string]MetricConfig{
"cpu.idle": {
Description: "percentage of time CPU is idle.",
Unit: "%",
Gauge: GaugeMetric{},
},
"no.counter": {
Description: "there is no counter or data for this metric",
Unit: "By",
Gauge: GaugeMetric{},
},
},
PerfCounters: []ObjectConfig{
{Object: "Processor", Instances: []string{"_Total"}, Counters: []CounterConfig{{Name: "% Idle Time", MetricRep: MetricRep{Name: "cpu.idle"}}}},
},
ControllerConfig: scraperhelper.ControllerConfig{CollectionInterval: time.Minute, InitialDelay: time.Second},
},
expectedMetricPath: filepath.Join("testdata", "scraper", "metric_not_scraped.yaml"),
},
}

for _, test := range testCases {
Expand Down Expand Up @@ -192,8 +214,16 @@ func Test_WindowsPerfCounterScraper(t *testing.T) {
expectedMetrics, err := golden.ReadMetrics(test.expectedMetricPath)
require.NoError(t, err)

// TODO: Metrics comparison is failing, not verifying the result until that is fixed.
_ = pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, pmetrictest.IgnoreMetricValues())
require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics,
// Scraping test host means static values, timestamps and instance counts are unreliable. ScopeMetrics order is also unpredictable.
// The check only takes the first instance of multi-instance counters and assumes that the other instances would be included.
pmetrictest.IgnoreSubsequentDataPoints("cpu.idle"),
pmetrictest.IgnoreSubsequentDataPoints("processor.time"),
pmetrictest.IgnoreScopeMetricsOrder(),
pmetrictest.IgnoreResourceMetricsOrder(),
pmetrictest.IgnoreMetricValues(),
pmetrictest.IgnoreTimestamp(),
))
})
}
}
Expand Down Expand Up @@ -473,18 +503,28 @@ func TestScrape(t *testing.T) {
metrics.Sort(func(a, b pmetric.Metric) bool {
return a.Name() < b.Name()
})

assert.Equal(t, len(test.mockPerfCounters)-len(expectedErrors), metrics.Len())

curMetricsNum := 0
for _, pc := range test.cfg.PerfCounters {

for counterIdx, counterCfg := range pc.Counters {
counterValues := test.mockPerfCounters[counterIdx].counterValues
scrapeErr := test.mockPerfCounters[counterIdx].scrapeErr

if scrapeErr != nil {
require.Empty(t, counterValues, "Invalid test case. Scrape error and counter values simultaneously.")
continue // no data for this counter.
}

metric := metrics.At(curMetricsNum)
assert.Equal(t, counterCfg.MetricRep.Name, metric.Name())
metricData := test.cfg.MetricMetaData[counterCfg.MetricRep.Name]
assert.Equal(t, metricData.Description, metric.Description())
assert.Equal(t, metricData.Unit, metric.Unit())
dps := metric.Gauge().DataPoints()

counterValues := test.mockPerfCounters[counterIdx].counterValues
assert.Equal(t, len(counterValues), dps.Len())
for dpIdx, val := range counterValues {
assert.Equal(t, val.Value, dps.At(dpIdx).DoubleValue())
Expand Down
Loading