Skip to content

Commit

Permalink
[v2] fix(ddtrace/tracer): avoid tests to fail with error "non-constan…
Browse files Browse the repository at this point in the history
…t format string" from upcoming Go 1.24 release (#3048) (#3082)
  • Loading branch information
darccio authored Jan 13, 2025
1 parent 0e34da9 commit 0880b0d
Show file tree
Hide file tree
Showing 11 changed files with 57 additions and 35 deletions.
3 changes: 1 addition & 2 deletions ddtrace/opentelemetry/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"encoding/binary"
"encoding/json"
"errors"
"fmt"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -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...)
Expand Down
30 changes: 15 additions & 15 deletions ddtrace/opentelemetry/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,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"])
Expand All @@ -126,7 +126,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"])
Expand Down Expand Up @@ -164,7 +164,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
Expand Down Expand Up @@ -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]

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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"])
Expand Down Expand Up @@ -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"])
Expand Down Expand Up @@ -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"])
Expand Down Expand Up @@ -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"])
Expand Down Expand Up @@ -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"])
Expand All @@ -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"])
Expand All @@ -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"])
Expand Down Expand Up @@ -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"])
Expand Down Expand Up @@ -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"])
Expand Down
2 changes: 1 addition & 1 deletion ddtrace/tracer/abandonedspans.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,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
Expand Down
2 changes: 1 addition & 1 deletion ddtrace/tracer/otel_dd_mappings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
13 changes: 6 additions & 7 deletions ddtrace/tracer/slog.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion internal/appsec/appsec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
})
Expand Down
2 changes: 1 addition & 1 deletion internal/appsec/emitter/waf/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions internal/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,15 +264,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(LevelError, msg)
printMsg(LevelError, "%v%s", report.err, extra)
}
for k := range erragg {
// compiler-optimized map-clearing post go1.11 (golang/go#20138)
Expand Down
2 changes: 1 addition & 1 deletion internal/telemetry/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
24 changes: 24 additions & 0 deletions internal/telemetry/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
}

0 comments on commit 0880b0d

Please sign in to comment.