From ec97000169e15de6d5870681498c71084c14f954 Mon Sep 17 00:00:00 2001
From: Karl Skewes <karl.skewes@gmail.com>
Date: Fri, 31 May 2019 20:11:28 +1200
Subject: [PATCH] Add silence ping logging flag

Add ability to silence logging of requests to /ping endpoint, reducing
log clutter

Pros:
- Don't have to change all handlers to set/not set silent ping logging
- Don't have to duplicate `loggingHandler` (this could be preferable yet)

Cons:
- Leaking oauth2proxy logic into `package logger`
- Defining default pingPath in two locations

Alternative:
- Add generic exclude path to `logger.go` and pass in `/ping`.
---
 CHANGELOG.md                        |  1 +
 docs/configuration/configuration.md |  3 +++
 logging_handler.go                  |  5 +++--
 logging_handler_test.go             | 16 +++++++++++----
 main.go                             |  1 +
 options.go                          |  6 ++++++
 pkg/logger/logger.go                | 32 +++++++++++++++++++++++++++++
 7 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8463401c..015a3152 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -31,6 +31,7 @@
 
 ## Changes since v3.2.0
 
+- [#178](https://github.com/pusher/outh2_proxy/pull/178) Add silence ping logging and exclude logging paths flags (@kskewes)
 - [#209](https://github.com/pusher/outh2_proxy/pull/209) Improve docker build caching of layers (@dekimsey)
 - [#186](https://github.com/pusher/oauth2_proxy/pull/186) Make config consistent (@JoelSpeed)
 - [#187](https://github.com/pusher/oauth2_proxy/pull/187) Move root packages to pkg folder (@JoelSpeed)
diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md
index dad9ea10..5fffc432 100644
--- a/docs/configuration/configuration.md
+++ b/docs/configuration/configuration.md
@@ -90,6 +90,7 @@ Usage of oauth2_proxy:
   -set-xauthrequest: set X-Auth-Request-User and X-Auth-Request-Email response headers (useful in Nginx auth_request mode)
   -set-authorization-header: set Authorization Bearer response header (useful in Nginx auth_request mode)
   -signature-key string: GAP-Signature request signature key (algorithm:secretkey)
+  -silence-ping-logging bool: disable logging of requests to ping endpoint (default false) 
   -skip-auth-preflight: will skip authentication for OPTIONS requests
   -skip-auth-regex value: bypass authentication for requests path's that match (may be given multiple times)
   -skip-jwt-bearer-tokens: will skip requests that have verified JWT bearer tokens
@@ -139,6 +140,8 @@ There are three different types of logging: standard, authentication, and HTTP r
 
 Each type of logging has their own configurable format and variables. By default these formats are similar to the Apache Combined Log.
 
+Logging of requests to the `/ping` endpoint can be disabled with `-silence-ping-logging` reducing log volume.
+
 ### Auth Log Format
 Authentication logs are logs which are guaranteed to contain a username or email address of a user attempting to authenticate. These logs are output by default in the below format:
 
diff --git a/logging_handler.go b/logging_handler.go
index b4f829d8..9915e277 100644
--- a/logging_handler.go
+++ b/logging_handler.go
@@ -75,18 +75,19 @@ func (l *responseLogger) Status() int {
 	return l.status
 }
 
-// Size returns teh response size
+// Size returns the response size
 func (l *responseLogger) Size() int {
 	return l.size
 }
 
+// Flush sends any buffered data to the client
 func (l *responseLogger) Flush() {
 	if flusher, ok := l.w.(http.Flusher); ok {
 		flusher.Flush()
 	}
 }
 
-// loggingHandler is the http.Handler implementation for LoggingHandlerTo and its friends
+// loggingHandler is the http.Handler implementation for LoggingHandler
 type loggingHandler struct {
 	handler http.Handler
 }
diff --git a/logging_handler_test.go b/logging_handler_test.go
index fd77e0f5..9d966b39 100644
--- a/logging_handler_test.go
+++ b/logging_handler_test.go
@@ -17,10 +17,17 @@ func TestLoggingHandler_ServeHTTP(t *testing.T) {
 
 	tests := []struct {
 		Format,
-		ExpectedLogMessage string
+		ExpectedLogMessage,
+		Path string
+		SilentPing bool
 	}{
-		{logger.DefaultRequestLoggingFormat, fmt.Sprintf("127.0.0.1 - - [%s] test-server GET - \"/foo/bar\" HTTP/1.1 \"\" 200 4 0.000\n", logger.FormatTimestamp(ts))},
-		{"{{.RequestMethod}}", "GET\n"},
+		{logger.DefaultRequestLoggingFormat, fmt.Sprintf("127.0.0.1 - - [%s] test-server GET - \"/foo/bar\" HTTP/1.1 \"\" 200 4 0.000\n", logger.FormatTimestamp(ts)), "/foo/bar", false},
+		{logger.DefaultRequestLoggingFormat, fmt.Sprintf("127.0.0.1 - - [%s] test-server GET - \"/foo/bar\" HTTP/1.1 \"\" 200 4 0.000\n", logger.FormatTimestamp(ts)), "/foo/bar", true},
+		{logger.DefaultRequestLoggingFormat, fmt.Sprintf("127.0.0.1 - - [%s] test-server GET - \"/ping\" HTTP/1.1 \"\" 200 4 0.000\n", logger.FormatTimestamp(ts)), "/ping", false},
+		{"{{.RequestMethod}}", "GET\n", "/foo/bar", false},
+		{"{{.RequestMethod}}", "GET\n", "/foo/bar", true},
+		{"{{.RequestMethod}}", "GET\n", "/ping", false},
+		{"{{.RequestMethod}}", "", "/ping", true},
 	}
 
 	for _, test := range tests {
@@ -36,9 +43,10 @@ func TestLoggingHandler_ServeHTTP(t *testing.T) {
 
 		logger.SetOutput(buf)
 		logger.SetReqTemplate(test.Format)
+		logger.SetSilentPing(test.SilentPing)
 		h := LoggingHandler(http.HandlerFunc(handler))
 
-		r, _ := http.NewRequest("GET", "/foo/bar", nil)
+		r, _ := http.NewRequest("GET", test.Path, nil)
 		r.RemoteAddr = "127.0.0.1"
 		r.Host = "test-server"
 
diff --git a/main.go b/main.go
index 4ccc25bc..395a55a9 100644
--- a/main.go
+++ b/main.go
@@ -98,6 +98,7 @@ func main() {
 
 	flagSet.Bool("request-logging", true, "Log HTTP requests")
 	flagSet.String("request-logging-format", logger.DefaultRequestLoggingFormat, "Template for HTTP request log lines")
+	flagSet.Bool("silence-ping-logging", false, "Disable logging of requests to ping endpoint")
 
 	flagSet.Bool("auth-logging", true, "Log authentication attempts")
 	flagSet.String("auth-logging-format", logger.DefaultAuthLoggingFormat, "Template for authentication log lines")
diff --git a/options.go b/options.go
index f1ca55d2..69919c3c 100644
--- a/options.go
+++ b/options.go
@@ -103,6 +103,8 @@ type Options struct {
 	StandardLoggingFormat string `flag:"standard-logging-format" cfg:"standard_logging_format" env:"OAUTH2_PROXY_STANDARD_LOGGING_FORMAT"`
 	RequestLogging        bool   `flag:"request-logging" cfg:"request_logging" env:"OAUTH2_PROXY_REQUEST_LOGGING"`
 	RequestLoggingFormat  string `flag:"request-logging-format" cfg:"request_logging_format" env:"OAUTH2_PROXY_REQUEST_LOGGING_FORMAT"`
+	PingPath              string `flag:"ping-path" cfg:"ping_path" env:"OAUTH2_PROXY_PING_PATH"`
+	SilencePingLogging    bool   `flag:"silence-ping-logging" cfg:"silence_ping_logging" env:"OAUTH2_PROXY_SILENCE_PING_LOGGING"`
 	AuthLogging           bool   `flag:"auth-logging" cfg:"auth_logging" env:"OAUTH2_PROXY_LOGGING_AUTH_LOGGING"`
 	AuthLoggingFormat     string `flag:"auth-logging-format" cfg:"auth_logging_format" env:"OAUTH2_PROXY_AUTH_LOGGING_FORMAT"`
 
@@ -165,6 +167,8 @@ func NewOptions() *Options {
 		LoggingMaxBackups:                0,
 		LoggingLocalTime:                 true,
 		LoggingCompress:                  false,
+		PingPath:                         "/ping",
+		SilencePingLogging:               false,
 		StandardLogging:                  true,
 		StandardLoggingFormat:            logger.DefaultStandardLoggingFormat,
 		RequestLogging:                   true,
@@ -567,6 +571,8 @@ func setupLogger(o *Options, msgs []string) []string {
 	logger.SetStandardEnabled(o.StandardLogging)
 	logger.SetAuthEnabled(o.AuthLogging)
 	logger.SetReqEnabled(o.RequestLogging)
+	logger.SetSilentPing(o.SilencePingLogging)
+	logger.SetPingPath(o.PingPath)
 	logger.SetStandardTemplate(o.StandardLoggingFormat)
 	logger.SetAuthTemplate(o.AuthLoggingFormat)
 	logger.SetReqTemplate(o.RequestLoggingFormat)
diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go
index 8e277339..89546131 100644
--- a/pkg/logger/logger.go
+++ b/pkg/logger/logger.go
@@ -88,6 +88,8 @@ type Logger struct {
 	stdEnabled     bool
 	authEnabled    bool
 	reqEnabled     bool
+	silentPing     bool
+	pingPath       string
 	stdLogTemplate *template.Template
 	authTemplate   *template.Template
 	reqTemplate    *template.Template
@@ -101,6 +103,8 @@ func New(flag int) *Logger {
 		stdEnabled:     true,
 		authEnabled:    true,
 		reqEnabled:     true,
+		silentPing:     false,
+		pingPath:       "/ping",
 		stdLogTemplate: template.Must(template.New("std-log").Parse(DefaultStandardLoggingFormat)),
 		authTemplate:   template.Must(template.New("auth-log").Parse(DefaultAuthLoggingFormat)),
 		reqTemplate:    template.Must(template.New("req-log").Parse(DefaultRequestLoggingFormat)),
@@ -177,6 +181,9 @@ func (l *Logger) PrintReq(username, upstream string, req *http.Request, url url.
 		return
 	}
 
+	if url.Path == l.pingPath && l.silentPing {
+		return
+	}
 	duration := float64(time.Now().Sub(ts)) / float64(time.Second)
 
 	if username == "" {
@@ -302,6 +309,20 @@ func (l *Logger) SetReqEnabled(e bool) {
 	l.reqEnabled = e
 }
 
+// SetPingPath sets the ping path.
+func (l *Logger) SetPingPath(s string) {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	l.pingPath = s
+}
+
+// SetSilentPing disables ping request logging.
+func (l *Logger) SetSilentPing(e bool) {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	l.silentPing = e
+}
+
 // SetStandardTemplate sets the template for standard logging.
 func (l *Logger) SetStandardTemplate(t string) {
 	l.mu.Lock()
@@ -365,6 +386,17 @@ func SetReqEnabled(e bool) {
 	std.SetReqEnabled(e)
 }
 
+// SetPingPath sets the healthcheck endpoint path.
+// FIXME: Seems wrong to define this
+func SetPingPath(s string) {
+	std.SetPingPath(s)
+}
+
+// SetSilentPing disables request logging for the ping endpoint.
+func SetSilentPing(e bool) {
+	std.SetSilentPing(e)
+}
+
 // SetStandardTemplate sets the template for standard logging for
 // the standard logger.
 func SetStandardTemplate(t string) {