Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(notifier): fix sending unwanted notifications on muted and deleted triggers and muted metrics #943

Merged
merged 33 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
d59ced8
fix fetchNotification function
almostinf Oct 5, 2023
a54ddee
fix notifier tests
almostinf Oct 5, 2023
ecf0a3a
returned previous settings in checker and notifier yamls
almostinf Oct 5, 2023
4d9ff22
few refactor last check in database
almostinf Oct 5, 2023
2ca9d07
fixed redis config and move check maintenance functions in datatypes
almostinf Oct 9, 2023
84fd6fc
change 5 min on 1 min delayedTime in NewTestDatabase and removed dela…
almostinf Oct 9, 2023
d1c6b12
add delayed_time in api and notifier config
almostinf Oct 9, 2023
8d71675
fix notifier config tests
almostinf Oct 9, 2023
ada472f
fix little bug in GetDelayedTimeInSeconds
almostinf Oct 10, 2023
15f1465
simple refactoring
almostinf Oct 11, 2023
116de69
fix some bugs in checker and notifier
almostinf Oct 12, 2023
24971c0
fix bug with LastSuccessfulCheckTimestamp and replaced it on Timestamp
almostinf Oct 12, 2023
31386d1
fix redis notifications tests
almostinf Oct 12, 2023
ee4c8df
make all fetch notifications operations in transactions
almostinf Oct 12, 2023
8ecf9e5
returned previous logic with same notifications, writing tests and re…
almostinf Oct 17, 2023
da5dcc6
merge with master
almostinf Oct 17, 2023
4ee20a1
fix transaction error handling
almostinf Oct 17, 2023
2152abc
add handling transaction error in getNotificationsInTxWithLimit and m…
almostinf Oct 17, 2023
1f69dd9
fix config, optimize get triggers check data, improve tests
almostinf Oct 18, 2023
a6998b3
removed unused info from configs
almostinf Oct 18, 2023
059bbbc
improved notification tests with maintenance
almostinf Oct 18, 2023
434dd93
fix tests naming
almostinf Oct 18, 2023
bd4d1bc
fix notification tests
almostinf Oct 18, 2023
5bd6f08
refactor validation function and checks, besides add notifications state
almostinf Oct 19, 2023
1be0ef1
fix error handling
almostinf Oct 23, 2023
56ba1a8
add notifier config and created at field
almostinf Oct 24, 2023
7addc49
removed unused code
almostinf Oct 24, 2023
0ae961a
fix doc
almostinf Oct 24, 2023
9c82f26
add transaction heuristic limit to notification config
almostinf Oct 24, 2023
fd3429d
add transaction heuristic limit to all files
almostinf Oct 24, 2023
e509234
remove notification config from api
almostinf Nov 3, 2023
342211a
add get trigger checks function and merge (#944)
almostinf Nov 13, 2023
887a4d7
feat(notifier, api): fix sending notifications on muted and deleted t…
almostinf Nov 13, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func main() {

databaseSettings := applicationConfig.Redis.GetSettings()
notificationHistorySettings := applicationConfig.NotificationHistory.GetSettings()
database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, redis.API)
database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, redis.NotificationConfig{}, redis.API)

// Start Index right before HTTP listener. Fail if index cannot start
searchIndex := index.NewSearchIndex(logger, database, telemetry.Metrics)
Expand Down
2 changes: 1 addition & 1 deletion cmd/checker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func main() {
logger.Info().Msg("Debug: checker started")

databaseSettings := config.Redis.GetSettings()
database := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.Checker)
database := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.NotificationConfig{}, redis.Checker)

remoteConfig := config.Remote.GetRemoteSourceSettings()
prometheusConfig := config.Prometheus.GetPrometheusSourceSettings()
Expand Down
2 changes: 1 addition & 1 deletion cmd/cli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ func initApp() (cleanupConfig, moira.Logger, moira.Database) {
}

databaseSettings := config.Redis.GetSettings()
dataBase := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.Cli)
dataBase := redis.NewDatabase(logger, databaseSettings, redis.NotificationHistoryConfig{}, redis.NotificationConfig{}, redis.Cli)
return config.Cleanup, logger, dataBase
}

Expand Down
24 changes: 24 additions & 0 deletions cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,30 @@ func (notificationHistoryConfig *NotificationHistoryConfig) GetSettings() redis.
}
}

// NotificationConfig is a config that stores the necessary configuration of the notifier
type NotificationConfig struct {
// Need to determine if notification is delayed - the difference between creation time and sending time
// is greater than DelayedTime
DelayedTime string `yaml:"delayed_time"`
// TransactionTimeout defines the timeout between fetch notifications transactions
TransactionTimeout string `yaml:"transaction_timeout"`
// TransactionMaxRetries defines the maximum number of attempts to make a transaction
TransactionMaxRetries int `yaml:"transaction_max_retries"`
// TransactionHeuristicLimit maximum allowable limit, after this limit all notifications
// without limit will be taken
TransactionHeuristicLimit int64 `yaml:"transaction_heuristic_limit"`
}

