Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/instrumentation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ jobs:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: ./.github/actions/instrumentations/test

instrumentation-anthropic-lifecycle:
runs-on: ubuntu-latest
permissions:
id-token: write
env:
PLUGINS: anthropic|anthropic-lifecycle
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

- uses: ./.github/actions/instrumentations/test

instrumentation-aws-sdk:
runs-on: ubuntu-latest
permissions:
Expand Down
191 changes: 180 additions & 11 deletions packages/datadog-instrumentations/src/anthropic.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,114 @@ const { addHook } = require('./helpers/instrument')

const anthropicTracingChannel = tracingChannel('apm:anthropic:request')
const onStreamedChunkCh = channel('apm:anthropic:request:chunk')
const messagesBeforeChannel = channel('dd-trace:anthropic:messages:before')
const messagesAfterChannel = channel('dd-trace:anthropic:messages:after')

/**
* Publishes a provider-native lifecycle payload to a cancelable lifecycle channel.
*
* Subscribers push async work into `pending` synchronously during publication and
* abort `abortController` with an error before the pushed promise resolves to block.
*
* @param {object} channel
* @param {object} payload
* @returns {Promise<void>}
*/
function publishLifecycle (channel, payload) {
const abortController = new AbortController()
const ctx = { ...payload, abortController, pending: [] }

channel.publish(ctx)

return Promise.all(ctx.pending).then(() => {
if (abortController.signal.aborted) {
throw abortController.signal.reason
}
})
}

/**
* @template T
* @param {Promise<T>} promise
* @param {Promise<void>|undefined} verdict
* @returns {Promise<T>}
*/
function waitForVerdict (promise, verdict) {
return verdict
? Promise.all([verdict, promise]).then(([, value]) => value)
: promise
}

/**
* @param {Array<unknown>} args
* @returns {Array<unknown>}
*/
function snapshotLifecycleArgs (args) {
const options = args[0]
if (!options || typeof options !== 'object') return args

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This keeps the actual arguments and I think we should skip inspection if something is wrong.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This will not cause an evaluation, when we have no valid messages array we're going to skip evaluations


const input = { messages: options.messages }
if (options.system !== undefined) input.system = options.system

const snapshot = [...args]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should move this into the try to make sure copying works, since it is not guaranteed to be an iterable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

args is guaranteed to be an array because it comes directly from wrapCreate, also make sense to keep it outside and reserve the try/catch for structuredClone failure only.

try {
snapshot[0] = { ...options, ...structuredClone(input) }
} catch {
// Custom non-cloneable message content is left untouched rather than breaking the request.
}
return snapshot
}

/**
* Runs the output verdict for a parsed response and finishes the span with it. Finishing after the
* verdict lets a block propagate to anthropic.request and keeps the span wrapping its child.
*
* @param {object} ctx
* @param {object} result
* @param {(body: object) => Promise<void>|undefined} getVerdict
* @param {object|string} [returnedResult]
* @returns {object|string|Promise<object|string>}
*/
function finishResult (ctx, result, getVerdict, returnedResult = result) {
const verdict = getVerdict(result)
if (!verdict) {
finish(ctx, result, null)
return returnedResult
}

return verdict.then(() => {
finish(ctx, result, null)
return returnedResult
})
}

/**
* @param {object} response
* @param {'json'|'text'} method
* @param {object} ctx
* @param {(body: object) => Promise<void>|undefined} getVerdict
*/
function wrapResponseReader (response, method, ctx, getVerdict) {
if (typeof response[method] !== 'function') return

shimmer.wrap(response, method, original => function (...args) {
return original.apply(this, args)
.then(body => {
if (method === 'json') return finishResult(ctx, body, getVerdict)

try {
return finishResult(ctx, JSON.parse(body), getVerdict, body)
} catch {
finish(ctx)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid finalizing nested text reads as success

When asResponse() returns a node-fetch Response, response.json() delegates to this.text(). Because both readers are wrapped, malformed JSON enters the inner text wrapper, this catch calls finish(ctx) as a success, and only afterward the outer json() rejects; its catch skips error publication because ctx.finished is already true. This records anthropic.request as successful even though the reader failed. Avoid finalizing from the nested text call, and cover invalid JSON through the real node-fetch response path.

AGENTS.md reference: AGENTS.md:L128-L129

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is not a bug on the supported node-fetch. node-fetch 2.7 consumes the body directly; it never calls this.text().

return body
}
})
.catch(error => {
if (!ctx.finished) finish(ctx, null, error)
throw error
})
})
}

