Add App Pool metrics to IIS module

[simonwillcock]

As discussed in #69 this adds metrics exposed by the Win32_PerfRawData_APPPOOLCountersProvider_APPPOOLWAS class

[carlpett]

Nice, really good work! This looks pretty good from a quick lookover. The one thing I would like to change is to map the current_application_pool_state to readable labels instead of a number. The recommended pattern is to have a gauge with a label for each state and the value 1 for the currently active state, so you'd get something like this:

current_application_pool_state{app="foo",state="uninitialized"} 0
current_application_pool_state{app="foo",state="initialized"} 0
current_application_pool_state{app="foo",state="running"} 1
...

So basically, add another label to CurrentApplicationPoolState, then do a bit of mapping before sending it down the channel.

[simonwillcock]

Ah good thinking. I'll see if I can work out how to do that.

[simonwillcock]

How does that look? I wasn't sure if the state values should be lower case and/or snake case? I've left them proper cased with spaces as they were described in the docs for now.

[carlpett]

Nice! Only one minor point, we should expose even the states which are not currently active, but with a value of 0. I think we're doing this somewhere already, let's see if I can find it.

Edit: Found it: f5365c9#diff-55a5701e10d779d0c16948d45e6c0422R86

[simonwillcock]

Ah beautiful - thanks for that. I'll make that change, and so should the value of the metric be just 1 for the active states or leave them as their current value? Having them as just 1 might make them more easier to use when querying perhaps?

[carlpett]

Exactly, set it to 1 if it is the active state, 0 otherwise. Makes for easy querying, as you say.

[simonwillcock]

I think that should do the trick. Thanks for the example too, that helped a lot 😄

[martinlindhe]

Looks fantastic! Great work @simonwillcock, I hope you found golang enjoyable

[martinlindhe]

I think this cast is unneeded, as you create a float value implicit already by isCurrentState := 0.0

[simonwillcock]

Ah, right you are. I'll fix that up

[martinlindhe]

Looks all good to me, but since @carlpett started the review, I'll let him merge it
Its just about lunch time in sweden now, hopefully he'll return soon :-)

[carlpett]

Took it for a test run on one of my machines, and I think there might be something off with the uptime-metrics:
wmi_iis_current_application_pool_uptime{app=".NET v4.5 Classic"} 1.3129207244518765e+17

In the service collector we do some normalization to get around this, but it doesn't look like that exact normalization works here (I get 17000 days of uptime). Trying to find some documentation.

[martinlindhe]

@carlpett days since Jan 1, 1970 = 17277, related?

[carlpett]

Aha, well spotted. The Timestamp_Object property contains the current timestamp, so (Timestamp_Object - TotalApplicationPoolUptime)/Frequency_Object should give the uptime in seconds, I think?

[simonwillcock]

Oh good pickup!

[brian-brazil]

If it's available, expose the time it started rather than uptime. This lets you use changes() to count restarts.

[simonwillcock]

I don't believe it is unfortunately - only uptime

[simonwillcock]

Quick question - do I need to cast the result of that equation? Or cast the value first and then do the calculation?

Something like this float64((Timestamp_Object - app.CurrentApplicationPoolUptime) / Frequency_Object), ?

[simonwillcock]

Ended up doing the same what is done in system.go and casting each property separately. Hope that is what we're going for :)

I'm not at work any more though, so I can't test these unfortunately

[martinlindhe]

@simonwillcock if the underlying type of a and b is an int, then float64(a / b) will first calc the integer result of a/b, then cast to float, so you'll loose precision. The way you solved it is correct!

[simonwillcock]

Ah rightio, thanks for explaining that for me!

[brian-brazil]

I don't believe it is unfortunately - only uptime

Are you sure CurrentApplicationPoolUptime isn't the time it started at?

That you're doing a subtraction indicates to me that that number is not uptime.

[simonwillcock]

Its possible that I'm misunderstanding.. the docs I found said this for CurrentApplicationPoolUptime: "The length of time, in seconds, that the application pool has been running since it was started" but they didn't appear to be official msdn docs though.

[carlpett]

@simonwillcock Yeah, the docs are for the processed metrics that Windows uses in ProcMon, so you'll have to take those with a pinch of salt...

[simonwillcock]

Interesting. I think it sounds like you guys know about this than me so I'll take your word for it!
Once I'm back at work after the weekend I'll check the value it's returning and can try and give a definitive answer if we don't have before then.

[simonwillcock]

It looks like @brian-brazil is on the money and CurrentApplicationPoolUptime is actually the start time! This is actually the same type of value as SystemUpTime in system.go.

I'll push an update to make it return the uptime instead so that it matches system.go for consistency.
Good pickup!

[carlpett]

@simonwillcock Great! Just one last thing (sorry): with this change, the metric should probably be renamed from _uptime to _start_time? Apart from that, I'm good to merge!

[carlpett]

@simonwillcock Thanks, and kudos on a very nice first Go PR!

[simonwillcock]

Thanks very much for guiding me through it 😊