Add the option to ignore starting some of the timers #28
Conversation
In case only some of the interval timers are desired, add an option to conditionally ignore starting some of them.
|
@csmarchbanks are you still maintaining this? |
|
Hi @rbarry82, I apologize for the slow response. I have had very little time to maintain this repo anymore, and there is a discussion going on to transfer it to prometheus-community: prometheus-community/community#39, depending on how that goes I will try to find some time to review this soon. |
| func IgnoredIntervals(s kingpin.Settings) (target map[string]bool) { | ||
| target = make(map[string]bool) | ||
| s.SetValue((ignoredIntervals)(target)) | ||
| return |
There was a problem hiding this comment.
the function signature shows a return type of map[string]bool but it does not seem like we are returning anything?
| @@ -70,8 +70,18 @@ func cycleValues(labelKeys []string, labelValues []string, seriesCount int, seri | |||
| } | |||
| } | |||
|
|
|||
| func maybeStartTicker(interval int, dontStart bool) (*time.Ticker, error) { | |||
There was a problem hiding this comment.
in both cases if dontStart is true or false the return value for error is nil. Why even return an error if it can only ever be nil?
| return nil | ||
| } | ||
|
|
||
| func (i ignoredIntervals) String() string { |
There was a problem hiding this comment.
this unused function is just to allow ignoreIntervals to be used as an input to kingpin.Settings.SetValue?
| metricTick := time.NewTicker(time.Duration(metricInterval) * time.Second) | ||
|
|
||
| valueTick, err := maybeStartTicker(valueInterval, ignoredIntervals["value"]) | ||
| if err != nil { |
There was a problem hiding this comment.
these nil checks become unnecessary if maybeStartTicker doesn't return an error
| @@ -23,6 +44,7 @@ var ( | |||
| valueInterval = kingpin.Flag("value-interval", "Change series values every {interval} seconds.").Default("30").Int() | |||
| labelInterval = kingpin.Flag("series-interval", "Change series_id label values every {interval} seconds.").Default("60").Int() | |||
| metricInterval = kingpin.Flag("metric-interval", "Change __name__ label values every {interval} seconds.").Default("120").Int() | |||
| ignoredTimers = IgnoredIntervals(kingpin.Flag("ignored-intervals", "Comma-separated list of intervals which should not be run")) | |||
There was a problem hiding this comment.
The flexibility of toggling specific intervals on/off is appreciated, but seems a little confusing how this should be used based on the help message. Thoughts on using a single bool flag to toggle both label and metric changing off? That seems more what the initial request in #27 is.
In case only some of the interval timers are desired, add an option to conditionally ignore starting some of them.
Closes #27