Improve request logging

Use httputil.DumpRequest as this is much cleaner than previous approach
and handles all requests (instead of just /data). With new approach we
can get rid of getInputParam and related tests.

Also, move slack link around.

Fixes #281

README.md

@@ -20,6 +20,6 @@ See [DEVELOPMENT.md](./docs/DEVELOPMENT.md) for development environment setup an
 - License: [Apache Version 2.0](https://raw.githubusercontent.com/open-policy-agent/opa/master/LICENSE)
 - Bugs: [Github Issues](https://github.com/open-policy-agent/opa/issues)
 - Features: [Github Issues](https://github.com/open-policy-agent/opa/issues)
-- Discussions: [![Slack Status](http://slack.openpolicyagent.org/badge.svg)](http://slack.openpolicyagent.org) [Google Groups](https://groups.google.com/forum/?hl=en#!forum/open-policy-agent)
+- Discussions: [Google Groups](https://groups.google.com/forum/?hl=en#!forum/open-policy-agent) [![Slack Status](http://slack.openpolicyagent.org/badge.svg)](http://slack.openpolicyagent.org)
 - Documentation: [Introduction](http://www.openpolicyagent.org/documentation/what-is-policy-enablement/), [Examples](http://www.openpolicyagent.org/examples/working-with-the-opa-repl/), [![GoDoc](https://godoc.org/github.com/open-policy-agent/opa?status.svg)](https://godoc.org/github.com/open-policy-agent/opa)
 - Continuous Integration: [![Build Status](https://travis-ci.org/open-policy-agent/opa.svg?branch=master)](https://travis-ci.org/open-policy-agent/opa) [![Go Report Card](https://goreportcard.com/badge/open-policy-agent/opa)](https://goreportcard.com/report/open-policy-agent/opa)

runtime/logging.go

@@ -7,8 +7,12 @@ package runtime
 import (
 	"net/http"
 	"net/url"
+	"strings"
+	"sync/atomic"
 	"time"
 
+	"net/http/httputil"
+
 	"github.com/golang/glog"
 	"github.com/open-policy-agent/opa/server/types"
 )
@@ -17,35 +21,43 @@ import (
 // containing the request information as well as response status and latency.
 type LoggingHandler struct {
 	inner http.Handler
+	rid   uint64
 }
 
 // NewLoggingHandler returns a new http.Handler.
 func NewLoggingHandler(inner http.Handler) http.Handler {
-	return &LoggingHandler{inner}
+	return &LoggingHandler{inner, uint64(0)}
 }
 
 func (h *LoggingHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	recorder := newRecorder(w)
 	t0 := time.Now()
+	rid := atomic.AddUint64(&h.rid, uint64(1))
+	if glog.V(3) {
+		dump, err := httputil.DumpRequest(r, true)
+		if err != nil {
+			glog.Infof("rid=%v: %v", rid, err)
+		} else {
+			for _, line := range strings.Split(string(dump), "\n") {
+				glog.Infof("rid=%v: %v", rid, line)
+			}
+		}
+	}
 	h.inner.ServeHTTP(recorder, r)
 	if glog.V(2) {
 		dt := time.Since(t0)
 		statusCode := 200
 		if recorder.statusCode != 0 {
 			statusCode = recorder.statusCode
 		}
-		glog.Infof("%v %v %v %v %v %vms",
+		glog.Infof("rid=%v: %v %v %v %v %v %vms",
+			rid,
 			r.RemoteAddr,
 			r.Method,
 			dropInputParam(r.URL),
 			statusCode,
 			recorder.bytesWritten,
 			float64(dt.Nanoseconds())/1e6)
-		if glog.V(3) {
-			for _, g := range getInputParam(r.URL) {
-				glog.Infoln(g)
-			}
-		}
 	}
 }
 
@@ -87,13 +99,3 @@ func dropInputParam(u *url.URL) string {
 	}
 	return u.Path + "?" + cpy.Encode()
 }
-
-func getInputParam(u *url.URL) (r []string) {
-	for _, g := range u.Query()[types.ParamInputV1] {
-		s, err := url.QueryUnescape(g)
-		if len(s) > 0 && err == nil {
-			r = append(r, s)
-		}
-	}
-	return r
-}

runtime/logging_test.go

@@ -7,7 +7,6 @@ package runtime
 import (
 	"fmt"
 	"net/url"
-	"reflect"
 	"testing"
 )
 
@@ -46,43 +45,3 @@ func TestDropInputParam(t *testing.T) {
 	}
 
 }
-
-func TestGetInputParam(t *testing.T) {
-
-	abc := `a.b.c:{"foo":[1,2,3,4]}`
-	def := `d.e.f:{"bar":{"baz":null}}`
-	abcEncoded := url.QueryEscape(abc)
-	defEncoded := url.QueryEscape(def)
-
-	uri, err := url.ParseRequestURI(fmt.Sprintf(`http://localhost:8181/v1/data/foo/bar?input=%v&pretty=true&input=%v`, abcEncoded, defEncoded))
-	if err != nil {
-		panic(err)
-	}
-
-	result := getInputParam(uri)
-	expected := []string{abc, def}
-
-	if !reflect.DeepEqual(result, expected) {
-		t.Errorf("Expected %v but got: %v", expected, result)
-	}
-
-	uri, err = url.ParseRequestURI(fmt.Sprintf(`http://localhost:8181/v1/data/foo?input`))
-	if err != nil {
-		panic(err)
-	}
-
-	result = getInputParam(uri)
-	if len(result) != 0 {
-		t.Errorf("Expected empty result but got: %v", result)
-	}
-
-	uri, err = url.ParseRequestURI(fmt.Sprintf(`http://localhost:8181/v1/data/foo?input=`))
-	if err != nil {
-		panic(err)
-	}
-
-	result = getInputParam(uri)
-	if len(result) != 0 {
-		t.Errorf("Expected empty result but got: %v", result)
-	}
-}