MM-20951 Add sentry support to mattermost-server (#14405)

app/diagnostics.go

@@ -66,6 +66,9 @@ const (
 	TRACK_PLUGINS  = "plugins"
 )
 
+// declaring this as var to allow overriding in tests
+var SENTRY_DSN = "placeholder_sentry_dsn"
+
 func (a *App) SendDailyDiagnostics() {
 	a.sendDailyDiagnostics(false)
 }

app/helper_test.go

@@ -59,6 +59,7 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo
 	}
 	*config.PluginSettings.Directory = filepath.Join(tempWorkspace, "plugins")
 	*config.PluginSettings.ClientDirectory = filepath.Join(tempWorkspace, "webapp")
+	*config.LogSettings.EnableSentry = false // disable error reporting during tests
 	memoryStore.Set(config)
 
 	buffer := &bytes.Buffer{}

app/server.go

@@ -17,11 +17,14 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/getsentry/sentry-go"
+	sentryhttp "github.com/getsentry/sentry-go/http"
 	"github.com/gorilla/mux"
 	"github.com/pkg/errors"
 	"github.com/rs/cors"
 	rudder "github.com/rudderlabs/analytics-go"
 	analytics "github.com/segmentio/analytics-go"
+
 	"github.com/throttled/throttled"
 	"golang.org/x/crypto/acme/autocert"
 
