adds whitelist for exchange collectors #642
Conversation
Signed-off-by: Björn Fischer <[email protected]>
fischerman
force-pushed
the
exchange_whitelist
branch
from
ce2044a
to
faea886
Compare
docs/collector.exchange.md
Outdated
| @@ -15,6 +15,9 @@ Enabled by default? | No | |||
| ### `--collectors.exchange.list` | |||
| Lists the Perflib Objects that are queried for data along with the perlfib object id | |||
|
|
|||
| ### `--collectors.exchange.whitelist` | |||
| Comma-separated list of collectors to use. Depending on the exchange installation not all performance counters are available. Use `--collectors.exchange.list` to obtain a list of available collectors. | |||
There was a problem hiding this comment.
An example of the flag would be good. E.G. --collectors.exchange.list=AvailabilityService,OutlookWebAccess.
Mentioning case-sensitivity of collectors would be good too.
collector/exchange.go
Outdated
| func find(slice []string, val string) (int, bool) { | ||
| for i, item := range slice { | ||
| if item == val { | ||
| return i, true | ||
| } | ||
| } | ||
| return -1, false | ||
| } |
There was a problem hiding this comment.
This function could be changed to return a bool only, as the slice index is never used.
Signed-off-by: Björn Fischer <[email protected]>
Signed-off-by: Björn Fischer <[email protected]>
|
@breed808 I've implemented your feedback. |
collector/exchange.go
Outdated
| func find(slice []string, val string) bool { | ||
| for _, item := range slice { | ||
| if item == val { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
Bit of a nit, but we've tried to keep generic utility functions in collector.go, since they are shared across the entire package.
collector/exchange.go
Outdated
| @@ -82,10 +82,17 @@ var ( | |||
| "RpcClientAccess", | |||
| } | |||
|
|
|||
| collectorWhitelist = make([]string, 0, len(exchangeAllCollectorNames)) | |||
There was a problem hiding this comment.
Could we make this a field on the exchangeCollector type? If not, it at least needs a more exchange-specific name, since it is a package level var.
Signed-off-by: Björn Fischer <[email protected]>
|
@carlpett Done! I've moved the whitelist in the struct. I don't have permissions to merge. |
|
Great! One minor thing that struck me now that I took a final pass was if whitelist is the right term here, since there is no corresponding blacklist? I think we usually just have |
Signed-off-by: Björn Fischer <[email protected]>
|
Agreed! I've renamed it. Ready to merge from my side. Looking forward to v0.15 :) |
|
Wow, this is great. Thanks for the work, @fischerman. We have not had the time to dig into this yet. Another benefit of providing upstream commits 👍 |
adds enable flag for exchange collectors Signed-off-by: Björn Fischer <[email protected]>
related to #614 and #569
The flag enables the windows_exporter to work with different exchange installations where not all performance counters are available. Detecting the installed roles would be even nicer, but is much more work. Either way this is a nice way of customizing the exporter.
As a side effect of this PR, the order in which the collectors are called is now fixed. As a result, if one of the sub collectors fails the returned metrics are now consistent.