Skip to content

Commit

Permalink
chore: Addressed code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
bizob2828 committed Aug 22, 2024
1 parent cb423d6 commit 21e4b5f
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 67 deletions.
7 changes: 4 additions & 3 deletions api.js
Original file line number Diff line number Diff line change
Expand Up @@ -1915,6 +1915,7 @@ API.prototype.ignoreApdex = function ignoreApdex() {
* @param {Function} callback The function to execute in context.
*/
API.prototype.withLlmCustomAttributes = function withLlmCustomAttributes(context, callback) {
context = context || {}
const metric = this.agent.metrics.getOrCreateMetric(
NAMES.SUPPORTABILITY.API + '/withLlmCustomAttributes'
)
Expand All @@ -1932,7 +1933,7 @@ API.prototype.withLlmCustomAttributes = function withLlmCustomAttributes(context
return callback()
}

for (const [key, value] of Object.entries(context || {})) {
for (const [key, value] of Object.entries(context)) {
if (typeof value === 'object' || typeof value === 'function') {
logger.warn(`Invalid attribute type for ${key}. Skipped.`)
delete context[key]
Expand All @@ -1944,9 +1945,9 @@ API.prototype.withLlmCustomAttributes = function withLlmCustomAttributes(context
}

transaction._llmContextManager = transaction._llmContextManager || new AsyncLocalStorage()
const parentContext = transaction._llmContextManager.getStore()
const parentContext = transaction._llmContextManager.getStore() || {}

const fullContext = Object.assign({}, parentContext || {}, context || {})
const fullContext = Object.assign({}, parentContext, context)
return transaction._llmContextManager.run(fullContext, callback)
}

Expand Down
48 changes: 27 additions & 21 deletions test/unit/api/api-llm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,22 +123,20 @@ tap.test('Agent API LLM methods', (t) => {

t.test('withLlmCustomAttributes should handle no active transaction', (t) => {
const { api } = t.context
t.autoend()

t.equal(
api.withLlmCustomAttributes({ test: 1 }, () => {
t.equal(loggerMock.warn.callCount, 1)
return 1
}),
1
)
t.end()
})

t.test('withLlmCustomAttributes should handle an empty store', (t) => {
const { api } = t.context
const agent = api.agent

t.autoend()
helper.runInTransaction(api.agent, (tx) => {
agent.tracer.getTransaction = () => {
return tx
Expand All @@ -149,26 +147,26 @@ tap.test('Agent API LLM methods', (t) => {
}),
1
)
t.end()
})
})

t.test('withLlmCustomAttributes should handle no callback', (t) => {
const { api } = t.context
const agent = api.agent
t.autoend()
helper.runInTransaction(api.agent, (tx) => {
agent.tracer.getTransaction = () => {
return tx
}
api.withLlmCustomAttributes({ test: 1 }, null)
t.equal(loggerMock.warn.callCount, 1)
t.end()
})
})

t.test('withLlmCustomAttributes should normalize attributes', (t) => {
const { api } = t.context
const agent = api.agent
t.autoend()
helper.runInTransaction(api.agent, (tx) => {
agent.tracer.getTransaction = () => {
return tx
Expand All @@ -191,6 +189,7 @@ tap.test('Agent API LLM methods', (t) => {
t.notOk(parentContext.toDelete3)
t.equal(parentContext['llm.number'], 1)
t.equal(parentContext['llm.boolean'], true)
t.end()
}
)
})
Expand All @@ -204,24 +203,31 @@ tap.test('Agent API LLM methods', (t) => {
agent.tracer.getTransaction = () => {
return tx
}
api.withLlmCustomAttributes({ 'llm.step': '1', 'llm.path': 'root' }, () => {
const contextManager = tx._llmContextManager
const context = contextManager.getStore()
t.equal(context[`llm.step`], '1')
t.equal(context['llm.path'], 'root')
api.withLlmCustomAttributes({ 'llm.step': '1.1', 'llm.path': 'root/1' }, () => {
const contextManager = tx._llmContextManager
const context = contextManager.getStore()
t.equal(context[`llm.step`], '1.1')
t.equal(context['llm.path'], 'root/1')
})
api.withLlmCustomAttributes({ 'llm.step': '1.2', 'llm.path': 'root/2' }, () => {
api.withLlmCustomAttributes(
{ 'llm.step': '1', 'llm.path': 'root', 'llm.name': 'root' },
() => {
const contextManager = tx._llmContextManager
const context = contextManager.getStore()
t.equal(context[`llm.step`], '1.2')
t.equal(context['llm.path'], 'root/2')
})
})
t.equal(context[`llm.step`], '1')
t.equal(context['llm.path'], 'root')
t.equal(context['llm.name'], 'root')
api.withLlmCustomAttributes({ 'llm.step': '1.1', 'llm.path': 'root/1' }, () => {
const contextManager = tx._llmContextManager
const context = contextManager.getStore()
t.equal(context[`llm.step`], '1.1')
t.equal(context['llm.path'], 'root/1')
t.equal(context['llm.name'], 'root')
})
api.withLlmCustomAttributes({ 'llm.step': '1.2', 'llm.path': 'root/2' }, () => {
const contextManager = tx._llmContextManager
const context = contextManager.getStore()
t.equal(context[`llm.step`], '1.2')
t.equal(context['llm.path'], 'root/2')
t.equal(context['llm.name'], 'root')
t.end()
})
}
)
})
})

Expand Down
20 changes: 11 additions & 9 deletions test/unit/util/llm-utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,49 +24,51 @@ tap.test('extractLlmAttributes', (t) => {
})

tap.test('extractLlmContext', (t) => {
let tx
let agent
t.autoend()
t.beforeEach(() => {
tx = {
t.beforeEach((t) => {
const tx = {
_llmContextManager: new AsyncLocalStorage()
}
agent = {
t.context.agent = {
tracer: {
getTransaction: () => {
return tx
}
}
}
t.context.tx = tx
})

t.test('handle empty context', (t) => {
t.autoend()
const { tx, agent } = t.context
tx._llmContextManager.run(null, () => {
const llmContext = extractLlmContext(agent)
t.equal(typeof llmContext, 'object')
t.equal(Object.entries(llmContext).length, 0)
t.end()
})
})

t.test('extract LLM context', (t) => {
t.autoend()
const { tx, agent } = t.context
tx._llmContextManager.run({ 'llm.test': 1, 'skip': 2 }, () => {
const llmContext = extractLlmContext(agent)
t.equal(llmContext['llm.test'], 1)
t.notOk(llmContext.skip)
t.end()
})
})

t.test('no transaction', (t) => {
t.autoend()
const { tx, agent } = t.context
agent.tracer.getTransaction = () => {
return null
}
tx._llmContextManager.run(null, () => {
const llmContext = extractLlmContext(agent)
t.equal(typeof llmContext, 'object')
t.equal(Object.entries(llmContext).length, 0)
t.end()
})
})
t.end()
})
34 changes: 0 additions & 34 deletions test/versioned/openai/chat-completions.tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -449,40 +449,6 @@ tap.test('OpenAI instrumentation - chat completions', (t) => {
})
})

t.test('should create chat completion message and summary for every message sent', (test) => {
const { client, agent } = t.context
helper.runInTransaction(agent, async (tx) => {
const model = 'gpt-3.5-turbo-0613'
const content = 'You are a mathematician.'
await client.chat.completions.create({
max_tokens: 100,
temperature: 0.5,
model,
messages: [
{ role: 'user', content },
{ role: 'user', content: 'What does 1 plus 1 equal?' }
]
})

const events = agent.customEventAggregator.events.toArray()
test.equal(events.length, 4, 'should create a chat completion message and summary event')
const chatMsgs = events.filter(([{ type }]) => type === 'LlmChatCompletionMessage')
test.llmMessages({
tx,
chatMsgs,
model,
id: 'chatcmpl-87sb95K4EF2nuJRcTs43Tm9ntTeat',
resContent: '1 plus 2 is 3.',
reqContent: content
})

const chatSummary = events.filter(([{ type }]) => type === 'LlmChatCompletionSummary')[0]
test.llmSummary({ tx, model, chatSummary, tokenUsage: true })
tx.end()
test.end()
})
})

t.test('should record LLM custom events with attributes', (test) => {
const { client, agent } = t.context
const api = helper.getAgentApi()
Expand Down

0 comments on commit 21e4b5f

Please sign in to comment.