Notifications: Address Code Review

- Bump IGL to latest changes in Icinga/icinga-go-library#145.
- Allow specifying which pipeline keys are relevant, ignore others.
- Allow specifying which pipeline key should be parsed in which type.
- Create history.DowntimeHistoryMeta as a chimera combining
  history.DowntimeHistory and history.HistoryDowntime to allow access
  event_type, distinguishing between downtime_start and downtime_end.
- Trace times for submission steps in the worker. Turns out, the single
  threaded worker blocks roughly two seconds for each
  Client.ProcessEvent method call. This might sum up to minutes if lots
  of events are processed at once. My current theory is that the delay
  results in the expensive bcrypt hash comparison on Notifications.

Author: oxzi

cmd/icingadb/main.go

@@ -170,23 +170,26 @@ func run() int {
 	sig := make(chan os.Signal, 1)
 	signal.Notify(sig, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP)
 
-	var notificationsSourceCallback func(database.Entity)
-	if cfg := cmd.Config.NotificationsSource; cfg.ApiBaseUrl != "" {
-		logger.Info("Starting Icinga Notifications source")
-
-		notificationsSource := notifications.NewNotificationsSource(
-			ctx,
-			db,
-			rc,
-			logs.GetChildLogger("notifications-source"),
-			cfg)
-		notificationsSourceCallback = notificationsSource.Submit
-	}
-
 	go func() {
+		var callback func(database.Entity)
+		var callbackKeyStructPtr map[string]any
+
+		if cfg := cmd.Config.NotificationsSource; cfg.ApiBaseUrl != "" {
+			logger.Info("Starting Icinga Notifications source")
+
+			notificationsSource := notifications.NewNotificationsClient(
+				ctx,
+				db,
+				rc,
+				logs.GetChildLogger("notifications-source"),
+				cfg)
+			callback = notificationsSource.Submit
+			callbackKeyStructPtr = notifications.SyncKeyStructPtrs
+		}
+
 		logger.Info("Starting history sync")
 
-		if err := hs.Sync(ctx, notificationsSourceCallback); err != nil && !utils.IsContextCanceled(err) {
+		if err := hs.Sync(ctx, callbackKeyStructPtr, callback); err != nil && !utils.IsContextCanceled(err) {
 			logger.Fatalf("%+v", err)
 		}
 	}()

internal/config/config.go

@@ -4,7 +4,7 @@ import (
 	"github.com/creasty/defaults"
 	"github.com/icinga/icinga-go-library/database"
 	"github.com/icinga/icinga-go-library/logging"
-	"github.com/icinga/icinga-go-library/notifications"
+	"github.com/icinga/icinga-go-library/notifications/source"
 	"github.com/icinga/icinga-go-library/redis"
 	"github.com/icinga/icingadb/pkg/icingadb/history"
 	"github.com/pkg/errors"
@@ -16,11 +16,11 @@ const DefaultConfigPath = "/etc/icingadb/config.yml"
 
 // Config defines Icinga DB config.
 type Config struct {
-	Database            database.Config      `yaml:"database" envPrefix:"DATABASE_"`
-	Redis               redis.Config         `yaml:"redis" envPrefix:"REDIS_"`
-	Logging             logging.Config       `yaml:"logging" envPrefix:"LOGGING_"`
-	Retention           RetentionConfig      `yaml:"retention" envPrefix:"RETENTION_"`
-	NotificationsSource notifications.Config `yaml:"notifications-source" envPrefix:"NOTIFICATIONS_SOURCE_"`
+	Database            database.Config `yaml:"database" envPrefix:"DATABASE_"`
+	Redis               redis.Config    `yaml:"redis" envPrefix:"REDIS_"`
+	Logging             logging.Config  `yaml:"logging" envPrefix:"LOGGING_"`
+	Retention           RetentionConfig `yaml:"retention" envPrefix:"RETENTION_"`
+	NotificationsSource source.Config   `yaml:"notifications-source" envPrefix:"NOTIFICATIONS_SOURCE_"`
 }
 
 func (c *Config) SetDefaults() {

pkg/icingadb/history/sync.go

@@ -39,9 +39,15 @@ func NewSync(db *database.DB, redis *redis.Client, logger *logging.Logger) *Sync
 
 // Sync synchronizes Redis history streams from s.redis to s.db and deletes the original data on success.
 //
-// If not nil, the callback function is appended to each synchronization pipeline and called before the entry is deleted
-// from Redis.
-func (s Sync) Sync(ctx context.Context, callback func(database.Entity)) error {
+// An optional callback and callbackKeyStructPtr might be given. Both most either be nil or not nil.
+//
+// The callbackKeyStructPtr says which pipeline keys should be mapped to which type, identified by a struct pointer. If
+// a key is missing from the map, it will not be used for the callback. The callback function itself shall not block.
+func (s Sync) Sync(ctx context.Context, callbackKeyStructPtr map[string]any, callback func(database.Entity)) error {
+	if (callbackKeyStructPtr == nil) != (callback == nil) {
+		return fmt.Errorf("either both callbackKeyStructPtr and callback must be nil or none")
+	}
+
 	g, ctx := errgroup.WithContext(ctx)
 
 	for key, pipeline := range syncPipelines {
@@ -67,8 +73,13 @@ func (s Sync) Sync(ctx context.Context, callback func(database.Entity)) error {
 		// forward the entry after it has completed its own sync so that later stages can rely on previous stages being
 		// executed successfully.
 
-		if callback != nil {
-			pipeline = append(pipeline, makeCallbackStageFunc(callback))
+		// Shadowed variable to allow appending custom callbacks.
+		pipeline := pipeline
+		if callbackKeyStructPtr != nil {
+			_, ok := callbackKeyStructPtr[key]
+			if ok {
+				pipeline = append(pipeline, makeCallbackStageFunc(callbackKeyStructPtr, callback))
+			}
 		}
 
 		ch := make([]chan redis.XMessage, len(pipeline)+1)
@@ -371,28 +382,17 @@ func userNotificationStage(ctx context.Context, s Sync, key string, in <-chan re
 
 // makeCallbackStageFunc creates a new stageFunc calling the given callback function for each message.
 //
+// The keyStructPtrs map decides what kind of database.Entity type will be used for the input data based on the key.
+//
 // The callback call is blocking and the message will be forwarded to the out channel after the function has returned.
 // Thus, please ensure this function does not block too long.
-func makeCallbackStageFunc(callback func(database.Entity)) stageFunc {
+func makeCallbackStageFunc(keyStructPtrs map[string]any, callback func(database.Entity)) stageFunc {
 	return func(ctx context.Context, _ Sync, key string, in <-chan redis.XMessage, out chan<- redis.XMessage) error {
 		defer close(out)
 
-		var structPtr database.Entity
-		switch key { // keep in sync with syncPipelines below
-		case "notification":
-			structPtr = (*v1.NotificationHistory)(nil)
-		case "state":
-			structPtr = (*v1.StateHistory)(nil)
-		case "downtime":
-			structPtr = (*v1.DowntimeHistory)(nil)
-		case "comment":
-			structPtr = (*v1.CommentHistory)(nil)
-		case "flapping":
-			structPtr = (*v1.FlappingHistory)(nil)
-		case "acknowledgement":
-			structPtr = (*v1.AcknowledgementHistory)(nil)
-		default:
-			return fmt.Errorf("unsupported key %q", key)
+		structPtr, ok := keyStructPtrs[key]
+		if !ok {
+			return fmt.Errorf("can't lookup struct pointer for key %q", key)
 		}
 
 		structifier := structify.MakeMapStructifier(
@@ -409,7 +409,7 @@ func makeCallbackStageFunc(callback func(database.Entity)) stageFunc {
 
 				val, err := structifier(msg.Values)
 				if err != nil {
-					return errors.Wrapf(err, "can't structify values %#v for %s", msg.Values, key)
+					return errors.Wrapf(err, "can't structify values %#v for %q", msg.Values, key)
 				}
 
 				entity, ok := val.(database.Entity)
@@ -427,32 +427,41 @@ func makeCallbackStageFunc(callback func(database.Entity)) stageFunc {
 	}
 }
 
+const (
+	SyncPipelineAcknowledgement = "acknowledgement"
+	SyncPipelineComment         = "comment"
+	SyncPipelineDowntime        = "downtime"
+	SyncPipelineFlapping        = "flapping"
+	SyncPipelineNotification    = "notification"
+	SyncPipelineState           = "state"
+)
+
 var syncPipelines = map[string][]stageFunc{
-	"notification": {
-		writeOneEntityStage((*v1.NotificationHistory)(nil)), // notification_history
-		userNotificationStage,                               // user_notification_history (depends on notification_history)
-		writeOneEntityStage((*v1.HistoryNotification)(nil)), // history (depends on notification_history)
+	SyncPipelineAcknowledgement: {
+		writeOneEntityStage((*v1.AcknowledgementHistory)(nil)), // acknowledgement_history
+		writeOneEntityStage((*v1.HistoryAck)(nil)),             // history (depends on acknowledgement_history)
 	},
-	"state": {
-		writeOneEntityStage((*v1.StateHistory)(nil)),   // state_history
-		writeOneEntityStage((*v1.HistoryState)(nil)),   // history (depends on state_history)
-		writeMultiEntityStage(stateHistoryToSlaEntity), // sla_history_state
+	SyncPipelineComment: {
+		writeOneEntityStage((*v1.CommentHistory)(nil)), // comment_history
+		writeOneEntityStage((*v1.HistoryComment)(nil)), // history (depends on comment_history)
 	},
-	"downtime": {
+	SyncPipelineDowntime: {
 		writeOneEntityStage((*v1.DowntimeHistory)(nil)),    // downtime_history
 		writeOneEntityStage((*v1.HistoryDowntime)(nil)),    // history (depends on downtime_history)
 		writeOneEntityStage((*v1.SlaHistoryDowntime)(nil)), // sla_history_downtime
 	},
-	"comment": {
-		writeOneEntityStage((*v1.CommentHistory)(nil)), // comment_history
-		writeOneEntityStage((*v1.HistoryComment)(nil)), // history (depends on comment_history)
-	},
-	"flapping": {
+	SyncPipelineFlapping: {
 		writeOneEntityStage((*v1.FlappingHistory)(nil)), // flapping_history
 		writeOneEntityStage((*v1.HistoryFlapping)(nil)), // history (depends on flapping_history)
 	},
-	"acknowledgement": {
-		writeOneEntityStage((*v1.AcknowledgementHistory)(nil)), // acknowledgement_history
-		writeOneEntityStage((*v1.HistoryAck)(nil)),             // history (depends on acknowledgement_history)
+	SyncPipelineNotification: {
+		writeOneEntityStage((*v1.NotificationHistory)(nil)), // notification_history
+		userNotificationStage,                               // user_notification_history (depends on notification_history)
+		writeOneEntityStage((*v1.HistoryNotification)(nil)), // history (depends on notification_history)
+	},
+	SyncPipelineState: {
+		writeOneEntityStage((*v1.StateHistory)(nil)),   // state_history
+		writeOneEntityStage((*v1.HistoryState)(nil)),   // history (depends on state_history)
+		writeMultiEntityStage(stateHistoryToSlaEntity), // sla_history_state
 	},
 }

pkg/icingadb/v1/history/downtime.go

@@ -88,6 +88,12 @@ func (*HistoryDowntime) TableName() string {
 	return "history"
 }
 
+type DowntimeHistoryMeta struct {
+	DowntimeHistoryEntity `json:",inline"`
+	DowntimeHistory       `json:",inline"`
+	HistoryMeta           `json:",inline"`
+}
+
 type SlaHistoryDowntime struct {
 	DowntimeHistoryEntity      `json:",inline"`
 	HistoryTableMeta           `json:",inline"`