Skip to content

Commit d986030

Browse files
alxblpjanotti
andauthored
[receiver/windowsperfcounters] fix: include instance index for multiple matches. (#32321)
**Description:** This PR adds the counter's instance index for counters that include multiple instances with the same name. A simple example of this case is for monitoring process ID when there are multiple instances of the same process running, but this can technically happen for any counter that uses instance names. This can potentially increase cardinality so I am open to adding a configuration to the receiver to toggle this behavior on and leave it disabled as a default. **Link to tracking Issue:** #32319 **Testing:** - See issue for details. Short version: two notepad processes running, scraping `Process\ID Process` to `debug` and `prometheusremotewrite` to validate that `instance` label now matches what is seen in `perfmon.exe` - Added unit test to watcher.go to validate instance name indexing **Documentation:** Added enhancement CHANGELOG Fixes #32319 --------- Co-authored-by: Paulo Janotti <[email protected]>
1 parent 501ef8b commit d986030

File tree

3 files changed

+199
-11
lines changed

3 files changed

+199
-11
lines changed
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: windowsperfcountersreceiver
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: "`windowsperfcountersreceiver` now appends an index number to additional instance names that share a name. An example of this is when scraping `\Process(*)` counters with multiple running instances of the same executable."
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [32319]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: |
19+
**NOTES**
20+
- This change can expose cardinality issues where the counters were previously collapsed under the non-indexed instance name.
21+
- The change mimics Windows Performance Monitor behavior: The first instance name remains unchanged, additional instances are suffixed with `#<N>` where `N=1` and is increased for each duplicate.
22+
- e.g. Given 3 powershell instances, this will return `powershell`, `powershell#1` and `powershell#2`.
23+
24+
# If your change doesn't affect end users or the exported elements of any package,
25+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
26+
# Optional: The change log or logs in which this entry should be included.
27+
# e.g. '[user]' or '[user, api]'
28+
# Include 'user' if the change is relevant to end users.
29+
# Include 'api' if there is a change to a library API.
30+
# Default: '[user]'
31+
change_logs: [user]

pkg/winperfcounters/watcher.go

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func (pc *perfCounter) ScrapeData() ([]CounterValue, error) {
142142
return nil, fmt.Errorf("failed to format data for performance counter '%s': %w", pc.path, err)
143143
}
144144

145-
vals = removeTotalIfMultipleValues(vals)
145+
vals = cleanupScrapedValues(vals)
146146
return vals, nil
147147
}
148148

@@ -151,24 +151,42 @@ func ExpandWildCardPath(counterPath string) ([]string, error) {
151151
return win_perf_counters.ExpandWildCardPath(counterPath)
152152
}
153153

