Skip to content

Commit 2cd13e3

Browse files
alexluongclaude
andauthored
fix: warn instead of crash when operator events topics configured without sink (#835)
* fix: warn instead of crash when operator events topics configured without sink When topics are configured but no sink is set, the server would crash on startup with a fatal error. Instead, log a warning and return a NoopSink so the server can start gracefully (operator events will be dropped). Co-Authored-By: Claude <noreply@anthropic.com> * chore: gofmt * refactor: use project logger instead of slog in opevents.NewSink Replace stdlib log/slog with the project's *logging.Logger for consistency. NewSink now accepts a logger parameter, and callers in builder.go pass b.logger. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent af17958 commit 2cd13e3

8 files changed

Lines changed: 37 additions & 34 deletions

File tree

cmd/outpost/migrate.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ func newMigrateCommand() *cli.Command {
7070
Action: runMigrateVerify,
7171
},
7272
{
73-
Name: "unlock",
74-
Usage: "Force clear the Redis migration lock (use with caution)",
73+
Name: "unlock",
74+
Usage: "Force clear the Redis migration lock (use with caution)",
7575
Flags: []cli.Flag{
7676
&cli.BoolFlag{
7777
Name: "yes",

internal/config/config.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ type Config struct {
5959
DeploymentID string `yaml:"deployment_id" env:"DEPLOYMENT_ID" desc:"Optional deployment identifier for multi-tenancy. Enables multiple deployments to share the same infrastructure while maintaining data isolation." required:"N"`
6060
AESEncryptionSecret string `yaml:"aes_encryption_secret" env:"AES_ENCRYPTION_SECRET" desc:"A 16, 24, or 32 byte secret key used for AES encryption of sensitive data at rest." required:"Y"`
6161
Topics []string `yaml:"topics" env:"TOPICS" envSeparator:"," desc:"Comma-separated list of topics that this Outpost instance should subscribe to for event processing." required:"N"`
62-
HTTPUserAgent string `yaml:"http_user_agent" env:"HTTP_USER_AGENT" desc:"Custom HTTP User-Agent string for outgoing webhook deliveries. If unset, defaults to 'Outpost/{version}'." required:"N"`
62+
HTTPUserAgent string `yaml:"http_user_agent" env:"HTTP_USER_AGENT" desc:"Custom HTTP User-Agent string for outgoing webhook deliveries. If unset, defaults to 'Outpost/{version}'." required:"N"`
6363

6464
// Infrastructure
6565
Redis RedisConfig `yaml:"redis"`
@@ -478,9 +478,9 @@ func (c *OperatorEventsConfig) ToConfig() opevents.Config {
478478
}
479479

480480
type AlertConfig struct {
481-
ConsecutiveFailureCount int `yaml:"consecutive_failure_count" env:"ALERT_CONSECUTIVE_FAILURE_COUNT" desc:"Number of consecutive delivery failures for a destination before triggering an alert and potentially disabling it." required:"N"`
482-
AutoDisableDestination bool `yaml:"auto_disable_destination" env:"ALERT_AUTO_DISABLE_DESTINATION" desc:"If true, automatically disables a destination after 'consecutive_failure_count' is reached." required:"N"`
483-
ExhaustedRetriesWindowSeconds int `yaml:"exhausted_retries_window_seconds" env:"ALERT_EXHAUSTED_RETRIES_WINDOW_SECONDS" desc:"Suppression window in seconds for exhausted_retries alerts. First exhaustion per destination emits an alert; subsequent exhaustions within the window are suppressed." required:"N"`
481+
ConsecutiveFailureCount int `yaml:"consecutive_failure_count" env:"ALERT_CONSECUTIVE_FAILURE_COUNT" desc:"Number of consecutive delivery failures for a destination before triggering an alert and potentially disabling it." required:"N"`
482+
AutoDisableDestination bool `yaml:"auto_disable_destination" env:"ALERT_AUTO_DISABLE_DESTINATION" desc:"If true, automatically disables a destination after 'consecutive_failure_count' is reached." required:"N"`
483+
ExhaustedRetriesWindowSeconds int `yaml:"exhausted_retries_window_seconds" env:"ALERT_EXHAUSTED_RETRIES_WINDOW_SECONDS" desc:"Suppression window in seconds for exhausted_retries alerts. First exhaustion per destination emits an alert; subsequent exhaustions within the window are suppressed." required:"N"`
484484
}
485485

486486
// ConfigFilePath returns the path of the config file that was used

internal/logstore/pglogstore/pglogstore.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -455,12 +455,12 @@ func scanAttemptRecords(rows pgx.Rows) ([]attemptRecordWithPosition, error) {
455455
attemptTime time.Time
456456
attemptNumber int
457457
manual bool
458-
code string
459-
responseDataStr string
460-
eventTime time.Time
461-
eligibleForRetry bool
462-
eventData string
463-
eventMetadata map[string]string
458+
code string
459+
responseDataStr string
460+
eventTime time.Time
461+
eligibleForRetry bool
462+
eventData string
463+
eventMetadata map[string]string
464464
)
465465

466466
if err := rows.Scan(
@@ -642,12 +642,12 @@ func (s *logStore) RetrieveAttempt(ctx context.Context, req driver.RetrieveAttem
642642
attemptTime time.Time
643643
attemptNumber int
644644
manual bool
645-
code string
646-
responseDataStr string
647-
eventTime time.Time
648-
eligibleForRetry bool
649-
eventData string
650-
eventMetadata map[string]string
645+
code string
646+
responseDataStr string
647+
eventTime time.Time
648+
eligibleForRetry bool
649+
eventData string
650+
eventMetadata map[string]string
651651
)
652652

653653
err := row.Scan(

internal/migrator/coordinator/coordinator.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ type Coordinator struct {
3838
// are expected to own the lifecycle of the SQL migrator and the Redis
3939
// client and close them when finished.
4040
type Config struct {
41-
SQLMigrator *migrator.Migrator
42-
RedisClient redis.Client
43-
RedisMigrations []migratorredis.Migration
44-
DeploymentID string
45-
Logger *logging.Logger
41+
SQLMigrator *migrator.Migrator
42+
RedisClient redis.Client
43+
RedisMigrations []migratorredis.Migration
44+
DeploymentID string
45+
Logger *logging.Logger
4646
}
4747

4848
// New constructs a Coordinator. Either SQLMigrator or the Redis fields

internal/migrator/coordinator/coordinator_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ type redisTestClient struct {
1919
*r.Client
2020
}
2121

22-
func (c *redisTestClient) Close() error { return c.Client.Close() }
22+
func (c *redisTestClient) Close() error { return c.Client.Close() }
2323
func (c *redisTestClient) Pipeline() redis.Pipeliner { return c.Client.Pipeline() }
2424

2525
// fakeMigration is a test-only migratorredis.Migration that records calls.
@@ -92,7 +92,7 @@ func (m *fakeMigration) Verify(ctx context.Context, state *migratorredis.State)
9292
}, nil
9393
}
9494

95-
func (m *fakeMigration) PlanCleanup(ctx context.Context) (int, error) { return 0, nil }
95+
func (m *fakeMigration) PlanCleanup(ctx context.Context) (int, error) { return 0, nil }
9696
func (m *fakeMigration) Cleanup(ctx context.Context, state *migratorredis.State) error {
9797
return nil
9898
}

internal/opevents/config.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ package opevents
33
import (
44
"fmt"
55

6+
"github.com/hookdeck/outpost/internal/logging"
67
"github.com/hookdeck/outpost/internal/mqs"
8+
"go.uber.org/zap"
79
)
810

911
// Config holds the configuration for the operator events system.
@@ -44,9 +46,9 @@ type RabbitMQSinkConfig struct {
4446

4547
// NewSink returns the appropriate Sink based on config.
4648
// Returns NoopSink if no sink is configured.
47-
// Returns an error if topics are specified but no sink is configured, as events
48-
// would be silently dropped.
49-
func NewSink(cfg Config) (Sink, error) {
49+
// If topics are specified but no sink is configured, it logs a warning and
50+
// returns NoopSink (operator events will be dropped).
51+
func NewSink(cfg Config, logger *logging.Logger) (Sink, error) {
5052
if cfg.HTTP != nil {
5153
return NewHTTPSink(cfg.HTTP.URL, cfg.HTTP.SigningSecret), nil
5254
}
@@ -60,7 +62,8 @@ func NewSink(cfg Config) (Sink, error) {
6062
return newMQSinkFromRabbitMQ(cfg.RabbitMQ)
6163
}
6264
if len(cfg.Topics) > 0 {
63-
return nil, fmt.Errorf("opevents: topics are configured (%v) but no sink is set; events would be silently dropped", cfg.Topics)
65+
logger.Warn("opevents: topics are configured but no sink is set; operator events will be dropped", zap.Any("topics", cfg.Topics))
66+
return &NoopSink{}, nil
6467
}
6568
return &NoopSink{}, nil
6669
}

internal/opevents/sink_noop.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ import "context"
55
// NoopSink is a sink that discards all events. Used when no sink is configured.
66
type NoopSink struct{}
77

8-
func (s *NoopSink) Init(ctx context.Context) error { return nil }
9-
func (s *NoopSink) Send(ctx context.Context, event *OperatorEvent) error { return nil }
10-
func (s *NoopSink) Close() error { return nil }
8+
func (s *NoopSink) Init(ctx context.Context) error { return nil }
9+
func (s *NoopSink) Send(ctx context.Context, event *OperatorEvent) error { return nil }
10+
func (s *NoopSink) Close() error { return nil }

internal/services/builder.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ func (b *ServiceBuilder) BuildAPIWorkers(baseRouter *gin.Engine) error {
193193

194194
// Create operator events emitter for subscription updates
195195
oeCfg := b.cfg.OperatorEvents.ToConfig()
196-
oeSink, err := opevents.NewSink(oeCfg)
196+
oeSink, err := opevents.NewSink(oeCfg, b.logger)
197197
if err != nil {
198198
return fmt.Errorf("failed to create operator events sink: %w", err)
199199
}
@@ -345,7 +345,7 @@ func (b *ServiceBuilder) BuildLogWorker(baseRouter *gin.Engine) error {
345345

346346
// Initialize alert monitor for operator events
347347
oeCfg := b.cfg.OperatorEvents.ToConfig()
348-
sink, err := opevents.NewSink(oeCfg)
348+
sink, err := opevents.NewSink(oeCfg, b.logger)
349349
if err != nil {
350350
return fmt.Errorf("failed to create operator events sink: %w", err)
351351
}

0 commit comments

Comments
 (0)