Skip to content

Commit

Permalink
Merge branch 'master' into khanayan123/trace_sampling_consistency
Browse files Browse the repository at this point in the history
  • Loading branch information
khanayan123 authored Feb 19, 2025
2 parents cd52ae3 + 3582be4 commit 33a41cf
Show file tree
Hide file tree
Showing 29 changed files with 365 additions and 79 deletions.
26 changes: 26 additions & 0 deletions .github/workflows/stale.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: 'Stale PR Bot'
on:
schedule:
- cron: '0 4 * * *'

permissions:
pull-requests: write

jobs:
stale:
runs-on: ubuntu-latest
steps:
- uses: actions/stale@5bef64f19d7facfb25b37b414482c7164d639639 # v9.1.0
with:
days-before-issue-stale: -1 # disabled for issues

days-before-pr-stale: 90 # 3 months
days-before-pr-close: 14 # 2 weeks
stale-pr-label: "stale"
exempt-pr-labels: "keep-open"
exempt-draft-pr: true
stale-pr-message: |
This pull request has been marked as stale due to 90 days of inactivity.
If this is still relevant, please update or comment to keep it open.
If this should be kept open indefinitely, please apply the label `keep-open`.
Otherwise, it will be automatically closed after 14 days.
2 changes: 1 addition & 1 deletion .github/workflows/system-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
uses: DataDog/system-tests/.github/workflows/compute-workflow-parameters.yml@main
with:
library: nodejs
scenarios_groups: essentials,appsec_rasp
scenarios_groups: essentials,appsec_rasp,debugger

system-tests:
runs-on: ${{ contains(fromJSON('["CROSSED_TRACING_LIBRARIES", "INTEGRATIONS"]'), matrix.scenario) && 'ubuntu-latest-16-cores' || 'ubuntu-latest' }}
Expand Down
6 changes: 2 additions & 4 deletions benchmark/sirun/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,11 @@ RUN wget -O sirun.tar.gz https://github.com/DataDog/sirun/releases/download/v0.1
RUN mkdir -p /usr/local/nvm \
&& wget -q -O - https://raw.githubusercontent.com/nvm-sh/nvm/v0.35.3/install.sh | bash \
&& . $NVM_DIR/nvm.sh \
&& nvm install --no-progress 14.21.3 \
&& nvm install --no-progress 16.20.1 \
&& nvm install --no-progress 18.16.1 \
&& nvm install --no-progress 20.4.0 \
&& nvm install --no-progress 22.10.0 \
&& nvm alias default 18 \
&& nvm use 18
&& nvm alias default 22 \
&& nvm use 22

RUN mkdir /opt/insecure-bank-js
RUN git clone --depth 1 https://github.com/hdiv/insecure-bank-js.git /opt/insecure-bank-js
Expand Down
4 changes: 2 additions & 2 deletions ci/init.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable no-console */
const tracer = require('../packages/dd-trace')
const { isTrue } = require('../packages/dd-trace/src/util')
const { isTrue, isFalse } = require('../packages/dd-trace/src/util')
const log = require('../packages/dd-trace/src/log')

const isJestWorker = !!process.env.JEST_WORKER_ID
Expand All @@ -23,7 +23,7 @@ const options = {
flushInterval: isJestWorker ? 0 : 5000
}

let shouldInit = true
let shouldInit = !isFalse(process.env.DD_CIVISIBILITY_ENABLED)