// GetSettings returns notification storage configuration
func (notificationConfig *NotificationConfig) GetSettings() redis.NotificationConfig {
return redis.NotificationConfig{
DelayedTime: to.Duration(notificationConfig.DelayedTime),
TransactionTimeout: to.Duration(notificationConfig.TransactionTimeout),
TransactionMaxRetries: notificationConfig.TransactionMaxRetries,
TransactionHeuristicLimit: notificationConfig.TransactionHeuristicLimit,
}
}

// GraphiteConfig is graphite metrics config structure that initialises at the start of moira
type GraphiteConfig struct {
// If true, graphite sender will be enabled.
Expand Down
2 changes: 1 addition & 1 deletion cmd/filter/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func main() {
}

filterMetrics := metrics.ConfigureFilterMetrics(telemetry.Metrics)
database := redis.NewDatabase(logger, config.Redis.GetSettings(), redis.NotificationHistoryConfig{}, redis.Filter)
database := redis.NewDatabase(logger, config.Redis.GetSettings(), redis.NotificationHistoryConfig{}, redis.NotificationConfig{}, redis.Filter)

retentionConfigFile, err := os.Open(config.Filter.RetentionConfig)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions cmd/notifier/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type config struct {
Prometheus cmd.PrometheusConfig `yaml:"prometheus"`
ImageStores cmd.ImageStoreConfig `yaml:"image_store"`
NotificationHistory cmd.NotificationHistoryConfig `yaml:"notification_history"`
Notification cmd.NotificationConfig `yaml:"notification"`
}

type entityLogConfig struct {
Expand Down Expand Up @@ -93,6 +94,12 @@ func getDefault() config {
NotificationHistoryTTL: "48h",
NotificationHistoryQueryLimit: int(notifier.NotificationsLimitUnlimited),
},
Notification: cmd.NotificationConfig{
DelayedTime: "1m",
TransactionTimeout: "200ms",
TransactionMaxRetries: 10,
TransactionHeuristicLimit: 10000,
},
Notifier: notifierConfig{
SenderTimeout: "10s",
ResendingTimeout: "1:00",
Expand Down
3 changes: 2 additions & 1 deletion cmd/notifier/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ func main() {

databaseSettings := config.Redis.GetSettings()
notificationHistorySettings := config.NotificationHistory.GetSettings()
database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, redis.Notifier)
notificationSettings := config.Notification.GetSettings()
database := redis.NewDatabase(logger, databaseSettings, notificationHistorySettings, notificationSettings, redis.Notifier)

remoteConfig := config.Remote.GetRemoteSourceSettings()
prometheusConfig := config.Prometheus.GetPrometheusSourceSettings()
Expand Down
14 changes: 14 additions & 0 deletions database/redis/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,17 @@ type NotificationHistoryConfig struct {
NotificationHistoryTTL time.Duration
NotificationHistoryQueryLimit int
}

// Notifier configuration in redis
type NotificationConfig struct {
// Need to determine if notification is delayed - the difference between creation time and sending time
// is greater than DelayedTime
DelayedTime time.Duration
// TransactionTimeout defines the timeout between fetch notifications transactions
TransactionTimeout time.Duration
// TransactionMaxRetries defines the maximum number of attempts to make a transaction
TransactionMaxRetries int
// TransactionHeuristicLimit maximum allowable limit, after this limit all notifications
// without limit will be taken
TransactionHeuristicLimit int64
}
1 change: 1 addition & 0 deletions database/redis/contact_notification_history_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ var inputScheduledNotification = moira.ScheduledNotification{
Throttled: false,
SendFail: 1,
Timestamp: time.Now().Unix(),
CreatedAt: time.Now().Unix(),
}

var eventsShouldBeInDb = []*moira.NotificationEventHistoryItem{
Expand Down
21 changes: 19 additions & 2 deletions database/redis/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,11 @@ type DbConnector struct {
source DBSource
clock moira.Clock
notificationHistory NotificationHistoryConfig
// Notifier configuration in redis
notification NotificationConfig
}

func NewDatabase(logger moira.Logger, config DatabaseConfig, nh NotificationHistoryConfig, source DBSource) *DbConnector {
func NewDatabase(logger moira.Logger, config DatabaseConfig, nh NotificationHistoryConfig, n NotificationConfig, source DBSource) *DbConnector {
client := redis.NewUniversalClient(&redis.UniversalOptions{
MasterName: config.MasterName,
Addrs: config.Addrs,
Expand Down Expand Up @@ -78,7 +80,9 @@ func NewDatabase(logger moira.Logger, config DatabaseConfig, nh NotificationHist
source: source,
clock: clock.NewSystemClock(),
notificationHistory: nh,
notification: n,
}

return &connector
}

Expand All @@ -90,7 +94,14 @@ func NewTestDatabase(logger moira.Logger) *DbConnector {
NotificationHistoryConfig{
NotificationHistoryTTL: time.Hour * 48,
NotificationHistoryQueryLimit: 1000,
}, testSource)
},
NotificationConfig{
DelayedTime: time.Minute,
TransactionTimeout: 200 * time.Millisecond,
TransactionMaxRetries: 10,
TransactionHeuristicLimit: 10000,
},
testSource)
}

// NewTestDatabaseWithIncorrectConfig use it only for tests
Expand All @@ -101,6 +112,12 @@ func NewTestDatabaseWithIncorrectConfig(logger moira.Logger) *DbConnector {
NotificationHistoryTTL: time.Hour * 48,
NotificationHistoryQueryLimit: 1000,
},
NotificationConfig{
DelayedTime: time.Minute,
TransactionTimeout: 200 * time.Millisecond,
TransactionMaxRetries: 10,
TransactionHeuristicLimit: 10000,
},
testSource)
}

Expand Down
22 changes: 22 additions & 0 deletions database/redis/last_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,28 @@ func (connector *DbConnector) checkDataScoreChanged(triggerID string, checkData
return oldScore != float64(checkData.Score)
}

// getTriggersLastCheck returns an array of trigger checks by the passed ids, if the trigger does not exist, it is nil
func (connector *DbConnector) getTriggersLastCheck(triggerIDs []string) ([]*moira.CheckData, error) {
ctx := connector.context
pipe := (*connector.client).TxPipeline()

results := make([]*redis.StringCmd, len(triggerIDs))
for i, id := range triggerIDs {
var result *redis.StringCmd
if id != "" {
result = pipe.Get(ctx, metricLastCheckKey(id))
}
results[i] = result
}

_, err := pipe.Exec(ctx)
if err != nil && err != redis.Nil {
return nil, err
}

return reply.Checks(results)
}

var badStateTriggersKey = "moira-bad-state-triggers"
var triggersChecksKey = "moira-triggers-checks"

Expand Down
101 changes: 101 additions & 0 deletions database/redis/last_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,107 @@ func TestLastCheckErrorConnection(t *testing.T) {
})
}

