Merge pull request #304 from pace/dumptripper-stuff

Extend DumpRoundTripper to allow disabling whole prefixes
pace · Apr 7, 2022 · d41da6b · d41da6b
2 parents a64a7c9 + 9d4a15d
commit d41da6b
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 22 deletions.
diff --git a/backend/couchdb/couchdb.go b/backend/couchdb/couchdb.go
@@ -6,12 +6,13 @@ package couchdb
 import "time"
 
 type Config struct {
-	URL                  string        `env:"COUCHDB_URL" envDefault:"http://couchdb:5984/"`
-	User                 string        `env:"COUCHDB_USER" envDefault:"admin"`
-	Password             string        `env:"COUCHDB_PASSWORD" envDefault:"secret"`
-	Database             string        `env:"COUCHDB_DB" envDefault:"test"`
-	DatabaseAutoCreate   bool          `env:"COUCHDB_DB_AUTO_CREATE" envDefault:"true"`
-	HealthCheckKey       string        `env:"COUCHDB_HEALTH_CHECK_KEY" envDefault:"$health_check"`
-	HealthCheckResultTTL time.Duration `env:"COUCHDB_HEALTH_CHECK_RESULT_TTL" envDefault:"10s"`
-	DisableHealthCheck   bool          `env:"COUCHDB_DISABLE_HEALTH_CHECK" envDefault:"false"`
+	URL                   string        `env:"COUCHDB_URL" envDefault:"http://couchdb:5984/"`
+	User                  string        `env:"COUCHDB_USER" envDefault:"admin"`
+	Password              string        `env:"COUCHDB_PASSWORD" envDefault:"secret"`
+	Database              string        `env:"COUCHDB_DB" envDefault:"test"`
+	DatabaseAutoCreate    bool          `env:"COUCHDB_DB_AUTO_CREATE" envDefault:"true"`
+	HealthCheckKey        string        `env:"COUCHDB_HEALTH_CHECK_KEY" envDefault:"$health_check"`
+	HealthCheckResultTTL  time.Duration `env:"COUCHDB_HEALTH_CHECK_RESULT_TTL" envDefault:"10s"`
+	DisableHealthCheck    bool          `env:"COUCHDB_DISABLE_HEALTH_CHECK" envDefault:"false"`
+	DisableRequestLogging bool          `env:"COUCHDB_DB_DISABLE_REQUEST_LOGGING" envDefault:"false"`
 }
