Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ddtrace/tracer: Enable 128bit trace ID logging by default #3086

Closed
wants to merge 9 commits into from
21 changes: 20 additions & 1 deletion contrib/log/slog/slog.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,32 @@ import (
"log/slog"
"strconv"

"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
"gopkg.in/DataDog/dd-trace-go.v1/internal"
"gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry"
)

const componentName = "log/slog"

var cfg = newConfig()

func init() {
telemetry.LoadIntegration(componentName)
tracer.MarkIntegrationImported("log/slog")
}

type config struct {
log128bits bool
}

func newConfig() *config {
return &config{
log128bits: internal.BoolEnv("DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED", true),
}
}

var _ slog.Handler = (*handler)(nil)

type group struct {
Expand Down Expand Up @@ -62,7 +76,12 @@ func (h *handler) Handle(ctx context.Context, rec slog.Record) error {
// set them at the root level.
span, ok := tracer.SpanFromContext(ctx)
if ok {
traceID := strconv.FormatUint(span.Context().TraceID(), 10)
var traceID string
if ctxW3c, ok := span.Context().(ddtrace.SpanContextW3C); ok && cfg.log128bits {
traceID = ctxW3c.TraceID128()
} else {
traceID = strconv.FormatUint(span.Context().TraceID(), 10)
}
spanID := strconv.FormatUint(span.Context().SpanID(), 10)

attrs := []slog.Attr{
Expand Down
70 changes: 65 additions & 5 deletions contrib/log/slog/slog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"encoding/json"
"io"
"log/slog"
"os"
"strconv"
"strings"
"testing"
Expand All @@ -19,12 +20,13 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
internallog "gopkg.in/DataDog/dd-trace-go.v1/internal/log"
)

func assertLogEntry(t *testing.T, rawEntry, wantMsg, wantLevel string, span tracer.Span, assertExtra func(t *testing.T, entry map[string]interface{})) {
func assertLogEntry(t *testing.T, rawEntry, wantMsg, wantLevel string, traceID string, spanID string, assertExtra func(t *testing.T, entry map[string]interface{})) {
t.Helper()

t.Log(rawEntry)
Expand All @@ -38,8 +40,6 @@ func assertLogEntry(t *testing.T, rawEntry, wantMsg, wantLevel string, span trac
assert.Equal(t, wantLevel, entry["level"])
assert.NotEmpty(t, entry["time"])

traceID := strconv.FormatUint(span.Context().TraceID(), 10)
spanID := strconv.FormatUint(span.Context().SpanID(), 10)
assert.Equal(t, traceID, entry[ext.LogKeyTraceID], "trace id not found")
assert.Equal(t, spanID, entry[ext.LogKeySpanID], "span id not found")

Expand All @@ -60,6 +60,18 @@ func testLogger(t *testing.T, createLogger func(b io.Writer) *slog.Logger, asser
span, ctx := tracer.StartSpanFromContext(context.Background(), "test")
defer span.Finish()

var traceID string
spanID := strconv.FormatUint(span.Context().SpanID(), 10)
if log128bits, ok := os.LookupEnv("DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED"); ok && log128bits == "false" {
// Re-initialize to account for race condition between setting env var in the test and reading it in the contrib
cfg = newConfig()
traceID = strconv.FormatUint(span.Context().TraceID(), 10)
} else {
ctxW3c, ok := span.Context().(ddtrace.SpanContextW3C)
assert.True(t, ok)
traceID = ctxW3c.TraceID128()
}

// log a message using the context containing span information
logger.Log(ctx, slog.LevelInfo, "this is an info log with tracing information")
logger.Log(ctx, slog.LevelError, "this is an error log with tracing information")
Expand All @@ -70,10 +82,44 @@ func testLogger(t *testing.T, createLogger func(b io.Writer) *slog.Logger, asser
)
// assert log entries contain trace information
require.Len(t, logs, 2)
assertLogEntry(t, logs[0], "this is an info log with tracing information", "INFO", span, assertExtra)
assertLogEntry(t, logs[1], "this is an error log with tracing information", "ERROR", span, assertExtra)

assertLogEntry(t, logs[0], "this is an info log with tracing information", "INFO", traceID, spanID, assertExtra)
assertLogEntry(t, logs[1], "this is an error log with tracing information", "ERROR", traceID, spanID, assertExtra)
}

// func testLogger128BitDisabled(t *testing.T, createLogger func(b io.Writer) *slog.Logger, assertExtra func(t *testing.T, entry map[string]interface{})) {
// tracer.Start(tracer.WithLogger(internallog.DiscardLogger{}))
// defer tracer.Stop()

// // create the application logger
// var b bytes.Buffer
// logger := createLogger(&b)

// // start a new span
// span, ctx := tracer.StartSpanFromContext(context.Background(), "test")
// defer span.Finish()

// t.Setenv("DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED", "false")
// // Re-initialize to account for race condition between setting env var in the test and reading it in the contrib
// cfg = newConfig()
// // log a message using the context containing span information
// logger.Log(ctx, slog.LevelInfo, "this is an info log with tracing information")
// logger.Log(ctx, slog.LevelError, "this is an error log with tracing information")

// logs := strings.Split(
// strings.TrimRight(b.String(), "\n"),
// "\n",
// )
// // assert log entries contain trace information
// require.Len(t, logs, 2)

// // Assert that 64 bit IDs are logged when 128bit logging is disabled
// traceID := strconv.FormatUint(span.Context().TraceID(), 10)
// spanID := strconv.FormatUint(span.Context().SpanID(), 10)
// assertLogEntry(t, logs[0], "this is an info log with tracing information", "INFO", traceID, spanID, assertExtra)
// assertLogEntry(t, logs[1], "this is an error log with tracing information", "ERROR", traceID, spanID, assertExtra)
// }

func TestNewJSONHandler(t *testing.T) {
testLogger(
t,
Expand Down Expand Up @@ -206,6 +252,20 @@ func TestRecordClone(t *testing.T) {
assert.True(t, foundSentinel)
}

func Test128BitLoggingDisabled(t *testing.T) {
os.Setenv("DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED", "false")
defer os.Unsetenv("DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED")
t.Run("testLogger", func(t *testing.T) {
testLogger(
t,
func(w io.Writer) *slog.Logger {
return slog.New(WrapHandler(slog.NewJSONHandler(w, nil)))
},
nil,
)
})
}

func BenchmarkHandler(b *testing.B) {
span, ctx := tracer.StartSpanFromContext(context.Background(), "test")
defer span.Finish()
Expand Down
4 changes: 2 additions & 2 deletions contrib/net/http/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,8 @@ func TestTraceAndServe(t *testing.T) {
t.Setenv("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "500")

cfg := &ServeConfig{
Service: "service",
Resource: "resource",
Service: "service",
Resource: "resource",
}

handler := func(w http.ResponseWriter, r *http.Request) {
Expand Down
24 changes: 22 additions & 2 deletions contrib/sirupsen/logrus/logrus.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@
package logrus

import (
"strconv"

"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
"gopkg.in/DataDog/dd-trace-go.v1/internal"
"gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry"

"github.com/sirupsen/logrus"
Expand All @@ -24,6 +28,18 @@ func init() {
// DDContextLogHook ensures that any span in the log context is correlated to log output.
type DDContextLogHook struct{}

type config struct {
log128bits bool
}

var cfg = newConfig()

func newConfig() *config {
return &config{
log128bits: internal.BoolEnv("DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED", true),
}
}

// Levels implements logrus.Hook interface, this hook applies to all defined levels
func (d *DDContextLogHook) Levels() []logrus.Level {
return []logrus.Level{logrus.PanicLevel, logrus.FatalLevel, logrus.ErrorLevel, logrus.WarnLevel, logrus.InfoLevel, logrus.DebugLevel, logrus.TraceLevel}
Expand All @@ -35,7 +51,11 @@ func (d *DDContextLogHook) Fire(e *logrus.Entry) error {
if !found {
return nil
}
e.Data[ext.LogKeyTraceID] = span.Context().TraceID()
e.Data[ext.LogKeySpanID] = span.Context().SpanID()
if ctxW3c, ok := span.Context().(ddtrace.SpanContextW3C); ok && cfg.log128bits {
e.Data[ext.LogKeyTraceID] = ctxW3c.TraceID128()
} else {
e.Data[ext.LogKeyTraceID] = strconv.FormatUint(span.Context().TraceID(), 10)
}
e.Data[ext.LogKeySpanID] = strconv.FormatUint(span.Context().SpanID(), 10)
return nil
}
37 changes: 33 additions & 4 deletions contrib/sirupsen/logrus/logrus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,54 @@ package logrus

import (
"context"
"strconv"
"testing"

"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
)

func TestFire(t *testing.T) {
func TestFire128BitEnabled(t *testing.T) {
// By default, trace IDs are logged in 128bit format
tracer.Start()
defer tracer.Stop()
_, sctx := tracer.StartSpanFromContext(context.Background(), "testSpan", tracer.WithSpanID(1234))
sp, sctx := tracer.StartSpanFromContext(context.Background(), "testSpan", tracer.WithSpanID(1234))

hook := &DDContextLogHook{}
e := logrus.NewEntry(logrus.New())
e.Context = sctx
err := hook.Fire(e)
assert.NoError(t, err)

ctxW3c, ok := sp.Context().(ddtrace.SpanContextW3C)
assert.True(t, ok)

assert.Equal(t, ctxW3c.TraceID128(), e.Data["dd.trace_id"])
assert.Equal(t, strconv.FormatUint(sp.Context().SpanID(), 10), e.Data["dd.span_id"])
}

func TestFire128BitDisabled(t *testing.T) {
// By default, trace IDs are logged in 128bit format

t.Setenv("DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED", "false")

// Re-initialize to account for race condition between setting env var in the test and reading it in the contrib
cfg = newConfig()

tracer.Start()
defer tracer.Stop()
id := 1234
_, sctx := tracer.StartSpanFromContext(context.Background(), "testSpan", tracer.WithSpanID(1234))

hook := &DDContextLogHook{}
e := logrus.NewEntry(logrus.New())
e.Context = sctx
err := hook.Fire(e)
assert.NoError(t, err)
assert.Equal(t, uint64(1234), e.Data["dd.trace_id"])
assert.Equal(t, uint64(1234), e.Data["dd.span_id"])

assert.Equal(t, strconv.Itoa(id), e.Data["dd.trace_id"])
assert.Equal(t, strconv.Itoa(id), e.Data["dd.span_id"])
}
6 changes: 4 additions & 2 deletions ddtrace/tracer/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,11 @@ func TestLogFormat(t *testing.T) {
defer stop()
tp.Reset()
tp.Ignore("appsec: ", telemetry.LogPrefix)
tracer.StartSpan("test", ServiceName("test-service"), ResourceName("/"), WithSpanID(12345))
sp := tracer.StartSpan("test", ServiceName("test-service"), ResourceName("/"), WithSpanID(12345))
sCtx, ok := sp.Context().(*spanContext)
assert.True(ok)
assert.Len(tp.Logs(), 1)
assert.Regexp(logPrefixRegexp+` DEBUG: Started Span: dd.trace_id="12345" dd.span_id="12345" dd.parent_id="0", Operation: test, Resource: /, Tags: map.*, map.*`, tp.Logs()[0])
assert.Regexp(logPrefixRegexp+` DEBUG: Started Span: dd.trace_id="`+sCtx.TraceID128()+`" dd.span_id="12345" dd.parent_id="0", Operation: test, Resource: /, Tags: map.*, map.*`, tp.Logs()[0])
}

func TestLogPropagators(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion ddtrace/tracer/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ func (s *span) Format(f fmt.State, c rune) {
}
}
var traceID string
if sharedinternal.BoolEnv("DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED", false) && s.context.traceID.HasUpper() {
if sharedinternal.BoolEnv("DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED", true) && s.context.traceID.HasUpper() {
traceID = s.context.TraceID128()
} else {
traceID = fmt.Sprintf("%d", s.TraceID)
Expand Down
Loading
Loading