Fix: nrslog integration with slog.Handler interface

v3/integrations/logcontext-v2/nrslog/example/main.go

@@ -21,7 +21,7 @@ func main() {
 	}
 
 	app.WaitForConnection(time.Second * 5)
-	log := slog.New(nrslog.TextHandler(app, os.Stdout, &slog.HandlerOptions{}))
+	log := slog.New(nrslog.WrapHandler(app, slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{})))
 
 	log.Info("I am a log message")
 

v3/integrations/logcontext-v2/nrslog/go.mod

@@ -2,7 +2,18 @@ module github.com/newrelic/go-agent/v3/integrations/logcontext-v2/nrslog
 
 go 1.21
 
-require github.com/newrelic/go-agent/v3 v3.35.0
+require (
+	github.com/newrelic/go-agent/v3 v3.35.0
+	github.com/pkg/errors v0.9.1
+)
 
+require (
+	golang.org/x/net v0.25.0 // indirect
+	golang.org/x/sys v0.20.0 // indirect
+	golang.org/x/text v0.15.0 // indirect
+	google.golang.org/genproto/googleapis/rpc v0.0.0-20240528184218-531527333157 // indirect
+	google.golang.org/grpc v1.65.0 // indirect
+	google.golang.org/protobuf v1.34.2 // indirect
+)
 
 replace github.com/newrelic/go-agent/v3 => ../../..

v3/integrations/logcontext-v2/nrslog/handler.go

@@ -1,100 +1,47 @@
 package nrslog
 
 import (
+	"bytes"
 	"context"
-	"io"
+	"fmt"
 	"log/slog"
+	"time"
 
 	"github.com/newrelic/go-agent/v3/newrelic"
 )
 
 // NRHandler is an Slog handler that includes logic to implement New Relic Logs in Context
 type NRHandler struct {
 	handler slog.Handler
-	w       *LogWriter
 	app     *newrelic.Application
 	txn     *newrelic.Transaction
 }
 
