From 9799bb5f747479f00a4faaf14ec211845fbebd61 Mon Sep 17 00:00:00 2001 From: Roch Devost Date: Wed, 13 Nov 2024 15:33:29 -0500 Subject: [PATCH] add crashtracking with libdatadog native binding (#4692) --- LICENSE-3rdparty.csv | 1 + package.json | 1 + packages/dd-trace/src/config.js | 6 ++ .../src/crashtracking/crashtracker.js | 98 +++++++++++++++++ packages/dd-trace/src/crashtracking/index.js | 15 +++ packages/dd-trace/src/crashtracking/noop.js | 8 ++ packages/dd-trace/src/proxy.js | 5 + packages/dd-trace/test/config.spec.js | 21 ++++ .../test/crashtracking/crashtracker.spec.js | 102 ++++++++++++++++++ .../dd-trace/test/crashtracking/index.spec.js | 87 +++++++++++++++ .../dd-trace/test/crashtracking/worker.js | 29 +++++ packages/dd-trace/test/proxy.spec.js | 1 + yarn.lock | 12 ++- 13 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 packages/dd-trace/src/crashtracking/crashtracker.js create mode 100644 packages/dd-trace/src/crashtracking/index.js create mode 100644 packages/dd-trace/src/crashtracking/noop.js create mode 100644 packages/dd-trace/test/crashtracking/crashtracker.spec.js create mode 100644 packages/dd-trace/test/crashtracking/index.spec.js create mode 100644 packages/dd-trace/test/crashtracking/worker.js diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv index 0ce2aba174a..772cd9b2553 100644 --- a/LICENSE-3rdparty.csv +++ b/LICENSE-3rdparty.csv @@ -1,4 +1,5 @@ Component,Origin,License,Copyright +require,@datadog/libdatadog,Apache license 2.0,Copyright 2024 Datadog Inc. require,@datadog/native-appsec,Apache license 2.0,Copyright 2018 Datadog Inc. require,@datadog/native-metrics,Apache license 2.0,Copyright 2018 Datadog Inc. require,@datadog/native-iast-rewriter,Apache license 2.0,Copyright 2018 Datadog Inc. diff --git a/package.json b/package.json index 388b36eb819..10c9826fdd3 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,7 @@ "node": ">=18" }, "dependencies": { + "@datadog/libdatadog": "^0.2.2", "@datadog/native-appsec": "8.2.1", "@datadog/native-iast-rewriter": "2.5.0", "@datadog/native-iast-taint-tracking": "3.2.0", diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index ec1df615627..c50c05f794a 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -467,6 +467,7 @@ class Config { this._setValue(defaults, 'ciVisibilityTestSessionName', '') this._setValue(defaults, 'clientIpEnabled', false) this._setValue(defaults, 'clientIpHeader', null) + this._setValue(defaults, 'crashtracking.enabled', false) this._setValue(defaults, 'codeOriginForSpans.enabled', false) this._setValue(defaults, 'dbmPropagationMode', 'disabled') this._setValue(defaults, 'dogstatsd.hostname', '127.0.0.1') @@ -586,6 +587,7 @@ class Config { DD_APPSEC_RASP_ENABLED, DD_APPSEC_TRACE_RATE_LIMIT, DD_APPSEC_WAF_TIMEOUT, + DD_CRASHTRACKING_ENABLED, DD_CODE_ORIGIN_FOR_SPANS_ENABLED, DD_DATA_STREAMS_ENABLED, DD_DBM_PROPAGATION_MODE, @@ -730,6 +732,7 @@ class Config { 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, 'crashtracking.enabled', DD_CRASHTRACKING_ENABLED) this._setBoolean(env, 'codeOriginForSpans.enabled', DD_CODE_ORIGIN_FOR_SPANS_ENABLED) this._setString(env, 'dbmPropagationMode', DD_DBM_PROPAGATION_MODE) this._setString(env, 'dogstatsd.hostname', DD_DOGSTATSD_HOSTNAME) @@ -1138,6 +1141,9 @@ class Config { if (iastEnabled || ['auto', 'true'].includes(profilingEnabled) || injectionIncludesProfiler) { this._setBoolean(calc, 'telemetry.logCollection', true) } + if (this._env.injectionEnabled?.length > 0) { + this._setBoolean(calc, 'crashtracking.enabled', true) + } } _applyRemote (options) { diff --git a/packages/dd-trace/src/crashtracking/crashtracker.js b/packages/dd-trace/src/crashtracking/crashtracker.js new file mode 100644 index 00000000000..0a35f0e0580 --- /dev/null +++ b/packages/dd-trace/src/crashtracking/crashtracker.js @@ -0,0 +1,98 @@ +'use strict' + +// Load binding first to not import other modules if it throws +const libdatadog = require('@datadog/libdatadog') +const binding = libdatadog.load('crashtracker') + +const log = require('../log') +const { URL } = require('url') +const pkg = require('../../../../package.json') + +class Crashtracker { + constructor () { + this._started = false + } + + configure (config) { + if (!this._started) return + + try { + binding.updateConfig(this._getConfig(config)) + binding.updateMetadata(this._getMetadata(config)) + } catch (e) { + log.error(e) + } + } + + start (config) { + if (this._started) return this.configure(config) + + this._started = true + + try { + binding.init( + this._getConfig(config), + this._getReceiverConfig(config), + this._getMetadata(config) + ) + } catch (e) { + log.error(e) + } + } + + // TODO: Send only configured values when defaults are fixed. + _getConfig (config) { + const { hostname = '127.0.0.1', port = 8126 } = config + const url = config.url || new URL(`http://${hostname}:${port}`) + + return { + additional_files: [], + create_alt_stack: true, + use_alt_stack: true, + endpoint: { + // TODO: Use the string directly when deserialization is fixed. + url: { + scheme: url.protocol.slice(0, -1), + authority: url.protocol === 'unix' + ? Buffer.from(url.pathname).toString('hex') + : url.host, + path_and_query: '' + }, + timeout_ms: 3000 + }, + timeout_ms: 0, + // TODO: Use `EnabledWithSymbolsInReceiver` instead for Linux when fixed. + resolve_frames: 'EnabledWithInprocessSymbols' + } + } + + _getMetadata (config) { + const tags = Object.keys(config.tags).map(key => `${key}:${config.tags[key]}`) + + return { + library_name: pkg.name, + library_version: pkg.version, + family: 'nodejs', + tags: [ + ...tags, + 'is_crash:true', + 'language:javascript', + `library_version:${pkg.version}`, + 'runtime:nodejs', + 'severity:crash' + ] + } + } + + _getReceiverConfig () { + return { + args: [], + env: [], + path_to_receiver_binary: libdatadog.find('crashtracker-receiver', true), + stderr_filename: null, + stdout_filename: null + } + } +} + +module.exports = new Crashtracker() diff --git a/packages/dd-trace/src/crashtracking/index.js b/packages/dd-trace/src/crashtracking/index.js new file mode 100644 index 00000000000..2ba38e72658 --- /dev/null +++ b/packages/dd-trace/src/crashtracking/index.js @@ -0,0 +1,15 @@ +'use strict' + +const { isMainThread } = require('worker_threads') +const log = require('../log') + +if (isMainThread) { + try { + module.exports = require('./crashtracker') + } catch (e) { + log.warn(e.message) + module.exports = require('./noop') + } +} else { + module.exports = require('./noop') +} diff --git a/packages/dd-trace/src/crashtracking/noop.js b/packages/dd-trace/src/crashtracking/noop.js new file mode 100644 index 00000000000..de1c555f4fa --- /dev/null +++ b/packages/dd-trace/src/crashtracking/noop.js @@ -0,0 +1,8 @@ +'use strict' + +class NoopCrashtracker { + configure () {} + start () {} +} + +module.exports = new NoopCrashtracker() diff --git a/packages/dd-trace/src/proxy.js b/packages/dd-trace/src/proxy.js index 32a7dcee10a..5c113399601 100644 --- a/packages/dd-trace/src/proxy.js +++ b/packages/dd-trace/src/proxy.js @@ -59,6 +59,11 @@ class Tracer extends NoopProxy { try { const config = new Config(options) // TODO: support dynamic code config + + if (config.crashtracking.enabled) { + require('./crashtracking').start(config) + } + telemetry.start(config, this._pluginManager) if (config.dogstatsd) { diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js index fa2734b206e..001ff8acf27 100644 --- a/packages/dd-trace/test/config.spec.js +++ b/packages/dd-trace/test/config.spec.js @@ -212,6 +212,7 @@ describe('Config', () => { expect(config).to.have.property('queryStringObfuscation').with.length(626) expect(config).to.have.property('clientIpEnabled', false) expect(config).to.have.property('clientIpHeader', null) + expect(config).to.have.nested.property('crashtracking.enabled', false) expect(config).to.have.property('sampleRate', undefined) expect(config).to.have.property('runtimeMetrics', false) expect(config.tags).to.have.property('service', 'node') @@ -440,6 +441,7 @@ describe('Config', () => { process.env.DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP = '.*' process.env.DD_TRACE_CLIENT_IP_ENABLED = 'true' process.env.DD_TRACE_CLIENT_IP_HEADER = 'x-true-client-ip' + process.env.DD_CRASHTRACKING_ENABLED = 'true' process.env.DD_RUNTIME_METRICS_ENABLED = 'true' process.env.DD_TRACE_REPORT_HOSTNAME = 'true' process.env.DD_ENV = 'test' @@ -529,6 +531,7 @@ describe('Config', () => { expect(config).to.have.property('queryStringObfuscation', '.*') expect(config).to.have.property('clientIpEnabled', true) expect(config).to.have.property('clientIpHeader', 'x-true-client-ip') + expect(config).to.have.nested.property('crashtracking.enabled', true) expect(config.grpc.client.error.statuses).to.deep.equal([3, 13, 400, 401, 402, 403]) expect(config.grpc.server.error.statuses).to.deep.equal([3, 13, 400, 401, 402, 403]) expect(config).to.have.property('runtimeMetrics', true) @@ -633,6 +636,7 @@ describe('Config', () => { { name: 'appsec.wafTimeout', value: '42', origin: 'env_var' }, { name: 'clientIpEnabled', value: true, origin: 'env_var' }, { name: 'clientIpHeader', value: 'x-true-client-ip', origin: 'env_var' }, + { name: 'crashtracking.enabled', value: true, origin: 'env_var' }, { name: 'codeOriginForSpans.enabled', value: true, origin: 'env_var' }, { name: 'dogstatsd.hostname', value: 'dsd-agent', origin: 'env_var' }, { name: 'dogstatsd.port', value: '5218', origin: 'env_var' }, @@ -738,6 +742,23 @@ describe('Config', () => { expect(config).to.have.nested.deep.property('tracePropagationStyle.extract', ['tracecontext']) }) + it('should enable crash tracking for SSI by default', () => { + process.env.DD_INJECTION_ENABLED = 'tracer' + + const config = new Config() + + expect(config).to.have.nested.deep.property('crashtracking.enabled', true) + }) + + it('should disable crash tracking for SSI when configured', () => { + process.env.DD_CRASHTRACKING_ENABLED = 'false' + process.env.DD_INJECTION_ENABLED = 'tracer' + + const config = new Config() + + expect(config).to.have.nested.deep.property('crashtracking.enabled', false) + }) + it('should initialize from the options', () => { const logger = {} const tags = { diff --git a/packages/dd-trace/test/crashtracking/crashtracker.spec.js b/packages/dd-trace/test/crashtracking/crashtracker.spec.js new file mode 100644 index 00000000000..9f1c0a81112 --- /dev/null +++ b/packages/dd-trace/test/crashtracking/crashtracker.spec.js @@ -0,0 +1,102 @@ +'use strict' + +const { expect } = require('chai') +const sinon = require('sinon') +const proxyquire = require('proxyquire').noCallThru() + +require('../setup/tap') + +describe('crashtracking', () => { + describe('crashtracker', () => { + let crashtracker + let binding + let config + let libdatadog + let log + + beforeEach(() => { + libdatadog = require('@datadog/libdatadog') + + binding = libdatadog.load('crashtracker') + + config = { + port: 7357, + tags: { + foo: 'bar' + } + } + + log = { + error: sinon.stub() + } + + sinon.spy(binding, 'init') + sinon.spy(binding, 'updateConfig') + sinon.spy(binding, 'updateMetadata') + + crashtracker = proxyquire('../../src/crashtracking/crashtracker', { + '../log': log + }) + }) + + afterEach(() => { + binding.init.restore() + binding.updateConfig.restore() + binding.updateMetadata.restore() + }) + + describe('start', () => { + it('should initialize the binding', () => { + crashtracker.start(config) + + expect(binding.init).to.have.been.called + expect(log.error).to.not.have.been.called + }) + + it('should initialize the binding only once', () => { + crashtracker.start(config) + crashtracker.start(config) + + expect(binding.init).to.have.been.calledOnce + }) + + it('should reconfigure when started multiple times', () => { + crashtracker.start(config) + crashtracker.start(config) + + expect(binding.updateConfig).to.have.been.called + expect(binding.updateMetadata).to.have.been.called + }) + + it('should handle errors', () => { + crashtracker.start(null) + + expect(() => crashtracker.start(config)).to.not.throw() + }) + }) + + describe('configure', () => { + it('should reconfigure the binding when started', () => { + crashtracker.start(config) + crashtracker.configure(config) + + expect(binding.updateConfig).to.have.been.called + expect(binding.updateMetadata).to.have.been.called + }) + + it('should reconfigure the binding only when started', () => { + crashtracker.configure(config) + + expect(binding.updateConfig).to.not.have.been.called + expect(binding.updateMetadata).to.not.have.been.called + }) + + it('should handle errors', () => { + crashtracker.start(config) + crashtracker.configure(null) + + expect(() => crashtracker.configure(config)).to.not.throw() + }) + }) + }) +}) diff --git a/packages/dd-trace/test/crashtracking/index.spec.js b/packages/dd-trace/test/crashtracking/index.spec.js new file mode 100644 index 00000000000..2d67f7428c8 --- /dev/null +++ b/packages/dd-trace/test/crashtracking/index.spec.js @@ -0,0 +1,87 @@ +'use strict' + +const { expect } = require('chai') +const sinon = require('sinon') +const proxyquire = require('proxyquire').noCallThru() +const path = require('node:path') +const { Worker } = require('node:worker_threads') + +require('../setup/tap') + +describe('crashtracking', () => { + let crashtracking + let crashtracker + let noop + let config + + beforeEach(() => { + crashtracker = { + start: sinon.stub(), + configure: sinon.stub() + } + + noop = { + start: sinon.stub(), + configure: sinon.stub() + } + + config = {} + }) + + describe('with a working crashtracker', () => { + beforeEach(() => { + crashtracking = proxyquire('../../src/crashtracking', { + './crashtracker': crashtracker + }) + }) + + it('should proxy to the crashtracker', () => { + crashtracking.start(config) + crashtracking.configure(config) + + expect(crashtracker.start).to.have.been.calledWith(config) + expect(crashtracker.configure).to.have.been.calledWith(config) + }) + }) + + describe('with an erroring crashtracker', () => { + beforeEach(() => { + crashtracking = proxyquire('../../src/crashtracking', { + './crashtracker': null, + './noop': noop + }) + }) + + it('should proxy to the noop', () => { + crashtracking.start(config) + crashtracking.configure(config) + + expect(noop.start).to.have.been.calledWith(config) + expect(noop.configure).to.have.been.calledWith(config) + }) + }) + + describe('when in a worker thread', () => { + let worker + + beforeEach(() => { + crashtracking = proxyquire('../../src/crashtracking', { + './crashtracker': null, + './noop': noop + }) + + worker = new Worker(path.join(__dirname, 'worker.js')) + }) + + it('should proxy to the noop', done => { + worker.on('error', done) + worker.on('exit', code => { + if (code === 0) { + done() + } else { + done(new Error(`Worker stopped with exit code ${code}`)) + } + }) + }) + }) +}) diff --git a/packages/dd-trace/test/crashtracking/worker.js b/packages/dd-trace/test/crashtracking/worker.js new file mode 100644 index 00000000000..ff12528e74c --- /dev/null +++ b/packages/dd-trace/test/crashtracking/worker.js @@ -0,0 +1,29 @@ +'use strict' + +const { expect } = require('chai') +const sinon = require('sinon') +const proxyquire = require('proxyquire').noCallThru() + +require('../setup/tap') + +const crashtracker = { + start: sinon.stub(), + configure: sinon.stub() +} + +const noop = { + start: sinon.stub(), + configure: sinon.stub() +} + +const crashtracking = proxyquire('../../src/crashtracking', { + './crashtracker': crashtracker, + './noop': noop + +}) + +crashtracking.start() +crashtracking.configure() + +expect(noop.start).to.have.been.called +expect(noop.configure).to.have.been.called diff --git a/packages/dd-trace/test/proxy.spec.js b/packages/dd-trace/test/proxy.spec.js index 3d7ebbc5a2a..4836e99787f 100644 --- a/packages/dd-trace/test/proxy.spec.js +++ b/packages/dd-trace/test/proxy.spec.js @@ -128,6 +128,7 @@ describe('TracerProxy', () => { profiling: {}, appsec: {}, iast: {}, + crashtracking: {}, remoteConfig: { enabled: true }, diff --git a/yarn.lock b/yarn.lock index 77dacb70614..de5a02ece2f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -401,6 +401,11 @@ resolved "https://registry.npmjs.org/@colors/colors/-/colors-1.5.0.tgz" integrity "sha1-u1BFecHK6SPmV2pPXaQ9Jfl729k= sha512-ooWCrlZP11i8GImSjTHYHLkvFDP48nS4+204nGb1RiX/WXYHmJA2III9/e2DWVabCESdW7hBAEzHRqUn9OUVvQ==" +"@datadog/libdatadog@^0.2.2": + version "0.2.2" + resolved "https://registry.yarnpkg.com/@datadog/libdatadog/-/libdatadog-0.2.2.tgz#ac02c76ac9a38250dca740727c7cdf00244ce3d3" + integrity sha512-rTWo96mEPTY5UbtGoFj8/wY0uKSViJhsPg/Z6aoFWBFXQ8b45Ix2e/yvf92AAwrhG+gPLTxEqTXh3kef2dP8Ow== + "@datadog/native-appsec@8.2.1": version "8.2.1" resolved "https://registry.yarnpkg.com/@datadog/native-appsec/-/native-appsec-8.2.1.tgz#e84f9ec7e5dddea2531970117744264a685da15a" @@ -679,11 +684,16 @@ "@jridgewell/sourcemap-codec" "^1.4.10" "@jridgewell/trace-mapping" "^0.3.9" -"@jridgewell/resolve-uri@^3.0.3", "@jridgewell/resolve-uri@^3.1.0": +"@jridgewell/resolve-uri@^3.0.3": version "3.1.2" resolved "https://registry.yarnpkg.com/@jridgewell/resolve-uri/-/resolve-uri-3.1.2.tgz#7a0ee601f60f99a20c7c7c5ff0c80388c1189bd6" integrity sha512-bRISgCIjP20/tbWSPWMEi54QVPRZExkuD9lJL+UIxUKtwVJA8wW1Trb1jMs1RFXo1CBTNZ/5hpC9QvmKWdopKw== +"@jridgewell/resolve-uri@^3.1.0": + version "3.1.1" + resolved "https://registry.yarnpkg.com/@jridgewell/resolve-uri/-/resolve-uri-3.1.1.tgz#c08679063f279615a3326583ba3a90d1d82cc721" + integrity sha512-dSYZh7HhCDtCKm4QakX0xFpsRDqjjtZf/kjI/v3T3Nwt5r8/qz/M19F9ySyOqU94SXBmeG9ttTul+YnR4LOxFA== + "@jridgewell/set-array@^1.0.1": version "1.1.2" resolved "https://registry.npmjs.org/@jridgewell/set-array/-/set-array-1.1.2.tgz"