fix(ddtrace/tracer): avoid tests to fail with error "non-constant format string" from upcoming Go 1.24 release

ddtrace/opentelemetry/span.go

@@ -9,7 +9,6 @@ import (
 	"encoding/binary"
 	"encoding/json"
 	"errors"
-	"fmt"
 	"strconv"
 	"strings"
 	"sync"
@@ -85,7 +84,7 @@ func (s *span) End(options ...oteltrace.SpanEndOption) {
 		if err == nil {
 			s.DD.SetTag("events", string(b))
 		} else {
-			log.Debug(fmt.Sprintf("Issue marshaling span events; events dropped from span meta\n%v", err))
+			log.Debug("Issue marshaling span events; events dropped from span meta\n%v", err)
 		}
 	}
 	var finishCfg = oteltrace.NewSpanEndConfig(options...)

ddtrace/opentelemetry/span_test.go

@@ -104,7 +104,7 @@ func TestSpanResourceNameDefault(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 	assert.Equal("internal", p[0]["name"])
@@ -127,7 +127,7 @@ func TestSpanSetName(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 	assert.Equal(strings.ToLower("NewName"), p[0]["name"])
@@ -165,7 +165,7 @@ func TestSpanLink(t *testing.T) {
 	tracer.Flush()
 	payload, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	assert.NotNil(payload)
 	assert.Len(payload, 1)    // only one trace
@@ -222,7 +222,7 @@ func TestSpanEnd(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 
@@ -285,7 +285,7 @@ func TestSpanSetStatus(t *testing.T) {
 				tracer.Flush()
 				traces, err := waitForPayload(payloads)
 				if err != nil {
-					t.Fatalf(err.Error())
+					t.Fatal(err.Error())
 				}
 				p := traces[0]
 				// An error description is set IFF the span has an error
@@ -425,7 +425,7 @@ func TestSpanContextWithStartOptions(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 	t.Logf("%v", p[0])
@@ -463,7 +463,7 @@ func TestSpanContextWithStartOptionsPriorityOrder(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 	assert.Equal("persisted_srv", p[0]["service"])
@@ -502,7 +502,7 @@ func TestSpanEndOptionsPriorityOrder(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 	assert.Equal(float64(duration.Nanoseconds()), p[0]["duration"])
@@ -531,7 +531,7 @@ func TestSpanEndOptions(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 	assert.Equal("ctx_srv", p[0]["service"])
@@ -579,7 +579,7 @@ func TestSpanSetAttributes(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 	meta := fmt.Sprintf("%v", p[0]["meta"])
@@ -620,7 +620,7 @@ func TestSpanSetAttributesWithRemapping(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 	assert.Equal("graphql.server.request", p[0]["name"])
@@ -638,7 +638,7 @@ func TestTracerStartOptions(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 	assert.Equal("test_serv", p[0]["service"])
@@ -661,7 +661,7 @@ func TestOperationNameRemapping(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 	assert.Equal("graphql.server.request", p[0]["name"])
@@ -788,7 +788,7 @@ func TestRemapName(t *testing.T) {
 			tracer.Flush()
 			traces, err := waitForPayload(payloads)
 			if err != nil {
-				t.Fatalf(err.Error())
+				t.Fatal(err.Error())
 			}
 			p := traces[0]
 			assert.Equal(test.out, p[0]["name"])
@@ -818,7 +818,7 @@ func TestRemapWithMultipleSetAttributes(t *testing.T) {
 	tracer.Flush()
 	traces, err := waitForPayload(payloads)
 	if err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 	p := traces[0]
 	assert.Equal("overriden.name", p[0]["name"])

ddtrace/tracer/abandonedspans.go

@@ -273,7 +273,7 @@ func (d *abandonedSpansDebugger) log(interval *time.Duration) {
 		log.Warn("Too many abandoned spans. Truncating message.")
 		sb.WriteString("...")
 	}
-	log.Warn(sb.String())
+	log.Warn("%s", sb.String())
 }
 
 // formatAbandonedSpans takes a bucket and returns a human-readable string representing

ddtrace/tracer/option_test.go

@@ -338,7 +338,7 @@ func TestIntegrationEnabled(t *testing.T) {
 		var out contribPkg
 		err := stream.Decode(&out)
 		if err != nil {
-			t.Fatalf(err.Error())
+			t.Fatal(err.Error())
 		}
 		packages = append(packages, out)
 	}

ddtrace/tracer/otel_dd_mappings.go

@@ -98,7 +98,7 @@ func getDDorOtelConfig(configName string) string {
 		} else {
 			v, err := config.remapper(otVal)
 			if err != nil {
-				log.Warn(err.Error())
+				log.Warn("%v", err)
 				telemetryTags := []string{ddPrefix + strings.ToLower(config.dd), otelPrefix + strings.ToLower(config.ot)}
 				telemetry.GlobalClient.Count(telemetry.NamespaceTracers, "otel.env.invalid", 1.0, telemetryTags, true)
 			}

ddtrace/tracer/slog.go

@@ -30,24 +30,23 @@ func (h slogHandler) Enabled(ctx context.Context, lvl slog.Level) bool {
 }
 
 func (h slogHandler) Handle(ctx context.Context, r slog.Record) error {
-	parts := make([]string, 0, 1+len(h.attrs)+r.NumAttrs())
-	parts = append(parts, r.Message)
+	parts := make([]string, 0, len(h.attrs)+r.NumAttrs())
 	parts = append(parts, h.attrs...)
 	r.Attrs(func(a slog.Attr) bool {
 		parts = append(parts, formatAttr(a, h.groups))
 		return true
 	})
 
-	msg := strings.Join(parts, " ")
+	extra := strings.Join(parts, " ")
 	switch r.Level {
 	case slog.LevelDebug:
-		log.Debug(msg)
+		log.Debug("%s %s", r.Message, extra)
 	case slog.LevelInfo:
-		log.Info(msg)
+		log.Info("%s %s", r.Message, extra)
 	case slog.LevelWarn:
-		log.Warn(msg)
+		log.Warn("%s %s", r.Message, extra)
 	case slog.LevelError:
-		log.Error(msg)
+		log.Error("%s %s", r.Message, extra)
 	}
 	return nil
 }

internal/appsec/appsec.go

@@ -222,7 +222,7 @@ func init() {
 		Warn:  log.Warn,
 		Errorf: func(s string, a ...any) error {
 			err := fmt.Errorf(s, a...)
-			log.Error(err.Error())
+			log.Error("%v", err)
 			return err
 		},
 	})

internal/appsec/emitter/waf/run.go

@@ -61,7 +61,7 @@ func (op *ContextOperation) Run(eventReceiver dyngo.Operation, addrs waf.RunAddr
 func RunSimple(ctx context.Context, addrs waf.RunAddressData, errorLog string) error {
 	parent, _ := dyngo.FromContext(ctx)
 	if parent == nil {
-		log.Error(errorLog)
+		log.Error("%s", errorLog)
 		return nil
 	}
 

internal/civisibility/integrations/civisibility_features.go

@@ -224,7 +224,7 @@ func uploadRepositoryChanges() (bytes int64, err error) {
 	hasBeenUnshallowed, err := utils.UnshallowGitRepository()
 	if err != nil || !hasBeenUnshallowed {
 		if err != nil {
-			log.Warn(err.Error())
+			log.Warn("%v", err)
 		}
 		// if unshallowing the repository failed or if there's nothing to unshallow then we try to upload the packfiles from
 		// the initial commit data

internal/log/log.go

@@ -239,15 +239,15 @@ func Flush() {
 
 func flushLocked() {
 	for _, report := range erragg {
-		msg := fmt.Sprintf("%v", report.err)
+		var extra string
 		if report.count > defaultErrorLimit {
-			msg += fmt.Sprintf(", %d+ additional messages skipped (first occurrence: %s)", defaultErrorLimit, report.first.Format(time.RFC822))
+			extra = fmt.Sprintf(", %d+ additional messages skipped (first occurrence: %s)", defaultErrorLimit, report.first.Format(time.RFC822))
 		} else if report.count > 1 {
-			msg += fmt.Sprintf(", %d additional messages skipped (first occurrence: %s)", report.count-1, report.first.Format(time.RFC822))
+			extra = fmt.Sprintf(", %d additional messages skipped (first occurrence: %s)", report.count-1, report.first.Format(time.RFC822))
 		} else {
-			msg += fmt.Sprintf(" (occurred: %s)", report.first.Format(time.RFC822))
+			extra = fmt.Sprintf(" (occurred: %s)", report.first.Format(time.RFC822))
 		}
-		printMsg("ERROR", msg)
+		printMsg("ERROR", "%v%s", report.err, extra)
 	}
 	for k := range erragg {
 		// compiler-optimized map-clearing post go1.11 (golang/go#20138)

internal/telemetry/client.go

@@ -151,7 +151,7 @@ type client struct {
 
 func log(msg string, args ...interface{}) {
 	// Debug level so users aren't spammed with telemetry info.
-	logger.Debug(fmt.Sprintf(LogPrefix+msg, args...))
+	logger.Debug(LogPrefix+msg, args...)
 }
 
 // RegisterAppConfig allows to register a globally-defined application configuration.

internal/telemetry/client_test.go

@@ -17,6 +17,8 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+
+	logging "gopkg.in/DataDog/dd-trace-go.v1/internal/log"
 )
 
 func TestClient(t *testing.T) {
@@ -449,3 +451,25 @@ func TestNoEmptyHeaders(t *testing.T) {
 	assertNotEmpty("Datadog-Container-ID")
 	assertNotEmpty("Datadog-Entity-ID")
 }
+
+func TestTelementryClientLogging(t *testing.T) {
+	var (
+		rl       = new(logging.RecordLogger)
+		oldLevel = logging.GetLevel()
+	)
+	logging.SetLevel(logging.LevelDebug)
+	undo := logging.UseLogger(rl)
+	defer func() {
+		logging.SetLevel(oldLevel)
+		undo()
+	}()
+
+	// We simulate a client that has already started
+	c := &client{
+		started: true,
+	}
+	c.start(nil, NamespaceTracers, true)
+
+	assert := assert.New(t)
+	assert.Contains(rl.Logs()[0], LogPrefix+"attempted to start telemetry client when client has already started - ignoring attempt")
+}