cleanups:
All checks were successful
continuous-integration/drone/push Build is passing
continuous-integration/drone/pr Build is passing

* middlewares in correct order now, now always throws 500 on panic
* only adds metrics middleware if metrics serving auth is configured
* only adds metrics serving route if metrics serving auth is configured
This commit is contained in:
Jeffrey Paul 2020-10-03 22:46:48 -07:00
parent 9230048097
commit 687e9accf8
4 changed files with 54 additions and 37 deletions

1
go.mod
View File

@ -3,6 +3,7 @@ module git.eeqj.de/sneak/gohttpserver
go 1.15 go 1.15
require ( require (
github.com/99designs/basicauth-go v0.0.0-20160802081356-2a93ba0f464d
github.com/getsentry/sentry-go v0.7.0 github.com/getsentry/sentry-go v0.7.0
github.com/go-chi/chi v4.1.2+incompatible github.com/go-chi/chi v4.1.2+incompatible
github.com/joho/godotenv v1.3.0 github.com/joho/godotenv v1.3.0

2
go.sum
View File

@ -12,6 +12,8 @@ cloud.google.com/go/pubsub v1.0.1/go.mod h1:R0Gpsv3s54REJCy4fxDixWD93lHJMoZTyQ2k
cloud.google.com/go/storage v1.0.0/go.mod h1:IhtSnM/ZTZV8YYJWCY8RULGVqBDmpoyjwiyrjsg+URw= cloud.google.com/go/storage v1.0.0/go.mod h1:IhtSnM/ZTZV8YYJWCY8RULGVqBDmpoyjwiyrjsg+URw=
contrib.go.opencensus.io/exporter/prometheus v0.1.0/go.mod h1:cGFniUXGZlKRjzOyuZJ6mgB+PgBcCIa79kEKR8YCW+A= contrib.go.opencensus.io/exporter/prometheus v0.1.0/go.mod h1:cGFniUXGZlKRjzOyuZJ6mgB+PgBcCIa79kEKR8YCW+A=
dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
github.com/99designs/basicauth-go v0.0.0-20160802081356-2a93ba0f464d h1:j6oB/WPCigdOkxtuPl1VSIiLpy7Mdsu6phQffbF19Ng=
github.com/99designs/basicauth-go v0.0.0-20160802081356-2a93ba0f464d/go.mod h1:3cARGAK9CfW3HoxCy1a0G4TKrdiKke8ftOMEOHyySYs=
github.com/AndreasBriese/bbloom v0.0.0-20190306092124-e2d15f34fcf9/go.mod h1:bOvUY6CB00SOBii9/FifXqc0awNKxLFCL/+pkDPuyl8= github.com/AndreasBriese/bbloom v0.0.0-20190306092124-e2d15f34fcf9/go.mod h1:bOvUY6CB00SOBii9/FifXqc0awNKxLFCL/+pkDPuyl8=
github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=

View File

@ -65,7 +65,6 @@ func Run(appname, version, buildarch string) int {
}) })
// this does nothing if SENTRY_DSN is unset in env. // this does nothing if SENTRY_DSN is unset in env.
s.enableSentry()
// TODO remove: // TODO remove:
if s.sentryEnabled { if s.sentryEnabled {
@ -75,6 +74,9 @@ func Run(appname, version, buildarch string) int {
s.configure() s.configure()
s.setupLogging() s.setupLogging()
// logging before sentry, because sentry logs
s.enableSentry()
s.databaseURL = viper.GetString("DBURL") s.databaseURL = viper.GetString("DBURL")
s.port = viper.GetInt("PORT") s.port = viper.GetInt("PORT")
@ -82,19 +84,21 @@ func Run(appname, version, buildarch string) int {
} }
func (s *server) enableSentry() { func (s *server) enableSentry() {
sentryDSN := os.Getenv("SENTRY_DSN")
if sentryDSN == "" {
s.sentryEnabled = false s.sentryEnabled = false
if viper.GetString("SENTRY_DSN") == "" {
return return
} }
err := sentry.Init(sentry.ClientOptions{ err := sentry.Init(sentry.ClientOptions{
Dsn: sentryDSN, Dsn: viper.GetString("SENTRY_DSN"),
Release: fmt.Sprintf("%s-%s", s.appname, s.version), Release: fmt.Sprintf("%s-%s", s.appname, s.version),
}) })
if err != nil { if err != nil {
log.Fatal().Err(err).Msg("sentry init failure") log.Fatal().Err(err).Msg("sentry init failure")
return
} }
s.log.Info().Msg("sentry error reporting activated")
s.sentryEnabled = true s.sentryEnabled = true
} }
@ -175,6 +179,9 @@ func (s *server) configure() {
viper.SetDefault("MAINTENANCE_MODE", "false") viper.SetDefault("MAINTENANCE_MODE", "false")
viper.SetDefault("PORT", "8080") viper.SetDefault("PORT", "8080")
viper.SetDefault("DBURL", "") viper.SetDefault("DBURL", "")
viper.SetDefault("SENTRY_DSN", "")
viper.SetDefault("METRICS_USERNAME", "")
viper.SetDefault("METRICS_PASSWORD", "")
if err := viper.ReadInConfig(); err != nil { if err := viper.ReadInConfig(); err != nil {
if _, ok := err.(viper.ConfigFileNotFoundError); ok { if _, ok := err.(viper.ConfigFileNotFoundError); ok {

View File

@ -4,10 +4,12 @@ import (
"net/http" "net/http"
"time" "time"
basicauth "github.com/99designs/basicauth-go"
sentryhttp "github.com/getsentry/sentry-go/http" sentryhttp "github.com/getsentry/sentry-go/http"
"github.com/go-chi/chi" "github.com/go-chi/chi"
"github.com/go-chi/chi/middleware" "github.com/go-chi/chi/middleware"
"github.com/prometheus/client_golang/prometheus/promhttp" "github.com/prometheus/client_golang/prometheus/promhttp"
"github.com/spf13/viper"
) )
func (s *server) routes() { func (s *server) routes() {
@ -20,32 +22,31 @@ func (s *server) routes() {
// can .Use() more than one) will be applied to every request into // can .Use() more than one) will be applied to every request into
// the service. // the service.
s.router.Use(middleware.Recoverer)
s.router.Use(middleware.RequestID) s.router.Use(middleware.RequestID)
s.router.Use(s.LoggingMiddleware()) s.router.Use(s.LoggingMiddleware())
// add metrics middleware only if we can serve them behind auth
if viper.GetString("METRICS_USERNAME") != "" {
s.router.Use(s.MetricsMiddleware()) s.router.Use(s.MetricsMiddleware())
}
// CHANGEME to suit your needs, or pull from config. // CHANGEME to suit your needs, or pull from config.
// timeout for request context: your handlers must finish within // timeout for request context; your handlers must finish within
// this window: // this window:
s.router.Use(middleware.Timeout(60 * time.Second)) s.router.Use(middleware.Timeout(60 * time.Second))
// this adds a sentry reporting middleware if and only if sentry is // this adds a sentry reporting middleware if and only if sentry is
// enabled via setting of SENTRY_DSN in env. this was at the // enabled via setting of SENTRY_DSN in env.
// bottom, but chi requires *all* middlewares applied before any
// routes are, so now it's up here. unfortunately this cannot
// coexist with the normal chi Recoverer handler which prints a nice
// colorful stack trace to the console
if s.sentryEnabled { if s.sentryEnabled {
// Options docs at // Options docs at
// https://docs.sentry.io/platforms/go/guides/http/ // https://docs.sentry.io/platforms/go/guides/http/
sentryHandler := sentryhttp.New(sentryhttp.Options{}) // we set sentry to repanic so that all panics bubble up to the
// Recoverer chi middleware above.
sentryHandler := sentryhttp.New(sentryhttp.Options{
Repanic: true,
})
s.router.Use(sentryHandler.Handle) s.router.Use(sentryHandler.Handle)
// FYI: the sentry panic-catcher seems to set the response
// code to 200.
} else {
// FYI: the chi Recoverer middleware sets the response code
// on panics to 500.
s.router.Use(middleware.Recoverer)
} }
//////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////
@ -58,34 +59,40 @@ func (s *server) routes() {
// if you want to use a general purpose middleware (http.Handler // if you want to use a general purpose middleware (http.Handler
// wrapper) on a specific HandleFunc route, you need to take the // wrapper) on a specific HandleFunc route, you need to take the
// .ServeHTTP of the http.Handler to get its HandleFunc, viz: // .ServeHTTP of the http.Handler to get its HandleFunc, viz:
authMiddleware := s.AuthMiddleware() authMiddleware := s.AuthMiddleware()
s.router.Get( s.router.Get(
"/login", "/login",
authMiddleware(s.handleLogin()).ServeHTTP, authMiddleware(s.handleLogin()).ServeHTTP,
) )
// route that panics for testing
// CHANGEME remove this
s.router.Get(
"/panic",
s.handlePanic(),
)
s.router.Get( s.router.Get(
"/.well-known/healthcheck.json", "/.well-known/healthcheck.json",
s.handleHealthCheck(), s.handleHealthCheck(),
) )
// route that panics for testing // set up authenticated /metrics route:
// CHANGEME remove this if viper.GetString("METRICS_USERNAME") != "" {
metricsAuthMiddleware := basicauth.New(
s.router.Get( "metrics",
"/panic", map[string][]string{
s.handlePanic(), viper.GetString("METRICS_USERNAME"): {
viper.GetString("METRICS_PASSWORD"),
},
},
) )
// CHANGEME you probably want to wrap the following in some kind of
// auth like http basic auth which is easy to set up on your
// rometheus collector
// TODO(sneak): read http basic auth user/pass for /metrics
// out of environment vars
s.router.Get( s.router.Get(
"/metrics", "/metrics",
metricsAuthMiddleware(
http.HandlerFunc(promhttp.Handler().ServeHTTP), http.HandlerFunc(promhttp.Handler().ServeHTTP),
).ServeHTTP,
) )
}
} }