-// TextHandler creates a wrapped Slog TextHandler, enabling it to both automatically capture logs
-// and to enrich logs locally depending on your logs in context configuration in your New Relic
-// application.
-func TextHandler(app *newrelic.Application, w io.Writer, opts *slog.HandlerOptions) NRHandler {
-	nrWriter := NewWriter(w, app)
-	textHandler := slog.NewTextHandler(nrWriter, opts)
-	wrappedHandler := WrapHandler(app, textHandler)
-	wrappedHandler.addWriter(&nrWriter)
-	return wrappedHandler
-}
-
-// JSONHandler creates a wrapped Slog JSONHandler, enabling it to both automatically capture logs
-// and to enrich logs locally depending on your logs in context configuration in your New Relic
-// application.
-func JSONHandler(app *newrelic.Application, w io.Writer, opts *slog.HandlerOptions) NRHandler {
-	nrWriter := NewWriter(w, app)
-	jsonHandler := slog.NewJSONHandler(nrWriter, opts)
-	wrappedHandler := WrapHandler(app, jsonHandler)
-	wrappedHandler.addWriter(&nrWriter)
-	return wrappedHandler
-}
-
-// WithTransaction creates a new Slog Logger object to be used for logging within a given transaction.
-// Calling this function with a logger having underlying TransactionFromContextHandler handler is a no-op.
-func WithTransaction(txn *newrelic.Transaction, logger *slog.Logger) *slog.Logger {
-	if txn == nil || logger == nil {
-		return logger
-	}
-
-	h := logger.Handler()
-	switch nrHandler := h.(type) {
-	case NRHandler:
-		txnHandler := nrHandler.WithTransaction(txn)
-		return slog.New(txnHandler)
-	default:
-		return logger
-	}
-}
-
-// WithTransaction creates a new Slog Logger object to be used for logging within a given transaction it its found
-// in a context.
-// Calling this function with a logger having underlying TransactionFromContextHandler handler is a no-op.
-func WithContext(ctx context.Context, logger *slog.Logger) *slog.Logger {
-	if ctx == nil {
-		return logger
-	}
-
-	txn := newrelic.FromContext(ctx)
-	return WithTransaction(txn, logger)
+// WithTransactionFromContext creates a wrapped NRHandler, enabling it to automatically reference New Relic
+//
+// Warning: This function is deprecated and will be removed in a future release.
+func WithTransactionFromContext(handler *NRHandler) *NRHandler {
+	return handler
 }
 
 // WrapHandler returns a new handler that is wrapped with New Relic tools to capture
 // log data based on your application's logs in context settings.
-func WrapHandler(app *newrelic.Application, handler slog.Handler) NRHandler {
-	return NRHandler{
+func WrapHandler(app *newrelic.Application, handler slog.Handler) *NRHandler {
+	return &NRHandler{
 		handler: handler,
 		app:     app,
 	}
 }
 
-// addWriter is an internal helper function to append an io.Writer to the NRHandler object
-func (h *NRHandler) addWriter(w *LogWriter) {
-	h.w = w
-}
-
 // WithTransaction returns a new handler that is configured to capture log data
 // and attribute it to a specific transaction.
-func (h *NRHandler) WithTransaction(txn *newrelic.Transaction) NRHandler {
-	handler := NRHandler{
+func (h *NRHandler) WithTransaction(txn *newrelic.Transaction) *NRHandler {
+	handler := &NRHandler{
 		handler: h.handler,
 		app:     h.app,
 		txn:     txn,
 	}
 
-	if h.w != nil {
-		writer := h.w.WithTransaction(txn)
-		handler.addWriter(&writer)
-	}
-
 	return handler
 }
 
@@ -107,7 +54,7 @@ func (h *NRHandler) WithTransaction(txn *newrelic.Transaction) NRHandler {
 // or the method does not take a context.
 // The context is passed so Enabled can use its values
 // to make a decision.
-func (h NRHandler) Enabled(ctx context.Context, lvl slog.Level) bool {
+func (h *NRHandler) Enabled(ctx context.Context, lvl slog.Level) bool {
 	return h.handler.Enabled(ctx, lvl)
 }
 
@@ -120,48 +67,98 @@ func (h NRHandler) Enabled(ctx context.Context, lvl slog.Level) bool {
 // cancellation-related problem.)
 //
 // Handle methods that produce output should observe the following rules:
-//   - If r.Time is the zero time, ignore the time.
-//   - If r.PC is zero, ignore it.
+//   - If r.Time is the zero time, time will not be added to your log print, but a timestamp will be sent to newrelic.
 //   - Attr's values should be resolved.
 //   - If an Attr's key and value are both the zero value, ignore the Attr.
 //     This can be tested with attr.Equal(Attr{}).
 //   - If a group's key is empty, inline the group's Attrs.
 //   - If a group has no Attrs (even if it has a non-empty key),
 //     ignore it.
-func (h NRHandler) Handle(ctx context.Context, record slog.Record) error {
+func (h *NRHandler) Handle(ctx context.Context, record slog.Record) error {
 	attrs := map[string]interface{}{}
 
 	record.Attrs(func(attr slog.Attr) bool {
-		attrs[attr.Key] = attr.Value.Any()
+		// ignore empty attributes
+		if !attr.Equal(slog.Attr{}) {
+			attrs[attr.Key] = attr.Value.Any()
+		}
 		return true
 	})
 
+	// timestamp must be sent to newrelic
+	logTime := record.Time.UnixMilli()
+	if record.Time.IsZero() {
+		logTime = time.Now().UnixMilli()
+	}
+
 	data := newrelic.LogData{
 		Severity:   record.Level.String(),
-		Timestamp:  record.Time.UnixMilli(),
+		Timestamp:  logTime,
 		Message:    record.Message,
 		Attributes: attrs,
 	}
-	if h.txn != nil {
-		h.txn.RecordLog(data)
+
+	// attempt to get the transaction from the context
+	txn := newrelic.FromContext(ctx)
+	if txn == nil {
+		txn = h.txn
+	}
+
+	if txn != nil {
+		txn.RecordLog(data)
 	} else {
 		h.app.RecordLog(data)
 	}
 
-	return h.handler.Handle(ctx, record)
+	var err error
+
+	// enrich log with newrelic metadata
+	// this will always return a valid log message even if an error occurs
+	enrichedRecord, enrichErr := enrichLog(record.Message, h.app, txn)
+	record.Message = enrichedRecord
+	if enrichErr != nil {
+		err = fmt.Errorf("failed to enrich logs with New Relic metadata: %v", enrichErr)
+	}
+	handleErr := h.handler.Handle(ctx, record)
+	if handleErr != nil {
+		if err != nil {
+			err = fmt.Errorf("%w; %w", err, handleErr)
+		} else {
+			err = handleErr
+		}
+	}
+
+	return err
+}
+
+// enrich log always returns a valid log message even if an error occurs
+func enrichLog(record string, app *newrelic.Application, txn *newrelic.Transaction) (string, error) {
+	var buf *bytes.Buffer
+	var err error
+
+	if txn != nil {
+		buf = bytes.NewBuffer([]byte(record))
+		err = newrelic.EnrichLog(buf, newrelic.FromTxn(txn))
+	} else if app != nil {
+		buf = bytes.NewBuffer([]byte(record))
+		err = newrelic.EnrichLog(buf, newrelic.FromApp(app))
+	} else {
+		return record, nil
+	}
+
+	return buf.String(), err
 }
 
 // WithAttrs returns a new Handler whose attributes consist of
 // both the receiver's attributes and the arguments.
 // The Handler owns the slice: it may retain, modify or discard it.
-func (h NRHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
+func (h *NRHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
 	handler := h.handler.WithAttrs(attrs)
-	return NRHandler{
+	return &NRHandler{
 		handler: handler,
 		app:     h.app,
 		txn:     h.txn,
 	}
-
 }
 
 // WithGroup returns a new Handler with the given group appended to
@@ -183,49 +180,11 @@ func (h NRHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
 //	logger.LogAttrs(level, msg, slog.Group("s", slog.Int("a", 1), slog.Int("b", 2)))
 //
 // If the name is empty, WithGroup returns the receiver.
-func (h NRHandler) WithGroup(name string) slog.Handler {
+func (h *NRHandler) WithGroup(name string) slog.Handler {
 	handler := h.handler.WithGroup(name)
-	return NRHandler{
+	return &NRHandler{
 		handler: handler,
 		app:     h.app,
 		txn:     h.txn,
 	}
 }
-
-// NRHandler is an Slog handler that includes logic to implement New Relic Logs in Context.
-// New Relic transaction value is taken from context. It cannot be set directly.
-// This serves as a quality of life improvement for cases where slog.Default global instance is
-// referenced, allowing to use slog methods directly and maintaining New Relic instrumentation.
-type TransactionFromContextHandler struct {
-	NRHandler
-}
-
-// WithTransactionFromContext creates a wrapped NRHandler, enabling it to automatically reference New Relic
-// transaction from context.
-func WithTransactionFromContext(handler NRHandler) TransactionFromContextHandler {
-	return TransactionFromContextHandler{handler}
-}
-
-// Handle handles the Record.
-// It will only be called when Enabled returns true.
-// The Context argument is as for Enabled and NewRelic transaction.
-// Canceling the context should not affect record processing.
-// (Among other things, log messages may be necessary to debug a
-// cancellation-related problem.)
-//
-// Handle methods that produce output should observe the following rules:
-//   - If r.Time is the zero time, ignore the time.
-//   - If r.PC is zero, ignore it.
-//   - Attr's values should be resolved.
-//   - If an Attr's key and value are both the zero value, ignore the Attr.
-//     This can be tested with attr.Equal(Attr{}).
-//   - If a group's key is empty, inline the group's Attrs.
-//   - If a group has no Attrs (even if it has a non-empty key),
-//     ignore it.
-func (h TransactionFromContextHandler) Handle(ctx context.Context, record slog.Record) error {
-	if txn := newrelic.FromContext(ctx); txn != nil {
-		return h.NRHandler.WithTransaction(txn).Handle(ctx, record)
-	}
-
-	return h.NRHandler.Handle(ctx, record)
-}