poc

fields.go

@@ -134,5 +134,5 @@ func NamedError(key string, err error) Field {
 	if err == nil {
 		return String(key, "<nil>")
 	}
-	return zap.NamedError(key, &encoders.ErrorEncoder{Source: err})
+	return zap.Inline(&encoders.ErrorEncoder{Key: key, Source: err})
 }

internal/encoders/config.go

@@ -64,18 +64,47 @@ func applyDevConfig(cfg zapcore.EncoderConfig) zapcore.EncoderConfig {
 	return cfg
 }
 
-func BuildEncoder(format output.Format, development bool) (enc zapcore.Encoder) {
+type EncoderOptions struct {
+	// Development enables options for dev environments
+	Development bool
+
+	// RedactErrors enables redaction of error fields, assuming the usage of
+	// cockroachdb/errors
+	RedactErrors bool
+}
+
+func BuildEncoder(format output.Format, opts EncoderOptions) (enc zapcore.Encoder) {
 	config := OpenTelemetryConfig
-	if development {
+	if opts.Development {
 		config = applyDevConfig(config)
 	}
 
+	var e zapcore.Encoder
 	switch format {
 	case output.FormatConsole:
-		return zapcore.NewConsoleEncoder(config)
+		e = zapcore.NewConsoleEncoder(config)
 	case output.FormatJSON:
-		return zapcore.NewJSONEncoder(config)
+		e = zapcore.NewJSONEncoder(config)
 	default:
 		panic("unknown output format")
 	}
+
+	// TODO: This doesn't work, somewhere deep inside zap we lose our wrapper
+	// when the error is encoded. We could just use a env var to control this
+	return &encoderWithOptions{
+		Encoder:      e,
+		RedactErrors: opts.RedactErrors,
+	}
+}
+
+type encoderWithOptions struct {
+	zapcore.Encoder
+	RedactErrors bool
+}
+
+func (e *encoderWithOptions) Clone() zapcore.Encoder {
+	return &encoderWithOptions{
+		Encoder:      e.Encoder.Clone(),
+		RedactErrors: e.RedactErrors,
+	}
 }

internal/encoders/encoders.go

@@ -1,8 +1,11 @@
 package encoders
 
 import (
+	"fmt"
+
 	"go.uber.org/zap/zapcore"
 
+	"github.com/cockroachdb/redact"
 	"github.com/sourcegraph/log/internal/otelfields"
 )
 
@@ -57,9 +60,29 @@ func (fields FieldsObjectEncoder) MarshalLogObject(enc zapcore.ObjectEncoder) er
 }
 
 type ErrorEncoder struct {
+	Key    string
 	Source error
 }
 
-func (l *ErrorEncoder) Error() string {
-	return l.Source.Error()
+var _ zapcore.ObjectMarshaler = &FieldsObjectEncoder{}
+
+func IsErrorEncoder(f zapcore.Field) (error, bool) {
+	if f.Type != zapcore.InlineMarshalerType {
+		return nil, false
+	}
+	if e, ok := f.Interface.(*ErrorEncoder); ok {
+		return e.Source, ok
+	}
+	return nil, false
+}
+
+func (e *ErrorEncoder) MarshalLogObject(enc zapcore.ObjectEncoder) error {
+	fmt.Printf("%T\n", enc)
+
+	if opts, ok := enc.(*encoderWithOptions); ok && opts.RedactErrors {
+		enc.AddString(e.Key, redact.Sprintf("%v", e.Source).Redact().StripMarkers())
+	} else {
+		enc.AddString(e.Key, e.Source.Error())
+	}
+	return nil
 }

internal/encoders/encoders_test.go

@@ -0,0 +1,46 @@
+package encoders_test
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/cockroachdb/errors"
+	"github.com/hexops/autogold/v2"
+	"github.com/stretchr/testify/assert"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
+
+	"github.com/sourcegraph/log"
+	"github.com/sourcegraph/log/internal/encoders"
+	"github.com/sourcegraph/log/output"
+)
+
+func TestIsErrorEncoder(t *testing.T) {
+	for _, f := range []log.Field{
+		log.Error(errors.New("foo")),
+		log.NamedError("named", errors.New("foo")),
+	} {
+		t.Run(f.Key, func(t *testing.T) {
+			e, ok := encoders.IsErrorEncoder(f)
+			assert.True(t, ok)
+			assert.Error(t, e)
+		})
+	}
+
+}
+
+func TestPOC(t *testing.T) {
+	buf := &bytes.Buffer{}
+	l := zap.New(zapcore.NewCore(
+		encoders.BuildEncoder(output.FormatJSON, encoders.EncoderOptions{
+			RedactErrors: true,
+		}),
+		zapcore.AddSync(buf),
+		zapcore.InfoLevel,
+	))
+
+	l.Error("foobar", log.Error(errors.Newf("safe: %s", "unsafe")))
+
+	autogold.Expect(`1686282238571598000 ERROR foobar {"error": "safe: unsafe"}`).Equal(t, strings.TrimSpace(buf.String()))
+}

internal/sinkcores/outputcore/core.go

@@ -20,7 +20,10 @@ func NewCore(
 ) zapcore.Core {
 	newCore := func(level zapcore.LevelEnabler) zapcore.Core {
 		return zapcore.NewCore(
-			encoders.BuildEncoder(format, development),
+			encoders.BuildEncoder(format, encoders.EncoderOptions{
+				Development:  development,
+				RedactErrors: false, // TODO: prop-drill from somewhere
+			}),
 			output,
 			level,
 		)

internal/sinkcores/sentrycore/core.go

@@ -90,13 +90,12 @@ func (c *Core) clone() *Core {
 func (c *Core) With(fields []zapcore.Field) zapcore.Core {
 	c = c.clone()
 	for _, f := range fields {
-		if f.Type == zapcore.ErrorType {
-			// Get original error, which we wrap on ErrorEncoder in log.Error
-			if enc, ok := f.Interface.(*encoders.ErrorEncoder); ok {
-				c.errs = append(c.errs, enc.Source)
-				continue
-			}
+		// Get original error, which we wrap on ErrorEncoder in log.Error
+		if err, ok := encoders.IsErrorEncoder(f); ok {
+			c.errs = append(c.errs, err)
+			continue
 		}
+		// Retain rest of fields as-is
 		c.base.Fields = append(c.base.Fields, f)
 	}
 	return c
@@ -120,22 +119,20 @@ func (c *Core) Write(entry zapcore.Entry, fields []zapcore.Field) error {
 	base.Message = entry.Message
 	base.Level = entry.Level
 
-	errs := make([]error, len(c.errs))
-	copy(errs, c.errs)
+	entryErrs := make([]error, len(c.errs))
+	copy(entryErrs, c.errs)
 
 	for _, f := range fields {
-		if f.Type == zapcore.ErrorType {
-			if enc, ok := f.Interface.(*encoders.ErrorEncoder); ok {
-				// If we find one of our errors, we remove it from the fields so our error reports are not including
-				// their own error as an attribute, which would a useless repetition.
-				errs = append(errs, enc.Source)
-				continue
-			}
+		// Get original error, which we wrap on ErrorEncoder in log.Error
+		if err, ok := encoders.IsErrorEncoder(f); ok {
+			entryErrs = append(entryErrs, err)
+			continue
 		}
+		// Retain rest of fields as-is
 		base.Fields = append(base.Fields, f)
 	}
 
-	for _, err := range errs {
+	for _, err := range entryErrs {
 		errC := errorContext{baseContext: base, Error: err}
 		select {
 		case c.w.C <- &errC: