diff --git a/packages/datadog-plugin-apollo/test/index.spec.js b/packages/datadog-plugin-apollo/test/index.spec.js index a72a0b291e7..e9cd809f02c 100644 --- a/packages/datadog-plugin-apollo/test/index.spec.js +++ b/packages/datadog-plugin-apollo/test/index.spec.js @@ -7,6 +7,7 @@ const sinon = require('sinon') const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants.js') const agent = require('../../dd-trace/test/plugins/agent.js') +const PublicSpan = require('../../dd-trace/src/opentracing/public/span') const { withNamingSchema, withVersions } = require('../../dd-trace/test/setup/mocha') const { assertObjectContains } = require('../../../integration-tests/helpers') const accounts = require('./fixtures.js') @@ -592,6 +593,11 @@ describe('Plugin', () => { const executeSpan = config.hooks.execute.firstCall.args[0] const postprocessingSpan = config.hooks.postprocessing.firstCall.args[0] + assert.ok(requestSpan instanceof PublicSpan) + assert.ok(planSpan instanceof PublicSpan) + assert.ok(executeSpan instanceof PublicSpan) + assert.ok(postprocessingSpan instanceof PublicSpan) + assert.strictEqual(requestSpan.context()._name, expectedSchema.server.opName) assert.strictEqual(planSpan.context()._name, 'apollo.gateway.plan') assert.strictEqual(executeSpan.context()._name, 'apollo.gateway.execute') @@ -627,6 +633,7 @@ describe('Plugin', () => { sinon.assert.notCalled(config.hooks.postprocessing) const validateSpan = config.hooks.validate.firstCall.args[0] + assert.ok(validateSpan instanceof PublicSpan) const validateCtx = config.hooks.validate.firstCall.args[1] assert.strictEqual(validateSpan.context()._name, 'apollo.gateway.validate') diff --git a/packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js b/packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js index b9bf5ecb86e..5a80935315b 100644 --- a/packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js +++ b/packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js @@ -7,6 +7,7 @@ const semver = require('semver') const { ERROR_MESSAGE, ERROR_STACK, ERROR_TYPE } = require('../../dd-trace/src/constants') const agent = require('../../dd-trace/test/plugins/agent') +const PublicSpan = require('../../dd-trace/src/opentracing/public/span') const { withVersions } = require('../../dd-trace/test/setup/mocha') const { assertObjectContains } = require('../../../integration-tests/helpers') const { setup, sort } = require('./spec_helpers') @@ -258,6 +259,7 @@ describe('Plugin', () => { service: 'test', hooks: { request (span, response) { + assert.ok(span instanceof PublicSpan) span.setTag('hook.operation', response.request.operation) span.addTags({ error: 0, diff --git a/packages/datadog-plugin-child_process/src/index.js b/packages/datadog-plugin-child_process/src/index.js index 799a5ad52c5..b58d61a9758 100644 --- a/packages/datadog-plugin-child_process/src/index.js +++ b/packages/datadog-plugin-child_process/src/index.js @@ -31,10 +31,6 @@ class ChildProcessPlugin extends TracingPlugin { static id = 'child_process' static prefix = 'tracing:datadog:child_process:execution' - get tracer () { - return this._tracer - } - start (ctx) { const { command, shell } = ctx diff --git a/packages/datadog-plugin-child_process/test/index.spec.js b/packages/datadog-plugin-child_process/test/index.spec.js index a76bb6ac4ca..67f09fe86ef 100644 --- a/packages/datadog-plugin-child_process/test/index.spec.js +++ b/packages/datadog-plugin-child_process/test/index.spec.js @@ -517,9 +517,9 @@ describe('Child process plugin', () => { beforeEach((done) => { if (hasParentSpan) { - parentSpan = tracer.startSpan('parent') + parentSpan = tracer._tracer.startSpan('parent') parentSpan.finish() - tracer.scope().activate(parentSpan, done) + storage('legacy').run({ span: parentSpan }, done) } else { storage('legacy').enterWith({}) done() @@ -659,9 +659,9 @@ describe('Child process plugin', () => { ] if (parentSpan) { beforeEach((done) => { - const parentSpan = tracer.startSpan('parent') - parentSpan.finish() - tracer.scope().activate(parentSpan, done) + const span = tracer._tracer.startSpan('parent') + span.finish() + storage('legacy').run({ span }, done) }) } diff --git a/packages/datadog-plugin-cypress/src/cypress-plugin.js b/packages/datadog-plugin-cypress/src/cypress-plugin.js index 52bb099345e..0fc5f12f24f 100644 --- a/packages/datadog-plugin-cypress/src/cypress-plugin.js +++ b/packages/datadog-plugin-cypress/src/cypress-plugin.js @@ -1103,7 +1103,8 @@ class CypressPlugin { const finishedTest = { testName, testStatus, - finishTime: this.activeTestSpan._getTime(), // we store the finish time here + // TODO: remove the need of accessing private span internals + finishTime: this.activeTestSpan._span._getTime(), // we store the finish time here testSpan: this.activeTestSpan, isEfdRetry, isAttemptToFix, diff --git a/packages/datadog-plugin-elasticsearch/test/index.spec.js b/packages/datadog-plugin-elasticsearch/test/index.spec.js index 93904155d45..e11b632c1ef 100644 --- a/packages/datadog-plugin-elasticsearch/test/index.spec.js +++ b/packages/datadog-plugin-elasticsearch/test/index.spec.js @@ -6,6 +6,7 @@ const { after, afterEach, before, beforeEach, describe, it } = require('mocha') const { ERROR_MESSAGE, ERROR_STACK, ERROR_TYPE } = require('../../dd-trace/src/constants') const agent = require('../../dd-trace/test/plugins/agent') +const PublicSpan = require('../../dd-trace/src/opentracing/public/span') const { breakThen, unbreakThen } = require('../../dd-trace/test/plugins/helpers') const { withNamingSchema, withPeerService, withVersions } = require('../../dd-trace/test/setup/mocha') const { assertObjectContains } = require('../../../integration-tests/helpers') @@ -346,6 +347,7 @@ describe('Plugin', () => { service: 'custom', hooks: { query: (span, params) => { + assert.ok(span instanceof PublicSpan) span.addTags({ 'elasticsearch.params': 'foo', 'elasticsearch.method': params.method }) }, }, diff --git a/packages/datadog-plugin-fetch/test/index.spec.js b/packages/datadog-plugin-fetch/test/index.spec.js index 51608c112b6..fd6dfcb544b 100644 --- a/packages/datadog-plugin-fetch/test/index.spec.js +++ b/packages/datadog-plugin-fetch/test/index.spec.js @@ -8,6 +8,7 @@ const tags = require('../../../ext/tags') const { storage } = require('../../datadog-core') const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants') const agent = require('../../dd-trace/test/plugins/agent') +const PublicSpan = require('../../dd-trace/src/opentracing/public/span') const { withNamingSchema } = require('../../dd-trace/test/setup/mocha') const { assertObjectContains } = require('../../../integration-tests/helpers') const { rawExpectedSchema } = require('./naming') @@ -516,6 +517,7 @@ describe('Plugin', function () { config = { hooks: { request: (span, req, res) => { + assert.ok(span instanceof PublicSpan) span.setTag('foo', '/foo') }, }, diff --git a/packages/datadog-plugin-graphql/test/index.spec.js b/packages/datadog-plugin-graphql/test/index.spec.js index 9d943372a45..c75f91460c8 100644 --- a/packages/datadog-plugin-graphql/test/index.spec.js +++ b/packages/datadog-plugin-graphql/test/index.spec.js @@ -13,6 +13,7 @@ const sinon = require('sinon') const { assertObjectContains } = require('../../../integration-tests/helpers') const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants') const agent = require('../../dd-trace/test/plugins/agent') +const PublicSpan = require('../../dd-trace/src/opentracing/public/span') const { withNamingSchema, withVersions } = require('../../dd-trace/test/setup/mocha') const plugin = require('../src') const { expectedSchema, rawExpectedSchema } = require('./naming') @@ -1819,6 +1820,7 @@ describe('Plugin', () => { sinon.assert.calledOnce(config.hooks.execute) const span = config.hooks.execute.firstCall.args[0] + assert.ok(span instanceof PublicSpan) const args = config.hooks.execute.firstCall.args[1] const res = config.hooks.execute.firstCall.args[2] @@ -1860,6 +1862,7 @@ describe('Plugin', () => { sinon.assert.calledOnce(config.hooks.validate) const span = config.hooks.validate.firstCall.args[0] + assert.ok(span instanceof PublicSpan) const hookDocument = config.hooks.validate.firstCall.args[1] const hookErrors = config.hooks.validate.firstCall.args[2] @@ -1885,6 +1888,7 @@ describe('Plugin', () => { sinon.assert.calledOnce(config.hooks.parse) const span = config.hooks.parse.firstCall.args[0] + assert.ok(span instanceof PublicSpan) const hookSource = config.hooks.parse.firstCall.args[1] const hookDocument = config.hooks.parse.firstCall.args[2] diff --git a/packages/datadog-plugin-http/test/client.spec.js b/packages/datadog-plugin-http/test/client.spec.js index 628d6ed5297..44c859f659d 100644 --- a/packages/datadog-plugin-http/test/client.spec.js +++ b/packages/datadog-plugin-http/test/client.spec.js @@ -8,6 +8,7 @@ const { satisfies } = require('semver') const tags = require('../../../ext/tags') const { storage } = require('../../datadog-core') const agent = require('../../dd-trace/test/plugins/agent') +const PublicSpan = require('../../dd-trace/src/opentracing/public/span') const { withNamingSchema, withPeerService } = require('../../dd-trace/test/setup/mocha') const key = fs.readFileSync(path.join(__dirname, './ssl/test.key')) const cert = fs.readFileSync(path.join(__dirname, './ssl/test.crt')) @@ -1291,6 +1292,7 @@ describe('Plugin', () => { client: { hooks: { request: (span, req, res) => { + assert.ok(span instanceof PublicSpan) span.setTag('resource.name', `${req.method} ${req._route}`) }, }, diff --git a/packages/datadog-plugin-sharedb/test/index.spec.js b/packages/datadog-plugin-sharedb/test/index.spec.js index 696b1d8f581..f04aa62c9a1 100644 --- a/packages/datadog-plugin-sharedb/test/index.spec.js +++ b/packages/datadog-plugin-sharedb/test/index.spec.js @@ -7,6 +7,7 @@ const sinon = require('sinon') const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants') const agent = require('../../dd-trace/test/plugins/agent') +const PublicSpan = require('../../dd-trace/src/opentracing/public/span') const { withVersions } = require('../../dd-trace/test/setup/mocha') describe('Plugin', () => { let ShareDB @@ -204,6 +205,8 @@ describe('Plugin', () => { sinon.match.object, sinon.match.object ) + assert.ok(receiveHookSpy.firstCall.args[0] instanceof PublicSpan) + assert.ok(replyHookSpy.firstCall.args[0] instanceof PublicSpan) }) .then(done) .catch(done) diff --git a/packages/datadog-plugin-undici/test/index.spec.js b/packages/datadog-plugin-undici/test/index.spec.js index 2bb66459143..86c6388455d 100644 --- a/packages/datadog-plugin-undici/test/index.spec.js +++ b/packages/datadog-plugin-undici/test/index.spec.js @@ -8,6 +8,7 @@ const tags = require('../../../ext/tags') const { NODE_MAJOR } = require('../../../version') const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants') const agent = require('../../dd-trace/test/plugins/agent') +const PublicSpan = require('../../dd-trace/src/opentracing/public/span') const { withNamingSchema, withVersions } = require('../../dd-trace/test/setup/mocha') const { assertObjectContains } = require('../../../integration-tests/helpers') const { rawExpectedSchema } = require('./naming') @@ -567,6 +568,7 @@ describe('Plugin', () => { config = { hooks: { request: (span, req, res) => { + assert.ok(span instanceof PublicSpan) span.setTag('foo', '/foo') }, }, diff --git a/packages/dd-trace/src/aiguard/sdk.js b/packages/dd-trace/src/aiguard/sdk.js index f817856bfc4..607f104d0e4 100644 --- a/packages/dd-trace/src/aiguard/sdk.js +++ b/packages/dd-trace/src/aiguard/sdk.js @@ -157,7 +157,7 @@ class AIGuard extends NoopAIGuard { const metaStruct = { messages: this.#buildMessagesForMetaStruct(messages), } - span.meta_struct = { + span._span.meta_struct = { [AI_GUARD_META_STRUCT_KEY]: metaStruct, } const rootSpan = span.context()?._trace?.started?.[0] diff --git a/packages/dd-trace/src/llmobs/sdk.js b/packages/dd-trace/src/llmobs/sdk.js index 8149b02ac77..a7c7ca558d7 100644 --- a/packages/dd-trace/src/llmobs/sdk.js +++ b/packages/dd-trace/src/llmobs/sdk.js @@ -7,6 +7,8 @@ const tracerVersion = require('../../../../package.json').version const logger = require('../log') const { getValueFromEnvSources } = require('../config/helper') const Span = require('../opentracing/span') + +const { storage: storageCore } = require('../../../datadog-core') const { SPAN_KIND, OUTPUT_VALUE, INPUT_VALUE } = require('./constants/tags') const { getFunctionArguments, @@ -110,12 +112,12 @@ class LLMObs extends NoopLLMObs { if (fn.length > 1) { return this._tracer.trace(name, spanOptions, (span, cb) => - this._activate(span, { kind, ...llmobsOptions }, () => fn(span, cb)) + this._activate(span._span, { kind, ...llmobsOptions }, () => fn(span, cb)) ) } return this._tracer.trace(name, spanOptions, span => - this._activate(span, { kind, ...llmobsOptions }, () => fn(span)) + this._activate(span._span, { kind, ...llmobsOptions }, () => fn(span)) ) } @@ -143,7 +145,7 @@ class LLMObs extends NoopLLMObs { function wrapped () { telemetry.incrementLLMObsSpanStartCount({ autoinstrumented: false, kind }) - const span = llmobs._tracer.scope().active() + const span = storageCore('legacy').getStore()?.span const fnArgs = arguments const lastArgId = fnArgs.length - 1 @@ -211,11 +213,11 @@ class LLMObs extends NoopLLMObs { if (!span) { span = this._active() - } - - if ((span && !options) && !(span instanceof Span)) { + } else if (!options && !(span?._span instanceof Span)) { options = span span = this._active() + } else { + span = span._span } let err = '' @@ -239,38 +241,7 @@ class LLMObs extends NoopLLMObs { throw new Error('Cannot annotate a finished span') } - const spanKind = LLMObsTagger.tagMap.get(span)[SPAN_KIND] - if (!spanKind) { - err = 'invalid_no_span_kind' - throw new Error('LLMObs span must have a span kind specified') - } - - const { inputData, outputData, metadata, metrics, tags, prompt } = options - - if (inputData || outputData) { - if (spanKind === 'llm') { - this._tagger.tagLLMIO(span, inputData, outputData) - } else if (spanKind === 'embedding') { - this._tagger.tagEmbeddingIO(span, inputData, outputData) - } else if (spanKind === 'retrieval') { - this._tagger.tagRetrievalIO(span, inputData, outputData) - } else { - this._tagger.tagTextIO(span, inputData, outputData) - } - } - - if (metadata) { - this._tagger.tagMetadata(span, metadata) - } - if (metrics) { - this._tagger.tagMetrics(span, metrics) - } - if (tags) { - this._tagger.tagSpanTags(span, tags) - } - if (prompt) { - this._tagger.tagPrompt(span, prompt) - } + this._annotate(span, options) } catch (e) { if (e.ddErrorTag) { err = e.ddErrorTag @@ -283,8 +254,46 @@ class LLMObs extends NoopLLMObs { } } + _annotate (span, options) { + if (!this.enabled) return + + const spanKind = LLMObsTagger.tagMap.get(span)[SPAN_KIND] + if (!spanKind) { + const err = new Error('LLMObs span must have a span kind specified') + err.ddErrorTag = 'invalid_no_span_kind' + throw err + } + + const { inputData, outputData, metadata, metrics, tags, prompt } = options + + if (inputData || outputData) { + if (spanKind === 'llm') { + this._tagger.tagLLMIO(span, inputData, outputData) + } else if (spanKind === 'embedding') { + this._tagger.tagEmbeddingIO(span, inputData, outputData) + } else if (spanKind === 'retrieval') { + this._tagger.tagRetrievalIO(span, inputData, outputData) + } else { + this._tagger.tagTextIO(span, inputData, outputData) + } + } + + if (metadata) { + this._tagger.tagMetadata(span, metadata) + } + if (metrics) { + this._tagger.tagMetrics(span, metrics) + } + if (tags) { + this._tagger.tagSpanTags(span, tags) + } + if (prompt) { + this._tagger.tagPrompt(span, prompt) + } + } + exportSpan (span) { - span = span || this._active() + span = span ? span._span : this._active() let err = '' try { if (!span) { @@ -516,7 +525,9 @@ class LLMObs extends NoopLLMObs { annotations.outputData = output } - this.annotate(span, annotations, true) + if ((annotations.inputData || annotations.outputData) && LLMObsTagger.tagMap.has(span)) { + this._annotate(span, annotations) + } } _active () { diff --git a/packages/dd-trace/src/noop/proxy.js b/packages/dd-trace/src/noop/proxy.js index 53dd1e709b1..f12ad2cee79 100644 --- a/packages/dd-trace/src/noop/proxy.js +++ b/packages/dd-trace/src/noop/proxy.js @@ -4,6 +4,8 @@ const NoopAppsecSdk = require('../appsec/sdk/noop') const NoopLLMObsSDK = require('../llmobs/noop') const NoopFlaggingProvider = require('../openfeature/noop') const NoopAIGuardSDK = require('../aiguard/noop') +const PublicSpan = require('../opentracing/public/span') +const { SVC_SRC_KEY } = require('../../src/constants') const NoopDogStatsDClient = require('./dogstatsd') const NoopTracer = require('./tracer') @@ -55,6 +57,12 @@ class NoopProxy { if (typeof fn !== 'function') return options = options || {} + if (options.service || options?.tags?.service || options?.tags?.['service.name']) { + options.tags = { + ...options.tags, + [SVC_SRC_KEY]: 'm', + } + } return this._tracer.trace(name, options, fn) } @@ -68,7 +76,12 @@ class NoopProxy { if (typeof fn !== 'function') return fn options = options || {} - + if (options.service || options?.tags?.service || options?.tags?.['service.name']) { + options.tags = { + ...options.tags, + [SVC_SRC_KEY]: 'm', + } + } return this._tracer.wrap(name, options, fn) } @@ -77,12 +90,21 @@ class NoopProxy { return this } - startSpan () { - return this._tracer.startSpan.apply(this._tracer, arguments) + startSpan (name, options) { + if (options?.tags?.service || options?.tags?.['service.name']) { + options.tags = { + ...options.tags, + [SVC_SRC_KEY]: 'm', + } + } + return new PublicSpan(this._tracer.startSpan.apply(this._tracer, arguments)) } - inject () { - return this._tracer.inject.apply(this._tracer, arguments) + inject (context, format, carrier) { + if (context instanceof PublicSpan) { + context = context._span + } + return this._tracer.inject(context, format, carrier) } extract () { diff --git a/packages/dd-trace/src/opentracing/public/span.js b/packages/dd-trace/src/opentracing/public/span.js new file mode 100644 index 00000000000..a59680e8598 --- /dev/null +++ b/packages/dd-trace/src/opentracing/public/span.js @@ -0,0 +1,63 @@ +'use strict' + +// A WeakMap cache at module scope ensures the same wrapper instance is returned +// for the same underlying span across all subclasses, so reference equality +// checks (===) in user code remain stable. +const cache = new WeakMap() + +const { SVC_SRC_KEY } = require('../../constants') +const DatadogSpan = require('../span') + +const SERVICE_KEY = 'service' +const SERVICE_NAME_KEY = 'service.name' + +/** + * This is a public wrapper of Span, this allows distinguishing internal usage from + * external usage and acting accordingly. + */ +class PublicSpan { + constructor (span) { + if (span instanceof PublicSpan) { + return span + } + const cached = cache.get(span) + if (cached !== undefined) { + return cached + } + this._span = span + try { + cache.set(span, this) + } catch {} + } + + setTag (key, value) { + if (key === SERVICE_KEY || key === SERVICE_NAME_KEY) { + this._span.setTag(SVC_SRC_KEY, 'm') + } + this._span.setTag(key, value) + return this + } + + addTags (tags) { + if (tags && (tags[SERVICE_KEY] || tags[SERVICE_NAME_KEY])) { + this._span.setTag(SVC_SRC_KEY, 'm') + } + this._span.addTags(tags) + return this + } +} + +// Whenever a method needs to be modified to have a unique public behavior, it +// should be implemented on `PublicSpan` directly so it is skipped here. +for (const method of Object.getOwnPropertyNames(DatadogSpan.prototype)) { + if (method === 'constructor' || method.startsWith('_') || PublicSpan.prototype[method]) { + continue + } + PublicSpan.prototype[method] = function (...args) { + const result = this._span[method](...args) + // always return wrapper span when the result is the span itself + return result === this._span ? this : result + } +} + +module.exports = PublicSpan diff --git a/packages/dd-trace/src/opentracing/tracer.js b/packages/dd-trace/src/opentracing/tracer.js index d26f720ddba..3c4d5aa1183 100644 --- a/packages/dd-trace/src/opentracing/tracer.js +++ b/packages/dd-trace/src/opentracing/tracer.js @@ -8,6 +8,7 @@ const log = require('../log') const runtimeMetrics = require('../runtime_metrics') const getExporter = require('../exporter') const Span = require('./span') +const PublicSpan = require('./public/span') const TextMapPropagator = require('./propagation/text_map') const DSMTextMapPropagator = require('./propagation/text_map_dsm') const HttpPropagator = require('./propagation/http') @@ -114,7 +115,7 @@ class DatadogTracer { * @returns {SpanContext} */ function getContext (spanContext) { - if (spanContext instanceof Span) { + if (spanContext instanceof Span || spanContext instanceof PublicSpan) { spanContext = spanContext.context() } diff --git a/packages/dd-trace/src/plugins/tracing.js b/packages/dd-trace/src/plugins/tracing.js index b50fab92d32..ce74cd14b2b 100644 --- a/packages/dd-trace/src/plugins/tracing.js +++ b/packages/dd-trace/src/plugins/tracing.js @@ -3,6 +3,7 @@ const { storage } = require('../../../datadog-core') const analyticsSampler = require('../analytics_sampler') const { COMPONENT, SVC_SRC_KEY } = require('../constants') +const PublicSpan = require('../opentracing/public/span') const Plugin = require('./plugin') class TracingPlugin extends Plugin { @@ -60,11 +61,17 @@ class TracingPlugin extends Plugin { * @returns {object} */ configure (config) { + const hooks = config.hooks || {} + const wrappedHooks = {} + for (const [name, hook] of Object.entries(hooks)) { + wrappedHooks[name] = (span, ...args) => hook(new PublicSpan(span), ...args) + } + return super.configure({ ...config, hooks: { [this.operation]: () => {}, - ...config.hooks, + ...wrappedHooks, }, }) } diff --git a/packages/dd-trace/src/scope.js b/packages/dd-trace/src/scope.js index cc3fd1a86c1..56e2224cab2 100644 --- a/packages/dd-trace/src/scope.js +++ b/packages/dd-trace/src/scope.js @@ -1,6 +1,7 @@ 'use strict' const { storage } = require('../../datadog-core') +const PublicSpan = require('./opentracing/public/span') // TODO: refactor bind to use shimmer once the new internal tracer lands @@ -9,13 +10,16 @@ const originals = new WeakMap() class Scope { active () { const store = storage('legacy').getStore() + const span = (store && store.span) || null - return (store && store.span) || null + return span ? new PublicSpan(span) : null } activate (span, callback) { if (typeof callback !== 'function') return callback + span = span?._span + const oldStore = storage('legacy').getStore() const newStore = span ? storage('legacy').getStore(span._store) : oldStore diff --git a/packages/dd-trace/src/tracer.js b/packages/dd-trace/src/tracer.js index 556d18652d6..87ed015cbee 100644 --- a/packages/dd-trace/src/tracer.js +++ b/packages/dd-trace/src/tracer.js @@ -5,6 +5,7 @@ const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/c const { flushStartupLogs } = require('../../datadog-instrumentations/src/helpers/check-require-cache') const Tracer = require('./opentracing/tracer') const Scope = require('./scope') +const PublicSpan = require('./opentracing/public/span') const { isError } = require('./util') const { setStartupLogConfig } = require('./startup-log') const { DataStreamsCheckpointer, DataStreamsManager, DataStreamsProcessor } = require('./datastreams') @@ -60,7 +61,7 @@ class DatadogTracer extends Tracer { trace (name, options, fn) { options = { childOf: this.scope().active(), ...options } - const span = this.startSpan(name, options) + const span = new PublicSpan(this.startSpan(name, options)) addTags(span, options) diff --git a/packages/dd-trace/test/appsec/sdk/utils.spec.js b/packages/dd-trace/test/appsec/sdk/utils.spec.js index b1d1131b910..61cf67dc54a 100644 --- a/packages/dd-trace/test/appsec/sdk/utils.spec.js +++ b/packages/dd-trace/test/appsec/sdk/utils.spec.js @@ -17,10 +17,10 @@ describe('Appsec SDK utils', () => { describe('getRootSpan', () => { it('should return root span if there are no childs', () => { - tracer.trace('parent', { }, parent => { + tracer.trace('parent', {}, parent => { const root = getRootSpan() - assert.strictEqual(root, parent) + assert.strictEqual(root, parent._span) }) }) @@ -49,7 +49,7 @@ describe('Appsec SDK utils', () => { const childOf = tracer.startSpan('parent') tracer.trace('child1.1', { childOf }, child11 => { - tracer.trace('child1.1.2', { childOf: child11 }, child112 => {}) + tracer.trace('child1.1.2', { childOf: child11 }, child112 => { }) }) tracer.trace('child1.2', { childOf }, child12 => { const root = getRootSpan() @@ -65,7 +65,7 @@ describe('Appsec SDK utils', () => { tracer.trace('child1', { childOf }, child1 => { const root = getRootSpan() - assert.strictEqual(root, child1) + assert.strictEqual(root, child1._span) }) }) @@ -90,7 +90,7 @@ describe('Appsec SDK utils', () => { const root = getRootSpan() - assert.strictEqual(root, child1) + assert.strictEqual(root, child1._span) }) }) @@ -98,12 +98,11 @@ describe('Appsec SDK utils', () => { const childOf = tracer.startSpan('parent') childOf.setTag('_inferred_span', {}) - tracer.trace('child1.1', { childOf }, child11 => {}) + tracer.trace('child1.1', { childOf }, child11 => { }) tracer.trace('child1.2', { childOf }, child12 => { tracer.trace('child1.2.1', { childOf: child12 }, child121 => { const root = getRootSpan() - - assert.strictEqual(root, child12) + assert.strictEqual(root, child12._span) }) }) }) @@ -111,7 +110,7 @@ describe('Appsec SDK utils', () => { it('should return root span discarding inferred spans if it is direct parent (mutiple childs)', () => { const childOf = tracer.startSpan('parent') - tracer.trace('child1.1', { childOf }, child11 => {}) + tracer.trace('child1.1', { childOf }, child11 => { }) tracer.trace('child1.2', { childOf }, child12 => { child12.setTag('_inferred_span', {}) @@ -126,7 +125,7 @@ describe('Appsec SDK utils', () => { it('should return root span discarding multiple inferred spans', () => { const childOf = tracer.startSpan('parent') - tracer.trace('child1.1', { childOf }, child11 => {}) + tracer.trace('child1.1', { childOf }, child11 => { }) tracer.trace('child1.2', { childOf }, child12 => { child12.setTag('_inferred_span', {}) @@ -146,7 +145,7 @@ describe('Appsec SDK utils', () => { const childOf = tracer.startSpan('parent') childOf.setTag('_inferred_span', {}) - tracer.trace('child1.1', { childOf }, child11 => {}) + tracer.trace('child1.1', { childOf }, child11 => { }) tracer.trace('child1.2', { childOf }, child12 => { child12.setTag('_inferred_span', {}) @@ -156,7 +155,7 @@ describe('Appsec SDK utils', () => { tracer.trace('child1.2.1.1', { childOf: child121 }, child1211 => { const root = getRootSpan() - assert.strictEqual(root, child1211) + assert.strictEqual(root, child1211._span) }) }) }) diff --git a/packages/dd-trace/test/llmobs/sdk/index.spec.js b/packages/dd-trace/test/llmobs/sdk/index.spec.js index ceeec2dd691..f90802036fa 100644 --- a/packages/dd-trace/test/llmobs/sdk/index.spec.js +++ b/packages/dd-trace/test/llmobs/sdk/index.spec.js @@ -56,6 +56,9 @@ describe('sdk', () => { clock = sinon.useFakeTimers({ toFake: ['Date', 'setTimeout', 'clearTimeout', 'setInterval', 'clearInterval'], }) + const tagMap = LLMObsTagger.tagMap + const originalGet = tagMap.get.bind(tagMap) + tagMap.get = (span) => originalGet(span?._span ?? span) }) afterEach(() => { @@ -289,11 +292,11 @@ describe('sdk', () => { it('maintains llmobs parentage separately from apm spans', () => { llmobs.trace({ kind: 'workflow', name: 'outer-llm' }, outerLLMSpan => { - assert.strictEqual(llmobs._active(), outerLLMSpan) + assert.strictEqual(llmobs._active(), outerLLMSpan._span) tracer.trace('apmSpan', apmSpan => { - assert.strictEqual(llmobs._active(), outerLLMSpan) + assert.strictEqual(llmobs._active(), outerLLMSpan._span) llmobs.trace({ kind: 'workflow', name: 'inner-llm' }, innerLLMSpan => { - assert.strictEqual(llmobs._active(), innerLLMSpan) + assert.strictEqual(llmobs._active(), innerLLMSpan._span) // llmobs span linkage assert.strictEqual( @@ -331,16 +334,18 @@ describe('sdk', () => { it('maintains the llmobs parentage when error callbacks are used', () => { llmobs.trace({ kind: 'workflow' }, outer => { llmobs.trace({ kind: 'task' }, (inner, cb) => { - assert.strictEqual(llmobs._active(), inner) - assert.strictEqual(LLMObsTagger.tagMap.get(inner)['_ml_obs.llmobs_parent_id'], outer.context().toSpanId()) + assert.strictEqual(llmobs._active(), inner._span) + assert.strictEqual(LLMObsTagger.tagMap.get(inner)['_ml_obs.llmobs_parent_id'], + outer.context().toSpanId()) cb() // finish the span }) - assert.strictEqual(llmobs._active(), outer) + assert.strictEqual(llmobs._active(), outer._span) llmobs.trace({ kind: 'task' }, (inner) => { - assert.strictEqual(llmobs._active(), inner) - assert.strictEqual(LLMObsTagger.tagMap.get(inner)['_ml_obs.llmobs_parent_id'], outer.context().toSpanId()) + assert.strictEqual(llmobs._active(), inner._span) + assert.strictEqual(LLMObsTagger.tagMap.get(inner)['_ml_obs.llmobs_parent_id'], + outer.context().toSpanId()) }) }) }) @@ -395,7 +400,7 @@ describe('sdk', () => { it('wraps a function', () => { let span const fn = llmobs.wrap({ kind: 'workflow' }, () => { - span = tracer.scope().active() + span = tracer.scope().active()._span sinon.spy(span, 'finish') }) @@ -409,7 +414,7 @@ describe('sdk', () => { let next const fn = llmobs.wrap({ kind: 'workflow' }, (_next) => { - span = tracer.scope().active() + span = tracer.scope().active()._span sinon.spy(span, 'finish') next = _next }) @@ -710,7 +715,7 @@ describe('sdk', () => { function outerLLMObs () { outerLLMObsSpan = llmobs._active() - assert.strictEqual(outerLLMObsSpan, tracer.scope().active()) + assert.strictEqual(outerLLMObsSpan, tracer.scope().active()._span) apmWrapped() } @@ -720,7 +725,7 @@ describe('sdk', () => { } function innerLLMObs () { innerLLMObsSpan = llmobs._active() - assert.strictEqual(innerLLMObsSpan, tracer.scope().active()) + assert.strictEqual(innerLLMObsSpan, tracer.scope().active()._span) assert.strictEqual( LLMObsTagger.tagMap.get(innerLLMObsSpan)['_ml_obs.llmobs_parent_id'], outerLLMObsSpan.context().toSpanId() @@ -768,12 +773,12 @@ describe('sdk', () => { function outer () { outerSpan = llmobs._active() wrappedInner1(() => {}) - assert.strictEqual(outerSpan, tracer.scope().active()) + assert.strictEqual(outerSpan, tracer.scope().active()._span) wrappedInner2() } function inner1 (cb) { - const inner = tracer.scope().active() + const inner = tracer.scope().active()._span assert.strictEqual(llmobs._active(), inner) assert.strictEqual( LLMObsTagger.tagMap.get(inner)['_ml_obs.llmobs_parent_id'], @@ -783,7 +788,7 @@ describe('sdk', () => { } function inner2 () { - const inner = tracer.scope().active() + const inner = tracer.scope().active()._span assert.strictEqual(llmobs._active(), inner) assert.strictEqual( LLMObsTagger.tagMap.get(inner)['_ml_obs.llmobs_parent_id'], @@ -898,7 +903,7 @@ describe('sdk', () => { const inputData = {} llmobs.annotate({ inputData }) - sinon.assert.calledWith(llmobs._tagger.tagTextIO, span, inputData, undefined) + sinon.assert.calledWith(llmobs._tagger.tagTextIO, span._span, inputData, undefined) }) llmobs._tagger.tagTextIO.restore() @@ -912,7 +917,7 @@ describe('sdk', () => { const inputData = {} llmobs.annotate({ inputData }) - sinon.assert.calledWith(llmobs._tagger.tagTextIO, llmobsSpan, inputData, undefined) + sinon.assert.calledWith(llmobs._tagger.tagTextIO, llmobsSpan._span, inputData, undefined) }) }) diff --git a/packages/dd-trace/test/proxy.spec.js b/packages/dd-trace/test/proxy.spec.js index 4c42a2f2a4f..3a10c4e320c 100644 --- a/packages/dd-trace/test/proxy.spec.js +++ b/packages/dd-trace/test/proxy.spec.js @@ -5,7 +5,6 @@ const assert = require('node:assert/strict') const { describe, it, beforeEach, afterEach } = require('mocha') const sinon = require('sinon') const proxyquire = require('proxyquire') - require('./setup/core') describe('TracerProxy', () => { @@ -66,7 +65,7 @@ describe('TracerProxy', () => { use: sinon.stub().returns('tracer'), trace: sinon.stub().returns('test'), wrap: sinon.stub().returns('fn'), - startSpan: sinon.stub().returns('span'), + startSpan: sinon.stub().returns({ id: 'span' }), inject: sinon.stub().returns('tracer'), extract: sinon.stub().returns('spanContext'), setUrl: sinon.stub(), @@ -77,7 +76,7 @@ describe('TracerProxy', () => { use: sinon.stub().returns('tracer'), trace: sinon.stub().returns('test'), wrap: sinon.stub().returns('fn'), - startSpan: sinon.stub().returns('span'), + startSpan: sinon.stub().returns({ id: 'span' }), inject: sinon.stub().returns('noop'), extract: sinon.stub().returns('spanContext'), setUrl: sinon.stub(), @@ -103,6 +102,9 @@ describe('TracerProxy', () => { flush: sinon.spy(), } + tracer.scope = sinon.stub() + noop.scope = sinon.stub() + { const dogstatsdIncrements = [] let dogstatsdConfig @@ -626,11 +628,22 @@ describe('TracerProxy', () => { it('should work without options', () => { const callback = () => 'test' const returnValue = proxy.trace('a', callback) - sinon.assert.calledWith(noop.trace, 'a', {}, callback) assert.strictEqual(returnValue, 'test') }) + it('should set service source override tag when service option is provided', () => { + const callback = () => 'test' + proxy.trace('a', { service: 'b' }, callback) + + sinon.assert.calledWith(noop.trace, 'a', { + service: 'b', + tags: { + '_dd.svc_src': 'm', + }, + }, callback) + }) + it('should ignore calls without an invalid callback', () => { proxy.wrap('a', 'b') @@ -668,7 +681,30 @@ describe('TracerProxy', () => { const returnValue = proxy.startSpan('a', 'b', 'c') sinon.assert.calledWith(noop.startSpan, 'a', 'b', 'c') - assert.strictEqual(returnValue, 'span') + assert.deepEqual(returnValue._span, { id: 'span' }) + }) + + it('should set service source override tag when returned span does a setTag', () => { + const spanStub = { setTag: sinon.spy() } + noop.startSpan.returns(spanStub) + const setTagSpy = spanStub.setTag + + const returnValue = proxy.startSpan('a', 'b', 'c') + returnValue.setTag('service', 'b') + assert.strictEqual(setTagSpy.callCount, 2) + sinon.assert.calledWith(setTagSpy.firstCall, '_dd.svc_src', 'm') + sinon.assert.calledWith(setTagSpy.secondCall, 'service', 'b') + }) + + it('should not set service source override tag when returned span does not setTag', () => { + const spanStub = { setTag: sinon.spy() } + noop.startSpan.returns(spanStub) + const setTagSpy = spanStub.setTag + + const returnValue = proxy.startSpan('a', 'b', 'c') + returnValue.setTag('randomTag', 'b') + assert.strictEqual(setTagSpy.callCount, 1) + sinon.assert.calledWith(setTagSpy.firstCall, 'randomTag', 'b') }) }) @@ -691,11 +727,28 @@ describe('TracerProxy', () => { }) describe('setUrl', () => { - it('should call the underlying DatadogTracer', () => { + it('should call the underlying NoopTracer', () => { const returnValue = proxy.setUrl('http://example.com') - sinon.assert.calledWith(noop.setUrl, 'http://example.com') assert.strictEqual(returnValue, proxy) + sinon.assert.calledWith(noop.setUrl, 'http://example.com') + }) + }) + + describe('scope', () => { + it('should return the underlying noop scope', () => { + const scope = { + active: sinon.stub().returns(null), + activate: sinon.stub(), + bind: sinon.stub(), + } + + noop.scope.returns(scope) + + const returnValue = proxy.scope() + + assert.strictEqual(returnValue, scope) + sinon.assert.calledOnce(noop.scope) }) }) @@ -848,6 +901,18 @@ describe('TracerProxy', () => { assert.strictEqual(returnValue, 'test') }) + it('should set service source override tag when service option is provided', () => { + const callback = () => 'test' + proxy.trace('a', { service: 'b' }, callback) + + sinon.assert.calledWith(tracer.trace, 'a', { + service: 'b', + tags: { + '_dd.svc_src': 'm', + }, + }, callback) + }) + it('should work without options', () => { const callback = () => 'test' const returnValue = proxy.trace('a', callback) @@ -866,6 +931,21 @@ describe('TracerProxy', () => { assert.strictEqual(returnValue, 'fn') }) + it('should add _dd.svc_src to tags when a service override is provided through options', () => { + const callback = () => 'test' + const returnValue = proxy.wrap('a', { + service: 'custom-service', + }, callback) + + sinon.assert.calledWith(tracer.wrap, 'a', { + service: 'custom-service', + tags: { + '_dd.svc_src': 'm', + }, + }, callback) + assert.strictEqual(returnValue, 'fn') + }) + it('should work without options', () => { const callback = () => 'test' const returnValue = proxy.wrap('a', callback) @@ -880,7 +960,30 @@ describe('TracerProxy', () => { const returnValue = proxy.startSpan('a', 'b', 'c') sinon.assert.calledWith(tracer.startSpan, 'a', 'b', 'c') - assert.strictEqual(returnValue, 'span') + assert.deepEqual(returnValue._span, { id: 'span' }) + }) + + it('should set service source override tag when returned span does a setTag', () => { + const spanStub = { setTag: sinon.spy() } + tracer.startSpan.returns(spanStub) + const setTagSpy = spanStub.setTag + + const returnValue = proxy.startSpan('a', 'b', 'c') + returnValue.setTag('service', 'b') + assert.strictEqual(setTagSpy.callCount, 2) + sinon.assert.calledWith(setTagSpy.firstCall, '_dd.svc_src', 'm') + sinon.assert.calledWith(setTagSpy.secondCall, 'service', 'b') + }) + + it('should not set service source override tag when returned span does not setTag', () => { + const spanStub = { setTag: sinon.spy() } + tracer.startSpan.returns(spanStub) + const setTagSpy = spanStub.setTag + + const returnValue = proxy.startSpan('a', 'b', 'c') + returnValue.setTag('randomTag', 'b') + assert.strictEqual(setTagSpy.callCount, 1) + sinon.assert.calledWith(setTagSpy.firstCall, 'randomTag', 'b') }) }) @@ -911,6 +1014,23 @@ describe('TracerProxy', () => { }) }) + describe('scope', () => { + it('should return the underlying tracer scope', () => { + const scope = { + active: sinon.stub().returns(null), + activate: sinon.stub(), + bind: sinon.stub(), + } + + tracer.scope.returns(scope) + + const returnValue = proxy.scope() + + assert.strictEqual(returnValue, scope) + sinon.assert.calledOnce(tracer.scope) + }) + }) + describe('appsec', () => { describe('trackUserLoginSuccessEvent', () => { it('should call the underlying AppsecSdk method', () => { diff --git a/packages/dd-trace/test/scope.spec.js b/packages/dd-trace/test/scope.spec.js index d84e3113a47..1aac37bbd53 100644 --- a/packages/dd-trace/test/scope.spec.js +++ b/packages/dd-trace/test/scope.spec.js @@ -6,6 +6,7 @@ const { describe, it, beforeEach } = require('mocha') const sinon = require('sinon') const { Span } = require('../../../vendor/dist/opentracing') +const PublicSpan = require('../src/opentracing/public/span') require('./setup/core') const Scope = require('../src/scope') @@ -15,13 +16,20 @@ describe('Scope', () => { beforeEach(() => { scope = new Scope() - span = new Span() + span = new PublicSpan(new Span()) }) describe('active()', () => { it('should return null by default', () => { assert.strictEqual(scope.active(), null) }) + + it('should return a PublicSpan wrapping the active span', () => { + scope.activate(span, () => { + const active = scope.active() + assert.strictEqual(active, span) + }) + }) }) describe('activate()', () => { @@ -51,6 +59,15 @@ describe('Scope', () => { assert.strictEqual(scope.active(), null) }) + it('should unwrap a PublicSpan before activating', () => { + scope.activate(span, () => { + const publicSpan = scope.active() + scope.activate(publicSpan, () => { + assert.strictEqual(scope.active(), span) + }) + }) + }) + it('should persist through setTimeout', done => { scope.activate(span, () => { setTimeout(() => { @@ -119,14 +136,14 @@ describe('Scope', () => { it('should handle errors', () => { const error = new Error('boom') - sinon.spy(span, 'setTag') + sinon.spy(span._span, 'setTag') try { scope.activate(span, () => { throw error }) } catch (e) { - sinon.assert.calledWith(span.setTag, 'error', e) + sinon.assert.calledWith(span._span.setTag, 'error', e) } }) }) diff --git a/packages/dd-trace/test/tracer.spec.js b/packages/dd-trace/test/tracer.spec.js index 871d195aa47..cdcf3376dad 100644 --- a/packages/dd-trace/test/tracer.spec.js +++ b/packages/dd-trace/test/tracer.spec.js @@ -7,6 +7,7 @@ const sinon = require('sinon') const { assertObjectContains } = require('../../../integration-tests/helpers') require('./setup/core') +const { storage } = require('../../datadog-core') const Tracer = require('../src/tracer') const Span = require('../src/opentracing/span') const getConfig = require('../src/config') @@ -52,7 +53,7 @@ describe('Tracer', () => { describe('trace', () => { it('should run the callback with a new span', () => { tracer.trace('name', {}, span => { - assert.ok(span instanceof Span) + assert.ok(span._span instanceof Span) assert.strictEqual(span.context()._name, 'name') }) }) @@ -68,7 +69,7 @@ describe('Tracer', () => { } tracer.trace('name', options, span => { - assert.ok(span instanceof Span) + assert.ok(span._span instanceof Span) assertObjectContains(span.context()._tags, options.tags) assertObjectContains(span.context()._tags, { [SERVICE_NAME]: 'service', @@ -110,7 +111,7 @@ describe('Tracer', () => { it('should start the span as a child of the active span', () => { const childOf = tracer.startSpan('parent') - tracer.scope().activate(childOf, () => { + storage('legacy').run({ span: childOf }, () => { tracer.trace('name', {}, span => { assert.strictEqual(span.context()._parentId.toString(10), childOf.context().toSpanId()) }) @@ -121,7 +122,7 @@ describe('Tracer', () => { const root = tracer.startSpan('root') const childOf = tracer.startSpan('parent') - tracer.scope().activate(root, () => { + storage('legacy').run({ span: root }, () => { tracer.trace('name', { childOf }, span => { assert.strictEqual(span.context()._parentId.toString(10), childOf.context().toSpanId()) }) @@ -138,7 +139,7 @@ describe('Tracer', () => { let span tracer.trace('name', {}, (_span) => { - span = _span + span = _span._span sinon.spy(span, 'finish') }) @@ -151,8 +152,8 @@ describe('Tracer', () => { try { tracer.trace('name', {}, _span => { - span = _span - tags = span.context()._tags + span = _span._span + tags = _span.context()._tags sinon.spy(span, 'finish') throw new Error('boom') }) @@ -172,7 +173,7 @@ describe('Tracer', () => { let done tracer.trace('name', {}, (_span, _done) => { - span = _span + span = _span._span sinon.spy(span, 'finish') done = _done }) @@ -191,8 +192,8 @@ describe('Tracer', () => { let done tracer.trace('name', {}, (_span, _done) => { - span = _span - tags = span.context()._tags + span = _span._span + tags = _span.context()._tags sinon.spy(span, 'finish') done = _done }) @@ -219,7 +220,7 @@ describe('Tracer', () => { tracer .trace('name', {}, _span => { - span = _span + span = _span._span sinon.spy(span, 'finish') return promise }) @@ -240,8 +241,8 @@ describe('Tracer', () => { tracer .trace('name', {}, _span => { - span = _span - tags = span.context()._tags + span = _span._span + tags = _span.context()._tags sinon.spy(span, 'finish') return Promise.reject(new Error('boom')) }) @@ -326,7 +327,7 @@ describe('Tracer', () => { it('should wait for the callback to be called before finishing the span', done => { const fn = tracer.wrap('name', {}, sinon.spy(function (cb) { - const span = tracer.scope().active() + const span = tracer.scope().active()._span sinon.spy(span, 'finish')