Skip to content
This repository has been archived by the owner on May 29, 2024. It is now read-only.

Adding log as destination for alerting #87

Closed
wants to merge 10 commits into from
2 changes: 1 addition & 1 deletion cmd/pessimism/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func initializeServer(ctx context.Context, cfg *config.Config,
// initializeAlerting ... Performs dependency injection to build alerting struct
func initializeAlerting(ctx context.Context, cfg *config.Config) alert.Manager {
sc := client.NewSlackClient(cfg.SlackURL)
return alert.NewManager(ctx, sc)
return alert.NewSlackManager(ctx, sc)
}

// initalizeETL ... Performs dependency injection to build etl struct
Expand Down
2 changes: 1 addition & 1 deletion internal/alert/interpolator.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func NewInterpolator() Interpolator {
return &interpolator{}
}

// InterpolateSlackMessage ... Interpolates a slack message with the given invariant session UUID and message
// InterpolateSlackMessage ... Interpolates a Slack message with the given invariant session UUID and message
func (*interpolator) InterpolateSlackMessage(sUUID core.SUUID, message string) string {
return fmt.Sprintf(SlackMsgFmt,
sUUID.PID.InvType().String(),
Expand Down
35 changes: 33 additions & 2 deletions internal/alert/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type alertManager struct {
}

// NewManager ... Instantiates a new alert manager
func NewManager(ctx context.Context, sc client.SlackClient) Manager {
func NewSlackManager(ctx context.Context, sc client.SlackClient) Manager {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious to know the nature/rationale behind changing this name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essentially, there's a new NewLogManager, keeping NewManager would indicate it can take in configuration of which type. Since that isn't the case now, specifying what the function can handle would be useful.

// NOTE - Consider constructing dependencies in higher level
// abstraction and passing them in

Expand All @@ -52,6 +52,21 @@ func NewManager(ctx context.Context, sc client.SlackClient) Manager {
return am
}

func NewLogManager(ctx context.Context) Manager {
ctx, cancel := context.WithCancel(ctx)

am := &alertManager{
ctx: ctx,
cancel: cancel,

sc: client.NewSlackClient(""), // will change when deps are constructed at higher level
interpolator: NewInterpolator(),
store: NewStore(),
alertTransit: make(chan core.Alert),
}
return am
}

// AddInvariantSession ... Adds an invariant session to the alert manager store
func (am *alertManager) AddInvariantSession(sUUID core.SUUID, alertDestination core.AlertDestination) error {
return am.store.AddAlertDestination(sUUID, alertDestination)
Expand All @@ -62,7 +77,7 @@ func (am *alertManager) Transit() chan core.Alert {
return am.alertTransit
}

// handleSlackPost ... Handles posting an alert to slack channel
// handleSlackPost ... Handles posting an alert to Slack channel
func (am *alertManager) handleSlackPost(alert core.Alert) error {
slackMsg := am.interpolator.InterpolateSlackMessage(alert.SUUID, alert.Content)

Expand All @@ -78,6 +93,17 @@ func (am *alertManager) handleSlackPost(alert core.Alert) error {
return nil
}

// handleSlackPost ... Handles posting an alert to console log
func (am *alertManager) handleLogPost(alert core.Alert) {
logger := logging.WithContext(am.ctx)

logger.Error("Pessimism Alert",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if a warning would be more appropriate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, i contemplated that. It essentially depends on how downstream monitoring of logs are setup. Figured usually errors are monitored and consumed, not so much warnings. But we can make the log type as an option in the future.

zap.String("Invariant Condition", alert.SUUID.PID.InvType().String()),
zap.String("Network", alert.SUUID.PID.Network().String()),
zap.String("Session UUID", alert.SUUID.String()),
zap.String("Assessment Content", alert.Content))
}

// EventLoop ... Event loop for alert manager subsystem
func (am *alertManager) EventLoop() error {
logger := logging.WithContext(am.ctx)
Expand Down Expand Up @@ -106,6 +132,11 @@ func (am *alertManager) EventLoop() error {
logger.Error("Could not post alert to slack", zap.Error(err))
}

case core.Log:
logger.Debug("Attempting to log alert to console")
am.handleLogPost(alert)
logger.Debug("Logging alert to console completed")
anupsv-cb marked this conversation as resolved.
Show resolved Hide resolved

case core.ThirdParty:
logger.Error("Attempting to post alert to third_party which is not yet supported")

Expand Down
10 changes: 7 additions & 3 deletions internal/core/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,18 @@ func StringToInvariantType(stringType string) InvariantType {
type AlertDestination uint8

const (
Slack AlertDestination = iota + 1
ThirdParty // 2
Slack AlertDestination = iota + 1
Log
ThirdParty // 2
anupsv-cb marked this conversation as resolved.
Show resolved Hide resolved
)

// String ... Converts an alerting destination type to a string
func (ad AlertDestination) String() string {
switch ad {
case Slack:
return "slack"
case Log:
return "log"
case ThirdParty:
return "third_party"
default:
Expand All @@ -119,7 +122,8 @@ func StringToAlertingDestType(stringType string) AlertDestination {
switch stringType {
case "slack":
return Slack

case "log":
return Log
case "third_party":
return ThirdParty
}
Expand Down