diff --git a/backend/couchdb/db.go b/backend/couchdb/db.go
@@ -75,16 +75,18 @@ func Client(cfg *Config) (*kivik.Client, error) {
 	if err != nil {
 		return nil, err
 	}
-
-	chain := transport.Chain(
+	rts := []transport.ChainableRoundTripper{
 		&AuthTransport{
 			Username: cfg.User,
 			Password: cfg.Password,
 		},
 		&transport.JaegerRoundTripper{},
 		transport.NewDumpRoundTripperEnv(),
-		&transport.LoggingRoundTripper{},
-	)
+	}
+	if !cfg.DisableRequestLogging {
+		rts = append(rts, &transport.LoggingRoundTripper{})
+	}
+	chain := transport.Chain(rts...)
 	tr := couchdb.SetTransport(chain)
 	err = client.Authenticate(ctx, tr)
 	if err != nil {

diff --git a/http/transport/dump_round_tripper.go b/http/transport/dump_round_tripper.go
@@ -28,14 +28,16 @@ type DumpRoundTripper struct {
 	DumpBody        bool
 	DumpNoRedact    bool
 
-	BlacklistPrefixes []string
+	BlacklistBodyDumpPrefixes []string
+	BlacklistAnyDumpPrefixes  []string
 }
 
 type DumpRoundTripperOption func(rt *DumpRoundTripper) (*DumpRoundTripper, error)
 
 type dumpRoundTripperConfig struct {
-	Options           []string `env:"HTTP_TRANSPORT_DUMP" envSeparator:"," envDefault:""`
-	BlacklistPrefixes []string `env:"HTTP_TRANSPORT_DUMP_DISABLE_DUMP_BODY_URL_PREFIX" envSeparator:"," envDefault:""`
+	Options                   []string `env:"HTTP_TRANSPORT_DUMP" envSeparator:"," envDefault:""`
+	BlacklistBodyDumpPrefixes []string `env:"HTTP_TRANSPORT_DUMP_DISABLE_DUMP_BODY_URL_PREFIX" envSeparator:"," envDefault:""`
+	BlacklistAnyDumpPrefixes  []string `env:"HTTP_TRANSPORT_DUMP_DISABLE_ALL_URL_PREFIX" envSeparator:"," envDefault:""`
 }
 
 func roundTripConfigViaEnv() DumpRoundTripperOption {
@@ -48,7 +50,8 @@ func roundTripConfigViaEnv() DumpRoundTripperOption {
 		if err := setRoundTripOptions(rt, cfg.Options...); err != nil {
 			return rt, err
 		}
-		rt.BlacklistPrefixes = cfg.BlacklistPrefixes
+		rt.BlacklistBodyDumpPrefixes = cfg.BlacklistBodyDumpPrefixes
+		rt.BlacklistAnyDumpPrefixes = cfg.BlacklistAnyDumpPrefixes
 		return rt, nil
 	}
 }
@@ -131,8 +134,11 @@ func (l DumpRoundTripper) AnyEnabled() bool {
 	return l.DumpRequest || l.DumpResponse || l.DumpRequestHEX || l.DumpResponseHEX
 }
 
-func (l DumpRoundTripper) ContainsBlacklistedPrefix(url *url.URL) bool {
-	for _, prefix := range l.BlacklistPrefixes {
+func (l DumpRoundTripper) ContainsBlacklistedPrefix(url *url.URL, blacklist []string) bool {
+	if len(blacklist) == 0 {
+		return false
+	}
+	for _, prefix := range blacklist {
 		// TODO (juf): Do benchmark and compare against using pre-constructed prefix-tree
 		if strings.HasPrefix(url.String(), prefix) {
 			return true
@@ -150,13 +156,17 @@ func (l *DumpRoundTripper) RoundTrip(req *http.Request) (*http.Response, error)
 		redactor = redact.Ctx(req.Context())
 	}
 
-	dumpBody := !l.ContainsBlacklistedPrefix(req.URL) && l.DumpBody
-
 	// fast path if logging is disabled
 	if !l.AnyEnabled() {
 		return l.Transport().RoundTrip(req)
 	}
 
+	if l.ContainsBlacklistedPrefix(req.URL, l.BlacklistAnyDumpPrefixes) {
+		return l.Transport().RoundTrip(req)
+	}
+
+	dumpBody := !l.ContainsBlacklistedPrefix(req.URL, l.BlacklistBodyDumpPrefixes) && l.DumpBody // move below !l.AnyEnabled to not clutter execution if everything is disabled
+
 	dl := log.Ctx(req.Context()).Debug()
 
 	// request logging

diff --git a/http/transport/dump_round_tripper_test.go b/http/transport/dump_round_tripper_test.go
@@ -35,7 +35,53 @@ func TestNewDumpRoundTripperEnv(t *testing.T) {
 	})
 }
 
-func TestNewDumpRoundTripperEnvDisablePrefixBased(t *testing.T) {
+func TestNewDumpRoundTripperEnvDisablePrefixBasedComplete(t *testing.T) {
+	out := &bytes.Buffer{}
+	ctx := log.Output(out).WithContext(context.Background())
+
+	require.NotPanics(t, func() {
+		defer os.Setenv("HTTP_TRANSPORT_DUMP_DISABLE_ALL_URL_PREFIX", os.Getenv("HTTP_TRANSPORT_DUMP_DISABLE_ALL_URL_PREFIX"))
+		os.Setenv("HTTP_TRANSPORT_DUMP_DISABLE_ALL_URL_PREFIX", "https://please-ignore-me")
+		rt, err := NewDumpRoundTripper(
+			roundTripConfigViaEnv(),
+			RoundTripConfig(
+				DumpRoundTripperOptionRequest,
+				DumpRoundTripperOptionRequestHEX,
+				DumpRoundTripperOptionResponse,
+				DumpRoundTripperOptionResponseHEX,
+				DumpRoundTripperOptionBody,
+			))
+		require.NoError(t, err)
+		assert.NotNil(t, rt)
+
+		req := httptest.NewRequest("GET", "/foo", bytes.NewBufferString("Foo"))
+		req = req.WithContext(ctx)
+		rt.SetTransport(&transportWithResponse{})
+
+		_, err = rt.RoundTrip(req)
+		assert.NoError(t, err)
+
+		assert.Contains(t, out.String(), `"level":"debug"`)
+		assert.Contains(t, out.String(), `"request":"GET /foo HTTP/1.1\r\nHost: example.com\r\n\r\nFoo"`)
+		assert.Contains(t, out.String(), `"request-hex":"474554202f666f6f20485454502f312e310d0a486f73743a206578616d706c652e636f6d0d0a0d0a466f6f"`)
+		assert.Contains(t, out.String(), `"response":"HTTP/0.0 000 status code 0\r\nContent-Length: 0\r\n\r\n"`)
+		assert.Contains(t, out.String(), `"response-hex":"485454502f302e30203030302073746174757320636f646520300d0a436f6e74656e742d4c656e6774683a20300d0a0d0a"`)
+		assert.Contains(t, out.String(), `"message":"HTTP Transport Dump"`)
+
+		out.Reset()
+
+		assert.Equal(t, "", out.String())
+
+		reqWithPrefix := httptest.NewRequest("GET", "https://please-ignore-me.org/foo/", bytes.NewBufferString("Foo"))
+		reqWithPrefix = reqWithPrefix.WithContext(ctx)
+
+		_, err = rt.RoundTrip(reqWithPrefix)
+		assert.NoError(t, err)
+		assert.Empty(t, out.String())
+	})
+}
+
+func TestNewDumpRoundTripperEnvDisablePrefixBasedBody(t *testing.T) {
 	out := &bytes.Buffer{}
 	ctx := log.Output(out).WithContext(context.Background())
 

diff --git a/http/transport/jaeger_round_tripper_test.go b/http/transport/jaeger_round_tripper_test.go
@@ -36,7 +36,7 @@ func TestJaegerRoundTripper(t *testing.T) {
 		exs := []string{`operationName:"GET /foo"`, "numericVal:202"}
 		for _, ex := range exs {
 			if !strings.Contains(spanString, ex) {
-				t.Errorf("Expected %q to be included in span %v", ex, spanString)
+				t.Errorf("Expected %q to be included in span %q", ex, spanString)
 			}
 		}
 	})