154-
func removeTotalIfMultipleValues(vals []CounterValue) []CounterValue {
154+
// cleanupScrapedValues handles instance name collisions and standardizes names.
155+
// It cleans up the list in-place to avoid unnecessary copies.
156+
func cleanupScrapedValues(vals []CounterValue) []CounterValue {
155157
if len(vals) == 0 {
156158
return vals
157159
}
158160

159-
if len(vals) == 1 {
160-
// if there is only one item & the instance name is "_Total", clear the instance name
161-
if vals[0].InstanceName == totalInstanceName {
162-
vals[0].InstanceName = ""
163-
}
161+
// If there is only one "_Total" instance, clear the instance name.
162+
if len(vals) == 1 && vals[0].InstanceName == totalInstanceName {
163+
vals[0].InstanceName = ""
164164
return vals
165165
}
166166

167-
// if there is more than one item, remove an item that has the instance name "_Total"
168-
for i, val := range vals {
169-
if val.InstanceName == totalInstanceName {
170-
return removeItemAt(vals, i)
167+
occurrences := map[string]int{}
168+
totalIndex := -1
169+
170+
for i := range vals {
171+
instanceName := vals[i].InstanceName
172+
173+
if instanceName == totalInstanceName {
174+
// Remember if a "_Total" instance was present.
175+
totalIndex = i
171176
}
177+
178+
if n, ok := occurrences[instanceName]; ok {
179+
// Append indices to duplicate instance names.
180+
occurrences[instanceName]++
181+
vals[i].InstanceName = fmt.Sprintf("%s#%d", instanceName, n)
182+
} else {
183+
occurrences[instanceName] = 1
184+
}
185+
}
186+
187+
// Remove the "_Total" instance, as it can be computed with a sum aggregation.
188+
if totalIndex >= 0 {
189+
return removeItemAt(vals, totalIndex)
172190
}
173191

174192
return vals

pkg/winperfcounters/watcher_test.go

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,3 +176,142 @@ func TestPerfCounter_ScrapeData(t *testing.T) {
176176
})
177177
}
178178
}
179+
180+
func Test_InstanceNameIndexing(t *testing.T) {
181+
type testCase struct {
182+
name string
183+
vals []CounterValue
184+
expected []CounterValue
185+
}
186+
187+
testCases := []testCase{
188+
{
189+
name: "Multiple distinct instances",
190+
vals: []CounterValue{
191+
{
192+
InstanceName: "A",
193+
Value: 1.0,
194+
},
195+
{
196+
InstanceName: "B",
197+
Value: 1.0,
198+
},
199+
{
200+
InstanceName: "C",
201+
Value: 1.0,
202+
},
203+
},
204+
expected: []CounterValue{
205+
{
206+
InstanceName: "A",
207+
Value: 1.0,
208+
},
209+
{
210+
InstanceName: "B",
211+
Value: 1.0,
212+
},
213+
{
214+
InstanceName: "C",
215+
Value: 1.0,
216+
},
217+
},
218+
},
219+
{
220+
name: "Single repeated instance name",
221+
vals: []CounterValue{
222+
{
223+
InstanceName: "A",
224+
Value: 1.0,
225+
},
226+
{
227+
InstanceName: "A",
228+
Value: 1.0,
229+
},
230+
{
231+
InstanceName: "A",
232+
Value: 1.0,
233+
},
234+
},
235+
expected: []CounterValue{
236+
{
237+
InstanceName: "A",
238+
Value: 1.0,
239+
},
240+
{
241+
InstanceName: "A#1",
242+
Value: 1.0,
243+
},
244+
{
245+
InstanceName: "A#2",
246+
Value: 1.0,
247+
},
248+
},
249+
},
250+
{
251+
name: "Multiple repeated instance name",
252+
vals: []CounterValue{
253+
{
254+
InstanceName: "A",
255+
Value: 1.0,
256+
},
257+
{
258+
InstanceName: "B",
259+
Value: 1.0,
260+
},
261+
{
262+
InstanceName: "A",
263+
Value: 1.0,
264+
},
265+
{
266+
InstanceName: "B",
267+
Value: 1.0,
268+
},
269+
{
270+
InstanceName: "B",
271+
Value: 1.0,
272+
},
273+
{
274+
InstanceName: "C",
275+
Value: 1.0,
276+
},
277+
},
278+
expected: []CounterValue{
279+
{
280+
InstanceName: "A",
281+
Value: 1.0,
282+
},
283+
{
284+
InstanceName: "B",
285+
Value: 1.0,
286+
},
287+
{
288+
InstanceName: "A#1",
289+
Value: 1.0,
290+
},
291+
{
292+
InstanceName: "B#1",
293+
Value: 1.0,
294+
},
295+
{
296+
InstanceName: "B#2",
297+
Value: 1.0,
298+
},
299+
{
300+
InstanceName: "C",
301+
Value: 1.0,
302+
},
303+
},
304+
},
305+
}
306+
307+
for _, test := range testCases {
308+
actual := cleanupScrapedValues(test.vals)
309+
t.Run(test.name, func(t *testing.T) {
310+
compareCounterValues(t, test.expected, actual)
311+
})
312+
}
313+
}
314+
315+
func compareCounterValues(t *testing.T, expected []CounterValue, actual []CounterValue) {
316+
assert.EqualValues(t, expected, actual)
317+
}

0 commit comments

Comments
 (0)