function wrapStreamIterator (iterator, ctx) {
return function (...args) {
Expand Down Expand Up @@ -34,40 +142,98 @@ function wrapStreamIterator (iterator, ctx) {

function wrapCreate (create) {
return function (...args) {
if (!anthropicTracingChannel.start.hasSubscribers) {
const options = args[0]
const stream = options?.stream

const hasLifecycle = !stream && (messagesBeforeChannel.hasSubscribers || messagesAfterChannel.hasSubscribers)
const lifecycleArgs = hasLifecycle ? snapshotLifecycleArgs(args) : args

if (!anthropicTracingChannel.start.hasSubscribers && !hasLifecycle) {
return create.apply(this, args)
}

const options = args[0]
const stream = options.stream

const ctx = { options, resource: 'create', baseUrl: this._client?.baseURL }

return anthropicTracingChannel.start.runStores(ctx, () => {
const parentSpan = hasLifecycle ? ctx.currentStore?.span : undefined

let apiPromise
try {
// Anthropic starts the request eagerly; the input verdict only gates result delivery.
apiPromise = create.apply(this, args)
} catch (error) {
finish(ctx, null, error)
throw error
}

shimmer.wrap(apiPromise, 'parse', parse => function (...args) {
return parse.apply(this, args)
let afterVerdict
let parseResult
let wrappedResponse

let beforeVerdict
function getBeforeVerdict () {
if (!hasLifecycle || beforeVerdict) return beforeVerdict
if (!messagesBeforeChannel.hasSubscribers) return

beforeVerdict = publishLifecycle(messagesBeforeChannel, { args: lifecycleArgs, parentSpan })
return beforeVerdict
}

/**
* @param {object|string} body
*/
function getAfterVerdict (body) {
if (!hasLifecycle || afterVerdict) return afterVerdict
if (!messagesAfterChannel.hasSubscribers) return

afterVerdict = publishLifecycle(messagesAfterChannel, { args: lifecycleArgs, body, parentSpan })
return afterVerdict
}

shimmer.wrap(apiPromise, 'parse', parse => function (...parseArgs) {
if (parseResult) return parseResult

const parsed = parse.apply(this, parseArgs)
parseResult = waitForVerdict(parsed, getBeforeVerdict())
Comment thread
IlyasShabi marked this conversation as resolved.
.then(response => {
if (stream) {
shimmer.wrap(response, Symbol.asyncIterator, iterator => wrapStreamIterator(iterator, ctx))
} else {
finish(ctx, response, null)
return response
}

return response
return finishResult(ctx, response, getAfterVerdict)
}).catch(error => {
finish(ctx, null, error)
if (!ctx.finished) finish(ctx, null, error)
throw error
})

return parseResult
})

if (typeof apiPromise.asResponse === 'function') {
shimmer.wrap(apiPromise, 'asResponse', origAsResponse => function (...asResponseArgs) {
return waitForVerdict(origAsResponse.apply(this, asResponseArgs), getBeforeVerdict())
.then(response => {
// Raw output evaluation supports the common json() and text() readers only.
if (!stream &&
(anthropicTracingChannel.start.hasSubscribers ||
afterVerdict ||
messagesAfterChannel.hasSubscribers) &&
wrappedResponse !== response) {
wrappedResponse = response
wrapResponseReader(response, 'json', ctx, getAfterVerdict)
wrapResponseReader(response, 'text', ctx, getAfterVerdict)
}

if (afterVerdict) return afterVerdict.then(() => response)
return response
})
Comment on lines +215 to +229

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
.then(response => {
// Raw output evaluation supports the common json() and text() readers only.
if (!stream &&
(anthropicTracingChannel.start.hasSubscribers ||
afterVerdict ||
messagesAfterChannel.hasSubscribers) &&
wrappedResponse !== response) {
wrappedResponse = response
wrapResponseReader(response, 'json', ctx, getAfterVerdict)
wrapResponseReader(response, 'text', ctx, getAfterVerdict)
}
if (afterVerdict) return afterVerdict.then(() => response)
return response
})

I believe this is the issue about asResponse being complained about by the AI findings.

What about removing this for now so that we can land partial support right away and land support for this afterwards as follow-up? :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added asResponse support in the initial PR and addressed some AI reviews such as #9492 (comment) as you noted.

For now, Im adding support only the common json() and text() and plan to add support for the remaining methods in a follow-up PR. If you think it's too large to review, I can limit it to parse() and move asResponse() to a separate PR.

.catch(error => {
if (!ctx.finished) finish(ctx, null, error)
throw error
})
})
}

anthropicTracingChannel.end.publish(ctx)

return apiPromise
Expand All @@ -76,13 +242,16 @@ function wrapCreate (create) {
}

function finish (ctx, result, error) {
if (ctx.finished) return

if (error) {
ctx.error = error
anthropicTracingChannel.error.publish(ctx)
}

// streamed responses are handled and set separately
ctx.result ??= result
ctx.finished = true

anthropicTracingChannel.asyncEnd.publish(ctx)
}
Expand Down
Loading
Loading