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

Win_perf_counters plugin regex doesn't work for instances that have back slashes #4499

Closed
jedthe3rd opened this issue Jul 31, 2018 · 4 comments · Fixed by #4572
Closed

Win_perf_counters plugin regex doesn't work for instances that have back slashes #4499

jedthe3rd opened this issue Jul 31, 2018 · 4 comments · Fixed by #4572
Labels
area/windows Related to windows plugins (win_eventlog, win_perf_counters, win_services) bug unexpected problem or unintended behavior
Milestone

Comments

@jedthe3rd
Copy link

jedthe3rd commented Jul 31, 2018

Relevant telegraf.conf:

[[inputs.win_perf_counters]]
  UseWildcardsExpansion=true 
 [[inputs.win_perf_counters.object]]
    ObjectName = "LogicalDisk"
    Instances = ["*"]
    Counters = [
      "% Disk Read Time",
      "% Disk Write Time",
      "Disk Read Bytes/sec",
      "Disk Write Bytes/sec",
      "Disk Reads/sec",
      "Disk Writes/sec",
      "Current Disk Queue Length",
      "Avg. Disk sec/Read",
      "Avg. Disk sec/Write"
    ]
    Measurement = "win_disk"
    IncludeTotal=false
    WarnOnMissing=true

System info:

Telegraf v1.7 - with changes (not affecting this problem)
Windows Server 2012 R2

Steps to reproduce:

  1. Get a system that has logicaldisk instances that are path names.
  2. Run telegraf with the above config input.
  3. Check output and you will see the instance and object for the path name instances are wrong.

Expected behavior:

\BOWSQLTST23A\LogicalDisk(E:\Data\Disk1)\Disk Reads/sec -> "instance":"E:\\Data\\Disk1" "objectname":"LogicalDisk"

Actual behavior:

\BOWSQLTST23A\LogicalDisk(E:\Data\Disk1)\Disk Reads/sec -> "objectname":"Disk1)"

Additional info:

Here is the fix for this from my testing: https://gist.github.com/jedthe3rd/4a361cc19ac4cfd4ccf7bf7547107ead

Relevant regex tests:
https://regex101.com/r/GAnd0s/2
https://regex101.com/r/GAnd0s/3

@jedthe3rd jedthe3rd changed the title win_perf_counters regex doesn't work for Instances that have back slashes Win_perf_counters plugin regex doesn't work for Instances that have back slashes Jul 31, 2018
@jedthe3rd jedthe3rd changed the title Win_perf_counters plugin regex doesn't work for Instances that have back slashes Win_perf_counters plugin regex doesn't work for instances that have back slashes Jul 31, 2018
@danielnelson
Copy link
Contributor

cc @vlastahajek

@danielnelson danielnelson added bug unexpected problem or unintended behavior area/windows Related to windows plugins (win_eventlog, win_perf_counters, win_services) labels Jul 31, 2018
@jedthe3rd
Copy link
Author

jedthe3rd commented Jul 31, 2018

You could technically use 1 regex \\(.*)\\([^()]*)\((.*)\)\\(.*) and condense the code more.
https://regex101.com/r/GAnd0s/5

@jedthe3rd
Copy link
Author

jedthe3rd commented Jul 31, 2018

Found another edge case introduced by my new regex.
Broken:
\BOWSQLTST23A\MSSQL$BOMSSTEST19A:Deprecated Features(sp_configure 'ft crawl bandwidth (max))\usage -> "instance" = max) "object" = MSSQL$BOMSSTEST19A:Deprecated Features(sp_configure 'ft crawl bandwidth

Fixed:
\BOWSQLTST23A\MSSQL$BOMSSTEST19A:Deprecated Features(sp_configure 'ft crawl bandwidth (max))\usage -> "instance" = sp_configure 'ft crawl bandwidth(max) "object" = MSSQL$BOMSSTEST19A:Deprecated Features

\\(.*)\\([^()]*)\((.*)\) is the fixed regex
https://regex101.com/r/GAnd0s/4

Updated gist to include it.

vlastahajek added a commit to bonitoo-io/telegraf that referenced this issue Aug 20, 2018
@vlastahajek
Copy link
Contributor

Thanks for discovering this bug. It is always surpsing what kind of text instance name could be :-).
However, regexp matching is a bit complicated, because the instance part is optional.

Even though playing with regexp if fun, regexp matching became too complex and slow, so I replaced it with a simple substring searching. Benchmark showed it is 40x faster than regexp.

@danielnelson danielnelson added this to the 1.8.0 milestone 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) bug unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants