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

Fix instance and object name in performance counters with backslashes #4572

Merged
merged 6 commits into from
Sep 2, 2018

Conversation

vlastahajek
Copy link
Contributor

@vlastahajek vlastahajek commented Aug 20, 2018

closes #4499

Overview

replaced complex regexp marching with simple index searching
Added unit test for counterPath parsing

Required for all PRs:

  • Signed CLA.
  • Has appropriate unit tests.

@danielnelson
Copy link
Contributor

@jedthe3rd Could you test this fix with your setup? telegraf-1.8.0~9f9bd961_windows_amd64.zip

@danielnelson danielnelson added area/windows Related to windows plugins (win_eventlog, win_perf_counters, win_services) fix pr to fix corresponding bug labels Aug 20, 2018
@danielnelson danielnelson added this to the 1.7.4 milestone Aug 20, 2018
@danielnelson danielnelson changed the title Fix #4499 Fix instance and object name in performance counters with backslashes Aug 20, 2018
@vlastahajek
Copy link
Contributor Author

I realized that there can be even more complex counter paths, e.g. \\host\object(d:\f\I(a)x)\counter(d:\f(d)\i).
I haven't found any restrictions of counter name, instance name or object name. I tested that complex path on Windows performance counters API function PdhParseCounterPathW and as it was able to parse it, I find such complex path valid.
So I have created better, more bulletproof, parser. It's ~6x slower than previous, but still a lot faster than regexp.
Unit test has been a lot more extended as well.

@danielnelson
Copy link
Contributor

@jedthe3rd Here is an updated build with the latest fixes for testing, if you are available to verify we can add this for 1.7.4:

@vlastahajek
Copy link
Contributor Author

I"m sorry for discontinuous commits. The latest one is just an improved unit test.

@danielnelson danielnelson modified the milestones: 1.7.4, 1.8.0 Aug 28, 2018
@danielnelson danielnelson merged commit 90b4a1e into influxdata:master Sep 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/windows Related to windows plugins (win_eventlog, win_perf_counters, win_services) fix pr to fix corresponding bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Win_perf_counters plugin regex doesn't work for instances that have back slashes
2 participants