From 8eaf3954b7d9aa585ef01b2ba73d600165efa348 Mon Sep 17 00:00:00 2001 From: noerw Date: Tue, 26 Jun 2018 23:30:08 +0200 Subject: [PATCH] refactor cache handling - do not update cache when notifications could not be sent - persist cache (in separate yaml file) --- cmd/config.go | 67 +-------------------------- cmd/debug.go | 6 ++- cmd/root.go | 4 +- core/cache.go | 62 +++++++++++++++++++++++++ core/checkrunner.go | 9 ++-- core/notifiers.go | 107 +++++++++++++++++++------------------------- utils/config.go | 72 +++++++++++++++++++++++++++++ 7 files changed, 193 insertions(+), 134 deletions(-) create mode 100644 core/cache.go create mode 100644 utils/config.go diff --git a/cmd/config.go b/cmd/config.go index daa25e1..450e273 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -2,69 +2,18 @@ package cmd import ( "os" - "path" "strings" + "github.com/noerw/osem_notify/utils" log "github.com/sirupsen/logrus" "github.com/spf13/viper" ) -/** - * config file handling, as it is kinda broken in spf13/viper - * mostly copied from https://github.com/TheThingsNetwork/ttn/blob/f623a6a/ttnctl/util/config.go - */ - -// GetConfigFile returns the location of the configuration file. -// It checks the following (in this order): -// the --config flag -// $XDG_CONFIG_HOME/osem_notify/config.yml (if $XDG_CONFIG_HOME is set) -// $HOME/.osem_notify.yml -func getConfigFile() string { - flag := viper.GetString("config") - - xdg := os.Getenv("XDG_CONFIG_HOME") - if xdg != "" { - xdg = path.Join(xdg, "osem_notify", "config.yml") - } - - home := os.Getenv("HOME") - homeyml := "" - homeyaml := "" - - if home != "" { - homeyml = path.Join(home, ".osem_notify.yml") - homeyaml = path.Join(home, ".osem_notify.yaml") - } - - try_files := []string{ - flag, - xdg, - homeyml, - homeyaml, - } - - // find a file that exists, and use that - for _, file := range try_files { - if file != "" { - if _, err := os.Stat(file); err == nil { - return file - } - } - } - - // no file found, set up correct fallback - if os.Getenv("XDG_CONFIG_HOME") != "" { - return xdg - } else { - return homeyml - } -} - // initConfig reads in config file and ENV variables if set. func initConfig() { theConfig := cfgFile if cfgFile == "" { - theConfig = getConfigFile() + theConfig = utils.GetConfigFile("osem_notify") } viper.SetConfigType("yaml") @@ -109,15 +58,3 @@ func validateConfig() { } } } - -func printConfig() { - log.Debug("Using config:") - printKV("config file", viper.ConfigFileUsed()) - for key, val := range viper.AllSettings() { - printKV(key, val) - } -} - -func printKV(key, val interface{}) { - log.Debugf("%20s: %v", key, val) -} diff --git a/cmd/debug.go b/cmd/debug.go index 5ef4c06..ad1232a 100644 --- a/cmd/debug.go +++ b/cmd/debug.go @@ -4,10 +4,12 @@ import ( "fmt" "os" - "github.com/noerw/osem_notify/core" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" + + "github.com/noerw/osem_notify/core" + "github.com/noerw/osem_notify/utils" ) func init() { @@ -23,7 +25,7 @@ var debugCmd = &cobra.Command{ log.SetLevel(log.DebugLevel) }, PersistentPostRun: func(cmd *cobra.Command, args []string) { - printConfig() + utils.PrintConfig() }, Run: func(cmd *cobra.Command, args []string) { cmd.Help() diff --git a/cmd/root.go b/cmd/root.go index 32638e7..5b19c9b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -17,7 +17,7 @@ var rootCmd = &cobra.Command{ log.SetOutput(os.Stdout) if viper.GetBool("debug") { log.SetLevel(log.DebugLevel) - printConfig() + utils.PrintConfig() } else { log.SetLevel(log.InfoLevel) } @@ -60,7 +60,7 @@ You might want to run 'osem_notify debug notifications' first to verify everythi func Execute() { // generate documentation - // err := doc.GenMarkdownTree(rootCmd, "./doc") + // err := doc.GenMarkdownTree(rootCmd, "./docs") // if err != nil { // log.Fatal(err) // } diff --git a/core/cache.go b/core/cache.go new file mode 100644 index 0000000..92eafc7 --- /dev/null +++ b/core/cache.go @@ -0,0 +1,62 @@ +package core + +import ( + "fmt" + "os" + + log "github.com/sirupsen/logrus" + "github.com/spf13/viper" + + "github.com/noerw/osem_notify/utils" +) + +/** + * in memory + yaml persisted cache for check results, ensuring we don't resend + * notifications on every check + + * TODO: reminder functionality: extract additional results with Status ERR + * from cache with time.Since(lastNotifyDate) > remindAfter. + * would require to serialize the full result.. + */ + +var cache = viper.New() + +func init() { + fileName := utils.GetConfigFile("osem_notify_cache") + + cache.SetConfigType("yaml") + cache.SetConfigFile(fileName) + + if _, err := os.Stat(fileName); err == nil { + err := cache.ReadInConfig() + if err != nil { + log.Error("Error when reading cache file:", err) + } + } +} + +func (results BoxCheckResults) filterChangedFromCache() BoxCheckResults { + remaining := BoxCheckResults{} + + for box, boxResults := range results { + // get results from cache. they are indexed by an event ID per boxId + // filter, so that only changed result.Status remain + remaining[box] = []CheckResult{} + for _, result := range boxResults { + cached := cache.GetStringMap(fmt.Sprintf("watchcache.%s.%s", box.Id, result.EventID())) + if result.Status != cached["laststatus"] { + remaining[box] = append(remaining[box], result) + } + } + } + + return remaining +} + +func updateCache(box *Box, results []CheckResult) error { + for _, result := range results { + key := fmt.Sprintf("watchcache.%s.%s", box.Id, result.EventID()) + cache.Set(key+".laststatus", result.Status) + } + return cache.WriteConfig() +} diff --git a/core/checkrunner.go b/core/checkrunner.go index b40cb33..ac4db93 100644 --- a/core/checkrunner.go +++ b/core/checkrunner.go @@ -10,10 +10,14 @@ import ( type BoxCheckResults map[*Box][]CheckResult -func (results BoxCheckResults) Size() int { +func (results BoxCheckResults) Size(status string) int { size := 0 for _, boxResults := range results { - size += len(boxResults) + for _, result := range boxResults { + if status == result.Status || status == "" { + size++ + } + } } return size } @@ -71,7 +75,6 @@ func CheckBoxes(boxIds []string, defaultConf *NotifyConfig) (BoxCheckResults, er } func checkBox(boxId string, defaultConf *NotifyConfig) (*Box, []CheckResult, error) { - osem := NewOsemClient(viper.GetString("api")) // get box data diff --git a/core/notifiers.go b/core/notifiers.go index 4249a65..4f157d2 100644 --- a/core/notifiers.go +++ b/core/notifiers.go @@ -6,7 +6,6 @@ import ( "time" log "github.com/sirupsen/logrus" - "github.com/spf13/viper" ) var Notifiers = map[string]AbstractNotifier{ @@ -41,21 +40,26 @@ func (box Box) GetNotifier() (AbstractNotifier, error) { } func (results BoxCheckResults) SendNotifications() error { - results = results.FilterChangedFromCache(false) - errs := []string{} + // TODO: expose flags to not use cache, and to notify for checks turned CheckOk as well - n := results.Size() - if n == 0 { + results = results.filterChangedFromCache() + + nErr := results.Size(CheckErr) + if nErr == 0 { log.Info("No notifications due.") - return nil } else { - log.Infof("Notifying for %v checks turned bad in total...", results.Size()) + log.Infof("Notifying for %v checks turned bad in total...", nErr) } + log.Debugf("%v checks turned OK!", results.Size(CheckOk)) - // FIXME: only update cache when notifications sent successfully - for box, resultsDue := range results { - if len(resultsDue) == 0 { - continue + errs := []string{} + for box, resultsBox := range results { + // only submit results which are errors + resultsDue := []CheckResult{} + for _, result := range resultsBox { + if result.Status != CheckOk { + resultsDue = append(resultsDue, result) + } } transport := box.NotifyConf.Notifications.Transport @@ -64,28 +68,40 @@ func (results BoxCheckResults) SendNotifications() error { "transport": transport, }) - notifier, err := box.GetNotifier() - if err != nil { - notifyLog.Error(err) - errs = append(errs, err.Error()) - continue + if len(resultsDue) != 0 { + notifier, err := box.GetNotifier() + if err != nil { + notifyLog.Error(err) + errs = append(errs, err.Error()) + continue + } + + notification := notifier.ComposeNotification(box, resultsDue) + + var submitErr error + submitErr = notifier.Submit(notification) + for retry := 1; submitErr != nil && retry < 3; retry++ { + time.Sleep(10 * time.Second) + notifyLog.Infof("trying to submit (retry %v)", retry) + } + if submitErr != nil { + notifyLog.Error(submitErr) + errs = append(errs, submitErr.Error()) + continue + } } - notification := notifier.ComposeNotification(box, resultsDue) - - var submitErr error - submitErr = notifier.Submit(notification) - for retry := 1; submitErr != nil && retry < 3; retry++ { - time.Sleep(10 * time.Second) - notifyLog.Debugf("trying to submit (retry %v)", retry) - } - if submitErr != nil { - notifyLog.Error(submitErr) - errs = append(errs, submitErr.Error()) - continue + // update cache (also with CheckOk results to reset status) + notifyLog.Debug("updating cache") + cacheError := updateCache(box, resultsBox) + if cacheError != nil { + notifyLog.Error("could not cache notification results: ", cacheError) + errs = append(errs, cacheError.Error()) } - notifyLog.Infof("Sent notification for %s via %s with %v new issues", box.Name, transport, len(resultsDue)) + if len(resultsDue) != 0 { + notifyLog.Infof("Sent notification for %s via %s with %v new issues", box.Name, transport, len(resultsDue)) + } } if len(errs) != 0 { @@ -93,36 +109,3 @@ func (results BoxCheckResults) SendNotifications() error { } return nil } - -func (results BoxCheckResults) FilterChangedFromCache(keepOk bool) BoxCheckResults { - remaining := BoxCheckResults{} - - for box, boxResults := range results { - // get results from cache. they are indexed by an event ID per boxId - // filter, so that only changed result.Status remain - remaining[box] = []CheckResult{} - for _, result := range boxResults { - cached := viper.GetStringMap(fmt.Sprintf("watchcache.%s.%s", box.Id, result.EventID())) - if result.Status != cached["laststatus"] { - if result.Status != CheckOk || keepOk { - remaining[box] = append(remaining[box], result) - } - } - } - - // TODO: reminder functionality: extract additional results with Status ERR - // from cache with time.Since(lastNotifyDate) > remindAfter. - // would require to serialize the full result.. - } - - // upate cache, setting lastNotifyDate to Now() - for box, boxResults := range results { - for _, result := range boxResults { - // FIXME: somehow this is not persisted? - key := fmt.Sprintf("watchcache.%s.%s", box.Id, result.EventID()) - viper.Set(key+".laststatus", result.Status) - } - } - - return remaining -} diff --git a/utils/config.go b/utils/config.go new file mode 100644 index 0000000..a0a68a5 --- /dev/null +++ b/utils/config.go @@ -0,0 +1,72 @@ +package utils + +import ( + "os" + "path" + + log "github.com/sirupsen/logrus" + "github.com/spf13/viper" +) + +/** + * config file handling, as it is kinda broken in spf13/viper + * mostly copied from https://github.com/TheThingsNetwork/ttn/blob/f623a6a/ttnctl/util/config.go + */ + +// GetConfigFile returns the location of the configuration file. +// It checks the following (in this order): +// the --config flag +// $XDG_CONFIG_HOME/osem_notify/config.yml (if $XDG_CONFIG_HOME is set) +// $HOME/.osem_notify.yml +func GetConfigFile(name string) string { + flag := viper.GetString("config") + + xdg := os.Getenv("XDG_CONFIG_HOME") + if xdg != "" { + xdg = path.Join(xdg, name, "config.yml") + } + + home := os.Getenv("HOME") + homeyml := "" + homeyaml := "" + + if home != "" { + homeyml = path.Join(home, "."+name+".yml") + homeyaml = path.Join(home, "."+name+".yaml") + } + + tryFiles := []string{ + flag, + xdg, + homeyml, + homeyaml, + } + + // find a file that exists, and use that + for _, file := range tryFiles { + if file != "" { + if _, err := os.Stat(file); err == nil { + return file + } + } + } + + // no file found, set up correct fallback + if os.Getenv("XDG_CONFIG_HOME") != "" { + return xdg + } else { + return homeyml + } +} + +func PrintConfig() { + log.Debug("Using config:") + printKV("config file", viper.ConfigFileUsed()) + for key, val := range viper.AllSettings() { + printKV(key, val) + } +} + +func printKV(key, val interface{}) { + log.Debugf("%20s: %v", key, val) +}