if (isPackageManager()) {
log.debug('dd-trace is not initialized in a package manager.')
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/pino/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const logger = require('pino')()
const server = http
.createServer((req, res) => {
const span = tracer.scope().active()
const contextTraceId = span.context().toTraceId()
const contextTraceId = span.context().toTraceId(true)
const contextSpanId = span.context().toSpanId()
logger.info(
{ custom: { trace_id: contextTraceId, span_id: contextSpanId } },
Expand Down
2 changes: 1 addition & 1 deletion packages/datadog-instrumentations/src/aws-sdk.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ function getMessage (request, error, result) {

function getChannelSuffix (name) {
// some resource identifiers have spaces between ex: bedrock runtime
name = name.replaceAll(' ', '')
name = String(name).replaceAll(' ', '')
return [
'cloudwatchlogs',
'dynamodb',
Expand Down
5 changes: 5 additions & 0 deletions packages/datadog-instrumentations/src/http/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ function patch (http, methodName) {
} catch (e) {
ctx.error = e
errorChannel.publish(ctx)
// if the initial request failed, ctx.req will be unset, we must close the span here
// fix for: https://github.com/DataDog/dd-trace-js/issues/5016
if (!ctx.req) {
finish()
}
throw e
} finally {
endChannel.publish(ctx)
Expand Down
86 changes: 53 additions & 33 deletions packages/datadog-instrumentations/src/openai.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,44 +306,33 @@ for (const shim of V4_PACKAGE_SHIMS) {
return ch.start.runStores(ctx, () => {
const apiProm = methodFn.apply(this, arguments)

if (baseResource === 'chat.completions' && typeof apiProm._thenUnwrap === 'function') {
// this should only ever be invoked from a client.beta.chat.completions.parse call
shimmer.wrap(apiProm, '_thenUnwrap', origApiPromThenUnwrap => function () {
// TODO(sam.brenner): I wonder if we can patch the APIPromise prototype instead, although
// we might not have access to everything we need...

// this is a new apipromise instance
const unwrappedPromise = origApiPromThenUnwrap.apply(this, arguments)

shimmer.wrap(unwrappedPromise, 'parse', origApiPromParse => function () {
const parsedPromise = origApiPromParse.apply(this, arguments)
.then(body => Promise.all([this.responsePromise, body]))

return handleUnwrappedAPIPromise(parsedPromise, ctx, stream, n)
})

return unwrappedPromise
})
}

// wrapping `parse` avoids problematic wrapping of `then` when trying to call
// `withResponse` in userland code after. This way, we can return the whole `APIPromise`
shimmer.wrap(apiProm, 'parse', origApiPromParse => function () {
return origApiPromParse.apply(this, arguments)
// the original response is wrapped in a promise, so we need to unwrap it
const parsedPromise = origApiPromParse.apply(this, arguments)
.then(body => Promise.all([this.responsePromise, body]))
.then(([{ response, options }, body]) => {
if (stream) {
if (body.iterator) {
shimmer.wrap(body, 'iterator', wrapStreamIterator(response, options, n, ctx))
} else {
shimmer.wrap(
body.response.body, Symbol.asyncIterator, wrapStreamIterator(response, options, n, ctx)
)
}
} else {
finish(ctx, {
headers: response.headers,
data: body,
request: {
path: response.url,
method: options.method
}
})
}

return body
})
.catch(error => {
finish(ctx, undefined, error)

throw error
})
.finally(() => {
// maybe we don't want to unwrap here in case the promise is re-used?
// other hand: we want to avoid resource leakage
shimmer.unwrap(apiProm, 'parse')
})
return handleUnwrappedAPIPromise(parsedPromise, ctx, stream, n)
})

ch.end.publish(ctx)
Expand All @@ -356,6 +345,37 @@ for (const shim of V4_PACKAGE_SHIMS) {
})
}

function handleUnwrappedAPIPromise (apiProm, ctx, stream, n) {
return apiProm
.then(([{ response, options }, body]) => {
if (stream) {
if (body.iterator) {
shimmer.wrap(body, 'iterator', wrapStreamIterator(response, options, n, ctx))
} else {
shimmer.wrap(
body.response.body, Symbol.asyncIterator, wrapStreamIterator(response, options, n, ctx)
)
}
} else {
finish(ctx, {
headers: response.headers,
data: body,
request: {
path: response.url,
method: options.method
}
})
}

return body
})
.catch(error => {
finish(ctx, undefined, error)

throw error
})
}

function finish (ctx, response, error) {
if (error) {
ctx.error = error
Expand Down
2 changes: 1 addition & 1 deletion packages/datadog-plugin-bunyan/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe('Plugin', () => {
const record = JSON.parse(stream.write.firstCall.args[0].toString())

expect(record.dd).to.deep.include({
trace_id: span.context().toTraceId(),
trace_id: span.context().toTraceId(true),
span_id: span.context().toSpanId()
})
})
Expand Down
4 changes: 3 additions & 1 deletion packages/datadog-plugin-http/src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ class HttpClientPlugin extends ClientPlugin {
addResponseHeaders(res, span, this.config)
}

addRequestHeaders(req, span, this.config)
if (req) {
addRequestHeaders(req, span, this.config)
}

this.config.hooks.request(span, req, res)

Expand Down
18 changes: 18 additions & 0 deletions packages/datadog-plugin-http/test/client.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,24 @@ describe('Plugin', () => {
})
})
}

it('should record unfinished http requests as error spans', done => {
agent
.use(traces => {
expect(traces[0][0]).to.have.property('error', 1)
expect(traces[0][0].meta).to.not.have.property('http.status_code')
})
.then(done)
.catch(done)

try {
http.request('http://httpbin.org/get', { headers: { BadHeader: 'a\nb' } }, res => {
res.on('data', () => { })
})
} catch {
// expected to throw error
}
})
})

describe('with late plugin initialization and an external subscriber', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/datadog-plugin-mongodb-core/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ function getQuery (cmd) {
if (!cmd || typeof cmd !== 'object' || Array.isArray(cmd)) return
if (cmd.query) return sanitizeBigInt(limitDepth(cmd.query))
if (cmd.filter) return sanitizeBigInt(limitDepth(cmd.filter))
if (cmd.pipeline) return sanitizeBigInt(limitDepth(cmd.pipeline))
}

function getResource (plugin, ns, query, operationName) {
Expand Down
19 changes: 19 additions & 0 deletions packages/datadog-plugin-mongodb-core/test/mongodb.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,25 @@ describe('Plugin', () => {
}).toArray()
})

it('should log the aggregate pipeline in mongodb.query', done => {
agent
.use(traces => {
const span = traces[0][0]
const resource = 'aggregate test.$cmd'
const query = '[{"$match":{"_id":"1234"}},{"$project":{"_id":1}}]'

expect(span).to.have.property('resource', resource)
expect(span.meta).to.have.property('mongodb.query', query)
})
.then(done)
.catch(done)

collection.aggregate([
{ $match: { _id: '1234' } },
{ $project: { _id: 1 } }
]).toArray()
})

it('should use the toJSON method of objects if it exists', done => {
const id = '123456781234567812345678'

Expand Down
54 changes: 53 additions & 1 deletion packages/datadog-plugin-openai/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2821,7 +2821,10 @@ describe('Plugin', () => {
}

if (semver.satisfies(realVersion, '>=4.0.0')) {
const result = await openai.chat.completions.create(params)
const prom = openai.chat.completions.create(params)
expect(prom).to.have.property('withResponse')

const result = await prom

expect(result.id).to.eql('chatcmpl-7GaWqyMTD9BLmkmy8SxyjUGX3KSRN')
expect(result.model).to.eql('gpt-3.5-turbo-0301')
Expand Down Expand Up @@ -3786,6 +3789,55 @@ describe('Plugin', () => {
}
})
}

if (semver.intersects('>=4.59.0', version)) {
it('makes a successful call with the beta chat completions', async () => {
nock('https://api.openai.com:443')
.post('/v1/chat/completions')
.reply(200, {
id: 'chatcmpl-7GaWqyMTD9BLmkmy8SxyjUGX3KSRN',
object: 'chat.completion',
created: 1684188020,
model: 'gpt-4o',
usage: {
prompt_tokens: 37,
completion_tokens: 10,
total_tokens: 47
},
choices: [
{
message: {
role: 'assistant',
content: 'I am doing well, how about you?'
},
finish_reason: 'stop',
index: 0
}
]
})

const checkTraces = agent
.use(traces => {
const span = traces[0][0]
expect(span).to.have.property('name', 'openai.request')
})

const prom = openai.beta.chat.completions.parse({
model: 'gpt-4o',
messages: [{ role: 'user', content: 'Hello, OpenAI!', name: 'hunter2' }],
temperature: 0.5,
stream: false
})

expect(prom).to.have.property('withResponse')

const response = await prom

expect(response.choices[0].message.content).to.eql('I am doing well, how about you?')

await checkTraces
})
}
})
})
})
6 changes: 3 additions & 3 deletions packages/datadog-plugin-pino/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe('Plugin', () => {
const record = JSON.parse(stream.write.firstCall.args[0].toString())

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

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

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

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

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

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

expect(record).to.include('message')
Expand Down
Loading

0 comments on commit 33a41cf

Please sign in to comment.