@@ -202,6 +205,20 @@ func NewServer(options ...Option) (*Server, error) {
 	// Use this app logger as the global logger (eventually remove all instances of global logging)
 	mlog.InitGlobalLogger(s.Log)
 
+	if *s.Config().LogSettings.EnableDiagnostics && *s.Config().LogSettings.EnableSentry {
+		if strings.Contains(SENTRY_DSN, "placeholder") {
+			mlog.Warn("Sentry reporting is enabled, but SENTRY_DSN is not set. Disabling reporting.")
+		} else {
+			if err := sentry.Init(sentry.ClientOptions{
+				Dsn:              SENTRY_DSN,
+				Release:          model.BuildHash,
+				AttachStacktrace: true,
+			}); err != nil {
+				mlog.Warn("Sentry could not be initiated, probably bad DSN?", mlog.Err(err))
+			}
+		}
+	}
+
 	if *s.Config().ServiceSettings.EnableOpenTracing {
 		tracer, err := tracing.New()
 		if err != nil {
@@ -433,6 +450,8 @@ func (s *Server) StopHTTPServer() {
 func (s *Server) Shutdown() error {
 	mlog.Info("Stopping Server...")
 
+	defer sentry.Flush(2 * time.Second)
+
 	s.RunOldAppShutdown()
 
 	if s.tracer != nil {
@@ -543,6 +562,14 @@ func (s *Server) Start() error {
 	mlog.Info("Starting Server...")
 
 	var handler http.Handler = s.RootRouter
+
+	if *s.Config().LogSettings.EnableDiagnostics && *s.Config().LogSettings.EnableSentry && !strings.Contains(SENTRY_DSN, "placeholder") {
+		sentryHandler := sentryhttp.New(sentryhttp.Options{
+			Repanic: true,
+		})
+		handler = sentryHandler.Handle(handler)
+	}
+
 	if allowedOrigins := *s.Config().ServiceSettings.AllowCorsFrom; allowedOrigins != "" {
 		exposedCorsHeaders := *s.Config().ServiceSettings.CorsExposedHeaders
 		allowCredentials := *s.Config().ServiceSettings.CorsAllowCredentials

app/server_test.go

@@ -6,15 +6,19 @@ package app
 import (
 	"bufio"
 	"crypto/tls"
+	"fmt"
 	"io/ioutil"
 	"net"
 	"net/http"
+	"net/http/httptest"
 	"os"
 	"path"
 	"strconv"
 	"strings"
 	"testing"
+	"time"
 
+	"github.com/getsentry/sentry-go"
 	"github.com/mattermost/mattermost-server/v5/mlog"
 
 	"github.com/mattermost/mattermost-server/v5/config"
@@ -306,7 +310,13 @@ func TestPanicLog(t *testing.T) {
 		panic("log this panic")
 	})
 
-	s.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ListenAddress = ":0" })
+	testDir, _ := fileutils.FindDir("tests")
+	s.UpdateConfig(func(cfg *model.Config) {
+		*cfg.ServiceSettings.ListenAddress = ":0"
+		*cfg.ServiceSettings.ConnectionSecurity = "TLS"
+		*cfg.ServiceSettings.TLSKeyFile = path.Join(testDir, "tls_test_key.pem")
+		*cfg.ServiceSettings.TLSCertFile = path.Join(testDir, "tls_test_cert.pem")
+	})
 	serverErr := s.Start()
 	require.NoError(t, serverErr)
 
@@ -347,3 +357,100 @@ func TestPanicLog(t *testing.T) {
 		t.Error("Panic was supposed to be logged")
 	}
 }
+
+func TestSentry(t *testing.T) {
+	if testing.Short() {
+		t.SkipNow()
+	}
+
+	client := &http.Client{Timeout: 5 * time.Second, Transport: &http.Transport{
+		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+	}}
+	data1 := make(chan bool, 1)
+
+	server1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Log("Received sentry request for some reason")
+		data1 <- true
+	}))
+	defer server1.Close()
+
+	// make sure we don't report anything when sentry is disabled
+	_, port, _ := net.SplitHostPort(server1.Listener.Addr().String())
+	SENTRY_DSN = fmt.Sprintf("http://test:test@localhost:%s/123", port)
+
+	testDir, _ := fileutils.FindDir("tests")
+	s, err := NewServer(func(server *Server) error {
+		configStore, _ := config.NewFileStore("config.json", true)
+		server.configStore = configStore
+		server.UpdateConfig(func(cfg *model.Config) {
+			*cfg.ServiceSettings.ListenAddress = ":0"
+			*cfg.LogSettings.EnableSentry = false
+			*cfg.ServiceSettings.ConnectionSecurity = "TLS"
+			*cfg.ServiceSettings.TLSKeyFile = path.Join(testDir, "tls_test_key.pem")
+			*cfg.ServiceSettings.TLSCertFile = path.Join(testDir, "tls_test_cert.pem")
+		})
+		return nil
+	})
+
+	require.NoError(t, err)
+	// Route for just panicing
+	s.Router.HandleFunc("/panic", func(writer http.ResponseWriter, request *http.Request) {
+		panic("log this panic")
+	})
+
+	serverErr := s.Start()
+	require.NoError(t, serverErr)
+	defer s.Shutdown()
+	resp, err := client.Get("https://localhost:" + strconv.Itoa(s.ListenAddr.Port) + "/panic")
+	require.Nil(t, resp)
+	require.Error(t, err)
+
+	sentry.Flush(time.Second * 1)
+	select {
+	case <-data1:
+		require.Fail(t, "Sentry received a message, even though it's disabled!")
+	case <-time.After(time.Second * 1):
+		t.Log("Sentry request didn't arrive. Good!")
+	}
+
+	// check successful report
+	data2 := make(chan bool, 1)
+	server2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Log("Received sentry request!")
+		data2 <- true
+	}))
+	defer server2.Close()
+	_, port, _ = net.SplitHostPort(server2.Listener.Addr().String())
+	SENTRY_DSN = fmt.Sprintf("http://test:test@localhost:%s/123", port)
+	s2, err := NewServer(func(server *Server) error {
+		configStore, _ := config.NewFileStore("config.json", true)
+		server.configStore = configStore
+		server.UpdateConfig(func(cfg *model.Config) {
+			*cfg.ServiceSettings.ListenAddress = ":0"
+			*cfg.ServiceSettings.ConnectionSecurity = "TLS"
+			*cfg.ServiceSettings.TLSKeyFile = path.Join(testDir, "tls_test_key.pem")
+			*cfg.ServiceSettings.TLSCertFile = path.Join(testDir, "tls_test_cert.pem")
+			*cfg.LogSettings.EnableSentry = true
+
+		})
+		return nil
+	})
+	require.NoError(t, err)
+	// Route for just panicing
+	s2.Router.HandleFunc("/panic", func(writer http.ResponseWriter, request *http.Request) {
+		panic("log this panic")
+	})
+
+	require.NoError(t, s2.Start())
+	defer s2.Shutdown()
+	resp, err = client.Get("https://localhost:" + strconv.Itoa(s2.ListenAddr.Port) + "/panic")
+	require.Nil(t, resp)
+	require.Error(t, err)
+	sentry.Flush(time.Second * 1)
+	select {
+	case <-data2:
+		t.Log("Sentry request arrived. Good!")
+	case <-time.After(time.Second * 2):
+		require.Fail(t, "Sentry report didn't arrive")
+	}
+}