Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add version tracking metric on OpenAI events #1882

Closed
wants to merge 3 commits into from
Closed
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
109 changes: 88 additions & 21 deletions lib/instrumentation/openai.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@ const {
const LlmTrackedIds = require('../../lib/llm-events/tracked-ids')

const MIN_VERSION = '4.0.0'
const MIN_STREAM_VERSION = '4.12.2'
const { AI } = require('../../lib/metrics/names')
const semver = require('semver')

let TRACKING_METRIC = AI.TRACKING_PREFIX

/**
* Checks if we should skip instrumentation.
* Currently it checks if `ai_monitoring.enabled` is true
Expand All @@ -31,10 +34,11 @@ function shouldSkipInstrumentation(config, shim) {
return true
}

const { version: pkgVersion } = shim.require('./package.json')
const { pkgVersion } = shim
return semver.lt(pkgVersion, MIN_VERSION)
}

// eslint-disable-next-line sonarjs/cognitive-complexity
module.exports = function initialize(agent, openai, moduleName, shim) {
if (shouldSkipInstrumentation(agent.config, shim)) {
shim.logger.debug(
Expand All @@ -43,6 +47,11 @@ module.exports = function initialize(agent, openai, moduleName, shim) {
return
}

// Update the tracking metric name with the version of the library
// being instrumented. We do not have access to the version when
// initially declaring the variable.
TRACKING_METRIC = `${TRACKING_METRIC}${shim.pkgVersion}`

/**
* Adds apiKey and response headers to the active segment
* on symbols
Expand All @@ -53,11 +62,13 @@ module.exports = function initialize(agent, openai, moduleName, shim) {
function decorateSegment(result, apiKey) {
const segment = shim.getActiveSegment()

if (segment) {
segment[openAiApiKey] = apiKey
segment[openAiHeaders] =
result?.response?.headers && Object.fromEntries(result.response.headers)
if (!segment) {
return
}

segment[openAiApiKey] = apiKey
segment[openAiHeaders] =
result?.response?.headers && Object.fromEntries(result.response.headers)
}

/**
Expand All @@ -67,6 +78,7 @@ module.exports = function initialize(agent, openai, moduleName, shim) {
* @param {object} msg LLM event
*/
function recordEvent(type, msg) {
agent.metrics.getOrCreateMetric(TRACKING_METRIC).incrementCallCount()
msg = agent?.llm?.metadata ? { ...agent.llm.metadata, ...msg } : msg
agent.customEventAggregator.add([{ type, timestamp: Date.now() }, msg])
}
Expand Down Expand Up @@ -129,7 +141,12 @@ module.exports = function initialize(agent, openai, moduleName, shim) {
* @param {object} params.response chat completion response
*/
function recordChatCompletionMessages({ segment, request, response }) {
response.headers = segment[openAiHeaders]
response.api_key = segment[openAiApiKey]
const tx = segment.transaction
// explicitly end segment to consistent duration
// for both LLM events and the segment
segment.end()
const completionSummary = new LlmChatCompletionSummary({
agent,
segment,
Expand All @@ -155,6 +172,60 @@ module.exports = function initialize(agent, openai, moduleName, shim) {
})

recordEvent('LlmChatCompletionSummary', completionSummary)
delete response.headers
delete response.api_key
}

/*
* Chat completions create can return a stream once promise resolves
* This wraps the iterator which is a generator function
* We will call the original iterator, intercept chunks and yield
* to the original. On complete we will construct the new message object
* with what we have seen in the stream and create the chat completion
* messages
*
*/
function instrumentStream({ request, response, segment }) {
if (semver.lt(shim.pkgVersion, MIN_STREAM_VERSION)) {
shim.logger.warn(
`instrumenting chat completion streams is only supported with openai version ${MIN_STREAM_VERSION}+`
)
return
}

shim.wrap(response, 'iterator', function wrapIterator(shim, orig) {
return async function* wrappedIterator() {
let content = ''
let role = ''
let chunk
try {
const iterator = orig.apply(this, arguments)

for await (chunk of iterator) {
if (chunk.choices[0]?.delta?.role) {
role = chunk.choices[0]?.delta?.role
}

content += chunk.choices[0]?.delta?.content ?? ''
yield chunk
}
} catch (err) {
// TODO: I cannot figure out how to get an error to throw
// this may be unused code
} finally {
chunk.choices[0].message = { role, content }
// update segment duration since we want to extend the time it took to
// handle the stream
segment.touch()

recordChatCompletionMessages({
segment,
request,
response: chunk
})
}
}
})
}

/**
Expand All @@ -173,22 +244,15 @@ module.exports = function initialize(agent, openai, moduleName, shim) {
promise: true,
// eslint-disable-next-line max-params
after(_shim, _fn, _name, err, response, segment) {
response.headers = segment[openAiHeaders]
response.api_key = segment[openAiApiKey]

// TODO: add LlmErrorMessage on failure
// and exit
// See: https://github.com/newrelic/node-newrelic/issues/1845
// if (err) {}

recordChatCompletionMessages({
segment,
request,
response
})
// cleanup keys on response before returning to user code
delete response.api_key
delete response.headers
if (request.stream) {
instrumentStream({ request, response, segment })
} else {
recordChatCompletionMessages({
segment,
request,
response
})
}
}
}
}
Expand All @@ -210,6 +274,9 @@ module.exports = function initialize(agent, openai, moduleName, shim) {
after(_shim, _fn, _name, err, response, segment) {
response.headers = segment[openAiHeaders]
response.api_key = segment[openAiApiKey]
// explicitly end segment to consistent duration
// for both LLM events and the segment
segment.end()
const embedding = new LlmEmbedding({
agent,
segment,
Expand Down
2 changes: 1 addition & 1 deletion lib/llm-events/openai/event.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ module.exports = class LlmEvent {
*/
if (responseAttrs) {
this['request.model'] = request.model || request.engine
this.duration = segment?.getExclusiveDurationInMillis()
this.duration = segment?.getDurationInMillis()
this.api_key_last_four_digits = response?.api_key && `sk-${response.api_key.slice(-4)}`
this.responseAttrs(response)
}
Expand Down
1 change: 1 addition & 0 deletions lib/metrics/names.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ const EXPRESS = {
}

const AI = {
TRACKING_PREFIX: 'Nodejs/ML/OpenAI/',
Copy link
Member

Choose a reason for hiding this comment

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

i'd omit OpenAI as the prefix is technically Nodejs/ML.

OPEN_AI: 'AI/OpenAI'
}

Expand Down
6 changes: 3 additions & 3 deletions lib/shim/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@
* @param resolvedName
* @param shimName
*/
function createShimFromType(type, agent, moduleName, resolvedName, shimName) {
function createShimFromType(type, agent, moduleName, resolvedName, shimName, pkgVersion) {

Check failure on line 35 in lib/shim/index.js

View workflow job for this annotation

GitHub Actions / lint (lts/*)

Function 'createShimFromType' has too many parameters (6). Maximum allowed is 5

Check failure on line 35 in lib/shim/index.js

View workflow job for this annotation

GitHub Actions / lint (lts/*)

Function 'createShimFromType' has too many parameters (6). Maximum allowed is 5
let shim = null
if (properties.hasOwn(SHIM_TYPE_MAP, type)) {
const ShimClass = SHIM_TYPE_MAP[type]
shim = new ShimClass(agent, moduleName, resolvedName, shimName)
shim = new ShimClass(agent, moduleName, resolvedName, shimName, pkgVersion)
} else {
shim = new Shim(agent, moduleName, resolvedName, shimName)
shim = new Shim(agent, moduleName, resolvedName, shimName, pkgVersion)
}
return shim
}
Expand Down
4 changes: 3 additions & 1 deletion lib/shim/shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ const fnApply = Function.prototype.apply
* @param {string} moduleName - The name of the module being instrumented.
* @param {string} resolvedName - The full path to the loaded module.
* @param {string} shimName - Used to persist shim ids across different instances. This is
* @param {string} pkgVersion - version of package getting instrumented
* applicable to instrument that compliments each other across libraries(i.e - koa + koa-route/koa-router)
*/
function Shim(agent, moduleName, resolvedName, shimName) {
function Shim(agent, moduleName, resolvedName, shimName, pkgVersion) {
if (!agent || !moduleName) {
throw new Error('Shim must be initialized with an agent and module name.')
}
Expand All @@ -44,6 +45,7 @@ function Shim(agent, moduleName, resolvedName, shimName) {
this._debug = false
this.defineProperty(this, 'moduleName', moduleName)
this.assignId(shimName)
this.pkgVersion = pkgVersion

// Determine the root directory of the module.
let moduleRoot = null
Expand Down
7 changes: 6 additions & 1 deletion lib/shimmer.js
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,8 @@ function instrumentPostLoad(agent, nodule, moduleName, resolvedName, esmResolver
agent,
moduleName,
resolvedName,
instrumentation.shimName
instrumentation.shimName,
pkgVersion
)

applyDebugState(shim, resolvedNodule, esmResolver)
Expand Down Expand Up @@ -714,6 +715,10 @@ function tryGetVersion(shim) {
return
}

if (shim.pkgVersion) {
return shim.pkgVersion
}

const packageInfo = shim.require('./package.json')
if (!packageInfo) {
return
Expand Down
4 changes: 2 additions & 2 deletions test/unit/instrumentation/openai.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ test('openai unit tests', (t) => {
agent.config.ai_monitoring = { enabled: true }
const shim = new GenericShim(agent, 'openai')
sandbox.stub(shim, 'require')
shim.require.returns({ version: '4.0.0' })
shim.pkgVersion = '4.0.0'
sandbox.stub(shim.logger, 'debug')

t.context.agent = agent
Expand Down Expand Up @@ -57,7 +57,7 @@ test('openai unit tests', (t) => {
t.test('should not register instrumentation if openai is < 4.0.0', (t) => {
const { shim, agent, initialize } = t.context
const MockOpenAi = getMockModule()
shim.require.returns({ version: '3.7.0' })
shim.pkgVersion = '3.7.0'
initialize(agent, MockOpenAi, 'openai', shim)
t.equal(shim.logger.debug.callCount, 1, 'should log 2 debug messages')
t.equal(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ tap.test('LlmChatCompletionSummary', (t) => {
helper.runInTransaction(agent, (tx) => {
api.startSegment('fakeSegment', false, () => {
const segment = api.shim.getActiveSegment()
segment.end()
const chatSummaryEvent = new LlmChatCompletionSummary({
agent,
segment,
Expand Down
2 changes: 1 addition & 1 deletion test/unit/llm-events/openai/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function getExpectedResult(tx, event, type, completionId) {
'ingest_source': 'Node'
}
const resKeys = {
'duration': trace.children[0].getExclusiveDurationInMillis(),
'duration': trace.children[0].getDurationInMillis(),
'request.model': 'gpt-3.5-turbo-0613',
'api_key_last_four_digits': 'sk-7890',
'response.organization': 'new-relic',
Expand Down
1 change: 1 addition & 0 deletions test/unit/llm-events/openai/embedding.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ tap.test('LlmEmbedding', (t) => {
helper.runInTransaction(agent, (tx) => {
api.startSegment('fakeSegment', false, () => {
const segment = api.shim.getActiveSegment()
segment.end()
const embeddingEvent = new LlmEmbedding({ agent, segment, request: req, response: res })
const expected = getExpectedResult(tx, embeddingEvent, 'embedding')
t.same(embeddingEvent, expected)
Expand Down
Loading
Loading