Restrict history and analytics to logs

Only log history and analytics actions, don't put them in a journal. It looks strange when every time you run `whistory` it adds a new entry to `whistory`. Fixes puppetlabs-toy-chest#652. Signed-off-by: Michael Smith <[email protected]>
MikaelSmith · Dec 10, 2019 · 596ed79 · 596ed79
1 parent 228eba5
commit 596ed79
Show file tree

Hide file tree

Showing 13 changed files with 49 additions and 44 deletions.
diff --git a/api/analytics.go b/api/analytics.go
@@ -25,7 +25,7 @@ import (
 //     Responses:
 //       200:
 //       404: errorResp
-var screenviewHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var screenviewHandler = handler{logOnly: true, fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	var body apitypes.ScreenviewBody
 	if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
 		return badRequestResponse(fmt.Sprintf("Error unmarshalling the request body: %v", err))
@@ -37,4 +37,4 @@ var screenviewHandler handler = func(w http.ResponseWriter, r *http.Request) *er
 		return badRequestResponse(err.Error())
 	}
 	return nil
-}
+}}
diff --git a/api/cache.go b/api/cache.go
@@ -23,7 +23,7 @@ import (
 //     Responses:
 //       200:
 //       500: errorResp
-var cacheHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var cacheHandler = handler{fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	path, errResp := getWashPathFromRequest(r)
 	if errResp != nil {
 		return errResp
@@ -37,4 +37,4 @@ var cacheHandler handler = func(w http.ResponseWriter, r *http.Request) *errorRe
 		return unknownErrorResponse(fmt.Errorf("Could not marshal deleted keys for %v: %v", path, err))
 	}
 	return nil
-}
+}}
diff --git a/api/delete.go b/api/delete.go
@@ -23,7 +23,7 @@ import (
 //       400: errorResp
 //       404: errorResp
 //       500: errorResp
-var deleteHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var deleteHandler = handler{fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	ctx := r.Context()
 	entry, path, errResp := getEntryFromRequest(r)
 	if errResp != nil {
@@ -42,4 +42,4 @@ var deleteHandler handler = func(w http.ResponseWriter, r *http.Request) *errorR
 		return unknownErrorResponse(fmt.Errorf("Could not marshal delete's result for %v: %v", path, err))
 	}
 	return nil
-}
+}}
diff --git a/api/exec.go b/api/exec.go
@@ -60,7 +60,7 @@ type execResponse struct {
 //       400: errorResp
 //       404: errorResp
 //       500: errorResp
-var execHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var execHandler = handler{fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	ctx := r.Context()
 	entry, path, errResp := getEntryFromRequest(r)
 	if errResp != nil {
@@ -123,4 +123,4 @@ var execHandler handler = func(w http.ResponseWriter, r *http.Request) *errorRes
 	sendPacket(ctx, enc, &packet)
 
 	return nil
-}
+}}
diff --git a/api/history.go b/api/history.go
@@ -12,6 +12,7 @@ import (
 	"github.com/gorilla/mux"
 	"github.com/puppetlabs/wash/activity"
 	apitypes "github.com/puppetlabs/wash/api/types"
+	log "github.com/sirupsen/logrus"
 )
 
 // swagger:parameters retrieveHistory getJournal
@@ -39,7 +40,7 @@ type historyParams struct {
 //       400: errorResp
 //       404: errorResp
 //       500: errorResp
-var historyHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var historyHandler = handler{logOnly: true, fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	follow, err := getBoolParam(r.URL, "follow")
 	if err != nil {
 		return err
@@ -83,7 +84,7 @@ var historyHandler handler = func(w http.ResponseWriter, r *http.Request) *error
 		}
 	}
 	return nil
-}
+}}
 
 func writeHistory(ctx context.Context, enc *json.Encoder, history []activity.Journal) *errorResponse {
 	var act apitypes.Activity
@@ -115,7 +116,7 @@ func writeHistory(ctx context.Context, enc *json.Encoder, history []activity.Jou
 //       400: errorResp
 //       404: errorResp
 //       500: errorResp
-var historyEntryHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var historyEntryHandler = handler{logOnly: true, fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	history := activity.History()
 	index := mux.Vars(r)["index"]
 
@@ -133,6 +134,10 @@ var historyEntryHandler handler = func(w http.ResponseWriter, r *http.Request) *
 	}
 
 	journal := history[idx]
+	streamCleanup := func(cleanup func() error) {
+		<-r.Context().Done()
+		log.Printf("API: Journal %v closed by completed context: %v", journal, cleanup())
+	}
 
 	if follow {
 		// Ensure every write is a flush.
@@ -147,7 +152,7 @@ var historyEntryHandler handler = func(w http.ResponseWriter, r *http.Request) *
 		}
 
 		// Ensure the reader is closed when context stops.
-		streamCleanup(r.Context(), "Journal "+journal.String(), func() error {
+		go streamCleanup(func() error {
 			rdr.Cleanup()
 			return rdr.Stop()
 		})
@@ -172,11 +177,11 @@ var historyEntryHandler handler = func(w http.ResponseWriter, r *http.Request) *
 			return journalUnavailableResponse(journal.String(), err.Error())
 		}
 
-		streamCleanup(r.Context(), "Journal "+journal.String(), rdr.Close)
+		go streamCleanup(rdr.Close)
 
 		if _, err := io.Copy(w, rdr); err != nil {
 			return unknownErrorResponse(fmt.Errorf("Could not read journal %v: %v", journal, err))
 		}
 	}
 	return nil
-}
+}}
diff --git a/api/info.go b/api/info.go
@@ -22,7 +22,7 @@ import (
 //       400: errorResp
 //       404: errorResp
 //       500: errorResp
-var infoHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var infoHandler = handler{fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	entry, path, errResp := getEntryFromRequest(r)
 	if errResp != nil {
 		return errResp
@@ -36,4 +36,4 @@ var infoHandler handler = func(w http.ResponseWriter, r *http.Request) *errorRes
 		return unknownErrorResponse(fmt.Errorf("Could not marshal %v: %v", path, err))
 	}
 	return nil
-}
+}}
diff --git a/api/list.go b/api/list.go
@@ -34,7 +34,7 @@ type entryList struct {
 //       400: errorResp
 //       404: errorResp
 //       500: errorResp
-var listHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var listHandler = handler{fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	ctx := r.Context()
 	entry, path, errResp := getEntryFromRequest(r)
 	if errResp != nil {
@@ -71,4 +71,4 @@ var listHandler handler = func(w http.ResponseWriter, r *http.Request) *errorRes
 		return unknownErrorResponse(fmt.Errorf("Could not marshal list results for %v: %v", path, err))
 	}
 	return nil
-}
+}}
diff --git a/api/metadata.go b/api/metadata.go
@@ -30,7 +30,7 @@ type entryMetadata struct {
 //       200: entryMetadata
 //       404: errorResp
 //       500: errorResp
-var metadataHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var metadataHandler = handler{fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	ctx := r.Context()
 	entry, path, errResp := getEntryFromRequest(r)
 	if errResp != nil {
@@ -49,4 +49,4 @@ var metadataHandler handler = func(w http.ResponseWriter, r *http.Request) *erro
 		return unknownErrorResponse(fmt.Errorf("Could not marshal metadata for %v: %v", path, err))
 	}
 	return nil
-}
+}}
diff --git a/api/schema.go b/api/schema.go
@@ -33,7 +33,7 @@ type schemaResponse struct {
 //       400: errorResp
 //       404: errorResp
 //       500: errorResp
-var schemaHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var schemaHandler = handler{fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	entry, path, errResp := getEntryFromRequest(r)
 	if errResp != nil {
 		return errResp
@@ -48,4 +48,4 @@ var schemaHandler handler = func(w http.ResponseWriter, r *http.Request) *errorR
 		return unknownErrorResponse(fmt.Errorf("Could not marshal schema for %v: %v", path, err))
 	}
 	return nil
-}
+}}
diff --git a/api/server.go b/api/server.go
@@ -41,13 +41,22 @@ type octetResponse struct {
 	Reader io.Reader
 }
 
-type handler func(http.ResponseWriter, *http.Request) *errorResponse
+type handler struct {
+	fn      func(http.ResponseWriter, *http.Request) *errorResponse
+	logOnly bool
+}
 
 func (handle handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	activity.Record(r.Context(), "API: %v %v", r.Method, r.URL)
+	var record func(msg string, a ...interface{})
+	if handle.logOnly {
+		record = log.Printf
+	} else {
+		record = func(msg string, a ...interface{}) { activity.Record(r.Context(), msg, a...) }
+	}
+	record("API: %v %v", r.Method, r.URL)
 
-	if err := handle(w, r); err != nil {
-		activity.Record(r.Context(), "API: %v %v: %v", r.Method, r.URL, err)
+	if err := handle.fn(w, r); err != nil {
+		record("API: %v %v: %v", r.Method, r.URL, err)
 		w.WriteHeader(err.statusCode)
 
 		// NOTE: Do not set these headers in the middleware because not
@@ -60,7 +69,7 @@ func (handle handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			log.Warnf("API: Failed writing error response: %v", err)
 		}
 	} else {
-		activity.Record(r.Context(), "API: %v %v complete", r.Method, r.URL)
+		record("API: %v %v complete", r.Method, r.URL)
 	}
 }
 

diff --git a/api/signal.go b/api/signal.go
@@ -26,7 +26,7 @@ import (
 //       400: errorResp
 //       404: errorResp
 //       500: errorResp
-var signalHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var signalHandler = handler{fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	ctx := r.Context()
 	entry, path, errResp := getEntryFromRequest(r)
 	if errResp != nil {
@@ -55,4 +55,4 @@ var signalHandler handler = func(w http.ResponseWriter, r *http.Request) *errorR
 
 	activity.Record(ctx, "API: Signal %v %v", path, body.Signal)
 	return nil
-}
+}}
diff --git a/api/stream.go b/api/stream.go
@@ -25,7 +25,7 @@ import (
 //       200: octetResponse
 //       404: errorResp
 //       500: errorResp
-var streamHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+var streamHandler = handler{fn: func(w http.ResponseWriter, r *http.Request) *errorResponse {
 	entry, path, errResp := getEntryFromRequest(r)
 	if errResp != nil {
 		return errResp
@@ -53,12 +53,15 @@ var streamHandler handler = func(w http.ResponseWriter, r *http.Request) *errorR
 	f.Flush()
 
 	// Ensure it's closed when the context is cancelled.
-	streamCleanup(ctx, "Stream "+path, rdr.Close)
+	go func() {
+		<-ctx.Done()
+		activity.Record(ctx, "API: Stream %v closed by completed context: %v", path, rdr.Close())
+	}()
 
 	// Ensure every write is a flush with streamableResponseWriter.
 	if _, err := io.Copy(&streamableResponseWriter{f}, rdr); err != nil {
 		// Common for copy to error when the caller closes the connection.
 		activity.Record(ctx, "API: Streaming %v errored: %v", path, err)
 	}
 	return nil
-}
+}}
diff --git a/api/streamableResponseWriter.go b/api/streamableResponseWriter.go
@@ -1,11 +1,8 @@
 package api
 
 import (
-	"context"
 	"io"
 	"net/http"
-
-	"github.com/puppetlabs/wash/activity"
 )
 
 // Inspired by Docker's WriteFlusher: https://github.com/moby/moby/blob/17.05.x/pkg/ioutils/writeflusher.go
@@ -23,12 +20,3 @@ func (wf *streamableResponseWriter) Write(b []byte) (n int, err error) {
 	wf.Flush()
 	return n, err
 }
-
-type cleanupFunc = func() error
-
-func streamCleanup(ctx context.Context, desc string, cleanup cleanupFunc) {
-	go func() {
-		<-ctx.Done()
-		activity.Record(ctx, "API: %v closed by completed context: %v", desc, cleanup())
-	}()
-}