Win perf counters expand wildcards #3018

vtsingaras · 2017-07-14T10:08:12Z

Required for all PRs:

Signed CLA.
Associated README.md updated.
Has appropriate unit tests.

Resolves conflicts against current master from: #2336

…tion expands wildcards in ObjectName, Instances and Counter.

…CounterPath.

bullshit · 2017-07-20T10:05:16Z

plugins/inputs/win_perf_counters/win_perf_counters.go

+	return "\\" + objectname + "(" + instance + ")\\" + counter
+}
+
+func expandQuery(query string, expand bool) ([]string, error) {


Hi it looks like this could be the solution for #2879 the only thing would be to check if the instance name equals "*" and than expand the query. The expandQuery and formatCounterQuery Method could handle this together.

I will be supplying a new patch today with both the changes you proposed and my comment at #2336

On second thought, we shouldn't only check for equality with '' as the ExpandWildcardPath function expands both counter and instance names and also partial wildcards, such as "chrome".

I will be supplying the first patch shortly, if you could review it and make comments, it would be really helpful.

Gather() now uses PdhGetFormattedCounterValue. Move character sanitization out of the Gather() function to the ParseConfig stage.

bullshit · 2017-07-24T10:02:17Z

Hi @vtsingaras thank you for your work. I created a fork and tried to simplify your logic a little bit. I think that this is all you need or do i miss something on the expand feature?!

bullshit · 2017-07-24T10:05:29Z

plugins/inputs/win_perf_counters/win_perf_counters.go

-				// return PDH_MORE_DATA and will not set the bufSize.
-				bufCount = 0
-				bufSize = 0
+			ret = PdhGetFormattedCounterValueDouble(metric.counterHandle, &emptyBuf[0], &counterValue)


I'm curious, can you explain why PdhGetFormattedCounterValueDouble don't need the removed code and why you are not sanitze the counter and measurment names?

Sure, the previous code used PdhGetFormattedCounterArrayDouble, my version expands the counter queries to a list of individual counter queries and handle all the removed logic in different parts now. I also sanitize the names in AddItem().

sanitized_measurement := sanitizedChars.Replace(measurement) if sanitized_measurement == "" { sanitized_measurement = "win_perf_counters" } sanitized_counter := sanitizedChars.Replace(counter)

Sorry my mistake

bullshit · 2017-07-24T13:08:04Z

plugins/inputs/win_perf_counters/win_perf_counters.go


 	"github.com/influxdata/telegraf"
 	"github.com/influxdata/telegraf/plugins/inputs"
+	"github.com/influxdata/telegraf/metric"


btw do you really need telegraf/metric? I'm getting an unused import error

Nope, it seems this import was inserted by my IDE prior to committing.

vtsingaras · 2017-07-24T18:34:54Z

@bullshit I'm currently on vacation with no access to a PC. Would it be possible for me to answer in detail on saturday/sunday? Thanks for the quick turnaround.

bullshit · 2017-07-25T12:24:23Z

@vtsingaras sure no problem, have fun

vtsingaras · 2017-07-31T07:12:32Z

@bullshit It seems your fork is missing my latest commit (f96e524)

bullshit · 2017-08-08T13:29:12Z

@vtsingaras i know i wanted to start from scratch and do minimum changes to the existing code.
I'm using your code for the last 2 weeks in a test enviroment and it looks good to me.

There is only one thing. For example:

[[inputs.win_perf_counters.object]]
    ObjectName = "Process"
    Counters = ["% Processor Time"]
    Instances = ["*"] 
    Measurement = "testmeasurement"

telegraf is running, everything is ok. When you start chrome after telegraf you will never get a query to this instance. So if you use wildcards in your telegraf configuration we should periodically refresh the handles. I think somebody tried to fix this a while ago #2345 but it was never merged

aventrax · 2017-09-19T11:45:30Z

Hello @vtsingaras @bullshit , how can I build telefraf for windows with your latest commits? I'm on version 1.4.0 but the issue persists so I would try if possible...
Thanks

vtsingaras · 2017-10-10T06:55:18Z

@bullshit Are you already working on it? I can make it reload the counters, add a ClearItems() func and calling ParseConfig again with locking after a configurable interval would be the easiest I think.

@aventrax You can do a go get for the original repo, cd to ..../influxdata/telegraf, change the remote to point to interworkslcoud/telegraf, do a git fetch and checkout this branch and finally do a go build.

Alternatively I can provide you with the build we use here, however take extra care as I don't like distributing binary builds and can guarantee neither safety and/or quality.

aventrax · 2017-10-20T14:56:51Z

@vtsingaras many thanks, anyway I do not understand where is interworkscloud:win_perf_counters-expand-wildcards. Is not on github... 😕

aventrax · 2017-11-07T16:14:29Z

@vtsingaras Finally I have been able to checkout that fork and compile it on windows. A little nightmare.
What can I say, it's working. Anyway, now there is another issue. It's not refreshing. Opening one more notepad.exe is not shown on my graphs until I restart Telegraf. Is there any patch for this? I saw #2345 but it seems a little bit old and never merged.....

My config is the following:

    # Process metrics
    ObjectName = "Process"
    Counters = ["% Processor Time","Working Set - Private"]
    Instances = ["notepad*"]
    Measurement = "notepad_processes"```

aventrax · 2017-11-17T12:38:07Z

I'm doing other tests.

With this branch the asterisk is working but I noticed that Telegraf does not start if there isn't at least one process active before starting.

# Process metrics
ObjectName = "Process"
Counters = ["% Processor Time","Working Set - Private"]
Instances = ["notepad*"]
Measurement = "notepad_processes"

Here, I must open at least one notepad.exe before Telegraf, otherwise it throws an error and it crashes.

@bullshit Any news regarding the periodic handles refresh?

bullshit · 2017-11-23T19:51:48Z

hi,
did you try the option "WarnOnMissing=true" ?
Sorry i don't have any news

danielnelson · 2018-05-25T02:24:38Z

This PR has been superseded by #4189

Oleg Grytsynevych and others added 5 commits January 29, 2017 16:44

Adds a new option "Expand" to a difinition of Windows counter. The op…

0edd85a

…tion expands wildcards in ObjectName, Instances and Counter.

Documentation is updated for the "Expand" option.

c797cb1

Use PdhExpandWildCardPath for wildcard expansion instead of PdhExpand…

2e72c15

…CounterPath.

Resolve conflicts from o7g8:win_perf_counters-expand-wildcards

0e5643a

Remove leftover debug statement change to log expanded queries.

4d6b4a7

bullshit reviewed Jul 20, 2017

View reviewed changes

Change the wildcard expansion logic.

f96e524

Gather() now uses PdhGetFormattedCounterValue. Move character sanitization out of the Gather() function to the ParseConfig stage.

bullshit added a commit to bullshit/telegraf that referenced this pull request Jul 24, 2017

alternative expand logic for influxdata#3018 and fix influxdata#2879

241de60

bullshit reviewed Jul 24, 2017

View reviewed changes

danielnelson added the area/windows Related to windows plugins (win_eventlog, win_perf_counters, win_services) label Aug 23, 2017

ivankudibal mentioned this pull request May 10, 2018

Cannot use instance wildcard in win_perf_counters #2879

Closed

danielnelson closed this May 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Win perf counters expand wildcards #3018

Win perf counters expand wildcards #3018

vtsingaras commented Jul 14, 2017 •

edited

Loading

bullshit Jul 20, 2017

vtsingaras Jul 20, 2017

vtsingaras Jul 20, 2017

bullshit commented Jul 24, 2017 •

edited

Loading

bullshit Jul 24, 2017

vtsingaras Jul 31, 2017

bullshit Aug 8, 2017

bullshit Jul 24, 2017

vtsingaras Jul 31, 2017

vtsingaras commented Jul 24, 2017

bullshit commented Jul 25, 2017 •

edited

Loading

vtsingaras commented Jul 31, 2017 •

edited

Loading

bullshit commented Aug 8, 2017

aventrax commented Sep 19, 2017

vtsingaras commented Oct 10, 2017 •

edited

Loading

aventrax commented Oct 20, 2017

aventrax commented Nov 7, 2017 •

edited

Loading

aventrax commented Nov 17, 2017

bullshit commented Nov 23, 2017

danielnelson commented May 25, 2018

Win perf counters expand wildcards #3018

Win perf counters expand wildcards #3018

Conversation

vtsingaras commented Jul 14, 2017 • edited Loading

Required for all PRs:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bullshit commented Jul 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vtsingaras commented Jul 24, 2017

bullshit commented Jul 25, 2017 • edited Loading

vtsingaras commented Jul 31, 2017 • edited Loading

bullshit commented Aug 8, 2017

aventrax commented Sep 19, 2017

vtsingaras commented Oct 10, 2017 • edited Loading

aventrax commented Oct 20, 2017

aventrax commented Nov 7, 2017 • edited Loading

aventrax commented Nov 17, 2017

bullshit commented Nov 23, 2017

danielnelson commented May 25, 2018

vtsingaras commented Jul 14, 2017 •

edited

Loading

bullshit commented Jul 24, 2017 •

edited

Loading

bullshit commented Jul 25, 2017 •

edited

Loading

vtsingaras commented Jul 31, 2017 •

edited

Loading

vtsingaras commented Oct 10, 2017 •

edited

Loading

aventrax commented Nov 7, 2017 •

edited

Loading