Skip to content

Commit

Permalink
Enable 128 bit trace id log injection by default (#5021)
Browse files Browse the repository at this point in the history
* enable 128 bit trace id log injection by default
  • Loading branch information
khanayan123 authored Feb 19, 2025
1 parent b27a955 commit f06fb27
Show file tree
Hide file tree
Showing 10 changed files with 43 additions and 17 deletions.
2 changes: 1 addition & 1 deletion integration-tests/pino/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const logger = require('pino')()
const server = http
.createServer((req, res) => {
const span = tracer.scope().active()
const contextTraceId = span.context().toTraceId()
const contextTraceId = span.context().toTraceId(true)
const contextSpanId = span.context().toSpanId()
logger.info(
{ custom: { trace_id: contextTraceId, span_id: contextSpanId } },
Expand Down
2 changes: 1 addition & 1 deletion packages/datadog-plugin-bunyan/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe('Plugin', () => {
const record = JSON.parse(stream.write.firstCall.args[0].toString())

expect(record.dd).to.deep.include({
trace_id: span.context().toTraceId(),
trace_id: span.context().toTraceId(true),
span_id: span.context().toSpanId()
})
})
Expand Down
6 changes: 3 additions & 3 deletions packages/datadog-plugin-pino/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe('Plugin', () => {
const record = JSON.parse(stream.write.firstCall.args[0].toString())

expect(record.dd).to.deep.include({
trace_id: span.context().toTraceId(),
trace_id: span.context().toTraceId(true),
span_id: span.context().toSpanId()
})

Expand Down Expand Up @@ -184,7 +184,7 @@ describe('Plugin', () => {
const record = JSON.parse(stream.write.firstCall.args[0].toString())

expect(record.dd).to.deep.include({
trace_id: span.context().toTraceId(),
trace_id: span.context().toTraceId(true),
span_id: span.context().toSpanId()
})

Expand All @@ -207,7 +207,7 @@ describe('Plugin', () => {

const record = stream.write.firstCall.args[0].toString()

expect(record).to.match(new RegExp(`trace_id\\W+?${span.context().toTraceId()}`))
expect(record).to.match(new RegExp(`trace_id\\W+?${span.context().toTraceId(true)}`))
expect(record).to.match(new RegExp(`span_id\\W+?${span.context().toSpanId()}`))

expect(record).to.include('message')
Expand Down
12 changes: 6 additions & 6 deletions packages/datadog-plugin-winston/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ describe('Plugin', () => {
it('should not alter the default behavior', () => {
const meta = {
dd: {
trace_id: span.context().toTraceId(),
trace_id: span.context().toTraceId(true),
span_id: span.context().toSpanId()
}
}
Expand Down Expand Up @@ -150,7 +150,7 @@ describe('Plugin', () => {
it('should add the trace identifiers to the default logger', async () => {
const meta = {
dd: {
trace_id: span.context().toTraceId(),
trace_id: span.context().toTraceId(true),
span_id: span.context().toSpanId()
}
}
Expand All @@ -170,7 +170,7 @@ describe('Plugin', () => {

const meta = {
dd: {
trace_id: span.context().toTraceId(),
trace_id: span.context().toTraceId(true),
span_id: span.context().toSpanId()
}
}
Expand All @@ -190,7 +190,7 @@ describe('Plugin', () => {
it('should support errors', async () => {
const meta = {
dd: {
trace_id: span.context().toTraceId(),
trace_id: span.context().toTraceId(true),
span_id: span.context().toSpanId()
}
}
Expand All @@ -216,7 +216,7 @@ describe('Plugin', () => {
transports: [transport, httpTransport]
})
const dd = {
trace_id: span.context().toTraceId(),
trace_id: span.context().toTraceId(true),
span_id: span.context().toSpanId()
}

Expand Down Expand Up @@ -287,7 +287,7 @@ describe('Plugin', () => {

const meta = {
dd: {
trace_id: span.context().toTraceId(),
trace_id: span.context().toTraceId(true),
span_id: span.context().toSpanId()
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/dd-trace/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ class Config {
this._setValue(defaults, 'telemetry.metrics', true)
this._setValue(defaults, 'traceEnabled', true)
this._setValue(defaults, 'traceId128BitGenerationEnabled', true)
this._setValue(defaults, 'traceId128BitLoggingEnabled', false)
this._setValue(defaults, 'traceId128BitLoggingEnabled', true)
this._setValue(defaults, 'tracePropagationExtractFirst', false)
this._setValue(defaults, 'tracePropagationStyle.inject', ['datadog', 'tracecontext', 'baggage'])
this._setValue(defaults, 'tracePropagationStyle.extract', ['datadog', 'tracecontext', 'baggage'])
Expand Down
3 changes: 2 additions & 1 deletion packages/dd-trace/src/opentracing/propagation/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ class LogPropagator {
carrier.dd = {}

if (spanContext) {
if (this._config.traceId128BitLoggingEnabled && spanContext._trace.tags['_dd.p.tid']) {
if (this._config.traceId128BitGenerationEnabled &&
this._config.traceId128BitLoggingEnabled && spanContext._trace.tags['_dd.p.tid']) {
carrier.dd.trace_id = spanContext.toTraceId(true)
} else {
carrier.dd.trace_id = spanContext.toTraceId()
Expand Down
3 changes: 2 additions & 1 deletion packages/dd-trace/src/telemetry/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,8 @@ function updateConfig (changes, config) {
service: 'DD_SERVICE',
clientIpHeader: 'DD_TRACE_CLIENT_IP_HEADER',
'grpc.client.error.statuses': 'DD_GRPC_CLIENT_ERROR_STATUSES',
'grpc.server.error.statuses': 'DD_GRPC_SERVER_ERROR_STATUSES'
'grpc.server.error.statuses': 'DD_GRPC_SERVER_ERROR_STATUSES',
traceId128BitLoggingEnabled: 'DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED'
}

const namesNeedFormatting = new Set(['DD_TAGS', 'peerServiceMapping', 'serviceMapping'])
Expand Down
4 changes: 2 additions & 2 deletions packages/dd-trace/test/config.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ describe('Config', () => {
expect(config).to.have.nested.deep.property('dynamicInstrumentation.redactedIdentifiers', [])
expect(config).to.have.nested.deep.property('dynamicInstrumentation.redactionExcludedIdentifiers', [])
expect(config).to.have.property('traceId128BitGenerationEnabled', true)
expect(config).to.have.property('traceId128BitLoggingEnabled', false)
expect(config).to.have.property('traceId128BitLoggingEnabled', true)
expect(config).to.have.property('spanAttributeSchema', 'v0')
expect(config.grpc.client.error.statuses).to.deep.equal(GRPC_CLIENT_ERROR_STATUSES)
expect(config.grpc.server.error.statuses).to.deep.equal(GRPC_SERVER_ERROR_STATUSES)
Expand Down Expand Up @@ -393,7 +393,7 @@ describe('Config', () => {
{ name: 'telemetry.logCollection', value: true, origin: 'default' },
{ name: 'telemetry.metrics', value: true, origin: 'default' },
{ name: 'traceId128BitGenerationEnabled', value: true, origin: 'default' },
{ name: 'traceId128BitLoggingEnabled', value: false, origin: 'default' },
{ name: 'traceId128BitLoggingEnabled', value: true, origin: 'default' },
{ name: 'tracing', value: true, origin: 'default' },
{ name: 'url', value: undefined, origin: 'default' },
{ name: 'version', value: '', origin: 'default' }
Expand Down
24 changes: 24 additions & 0 deletions packages/dd-trace/test/opentracing/propagation/log.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ describe('LogPropagator', () => {

it('should inject 128-bit trace IDs when enabled', () => {
config.traceId128BitLoggingEnabled = true
config.traceId128BitGenerationEnabled = true

const carrier = {}
const traceId = id('1234567812345678')
Expand All @@ -78,6 +79,7 @@ describe('LogPropagator', () => {

it('should correctly inject 128 bit trace ids when _dd.p.tid is present', () => {
config.traceId128BitLoggingEnabled = true
config.traceId128BitGenerationEnabled = true
const carrier = {}
const traceId = id('4e2a9c1573d240b1a3b7e3c1d4c2f9a7', 16)
const traceIdTag = '8765432187654321'
Expand All @@ -96,6 +98,28 @@ describe('LogPropagator', () => {
})

it('should not inject 128-bit trace IDs when disabled', () => {
config.traceId128BitLoggingEnabled = false
config.traceId128BitGenerationEnabled = true
const carrier = {}
const traceId = id('123', 10)
const traceIdTag = '8765432187654321'
const spanContext = new SpanContext({
traceId,
spanId: id('456', 10)
})

spanContext._trace.tags['_dd.p.tid'] = traceIdTag

propagator.inject(spanContext, carrier)

expect(carrier).to.have.property('dd')
expect(carrier.dd).to.have.property('trace_id', '123')
expect(carrier.dd).to.have.property('span_id', '456')
})

it('should not inject 128-bit trace IDs when 128 bit generation is disabled', () => {
config.traceId128BitLoggingEnabled = true
config.traceId128BitGenerationEnabled = false
const carrier = {}
const traceId = id('123', 10)
const traceIdTag = '8765432187654321'
Expand Down
2 changes: 1 addition & 1 deletion packages/dd-trace/test/plugins/log_plugin.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('LogPlugin', () => {
expect(message.dd).to.contain(config)

// Should have trace/span data when none is active
expect(message.dd).to.have.property('trace_id', span.context().toTraceId())
expect(message.dd).to.have.property('trace_id', span.context().toTraceId(true))
expect(message.dd).to.have.property('span_id', span.context().toSpanId())
})
})
Expand Down

0 comments on commit f06fb27

Please sign in to comment.