From 4477ce79f8c3b12d5bc5f140db6eb5fc504e4aff Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Tue, 1 Jul 2025 14:19:12 +0200 Subject: [PATCH 1/5] feat: add recordException API --- packages/dd-trace/src/opentracing/span.js | 80 ++++++++++++---- .../dd-trace/test/opentracing/span.spec.js | 91 ++++++++++++++++++- 2 files changed, 152 insertions(+), 19 deletions(-) diff --git a/packages/dd-trace/src/opentracing/span.js b/packages/dd-trace/src/opentracing/span.js index 10c81a1eb95..ee1283e4a72 100644 --- a/packages/dd-trace/src/opentracing/span.js +++ b/packages/dd-trace/src/opentracing/span.js @@ -14,6 +14,7 @@ const telemetryMetrics = require('../telemetry/metrics') const { channel } = require('dc-polyfill') const util = require('util') const { getEnvironmentVariable } = require('../config-helper') +const { isError } = require('../util') const tracerMetrics = telemetryMetrics.manager.namespace('tracers') @@ -26,6 +27,10 @@ const finishedRegistry = createRegistry('finished') const OTEL_ENABLED = !!getEnvironmentVariable('DD_TRACE_OTEL_ENABLED') const ALLOWED = new Set(['string', 'number', 'boolean']) +const MIN_INT_64BITS = -(2n ** 63n); +const MAX_INT_64BITS = (2n ** 63n) - 1n; + + const integrationCounters = { spans_created: {}, spans_finished: {} @@ -230,6 +235,18 @@ class DatadogSpan { this._events.push(event) } + recordException(exception, attributes={}) { + if (!isError(exception)) return + + const event_attributes = { + "exception.type": exception.name, + "exception.message": exception.message, + "exception.stacktrace": exception.stack + } + + this.addEvent('exception', {...event_attributes, ...attributes}) + } + finish (finishTime) { if (this._duration !== undefined) { return @@ -290,25 +307,54 @@ class DatadogSpan { return sanitizedAttributes } - _sanitizeEventAttributes (attributes = {}) { - const sanitizedAttributes = {} - for (const key in attributes) { - const value = attributes[key] - if (Array.isArray(value)) { - const newArray = [] - for (const subkey in value) { - if (ALLOWED.has(typeof value[subkey])) { - newArray.push(value[subkey]) - } else { - log.warn('Dropping span event attribute. It is not of an allowed type') - } + _validateEventAttributes (key, value) { + if (ALLOWED.has(typeof value)) { + return this._validateScalar(key, value) + } + + if (Array.isArray(value)) { + if (value.length === 0) return true + + const expectedType = typeof value[0] + if (!ALLOWED.has(expectedType)) { + log.warn(`Dropping span event attribute. List values ${key} must be string, number, or boolean: ${value}`) + return false + } + + for (let i = 0; i < value.length; i++) { + if (typeof value[i] !== expectedType || !this._validateScalar(key, value[i])) { + log.warn(`Dropping span event attribute. Attribute ${key} array values are not homogenous or valid: ${value}`) + return false } - sanitizedAttributes[key] = newArray - } else if (ALLOWED.has(typeof value)) { - sanitizedAttributes[key] = value - } else { - log.warn('Dropping span event attribute. It is not of an allowed type') + } + return true + } + + log.warn(`Dropping span event attribute. Attribute ${key} must be (array of) string, number, or boolean: ${value}`) + return false + } + + _validateScalar (key, value) { + if (typeof value !== 'number') { + return true + } + + if (Number.isInteger(value) && (value < MIN_INT_64BITS || value > MAX_INT_64BITS)) { + log.warn(`Dropping span event attribute. Attribute ${key} must be within the range of a signed 64-bit integer: ${value}`) + return false + } else if (!Number.isFinite(value)) { + log.warn(`Dropping span event attribute. Attribute ${key} must be a finite number: ${value}`) + return false + } + return true + } + + _sanitizeEventAttributes (attributes = {}) { + const sanitizedAttributes = {} + for (const [k, v] of Object.entries(attributes)) { + if (this._validateEventAttributes(k, v)) { + sanitizedAttributes[k] = v } } return sanitizedAttributes diff --git a/packages/dd-trace/test/opentracing/span.spec.js b/packages/dd-trace/test/opentracing/span.spec.js index e2cea9a09da..680d0bb7acf 100644 --- a/packages/dd-trace/test/opentracing/span.spec.js +++ b/packages/dd-trace/test/opentracing/span.spec.js @@ -19,6 +19,7 @@ describe('Span', () => { let handle let id let tagger + let log beforeEach(() => { sinon.stub(Date, 'now').returns(1500000000000) @@ -48,6 +49,11 @@ describe('Span', () => { add: sinon.spy() } + log = { + warn: sinon.stub(), + error: sinon.stub() + } + Span = proxyquire('../src/opentracing/span', { perf_hooks: { performance: { @@ -56,7 +62,8 @@ describe('Span', () => { }, '../id': id, '../tagger': tagger, - '../metrics': metrics + '../metrics': metrics, + '../log': log }) }) @@ -336,7 +343,7 @@ describe('Span', () => { span = new Span(tracer, processor, prioritySampler, { operationName: 'operation' }) span.addEvent('Web page unresponsive', - { 'error.code': '403', 'unknown values': [1, ['h', 'a', [false]]] }, 1714536311886) + { 'error.code': '403', 'unknown values': [1] }, 1714536311886) span.addEvent('Web page loaded') span.addEvent('Button changed color', { colors: [112, 215, 70], 'response.time': 134.3, success: true }) @@ -368,6 +375,86 @@ describe('Span', () => { }) }) + describe('recordException', () => { + it('should record exception', () => { + span = new Span(tracer, processor, prioritySampler, { operationName: 'operation' }) + + try { + throw new TypeError("foo") + } catch(error) { + span.recordException(error) + } + + try { + throw new Error("bar") + } catch(error) { + span.recordException(error) + } + + const events = span._events + expect(events).to.have.lengthOf(2) + + expect(events[0].name).to.equal('exception') + expect(events[0].attributes).to.have.property('exception.type', 'TypeError') + expect(events[0].attributes).to.have.property('exception.message', 'foo') + + expect(events[1].name).to.equal('exception') + expect(events[1].attributes).to.have.property('exception.type', 'Error') + expect(events[1].attributes).to.have.property('exception.message', 'bar') + }) + + it('should record exception with custom attributes', () => { + span = new Span(tracer, processor, prioritySampler, { operationName: 'operation' }) + + try { + throw new TypeError("foo") + } catch(error) { + span.recordException(error, {'foo': 'bar'}) + } + + const events = span._events + expect(events).to.have.lengthOf(1) + + expect(events[0].name).to.equal('exception') + expect(events[0].attributes).to.have.property('exception.type', 'TypeError') + expect(events[0].attributes).to.have.property('exception.message', 'foo') + expect(events[0].attributes).to.have.property('foo', 'bar') + }) + + it('should record exception with invalid attributes', () => { + span = new Span(tracer, processor, prioritySampler, { operationName: 'operation' }) + + try { + throw new TypeError("foo") + } catch(error) { + span.recordException(error, + {'foo': 'bar', + 'invalid1': [1, false], + 'invalid2': [[1]], + 'invalid3': {"foo": "bar"}, + "invalid4": NaN, + "invalid5": 9223372036854775808n, + }) + } + + const events = span._events + expect(events).to.have.lengthOf(1) + + expect(events[0].name).to.equal('exception') + expect(Object.keys(events[0].attributes)).to.have.lengthOf(4) + expect(events[0].attributes).to.have.property('exception.type', 'TypeError') + expect(events[0].attributes).to.have.property('exception.message', 'foo') + expect(events[0].attributes).to.have.property('foo', 'bar') + + // Check that warning logs were called for invalid attributes + expect(log.warn).to.have.been.calledWith('Dropping span event attribute. Attribute invalid1 array values are not homogenous or valid: 1,false') + expect(log.warn).to.have.been.calledWith('Dropping span event attribute. List values invalid2 must be string, number, or boolean: 1') + expect(log.warn).to.have.been.calledWith('Dropping span event attribute. Attribute invalid3 must be (array of) string, number, or boolean: [object Object]') + expect(log.warn).to.have.been.calledWith('Dropping span event attribute. Attribute invalid4 must be a finite number: NaN') + expect(log.warn).to.have.been.calledWith('Dropping span event attribute. Attribute invalid5 must be (array of) string, number, or boolean: 9223372036854775808') + }) + }) + describe('getBaggageItem', () => { it('should get a baggage item', () => { span = new Span(tracer, processor, prioritySampler, { operationName: 'operation' }) From 1b085bed1bbe02ff9e638e56c6ef0ac5df1c0680 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Tue, 1 Jul 2025 16:24:30 +0200 Subject: [PATCH 2/5] update index.d.ts --- index.d.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/index.d.ts b/index.d.ts index 8a304e017be..6311d3104a4 100644 --- a/index.d.ts +++ b/index.d.ts @@ -271,6 +271,15 @@ declare namespace tracer { * @returns {void} */ addLink (context: SpanContext, attributes?: Object): void; + + /** + * Records an exception as a span event. + * + * @param exception the exception to record. + * @param [attributes] additional attributes to add to the span event. + */ + recordException(exception: Error | Object, attributes?: SpanAttributes): this; + } /** From 5c85e13614a32995b00bd94f3397b3cc156c6187 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Wed, 2 Jul 2025 11:43:37 +0200 Subject: [PATCH 3/5] better typing and test coverage --- index.d.ts | 15 ++++++++++++--- packages/dd-trace/test/opentracing/span.spec.js | 13 +++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/index.d.ts b/index.d.ts index 6311d3104a4..f0c6f080123 100644 --- a/index.d.ts +++ b/index.d.ts @@ -272,13 +272,22 @@ declare namespace tracer { */ addLink (context: SpanContext, attributes?: Object): void; + /** + * Represents an exception object that can be recorded on a span. + */ + Exception?: { + message: string; + name?: string; + stack?: string; + }; + /** * Records an exception as a span event. * - * @param exception the exception to record. - * @param [attributes] additional attributes to add to the span event. + * @param {Error | Span['Exception']} exception the exception to record. + * @param {SpanAttributes} [attributes] additional attributes to add to the span event. */ - recordException(exception: Error | Object, attributes?: SpanAttributes): this; + recordException(exception: Error | Span['Exception'], attributes?: SpanAttributes): this; } diff --git a/packages/dd-trace/test/opentracing/span.spec.js b/packages/dd-trace/test/opentracing/span.spec.js index 680d0bb7acf..88fe5a8e3c8 100644 --- a/packages/dd-trace/test/opentracing/span.spec.js +++ b/packages/dd-trace/test/opentracing/span.spec.js @@ -403,6 +403,19 @@ describe('Span', () => { expect(events[1].attributes).to.have.property('exception.message', 'bar') }) + it('should record exception when error is not an error object', async () => { + span = new Span(tracer, processor, prioritySampler, { operationName: 'operation' }) + + await Promise.reject({ message: "something went wrong" }).catch(error => span.recordException(error)); + + const events = span._events + expect(events).to.have.lengthOf(1) + + expect(events[0].name).to.equal('exception') + expect(Object.keys(events[0].attributes)).to.have.lengthOf(1) + expect(events[0].attributes).to.have.property('exception.message', 'something went wrong') + }) + it('should record exception with custom attributes', () => { span = new Span(tracer, processor, prioritySampler, { operationName: 'operation' }) From ec51b7a116f89000df1a7d4d8c40b292e66348a3 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Wed, 2 Jul 2025 11:54:12 +0200 Subject: [PATCH 4/5] fix: lint --- packages/dd-trace/src/opentracing/span.js | 21 ++++--- .../dd-trace/test/opentracing/span.spec.js | 55 ++++++++++++------- 2 files changed, 44 insertions(+), 32 deletions(-) diff --git a/packages/dd-trace/src/opentracing/span.js b/packages/dd-trace/src/opentracing/span.js index ee1283e4a72..8fe1485bd1a 100644 --- a/packages/dd-trace/src/opentracing/span.js +++ b/packages/dd-trace/src/opentracing/span.js @@ -27,9 +27,8 @@ const finishedRegistry = createRegistry('finished') const OTEL_ENABLED = !!getEnvironmentVariable('DD_TRACE_OTEL_ENABLED') const ALLOWED = new Set(['string', 'number', 'boolean']) -const MIN_INT_64BITS = -(2n ** 63n); -const MAX_INT_64BITS = (2n ** 63n) - 1n; - +const MIN_INT_64BITS = -(2n ** 63n) +const MAX_INT_64BITS = (2n ** 63n) - 1n const integrationCounters = { spans_created: {}, @@ -235,16 +234,16 @@ class DatadogSpan { this._events.push(event) } - recordException(exception, attributes={}) { + recordException (exception, attributes = {}) { if (!isError(exception)) return - const event_attributes = { - "exception.type": exception.name, - "exception.message": exception.message, - "exception.stacktrace": exception.stack + const eventAttributes = { + 'exception.type': exception.name, + 'exception.message': exception.message, + 'exception.stacktrace': exception.stack } - this.addEvent('exception', {...event_attributes, ...attributes}) + this.addEvent('exception', { ...eventAttributes, ...attributes }) } finish (finishTime) { @@ -307,7 +306,6 @@ class DatadogSpan { return sanitizedAttributes } - _validateEventAttributes (key, value) { if (ALLOWED.has(typeof value)) { return this._validateScalar(key, value) @@ -341,7 +339,8 @@ class DatadogSpan { } if (Number.isInteger(value) && (value < MIN_INT_64BITS || value > MAX_INT_64BITS)) { - log.warn(`Dropping span event attribute. Attribute ${key} must be within the range of a signed 64-bit integer: ${value}`) + log.warn(`Dropping span event attribute. Attribute ${key} must be within the range of a + signed 64-bit integer: ${value}`) return false } else if (!Number.isFinite(value)) { log.warn(`Dropping span event attribute. Attribute ${key} must be a finite number: ${value}`) diff --git a/packages/dd-trace/test/opentracing/span.spec.js b/packages/dd-trace/test/opentracing/span.spec.js index 88fe5a8e3c8..26ebb3115ba 100644 --- a/packages/dd-trace/test/opentracing/span.spec.js +++ b/packages/dd-trace/test/opentracing/span.spec.js @@ -380,14 +380,14 @@ describe('Span', () => { span = new Span(tracer, processor, prioritySampler, { operationName: 'operation' }) try { - throw new TypeError("foo") - } catch(error) { + throw new TypeError('foo') + } catch (error) { span.recordException(error) } try { - throw new Error("bar") - } catch(error) { + throw new Error('bar') + } catch (error) { span.recordException(error) } @@ -406,7 +406,7 @@ describe('Span', () => { it('should record exception when error is not an error object', async () => { span = new Span(tracer, processor, prioritySampler, { operationName: 'operation' }) - await Promise.reject({ message: "something went wrong" }).catch(error => span.recordException(error)); + span.recordException({ message: 'something went wrong' }) const events = span._events expect(events).to.have.lengthOf(1) @@ -420,9 +420,9 @@ describe('Span', () => { span = new Span(tracer, processor, prioritySampler, { operationName: 'operation' }) try { - throw new TypeError("foo") - } catch(error) { - span.recordException(error, {'foo': 'bar'}) + throw new TypeError('foo') + } catch (error) { + span.recordException(error, { foo: 'bar' }) } const events = span._events @@ -438,15 +438,16 @@ describe('Span', () => { span = new Span(tracer, processor, prioritySampler, { operationName: 'operation' }) try { - throw new TypeError("foo") - } catch(error) { + throw new TypeError('foo') + } catch (error) { span.recordException(error, - {'foo': 'bar', - 'invalid1': [1, false], - 'invalid2': [[1]], - 'invalid3': {"foo": "bar"}, - "invalid4": NaN, - "invalid5": 9223372036854775808n, + { + foo: 'bar', + invalid1: [1, false], + invalid2: [[1]], + invalid3: { foo: 'bar' }, + invalid4: NaN, + invalid5: 9223372036854775808n, }) } @@ -460,11 +461,23 @@ describe('Span', () => { expect(events[0].attributes).to.have.property('foo', 'bar') // Check that warning logs were called for invalid attributes - expect(log.warn).to.have.been.calledWith('Dropping span event attribute. Attribute invalid1 array values are not homogenous or valid: 1,false') - expect(log.warn).to.have.been.calledWith('Dropping span event attribute. List values invalid2 must be string, number, or boolean: 1') - expect(log.warn).to.have.been.calledWith('Dropping span event attribute. Attribute invalid3 must be (array of) string, number, or boolean: [object Object]') - expect(log.warn).to.have.been.calledWith('Dropping span event attribute. Attribute invalid4 must be a finite number: NaN') - expect(log.warn).to.have.been.calledWith('Dropping span event attribute. Attribute invalid5 must be (array of) string, number, or boolean: 9223372036854775808') + expect(log.warn).to.have.been.calledWith( + 'Dropping span event attribute. Attribute invalid1 array values are not homogenous or valid: 1,false' + ) + expect(log.warn).to.have.been.calledWith( + 'Dropping span event attribute. List values invalid2 must be string, number, or boolean: 1' + ) + expect(log.warn).to.have.been.calledWith( + 'Dropping span event attribute. Attribute invalid3 must be (array of) string, number, or boolean: ' + + '[object Object]' + ) + expect(log.warn).to.have.been.calledWith( + 'Dropping span event attribute. Attribute invalid4 must be a finite number: NaN' + ) + expect(log.warn).to.have.been.calledWith( + 'Dropping span event attribute. Attribute invalid5 must be (array of) string, number, or boolean: ' + + '9223372036854775808' + ) }) }) From 2989260fbe2856010e026130ea07bb6abe3a95d8 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Wed, 2 Jul 2025 13:52:12 +0200 Subject: [PATCH 5/5] fix: ci --- index.d.ts | 4 ++-- packages/dd-trace/src/opentracing/span.js | 3 ++- packages/dd-trace/test/opentelemetry/span.spec.js | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/index.d.ts b/index.d.ts index f0c6f080123..7fc6617e468 100644 --- a/index.d.ts +++ b/index.d.ts @@ -285,9 +285,9 @@ declare namespace tracer { * Records an exception as a span event. * * @param {Error | Span['Exception']} exception the exception to record. - * @param {SpanAttributes} [attributes] additional attributes to add to the span event. + * @param {otel.Attributes} [attributes] additional attributes to add to the span event. */ - recordException(exception: Error | Span['Exception'], attributes?: SpanAttributes): this; + recordException(exception: Error | Span['Exception'], attributes?: otel.Attributes): this; } diff --git a/packages/dd-trace/src/opentracing/span.js b/packages/dd-trace/src/opentracing/span.js index 8fe1485bd1a..b2f5a4d5818 100644 --- a/packages/dd-trace/src/opentracing/span.js +++ b/packages/dd-trace/src/opentracing/span.js @@ -321,7 +321,8 @@ class DatadogSpan { } for (let i = 0; i < value.length; i++) { - if (typeof value[i] !== expectedType || !this._validateScalar(key, value[i])) { + const currentType = typeof value[i] + if (currentType !== expectedType || !this._validateScalar(key, value[i])) { log.warn(`Dropping span event attribute. Attribute ${key} array values are not homogenous or valid: ${value}`) return false } diff --git a/packages/dd-trace/test/opentelemetry/span.spec.js b/packages/dd-trace/test/opentelemetry/span.spec.js index 3f34189caa2..31411d6ffe8 100644 --- a/packages/dd-trace/test/opentelemetry/span.spec.js +++ b/packages/dd-trace/test/opentelemetry/span.spec.js @@ -503,7 +503,7 @@ describe('OTel Span', () => { const span2 = makeSpan('span2') const datenow = Date.now() span1.addEvent('Web page unresponsive', - { 'error.code': '403', 'unknown values': [1, ['h', 'a', [false]]] }, datenow) + { 'error.code': '403', 'unknown values': [1] }, datenow) span2.addEvent('Web page loaded') span2.addEvent('Button changed color', { colors: [112, 215, 70], 'response.time': 134.3, success: true }) const events1 = span1._ddSpan._events