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

Baggage support #4563

Merged
merged 45 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
1d46ff2
otel baggage support
ida613 Jul 31, 2024
951e4cb
modified otel baggage support
ida613 Jul 31, 2024
d14fe44
bug fix
ida613 Jul 31, 2024
55c9880
removed undefined function
ida613 Jul 31, 2024
0d626b2
unit test
ida613 Jul 31, 2024
87d8174
update implementation
ida613 Aug 1, 2024
f49cb62
added encoding
ida613 Aug 2, 2024
700f022
add span formatting
ida613 Aug 5, 2024
c0f74d8
revert format changes
ida613 Aug 6, 2024
d6672cc
update config options
ida613 Aug 7, 2024
d98648b
updated http encoding
ida613 Aug 13, 2024
7ffc64a
remove comments
ida613 Aug 13, 2024
4880dea
add noop and otel baggage support
ida613 Aug 13, 2024
a4b90c7
Merge branch 'master' into ida613/baggage
ida613 Aug 13, 2024
b2ff71c
updated according to rfc
ida613 Sep 19, 2024
d1a2be3
Merge branch 'master' into ida613/baggage
ida613 Sep 19, 2024
49aaa7a
fix unit test
ida613 Sep 19, 2024
d514bed
Merge branch 'ida613/baggage' of github.com:DataDog/dd-trace-js into …
ida613 Sep 19, 2024
32e91a7
Merge branch 'master' into ida613/baggage
ida613 Sep 19, 2024
8b9b917
update span api
ida613 Sep 20, 2024
de79a4e
logging debugging message
ida613 Sep 20, 2024
dae0866
Merge branch 'master' into ida613/baggage
ida613 Sep 20, 2024
089353f
bug fix
ida613 Sep 23, 2024
ac0f133
remove limits from baggage extraction
ida613 Sep 26, 2024
1ed2206
fix unit test
ida613 Sep 26, 2024
0b753fd
remove unused tests
ida613 Sep 26, 2024
489a81e
Merge branch 'master' into ida613/baggage
ida613 Sep 26, 2024
052e693
add baggage propagation style
ida613 Oct 20, 2024
6359a79
Merge branch 'master' into ida613/baggage
ida613 Oct 20, 2024
78c87d5
fix lint error
ida613 Oct 20, 2024
3719d66
account for when spanId and traceId are missing
ida613 Oct 20, 2024
54d926d
test for undefined traceId and spanId
ida613 Oct 20, 2024
098ac33
remove break statement
ida613 Oct 20, 2024
2809fe4
removed handling of undefined traceId and spandId
ida613 Oct 20, 2024
511fe14
handle baggage propagator
ida613 Oct 21, 2024
7109626
test theory
ida613 Oct 21, 2024
1f3ebc0
Merge branch 'master' into ida613/baggage
ida613 Oct 21, 2024
9f1bfac
bug fix
ida613 Oct 22, 2024
2e6afab
reverse yarn.lock changes
ida613 Oct 22, 2024
cadc687
remove yarn.lock changes
ida613 Oct 22, 2024
97ccd23
Merge branch 'master' into ida613/baggage
ida613 Oct 22, 2024
1249935
revert otel changes
ida613 Oct 23, 2024
c27f768
removed dead code and updated baggage pair limits
ida613 Oct 30, 2024
58d1b7f
Merge branch 'master' into ida613/baggage
ida613 Oct 30, 2024
f898c98
fix linter errors
ida613 Oct 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/datadog-plugin-aws-sdk/test/eventbridge.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('EventBridge', () => {
_traceFlags: {
sampled: 1
},
_baggageItems: {},
'x-datadog-trace-id': traceId,
'x-datadog-parent-id': parentId,
'x-datadog-sampling-priority': '1',
Expand Down
17 changes: 15 additions & 2 deletions packages/dd-trace/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,9 @@ class Config {
this._setValue(defaults, 'appsec.stackTrace.maxDepth', 32)
this._setValue(defaults, 'appsec.stackTrace.maxStackTraces', 2)
this._setValue(defaults, 'appsec.wafTimeout', 5e3) // µs
this._setValue(defaults, 'baggageMaxBytes', 8192)
rochdev marked this conversation as resolved.
Show resolved Hide resolved
this._setValue(defaults, 'baggageMaxItems', 64)
this._setValue(defaults, 'ciVisibilityTestSessionName', '')
this._setValue(defaults, 'clientIpEnabled', false)
this._setValue(defaults, 'clientIpHeader', null)
this._setValue(defaults, 'codeOriginForSpans.enabled', false)
Expand Down Expand Up @@ -501,6 +504,7 @@ class Config {
this._setValue(defaults, 'isManualApiEnabled', false)
this._setValue(defaults, 'ciVisibilityTestSessionName', '')
this._setValue(defaults, 'ciVisAgentlessLogSubmissionEnabled', false)
this._setValue(defaults, 'legacyBaggageEnabled', true)
rochdev marked this conversation as resolved.
Show resolved Hide resolved
this._setValue(defaults, 'logInjection', false)
this._setValue(defaults, 'lookup', undefined)
this._setValue(defaults, 'memcachedCommandEnabled', false)
Expand Down Expand Up @@ -545,8 +549,8 @@ class Config {
this._setValue(defaults, 'traceId128BitGenerationEnabled', true)
this._setValue(defaults, 'traceId128BitLoggingEnabled', false)
this._setValue(defaults, 'tracePropagationExtractFirst', false)
this._setValue(defaults, 'tracePropagationStyle.inject', ['datadog', 'tracecontext'])
this._setValue(defaults, 'tracePropagationStyle.extract', ['datadog', 'tracecontext'])
this._setValue(defaults, 'tracePropagationStyle.inject', ['datadog', 'tracecontext', 'baggage'])
rochdev marked this conversation as resolved.
Show resolved Hide resolved
this._setValue(defaults, 'tracePropagationStyle.extract', ['datadog', 'tracecontext', 'baggage'])
this._setValue(defaults, 'tracePropagationStyle.otelPropagators', false)
this._setValue(defaults, 'tracing', true)
this._setValue(defaults, 'url', undefined)
Expand Down Expand Up @@ -626,6 +630,8 @@ class Config {
DD_TRACE_AGENT_HOSTNAME,
DD_TRACE_AGENT_PORT,
DD_TRACE_AGENT_PROTOCOL_VERSION,
DD_TRACE_BAGGAGE_MAX_BYTES,
DD_TRACE_BAGGAGE_MAX_ITEMS,
DD_TRACE_CLIENT_IP_ENABLED,
DD_TRACE_CLIENT_IP_HEADER,
DD_TRACE_ENABLED,
Expand All @@ -635,6 +641,7 @@ class Config {
DD_TRACE_GIT_METADATA_ENABLED,
DD_TRACE_GLOBAL_TAGS,
DD_TRACE_HEADER_TAGS,
DD_TRACE_LEGACY_BAGGAGE_ENABLED,
DD_TRACE_MEMCACHED_COMMAND_ENABLED,
DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP,
DD_TRACE_PARTIAL_FLUSH_MIN_SPANS,
Expand Down Expand Up @@ -706,6 +713,8 @@ class Config {
this._envUnprocessed['appsec.stackTrace.maxStackTraces'] = DD_APPSEC_MAX_STACK_TRACES
this._setValue(env, 'appsec.wafTimeout', maybeInt(DD_APPSEC_WAF_TIMEOUT))
this._envUnprocessed['appsec.wafTimeout'] = DD_APPSEC_WAF_TIMEOUT
this._setValue(env, 'baggageMaxBytes', DD_TRACE_BAGGAGE_MAX_BYTES)
this._setValue(env, 'baggageMaxItems', DD_TRACE_BAGGAGE_MAX_ITEMS)
this._setBoolean(env, 'clientIpEnabled', DD_TRACE_CLIENT_IP_ENABLED)
this._setString(env, 'clientIpHeader', DD_TRACE_CLIENT_IP_HEADER)
this._setBoolean(env, 'codeOriginForSpans.enabled', DD_CODE_ORIGIN_FOR_SPANS_ENABLED)
Expand Down Expand Up @@ -744,6 +753,7 @@ class Config {
this._setArray(env, 'injectionEnabled', DD_INJECTION_ENABLED)
this._setBoolean(env, 'isAzureFunction', getIsAzureFunction())
this._setBoolean(env, 'isGCPFunction', getIsGCPFunction())
this._setBoolean(env, 'legacyBaggageEnabled', DD_TRACE_LEGACY_BAGGAGE_ENABLED)
this._setBoolean(env, 'logInjection', DD_LOGS_INJECTION)
// Requires an accompanying DD_APM_OBFUSCATION_MEMCACHED_KEEP_COMMAND=true in the agent
this._setBoolean(env, 'memcachedCommandEnabled', DD_TRACE_MEMCACHED_COMMAND_ENABLED)
Expand Down Expand Up @@ -877,6 +887,8 @@ class Config {
this._optsUnprocessed['appsec.wafTimeout'] = options.appsec.wafTimeout
this._setBoolean(opts, 'clientIpEnabled', options.clientIpEnabled)
this._setString(opts, 'clientIpHeader', options.clientIpHeader)
this._setValue(opts, 'baggageMaxBytes', options.baggageMaxBytes)
this._setValue(opts, 'baggageMaxItems', options.baggageMaxItems)
this._setBoolean(opts, 'codeOriginForSpans.enabled', options.codeOriginForSpans?.enabled)
this._setString(opts, 'dbmPropagationMode', options.dbmPropagationMode)
if (options.dogstatsd) {
Expand Down Expand Up @@ -914,6 +926,7 @@ class Config {
}
this._setString(opts, 'iast.telemetryVerbosity', options.iast && options.iast.telemetryVerbosity)
this._setBoolean(opts, 'isCiVisibility', options.isCiVisibility)
this._setBoolean(opts, 'legacyBaggageEnabled', options.legacyBaggageEnabled)
this._setBoolean(opts, 'logInjection', options.logInjection)
this._setString(opts, 'lookup', options.lookup)
this._setBoolean(opts, 'openAiLogsEnabled', options.openAiLogsEnabled)
Expand Down
3 changes: 3 additions & 0 deletions packages/dd-trace/src/noop/span.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ class NoopSpan {
setOperationName (name) { return this }
setBaggageItem (key, value) { return this }
getBaggageItem (key) {}
getAllBaggageItems () {}
removeBaggageItem (key) { return this }
removeAllBaggageItems () { return this }
setTag (key, value) { return this }
addTags (keyValueMap) { return this }
addLink (link) { return this }
Expand Down
74 changes: 63 additions & 11 deletions packages/dd-trace/src/opentracing/propagation/text_map.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,28 @@ class TextMapPropagator {
}
}

_encodeOtelBaggageKey (key) {
let encoded = encodeURIComponent(key)
encoded = encoded.replaceAll('(', '%28')
encoded = encoded.replaceAll(')', '%29')
return encoded
}

_injectBaggageItems (spanContext, carrier) {
spanContext._baggageItems && Object.keys(spanContext._baggageItems).forEach(key => {
carrier[baggagePrefix + key] = String(spanContext._baggageItems[key])
})
if (this._config.legacyBaggageEnabled) {
spanContext._baggageItems && Object.keys(spanContext._baggageItems).forEach(key => {
carrier[baggagePrefix + key] = String(spanContext._baggageItems[key])
})
}
if (this._hasPropagationStyle('inject', 'baggage')) {
if (Object.keys(spanContext._baggageItems).length > this._config.baggageMaxItems) return
const baggage = Object.entries(spanContext._baggageItems)
.map(([key, value]) =>
`${this._encodeOtelBaggageKey(String(key).trim())}=${encodeURIComponent(String(value).trim())}`).join(',')
const buf = Buffer.from(baggage)
if (buf.length > this._config.baggageMaxBytes) return
if (baggage) carrier.baggage = baggage
}
}

_injectTags (spanContext, carrier) {
Expand Down Expand Up @@ -299,6 +317,11 @@ class TextMapPropagator {
default:
log.warn(`Unknown propagation style: ${extractor}`)
}

if (this._config.tracePropagationStyle.extract.includes('baggage') && carrier.baggage) {
spanContext = spanContext || new DatadogSpanContext()
this._extractBaggageItems(carrier, spanContext)
}
}

return spanContext || this._extractSqsdContext(carrier)
Expand All @@ -310,7 +333,7 @@ class TextMapPropagator {
if (!spanContext) return spanContext

this._extractOrigin(carrier, spanContext)
this._extractBaggageItems(carrier, spanContext)
this._extractLegacyBaggageItems(carrier, spanContext)
this._extractSamplingPriority(carrier, spanContext)
this._extractTags(carrier, spanContext)

Expand Down Expand Up @@ -444,7 +467,7 @@ class TextMapPropagator {
}
})

this._extractBaggageItems(carrier, spanContext)
this._extractLegacyBaggageItems(carrier, spanContext)
return spanContext
}
return null
Expand Down Expand Up @@ -528,14 +551,43 @@ class TextMapPropagator {
}
}

_extractBaggageItems (carrier, spanContext) {
Object.keys(carrier).forEach(key => {
const match = key.match(baggageExpr)
_decodeOtelBaggageKey (key) {
let decoded = decodeURIComponent(key)
decoded = decoded.replaceAll('%28', '(')
decoded = decoded.replaceAll('%29', ')')
return decoded
}

_extractLegacyBaggageItems (carrier, spanContext) {
if (this._config.legacyBaggageEnabled) {
Object.keys(carrier).forEach(key => {
const match = key.match(baggageExpr)

if (match) {
spanContext._baggageItems[match[1]] = carrier[key]
if (match) {
spanContext._baggageItems[match[1]] = carrier[key]
}
})
}
}

_extractBaggageItems (carrier, spanContext) {
const baggages = carrier.baggage.split(',')
for (const keyValue of baggages) {
if (!keyValue.includes('=')) {
spanContext._baggageItems = {}
return
}
})
let [key, value] = keyValue.split('=')
key = this._decodeOtelBaggageKey(key.trim())
value = decodeURIComponent(value.trim())
if (!key || !value) {
spanContext._baggageItems = {}
return
}
// the current code assumes precedence of ot-baggage- (legacy opentracing baggage) over baggage
if (key in spanContext._baggageItems) return
spanContext._baggageItems[key] = value
}
}

_extractSamplingPriority (carrier, spanContext) {
Expand Down
12 changes: 12 additions & 0 deletions packages/dd-trace/src/opentracing/span.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,18 @@ class DatadogSpan {
return this._spanContext._baggageItems[key]
}

getAllBaggageItems () {
return JSON.stringify(this._spanContext._baggageItems)
}

removeBaggageItem (key) {
delete this._spanContext._baggageItems[key]
}

removeAllBaggageItems () {
this._spanContext._baggageItems = {}
}

setTag (key, value) {
this._addTags({ [key]: value })
return this
Expand Down
2 changes: 2 additions & 0 deletions packages/dd-trace/src/opentracing/span_context.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@ class DatadogSpanContext {
? this._trace.tags[TRACE_ID_128] + this._traceId.toString(16).padStart(16, '0')
: this._traceId.toString(16).padStart(32, '0')
}
// if (!this._traceId) return ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code should be removed and not just commented out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

return this._traceId.toString(10)
}

toSpanId (get128bitId = false) {
if (get128bitId) {
return this._spanId.toString(16).padStart(16, '0')
}
// if (!this._spanId) return ''
return this._spanId.toString(10)
}

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 @@ -229,8 +229,8 @@ describe('Config', () => {
expect(config).to.have.property('spanRemoveIntegrationFromService', false)
expect(config).to.have.property('instrumentation_config_id', undefined)
expect(config).to.have.deep.property('serviceMapping', {})
expect(config).to.have.nested.deep.property('tracePropagationStyle.inject', ['datadog', 'tracecontext'])
expect(config).to.have.nested.deep.property('tracePropagationStyle.extract', ['datadog', 'tracecontext'])
expect(config).to.have.nested.deep.property('tracePropagationStyle.inject', ['datadog', 'tracecontext', 'baggage'])
expect(config).to.have.nested.deep.property('tracePropagationStyle.extract', ['datadog', 'tracecontext', 'baggage'])
expect(config).to.have.nested.property('experimental.runtimeId', false)
expect(config).to.have.nested.property('experimental.exporter', undefined)
expect(config).to.have.nested.property('experimental.enableGetRumData', false)
Expand Down
102 changes: 93 additions & 9 deletions packages/dd-trace/test/opentracing/propagation/text_map.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ describe('TextMapPropagator', () => {
textMap = {
'x-datadog-trace-id': '123',
'x-datadog-parent-id': '456',
'ot-baggage-foo': 'bar'
'ot-baggage-foo': 'bar',
baggage: 'foo=bar'
}
baggageItems = {}
})
Expand All @@ -67,25 +68,60 @@ describe('TextMapPropagator', () => {
expect(carrier).to.have.property('x-datadog-trace-id', '123')
expect(carrier).to.have.property('x-datadog-parent-id', '456')
expect(carrier).to.have.property('ot-baggage-foo', 'bar')
expect(carrier).to.have.property('baggage', 'foo=bar')
})

it('should handle non-string values', () => {
const carrier = {}
const spanContext = createContext({
baggageItems: {
number: 1.23,
bool: true,
array: ['foo', 'bar'],
object: {}
}
})
const baggageItems = {
number: 1.23,
bool: true,
array: ['foo', 'bar'],
object: {}
}
const spanContext = createContext({ baggageItems })

propagator.inject(spanContext, carrier)

expect(carrier['ot-baggage-number']).to.equal('1.23')
expect(carrier['ot-baggage-bool']).to.equal('true')
expect(carrier['ot-baggage-array']).to.equal('foo,bar')
expect(carrier['ot-baggage-object']).to.equal('[object Object]')
expect(carrier.baggage).to.be.equal('number=1.23,bool=true,array=foo%2Cbar,object=%5Bobject%20Object%5D')
lucaspimentel marked this conversation as resolved.
Show resolved Hide resolved
})

it('should handle special characters in baggage', () => {
const carrier = {}
const baggageItems = {
'",;\\()/:<=>?@[]{}': '",;\\'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add non-ASCII characters like é or or 🐶 to ensure those are encoded correctly, too?

(I thought I added this comment before, but I don't see it. Sorry if it's a duplicate.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops merged before I saw your comments, will create a separate PR to address your comments

}
const spanContext = createContext({ baggageItems })

propagator.inject(spanContext, carrier)
expect(carrier.baggage).to.be.equal('%22%2C%3B%5C%28%29%2F%3A%3C%3D%3E%3F%40%5B%5D%7B%7D=%22%2C%3B%5C')
})

it('should not inject baggage when it contains too many key-value pairs', () => {
const carrier = {}
const baggageItems = {}
for (let i = 0; i < config.baggageMaxItems + 1; i++) {
baggageItems[`key-${i}`] = i
}
const spanContext = createContext({ baggageItems })

propagator.inject(spanContext, carrier)
expect(carrier.baggage).to.be.undefined
})

it('should not inject baggage when it contains too many bytes', () => {
const carrier = {}
const baggageItems = {
foo: Buffer.alloc(config.baggageMaxBytes).toString()
}
const spanContext = createContext({ baggageItems })

propagator.inject(spanContext, carrier)
expect(carrier.baggage).to.be.undefined
})

it('should inject an existing sampling priority', () => {
Expand Down Expand Up @@ -353,9 +389,57 @@ describe('TextMapPropagator', () => {
expect(spanContext.toTraceId()).to.equal(carrier['x-datadog-trace-id'])
expect(spanContext.toSpanId()).to.equal(carrier['x-datadog-parent-id'])
expect(spanContext._baggageItems.foo).to.equal(carrier['ot-baggage-foo'])
expect(spanContext._baggageItems).to.deep.equal({ foo: 'bar' })
expect(spanContext._isRemote).to.equal(true)
})

it('should extract otel baggage items with special characters', () => {
process.env.DD_TRACE_BAGGAGE_ENABLED = true
config = new Config()
propagator = new TextMapPropagator(config)
const carrier = {
'x-datadog-trace-id': '123',
'x-datadog-parent-id': '456',
baggage: '%22%2C%3B%5C%28%29%2F%3A%3C%3D%3E%3F%40%5B%5D%7B%7D=%22%2C%3B%5C'
}
const spanContext = propagator.extract(carrier)
expect(spanContext._baggageItems).to.deep.equal({ '",;\\()/:<=>?@[]{}': '",;\\' })
})

it('should not extract baggage when the header is malformed', () => {
const carrierA = {
'x-datadog-trace-id': '123',
'x-datadog-parent-id': '456',
baggage: 'no-equal-sign,foo=gets-dropped-because-previous-pair-is-malformed'
}
const spanContextA = propagator.extract(carrierA)
expect(spanContextA._baggageItems).to.deep.equal({})

const carrierB = {
'x-datadog-trace-id': '123',
'x-datadog-parent-id': '456',
baggage: 'foo=gets-dropped-because-subsequent-pair-is-malformed,='
}
const spanContextB = propagator.extract(carrierB)
expect(spanContextB._baggageItems).to.deep.equal({})

const carrierC = {
'x-datadog-trace-id': '123',
'x-datadog-parent-id': '456',
baggage: '=no-key'
}
const spanContextC = propagator.extract(carrierC)
expect(spanContextC._baggageItems).to.deep.equal({})

const carrierD = {
'x-datadog-trace-id': '123',
'x-datadog-parent-id': '456',
baggage: 'no-value='
}
const spanContextD = propagator.extract(carrierD)
expect(spanContextD._baggageItems).to.deep.equal({})
})

it('should convert signed IDs to unsigned', () => {
textMap['x-datadog-trace-id'] = '-123'
textMap['x-datadog-parent-id'] = '-456'
Expand Down
Loading
Loading