From 51efa775a389ada9f2845b5363932f0764afe359 Mon Sep 17 00:00:00 2001
From: Chris Banks <chris.banks@digital.cabinet-office.gov.uk>
Date: Sun, 20 Aug 2023 16:08:24 +0100
Subject: [PATCH] Lightly clean up Sentry event logging.

We're still quite some way from using the Sentry API as intended, but
this at least makes our usage a bit less weird/buggy and more readable.

- Send events to Sentry asynchronously, instead of blocking.
- Allow up to 2 s to finish sending events to Sentry on exit.
- Manage the lifecycle of the sentry.Client, Hub and Scope objects the
  way we're supposed to, rather than reinitialising Sentry every time we
  send an event.
- Remove the unexported methods on our (still somewhat sketchy)
  ReportableError type; these weren't helping readability.

Relevant docs:

- https://docs.sentry.io/platforms/go/usage/
- https://docs.sentry.io/platforms/go/enriching-events/scopes/
- https://docs.sentry.io/platforms/go/concurrency/
- https://docs.sentry.io/platforms/go/panics/
- https://docs.sentry.io/platforms/go/guides/http/
- https://github.com/getsentry/sentry-go/blob/master/_examples/http/main.go
---
 logger/sentry.go | 57 ++++++++++++++++++------------------------------
 main.go          |  6 +++++
 2 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/logger/sentry.go b/logger/sentry.go
index 5c5bb199..b77d20b0 100644
--- a/logger/sentry.go
+++ b/logger/sentry.go
@@ -5,11 +5,13 @@ import (
 	"log"
 	"net"
 	"net/http"
-	"time"
 
 	sentry "github.com/getsentry/sentry-go"
 )
 
+// TODO: use the Sentry API as intended + remove these wonky, reinvented wheels.
+// See https://docs.sentry.io/platforms/go/guides/http/.
+
 type RecoveredError struct {
 	ErrorMessage string
 }
@@ -24,25 +26,14 @@ type ReportableError struct {
 	Response *http.Response
 }
 
-func (re ReportableError) hint() *sentry.EventHint {
-	return &sentry.EventHint{
-		Request:  re.Request,
-		Response: re.Response,
-	}
-}
-
-func (re ReportableError) scope() *sentry.Scope {
-	scope := sentry.NewScope()
-	if re.hint().Request != nil {
-		scope.SetRequest(re.hint().Request)
-	}
-	if re.hint().Response != nil {
-		scope.SetExtra("Response Status", re.hint().Response.Status)
+func InitSentry() {
+	if err := sentry.Init(sentry.ClientOptions{}); err != nil {
+		log.Printf("sentry.Init failed: %v\n", err)
 	}
-	return scope
 }
 
-func (re ReportableError) timeoutError() bool {
+// Timeout returns true if and only if this ReportableError is a timeout.
+func (re ReportableError) Timeout() bool {
 	var oerr *net.OpError
 	if errors.As(re.Error, &oerr) {
 		return oerr.Timeout()
@@ -50,27 +41,21 @@ func (re ReportableError) timeoutError() bool {
 	return false
 }
 
-func (re ReportableError) ignorableError() bool {
-	// We don't want to hear about timeouts. These get visibility elsewhere.
-	return re.timeoutError()
-}
-
+// NotifySentry sends an event to sentry.io. Sentry is configurable via the
+// environment variables SENTRY_ENVIRONMENT, SENTRY_DSN, SENTRY_RELEASE.
 func NotifySentry(re ReportableError) {
-	if re.ignorableError() {
-		return
-	}
-
-	// We don't need to set SENTRY_ENVIRONMENT, SENTRY_DSN or SENTRY_RELEASE
-	// in ClientOptions as they are automatically picked up as env vars.
-	// https://docs.sentry.io/platforms/go/config/
-	client, err := sentry.NewClient(sentry.ClientOptions{})
-
-	if err != nil {
-		log.Printf("router: Sentry initialization failed: %v\n", err)
+	if re.Timeout() {
 		return
 	}
 
-	hub := sentry.NewHub(client, re.scope())
-	hub.CaptureException(re.Error)
-	sentry.Flush(time.Second * 5)
+	hub := sentry.CurrentHub().Clone()
+	hub.WithScope(func(s *sentry.Scope) {
+		if re.Request != nil {
+			s.SetRequest(re.Request)
+		}
+		if re.Response != nil {
+			s.SetExtra("Response Status", re.Response.Status)
+		}
+		hub.CaptureException(re.Error)
+	})
 }
diff --git a/main.go b/main.go
index f2d878e3..d2ca5288 100644
--- a/main.go
+++ b/main.go
@@ -11,6 +11,8 @@ import (
 
 	"github.com/alphagov/router/handlers"
 	router "github.com/alphagov/router/lib"
+	"github.com/alphagov/router/logger"
+	sentry "github.com/getsentry/sentry-go"
 	"github.com/prometheus/client_golang/prometheus"
 )
 
@@ -130,6 +132,10 @@ func main() {
 	if err != nil {
 		log.Fatal(err)
 	}
+
+	logger.InitSentry()
+	defer sentry.Flush(2 * time.Second)
+
 	log.Printf("router: listening for API requests on %v", apiAddr)
 	listenAndServeOrFatal(apiAddr, api, feReadTimeout, feWriteTimeout)
 }