cmd/scollector: negative -f filters and total_time metric for httpunit collector #1630

Merged
merged 4 commits into from Mar 2, 2016

Projects

None yet

3 participants

@gbrayut
Contributor
gbrayut commented Mar 1, 2016

Updated version of #1263 rebased on current master and with more documentation

anderejd and others added some commits Aug 19, 2015
@anderejd @gbrayut anderejd Scollector features: "-" patterns, httpunit time_total + Freq setting.
Negative filter patterns ("-" prefix),
Added time_total metric to httpunit collector,
Freq setting for HTTPUnit collectors.
43cbe7c
@gbrayut gbrayut cmd/scollector: sort constants in collectors.go to try and prevent me…
…rge conflicts
39ebce9
@gbrayut gbrayut cmd/scollector: Use * in -f to include all non-excluded collectors
also add a log message when metrics are being filtered since I spent 20 minutes trying to figure out why my -p and -f tests were not working

closes #1263
0370518
@gbrayut gbrayut added the scollector label Mar 1, 2016
@captncraig captncraig was assigned by gbrayut Mar 1, 2016
@captncraig captncraig commented on an outdated diff Mar 1, 2016
cmd/scollector/collectors/httpunit.go
return nil
}
-func HTTPUnitPlans(name string, plans *httpunit.Plans) {
+func HTTPUnitPlans(name string, plans *httpunit.Plans, freq time.Duration) {
+ if freq < time.Second {
@captncraig
captncraig Mar 1, 2016 Contributor

this seems a bit odd to me. If they didn't set it, and it was zero, I would understand. But if they set 15 expecting it to be seconds or minutes or who knows what, 5 mins is an odd default. Maybe error with a helpful message?

@captncraig captncraig commented on an outdated diff Mar 1, 2016
cmd/scollector/main.go
@@ -124,11 +125,12 @@ func main() {
check(collectors.AddProcessDotNetConfig(p))
}
for _, h := range conf.HTTPUnit {
+ freq := time.Second * time.Duration(h.Freq)
@captncraig
captncraig Mar 1, 2016 Contributor

We use time.ParseDuration in some places, but apparently not others. Is a number really better than a duration string if you have to look up the docs to know it is seconds?

@captncraig
Contributor

Other than craig being confused about timings, it looks good to me.

@gbrayut
Contributor
gbrayut commented Mar 1, 2016

@captncraig I changed it to use a duration string... will be clearer in the config files

@gbrayut gbrayut merged commit 51a6929 into master Mar 2, 2016
@gbrayut gbrayut deleted the rajder branch Mar 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment