feat: Log result of HTTP requests & artifacts load/saves. Closes #8257 (

#8394)

Signed-off-by: Alex Collins <alex_collins@intuit.com>

server/apiserver/accesslog/interceptor.go

@@ -0,0 +1,30 @@
+package accesslog
+
+import (
+	"net/http"
+	"time"
+
+	log "github.com/sirupsen/logrus"
+)
+
+// Interceptor returns a handler that provides access logging.
+//
+// github.com/gorilla/handlers/logging.go
+// https://arunvelsriram.medium.com/simple-golang-http-logging-middleware-315656ff8722
+func Interceptor(h http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t := time.Now()
+
+		rcw := &resultCapturingWriter{ResponseWriter: w}
+
+		h.ServeHTTP(rcw, r)
+
+		log.WithFields(log.Fields{
+			"path":     r.URL.Path, // log the path not the URL, to avoid logging sensitive data that could be in the query params
+			"method":   r.Method,   // log the method, so we can differentiate create/update from get/list
+			"status":   rcw.status,
+			"size":     rcw.size,
+			"duration": time.Since(t),
+		}).Info()
+	})
+}

server/apiserver/accesslog/result_capturing_writer.go

@@ -0,0 +1,29 @@
+package accesslog
+
+import (
+	"net/http"
+)
+
+// resultCapturingWriter captures the size and status code of the response.
+// Because http.response implements http.Flusher, we must do so too, otherwise Watch* methods don't work.
+// We do not implement http.Hijacker, as HTTP/2 requests should not allow it.
+type resultCapturingWriter struct {
+	http.ResponseWriter // MUST also be http.Flusher
+	status              int
+	size                int
+}
+
+func (r *resultCapturingWriter) Write(b []byte) (int, error) {
+	size, err := r.ResponseWriter.Write(b)
+	r.size += size
+	return size, err
+}
+
+func (r *resultCapturingWriter) WriteHeader(v int) {
+	r.ResponseWriter.WriteHeader(v)
+	r.status = v
+}
+
+func (r *resultCapturingWriter) Flush() {
+	r.ResponseWriter.(http.Flusher).Flush()
+}

server/apiserver/argoserver.go

@@ -40,6 +40,7 @@ import (
 	workflowarchivepkg "github.com/argoproj/argo-workflows/v3/pkg/apiclient/workflowarchive"
 	workflowtemplatepkg "github.com/argoproj/argo-workflows/v3/pkg/apiclient/workflowtemplate"
 	"github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
+	"github.com/argoproj/argo-workflows/v3/server/apiserver/accesslog"
 	"github.com/argoproj/argo-workflows/v3/server/artifacts"
 	"github.com/argoproj/argo-workflows/v3/server/auth"
 	"github.com/argoproj/argo-workflows/v3/server/auth/sso"
@@ -299,7 +300,7 @@ func (as *argoServer) newHTTPServer(ctx context.Context, port int, artifactServe
 	mux := http.NewServeMux()
 	httpServer := http.Server{
 		Addr:      endpoint,
-		Handler:   mux,
+		Handler:   accesslog.Interceptor(mux),
 		TLSConfig: as.tlsConfig,
 	}
 	dialOpts := []grpc.DialOption{

server/artifacts/artifact_server.go

@@ -219,7 +219,6 @@ func (a *ArtifactServer) returnArtifact(ctx context.Context, w http.ResponseWrit
 
 	key, _ := art.GetKey()
 	w.Header().Add("Content-Disposition", fmt.Sprintf(`filename="%s"`, path.Base(key)))
-	w.WriteHeader(200)
 
 	http.ServeContent(w, r, "", time.Time{}, file)
 

workflow/artifacts/artifacts.go

@@ -10,6 +10,7 @@ import (
 	"github.com/argoproj/argo-workflows/v3/workflow/artifacts/git"
 	"github.com/argoproj/argo-workflows/v3/workflow/artifacts/hdfs"
 	"github.com/argoproj/argo-workflows/v3/workflow/artifacts/http"
+	"github.com/argoproj/argo-workflows/v3/workflow/artifacts/logging"
 	"github.com/argoproj/argo-workflows/v3/workflow/artifacts/oss"
 	"github.com/argoproj/argo-workflows/v3/workflow/artifacts/raw"
 	"github.com/argoproj/argo-workflows/v3/workflow/artifacts/resource"
@@ -22,6 +23,14 @@ type NewDriverFunc func(ctx context.Context, art *wfv1.Artifact, ri resource.Int
 
 // NewDriver initializes an instance of an artifact driver
 func NewDriver(ctx context.Context, art *wfv1.Artifact, ri resource.Interface) (common.ArtifactDriver, error) {
+	drv, err := newDriver(ctx, art, ri)
+	if err != nil {
+		return nil, err
+	}
+	return logging.New(drv), nil
+
+}
+func newDriver(ctx context.Context, art *wfv1.Artifact, ri resource.Interface) (common.ArtifactDriver, error) {
 	if art.S3 != nil {
 		var accessKey string
 		var secretKey string

workflow/artifacts/logging/driver.go

@@ -0,0 +1,55 @@
+package logging
+
+import (
+	"time"
+
+	log "github.com/sirupsen/logrus"
+
+	wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
+	"github.com/argoproj/argo-workflows/v3/workflow/artifacts/common"
+)
+
+// driver adds a logging interceptor to help diagnose issues with artifacts
+type driver struct {
+	common.ArtifactDriver
+}
+
+func New(d common.ArtifactDriver) common.ArtifactDriver {
+	return &driver{d}
+}
+
+func (d driver) Load(a *wfv1.Artifact, path string) error {
+	t := time.Now()
+	key, _ := a.GetKey()
+	err := d.ArtifactDriver.Load(a, path)
+	log.WithField("artifactName", a.Name).
+		WithField("key", key).
+		WithField("duration", time.Since(t)).
+		WithError(err).
+		Info("Load artifact")
+	return err
+}
+
+func (d driver) Save(path string, a *wfv1.Artifact) error {
+	t := time.Now()
+	key, _ := a.GetKey()
+	err := d.ArtifactDriver.Save(path, a)
+	log.WithField("artifactName", a.Name).
+		WithField("key", key).
+		WithField("duration", time.Since(t)).
+		WithError(err).
+		Info("Save artifact")
+	return err
+}
+
+func (d driver) ListObjects(a *wfv1.Artifact) ([]string, error) {
+	t := time.Now()
+	key, _ := a.GetKey()
+	list, err := d.ArtifactDriver.ListObjects(a)
+	log.WithField("artifactName", a.Name).
+		WithField("key", key).
+		WithField("duration", time.Since(t)).
+		WithError(err).
+		Info("List objects")
+	return list, err
+}