func TestGetTriggersLastCheck(t *testing.T) {
logger, _ := logging.GetLogger("dataBase")
dataBase := NewTestDatabase(logger)
dataBase.Flush()
defer dataBase.Flush()

_ = dataBase.SetTriggerLastCheck("test1", &moira.CheckData{
Timestamp: 1,
}, moira.TriggerSourceNotSet)

_ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{
Timestamp: 2,
}, moira.TriggerSourceNotSet)

_ = dataBase.SetTriggerLastCheck("test3", &moira.CheckData{
Timestamp: 3,
}, moira.TriggerSourceNotSet)

Convey("getTriggersLastCheck manipulations", t, func() {
Convey("Test with nil id array", func() {
actual, err := dataBase.getTriggersLastCheck(nil)
So(err, ShouldBeNil)
So(actual, ShouldResemble, []*moira.CheckData{})
})

Convey("Test with correct id array", func() {
actual, err := dataBase.getTriggersLastCheck([]string{"test1", "test2", "test3"})
So(err, ShouldBeNil)
So(actual, ShouldResemble, []*moira.CheckData{
{
Timestamp: 1,
MetricsToTargetRelation: map[string]string{},
},
{
Timestamp: 2,
MetricsToTargetRelation: map[string]string{},
},
{
Timestamp: 3,
MetricsToTargetRelation: map[string]string{},
},
})
})

Convey("Test with deleted trigger", func() {
dataBase.RemoveTriggerLastCheck("test2") //nolint
defer func() {
_ = dataBase.SetTriggerLastCheck("test2", &moira.CheckData{
Timestamp: 2,
}, moira.TriggerSourceNotSet)
}()

actual, err := dataBase.getTriggersLastCheck([]string{"test1", "test2", "test3"})
So(err, ShouldBeNil)
So(actual, ShouldResemble, []*moira.CheckData{
{
Timestamp: 1,
MetricsToTargetRelation: map[string]string{},
},
nil,
{
Timestamp: 3,
MetricsToTargetRelation: map[string]string{},
},
})
})

Convey("Test with a nonexistent trigger id", func() {
actual, err := dataBase.getTriggersLastCheck([]string{"test1", "test2", "test4"})
So(err, ShouldBeNil)
So(actual, ShouldResemble, []*moira.CheckData{
{
Timestamp: 1,
MetricsToTargetRelation: map[string]string{},
},
{
Timestamp: 2,
MetricsToTargetRelation: map[string]string{},
},
nil,
})
})

Convey("Test with an empty trigger id", func() {
actual, err := dataBase.getTriggersLastCheck([]string{"", "test2", "test3"})
So(err, ShouldBeNil)
So(actual, ShouldResemble, []*moira.CheckData{
nil,
{
Timestamp: 2,
MetricsToTargetRelation: map[string]string{},
},
{
Timestamp: 3,
MetricsToTargetRelation: map[string]string{},
},
})
})
})
}

func TestMaintenanceUserSave(t *testing.T) {
logger, _ := logging.GetLogger("dataBase")
dataBase := NewTestDatabase(logger)
Expand Down
Loading
Loading