-
Notifications
You must be signed in to change notification settings - Fork 19
Refactor Sentry error reporting pattern with flexible options across application layers #62
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
Changes from all commits
b574871
cde5e37
123890a
60e0e2b
4c61601
834bb2f
ee84715
a1bf3eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,9 +16,9 @@ import ( | |
|
|
||
| "github.com/getsentry/sentry-go" | ||
| "watchdog.onebusaway.org/internal/models" | ||
| "watchdog.onebusaway.org/internal/report" | ||
| "watchdog.onebusaway.org/internal/server" | ||
| "watchdog.onebusaway.org/internal/utils" | ||
| "watchdog.onebusaway.org/internal/report" | ||
| ) | ||
|
|
||
| // Declare a string containing the application version number. Later in the book we'll | ||
|
|
@@ -31,9 +31,10 @@ const version = "1.0.0" | |
| // logger, but it will grow to include a lot more as our build progresses. | ||
|
|
||
| type application struct { | ||
| config server.Config | ||
| logger *slog.Logger | ||
| mu sync.RWMutex | ||
| config server.Config | ||
| logger *slog.Logger | ||
| reporter *report.Reporter | ||
| mu sync.RWMutex | ||
| } | ||
|
|
||
| func main() { | ||
|
|
@@ -53,20 +54,25 @@ func main() { | |
| configAuthPass := os.Getenv("CONFIG_AUTH_PASS") | ||
|
|
||
| var err error | ||
| if err = validateConfigFlags(configFile, configURL); err != nil{ | ||
| fmt.Println("Error:",err) | ||
|
|
||
| if err = validateConfigFlags(configFile, configURL); err != nil { | ||
| fmt.Println("Error:", err) | ||
| flag.Usage() | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| report.SetupSentry() | ||
| defer report.FlushSentry() | ||
|
|
||
| reporter := report.NewReporter(cfg.Env, version) | ||
| reporter.ConfigureScope() | ||
|
|
||
| var servers []models.ObaServer | ||
|
|
||
|
|
||
| if *configFile != "" { | ||
| servers, err = loadConfigFromFile(*configFile) | ||
| servers, err = loadConfigFromFile(*configFile, reporter) | ||
| } else if *configURL != "" { | ||
| servers, err = loadConfigFromURL(*configURL, configAuthUser, configAuthPass) | ||
| servers, err = loadConfigFromURL(*configURL, configAuthUser, configAuthPass, reporter) | ||
| } else { | ||
| fmt.Println("Error: No configuration provided. Use --config-file or --config-url.") | ||
| flag.Usage() | ||
|
|
@@ -87,35 +93,29 @@ func main() { | |
|
|
||
| logger := slog.New(slog.NewTextHandler(os.Stdout, nil)) | ||
|
|
||
| report.SetupSentry() | ||
| defer report.FlushSentry() | ||
|
|
||
|
|
||
| reporter := report.NewReporter(cfg.Env, version) | ||
| reporter.ConfigureScope() | ||
|
|
||
| cacheDir := "cache" | ||
| if err = createCacheDirectory(cacheDir, logger); err != nil { | ||
| if err = createCacheDirectory(cacheDir, logger, reporter); err != nil { | ||
| logger.Error("Failed to create cache directory", "error", err) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| // Download GTFS bundles for all servers on startup | ||
| downloadGTFSBundles(servers, cacheDir, logger) | ||
| downloadGTFSBundles(servers, cacheDir, logger, reporter) | ||
|
|
||
| app := &application{ | ||
| config: cfg, | ||
| logger: logger, | ||
| config: cfg, | ||
| logger: logger, | ||
| reporter: reporter, | ||
| } | ||
|
|
||
| app.startMetricsCollection() | ||
|
|
||
| // Cron job to download GTFS bundles for all servers every 24 hours | ||
| go refreshGTFSBundles(servers, cacheDir, logger , 24 * time.Hour) | ||
| go refreshGTFSBundles(servers, cacheDir, logger, 24*time.Hour, reporter) | ||
|
|
||
| // If a remote URL is specified, refresh the configuration every minute | ||
| if *configURL != "" { | ||
| go refreshConfig(*configURL, configAuthUser, configAuthPass, app, logger, time.Minute) | ||
| go refreshConfig(*configURL, configAuthUser, configAuthPass, app, logger, time.Minute, reporter) | ||
| } | ||
|
|
||
| srv := &http.Server{ | ||
|
|
@@ -129,52 +129,69 @@ func main() { | |
|
|
||
| logger.Info("starting server", "addr", srv.Addr, "env", cfg.Env) | ||
| err = srv.ListenAndServe() | ||
| reporter.ReportIfProd(err, map[string]interface{}{ | ||
| "addr": srv.Addr, | ||
| "env": cfg.Env, | ||
| }, sentry.LevelFatal) | ||
| reporter.ReportError(err, sentry.LevelFatal) | ||
| report.FlushSentry() | ||
| logger.Error(err.Error()) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| // validateConfigFlags checks that only one of --config-file, --config-url, or an additional argument is provided. | ||
| func validateConfigFlags(configFile, configURL *string) error{ | ||
| func validateConfigFlags(configFile, configURL *string) error { | ||
| if (*configFile != "" && *configURL != "") || (*configFile != "" && len(flag.Args()) > 0) || (*configURL != "" && len(flag.Args()) > 0) { | ||
| return fmt.Errorf("only one of --config-file or --config-url can be specified") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
|
|
||
| // createCacheDirectory ensures the cache directory exists, creating it if necessary. | ||
| func createCacheDirectory(cacheDir string , logger *slog.Logger) error{ | ||
| stat, err := os.Stat(cacheDir); | ||
| func createCacheDirectory(cacheDir string, logger *slog.Logger, reporter *report.Reporter) error { | ||
| stat, err := os.Stat(cacheDir) | ||
|
|
||
| if err != nil { | ||
| if os.IsNotExist(err){ | ||
| if os.IsNotExist(err) { | ||
| if err := os.MkdirAll(cacheDir, os.ModePerm); err != nil { | ||
| reporter.ReportErrorWithSentryOptions(err, report.SentryReportOptions{ | ||
| Level: sentry.LevelError, | ||
| ExtraContext: map[string]interface{}{ | ||
| "cache_dir": cacheDir, | ||
| }, | ||
| }) | ||
| return err | ||
| } | ||
| return nil | ||
| } | ||
| return err | ||
|
|
||
| } | ||
| if !stat.IsDir() { | ||
| return fmt.Errorf("%s is not a directory", cacheDir) | ||
| err := fmt.Errorf("%s is not a directory", cacheDir) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here on formatting. |
||
| reporter.ReportErrorWithSentryOptions(err, report.SentryReportOptions{ | ||
| Level: sentry.LevelError, | ||
| ExtraContext: map[string]interface{}{ | ||
| "cache_dir": cacheDir, | ||
| }, | ||
| }) | ||
| return err | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // downloadGTFSBundles downloads GTFS bundles for each server and caches them locally. | ||
| func downloadGTFSBundles(servers []models.ObaServer, cacheDir string, logger *slog.Logger) { | ||
| func downloadGTFSBundles(servers []models.ObaServer, cacheDir string, logger *slog.Logger, reporter *report.Reporter) { | ||
| for _, server := range servers { | ||
| hash := sha1.Sum([]byte(server.GtfsUrl)) | ||
| hashStr := hex.EncodeToString(hash[:]) | ||
| cachePath := filepath.Join(cacheDir, fmt.Sprintf("server_%d_%s.zip", server.ID, hashStr)) | ||
|
|
||
| _, err := utils.DownloadGTFSBundle(server.GtfsUrl, cacheDir, server.ID, hashStr) | ||
| if err != nil { | ||
| reporter.ReportErrorWithSentryOptions(err, report.SentryReportOptions{ | ||
| Tags: utils.MakeMap("server_id", fmt.Sprintf("%d", server.ID)), | ||
| ExtraContext: map[string]interface{}{ | ||
| "gtfs_url": server.GtfsUrl, | ||
| }, | ||
| Level: sentry.LevelError, | ||
| }) | ||
| logger.Error("Failed to download GTFS bundle", "server_id", server.ID, "error", err) | ||
| } else { | ||
| logger.Info("Successfully downloaded GTFS bundle", "server_id", server.ID, "path", cachePath) | ||
|
|
@@ -183,19 +200,23 @@ func downloadGTFSBundles(servers []models.ObaServer, cacheDir string, logger *sl | |
| } | ||
|
|
||
| // refreshGTFSBundles periodically downloads GTFS bundles at the specified interval. | ||
| func refreshGTFSBundles(servers []models.ObaServer, cacheDir string, logger *slog.Logger , interval time.Duration) { | ||
| func refreshGTFSBundles(servers []models.ObaServer, cacheDir string, logger *slog.Logger, interval time.Duration, reporter *report.Reporter) { | ||
| for { | ||
| time.Sleep(interval) | ||
| downloadGTFSBundles(servers, cacheDir, logger) | ||
| downloadGTFSBundles(servers, cacheDir, logger, reporter) | ||
| } | ||
| } | ||
|
|
||
| // refreshConfig periodically fetches remote config and updates the application servers. | ||
| func refreshConfig(configURL, configAuthUser, configAuthPass string, app *application, logger *slog.Logger , interval time.Duration) { | ||
| func refreshConfig(configURL, configAuthUser, configAuthPass string, app *application, logger *slog.Logger, interval time.Duration, reporter *report.Reporter) { | ||
| for { | ||
| time.Sleep(interval) | ||
| newServers, err := loadConfigFromURL(configURL, configAuthUser, configAuthPass) | ||
| newServers, err := loadConfigFromURL(configURL, configAuthUser, configAuthPass, reporter) | ||
| if err != nil { | ||
| reporter.ReportErrorWithSentryOptions(err, report.SentryReportOptions{ | ||
| Tags: utils.MakeMap("config_url", configURL), | ||
| Level: sentry.LevelError, | ||
| }) | ||
| logger.Error("Failed to refresh remote config", "error", err) | ||
| continue | ||
| } | ||
|
|
@@ -212,25 +233,36 @@ func (app *application) updateConfig(newServers []models.ObaServer) { | |
| app.config.Servers = newServers | ||
| } | ||
|
|
||
|
|
||
| func loadConfigFromFile(filePath string) ([]models.ObaServer, error) { | ||
| func loadConfigFromFile(filePath string, reporter *report.Reporter) ([]models.ObaServer, error) { | ||
| data, err := os.ReadFile(filePath) | ||
| if err != nil { | ||
| reporter.ReportErrorWithSentryOptions(err, report.SentryReportOptions{ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. formatting |
||
| Tags: utils.MakeMap("file_path", filePath), | ||
| Level: sentry.LevelError, | ||
| }) | ||
| return nil, fmt.Errorf("failed to read config file: %v", err) | ||
| } | ||
|
|
||
| var servers []models.ObaServer | ||
| if err := json.Unmarshal(data, &servers); err != nil { | ||
| reporter.ReportErrorWithSentryOptions(err, report.SentryReportOptions{ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. formatting |
||
| Tags: utils.MakeMap("file_path", filePath), | ||
| Level: sentry.LevelError, | ||
| }) | ||
| return nil, fmt.Errorf("failed to unmarshal JSON: %v", err) | ||
| } | ||
|
|
||
| return servers, nil | ||
| } | ||
|
|
||
| func loadConfigFromURL(url, authUser, authPass string) ([]models.ObaServer, error) { | ||
| func loadConfigFromURL(url, authUser, authPass string, reporter *report.Reporter) ([]models.ObaServer, error) { | ||
| client := &http.Client{} | ||
| req, err := http.NewRequest("GET", url, nil) | ||
| if err != nil { | ||
| reporter.ReportErrorWithSentryOptions(err, report.SentryReportOptions{ | ||
| Tags: utils.MakeMap("config_url", url), | ||
| Level: sentry.LevelError, | ||
| }) | ||
| return nil, fmt.Errorf("failed to create request: %v", err) | ||
| } | ||
|
|
||
|
|
@@ -240,24 +272,40 @@ func loadConfigFromURL(url, authUser, authPass string) ([]models.ObaServer, erro | |
|
|
||
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| reporter.ReportErrorWithSentryOptions(err, report.SentryReportOptions{ | ||
| Tags: utils.MakeMap("config_url", url), | ||
| Level: sentry.LevelError, | ||
| }) | ||
| return nil, fmt.Errorf("failed to fetch remote config: %v", err) | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode != http.StatusOK { | ||
| return nil, fmt.Errorf("remote config returned status: %d", resp.StatusCode) | ||
| statusErr := fmt.Errorf("remote config returned status: %d", resp.StatusCode) | ||
| reporter.ReportErrorWithSentryOptions(statusErr, report.SentryReportOptions{ | ||
| Tags: utils.MakeMap("config_url", url), | ||
| Level: sentry.LevelError, | ||
| }) | ||
| return nil, statusErr | ||
| } | ||
|
|
||
| data, err := io.ReadAll(resp.Body) | ||
| if err != nil { | ||
| reporter.ReportErrorWithSentryOptions(err, report.SentryReportOptions{ | ||
| Tags: utils.MakeMap("config_url", url), | ||
| Level: sentry.LevelError, | ||
| }) | ||
| return nil, fmt.Errorf("failed to read remote config: %v", err) | ||
| } | ||
|
|
||
| var servers []models.ObaServer | ||
| if err := json.Unmarshal(data, &servers); err != nil { | ||
| reporter.ReportErrorWithSentryOptions(err, report.SentryReportOptions{ | ||
| Tags: utils.MakeMap("config_url", url), | ||
| Level: sentry.LevelError, | ||
| }) | ||
| return nil, fmt.Errorf("failed to unmarshal JSON: %v", err) | ||
| } | ||
|
|
||
| return servers, nil | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to run
go fmton this code. the indentation is strange.