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
Closed

Adding log as destination for alerting #87

wants to merge 10 commits into from

Conversation

anupsv-cb
Copy link
Contributor

Fixes Issue

Closes #62

Changes proposed

Adding log as the destination for alerting.

@anupsv-cb anupsv-cb requested a review from epociask June 26, 2023 16:28
@anupsv-cb anupsv-cb changed the title Issue 62 Adding log as destination for alerting Jun 26, 2023
@@ -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.

internal/core/constants.go Outdated Show resolved Hide resolved
internal/alert/manager.go Outdated Show resolved Hide resolved
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.

@github-actions
Copy link

Current Test Coverage

ok  	github.com/base-org/pessimism/internal/api/handlers	0.028s	coverage: 95.8% of statements
?   	github.com/base-org/pessimism/internal/api/handlers/middleware	[no test files]
ok  	github.com/base-org/pessimism/internal/api/models	0.009s	coverage: 70.0% of statements
ok  	github.com/base-org/pessimism/internal/api/server	0.020s	coverage: 53.3% of statements
ok  	github.com/base-org/pessimism/internal/api/service	0.020s	coverage: 89.7% of statements
?   	github.com/base-org/pessimism/internal/app	[no test files]
?   	github.com/base-org/pessimism/internal/client	[no test files]
ok  	github.com/base-org/pessimism/internal/common	0.002s	coverage: 100.0% of statements
ok  	github.com/base-org/pessimism/internal/config	0.019s	coverage: 69.6% of statements
ok  	github.com/base-org/pessimism/internal/core	0.011s	coverage: 48.5% of statements
ok  	github.com/base-org/pessimism/internal/engine	0.024s	coverage: 68.8% of statements
ok  	github.com/base-org/pessimism/internal/engine/invariant	0.003s	coverage: 81.8% of statements
ok  	github.com/base-org/pessimism/internal/engine/registry	0.023s	coverage: 52.0% of statements
ok  	github.com/base-org/pessimism/internal/etl/component	0.027s	coverage: 44.4% of statements
ok  	github.com/base-org/pessimism/internal/etl/pipeline	0.019s	coverage: 40.6% of statements
ok  	github.com/base-org/pessimism/internal/etl/registry	0.016s	coverage: 90.5% of statements
ok  	github.com/base-org/pessimism/internal/etl/registry/oracle	7.020s	coverage: 51.6% of statements
?   	github.com/base-org/pessimism/internal/etl/registry/pipe	[no test files]
?   	github.com/base-org/pessimism/internal/logging	[no test files]
?   	github.com/base-org/pessimism/internal/metrics	[no test files]
?   	github.com/base-org/pessimism/internal/mocks	[no test files]
ok  	github.com/base-org/pessimism/internal/state	0.007s	coverage: 53.1% of statements
ok  	github.com/base-org/pessimism/internal/subsystem	0.020s	coverage: 52.8% of statements
55.5%

@epociask epociask closed this Aug 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Log as Default Alert Destination Type
2 participants