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: removed api.setLlmMetadata as llm metadata will be assigned via api.addCustomAttribute with a prefix of llm. #1918

Merged
merged 1 commit into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 0 additions & 24 deletions api.js
Original file line number Diff line number Diff line change
Expand Up @@ -1544,7 +1544,7 @@
* @param {object} params Input parameters.
* @param {string} params.responseId The LLM generated identifier for the
* response.
* @returns {LlmTrackedIds|undefined} The tracked identifiers.

Check warning on line 1547 in api.js

View workflow job for this annotation

GitHub Actions / lint (lts/*)

The type 'LlmTrackedIds' is undefined

Check warning on line 1547 in api.js

View workflow job for this annotation

GitHub Actions / lint (lts/*)

The type 'LlmTrackedIds' is undefined
*/
API.prototype.getLlmMessageIds = function getLlmMessageIds({ responseId } = {}) {
this.agent.metrics
Expand Down Expand Up @@ -1869,28 +1869,4 @@
this.agent.errors.errorGroupCallback = callback
}

/**
* Function for assigning metadata to all LLM events. This should only
* be called once in your application and before executing any openai, bedrock,
* langchain, generative AI methods.
*
* If passing in metadata via `api.recordLlmFeedbackEvent`, it will take precedence
* over what is assigned via this method.
*
* @param {object} metadata key/value metadata to be assigned to all LLM Events on creation
* @example
* newrelic.setLlmMetadata({ type: 'openai', framework: 'express', region: 'us' })
*/
API.prototype.setLlmMetadata = function setLlmMetadata(metadata) {
const metric = this.agent.metrics.getOrCreateMetric(NAMES.SUPPORTABILITY.API + '/setLlmMetadata')
metric.incrementCallCount()

if (!isSimpleObject(metadata)) {
logger.warn('metadata must be an object, not assigning LLM metadata.')
return
}

this.agent.llm.metadata = metadata
}

module.exports = API
2 changes: 1 addition & 1 deletion lib/llm-events/openai/event.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ module.exports = class LlmEvent {
conversationId(agent) {
const transaction = agent.tracer.getTransaction()
const attrs = transaction?.trace?.custom.get(DESTINATIONS.TRANS_SCOPE)
return attrs?.conversation_id
return attrs?.['llm.conversation_id']
}
}
25 changes: 0 additions & 25 deletions test/unit/api/api-llm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const tap = require('tap')
const sinon = require('sinon')
const proxyquire = require('proxyquire')
const helper = require('../../lib/agent_helper')
const NAMES = require('../../../lib/metrics/names')

tap.test('Agent API LLM methods', (t) => {
t.autoend()
Expand All @@ -36,30 +35,6 @@ tap.test('Agent API LLM methods', (t) => {
helper.unloadAgent(t.context.api.agent)
})

t.test('should assign llm metadata when it is an object', (t) => {
const { api } = t.context
const meta = { user: 'bob', env: 'prod', random: 'data' }
api.setLlmMetadata(meta)

t.equal(loggerMock.warn.callCount, 0, 'should not log warnings when successful')
t.equal(
api.agent.metrics.getOrCreateMetric(NAMES.SUPPORTABILITY.API + '/setLlmMetadata').callCount,
1,
'should increment the API tracking metric'
)
t.same(api.agent.llm.metadata, meta)
t.end()
})
;['string', 10, true, null, undefined, [1, 2, 3, 4], [{ collection: true }]].forEach((meta) => {
t.test(`should not assign llm metadata when ${meta} is not an object`, (t) => {
const { api } = t.context
api.setLlmMetadata(meta)
t.equal(loggerMock.warn.callCount, 1, 'should log warning when metadata is not an object')
t.same(api.agent.llm, {})
t.end()
})
})

t.test('getLlmMessageIds is no-op when ai_monitoring is disabled', async (t) => {
const { api } = t.context
api.agent.config.ai_monitoring.enabled = false
Expand Down
2 changes: 1 addition & 1 deletion test/unit/api/stub.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const tap = require('tap')
const API = require('../../../stub_api')

const EXPECTED_API_COUNT = 36
const EXPECTED_API_COUNT = 35

tap.test('Agent API - Stubbed Agent API', (t) => {
t.autoend()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ tap.test('LlmChatCompletionMessage', (t) => {
const api = helper.getAgentApi()
const conversationId = 'convo-id'
helper.runInTransaction(agent, () => {
api.addCustomAttribute('conversation_id', conversationId)
api.addCustomAttribute('llm.conversation_id', conversationId)
const chatMessageEvent = new LlmChatCompletionMessage({
agent,
segment: {},
Expand Down
4 changes: 2 additions & 2 deletions test/unit/llm-events/openai/chat-completion-summary.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ tap.test('LlmChatCompletionSummary', (t) => {
const api = helper.getAgentApi()
const conversationId = 'convo-id'
helper.runInTransaction(agent, () => {
api.addCustomAttribute('conversation_id', conversationId)
api.addCustomAttribute('llm.conversation_id', conversationId)
const chatSummaryEvent = new LlmChatCompletionSummary({
agent,
segment: null,
Expand All @@ -61,7 +61,7 @@ tap.test('LlmChatCompletionSummary', (t) => {
const api = helper.getAgentApi()
const conversationId = 'convo-id'
helper.runInTransaction(agent, () => {
api.addCustomAttribute('conversation_id', conversationId)
api.addCustomAttribute('llm.conversation_id', conversationId)
const chatSummaryEvent = new LlmChatCompletionSummary({
agent,
segment: null,
Expand Down
27 changes: 0 additions & 27 deletions test/versioned/openai/chat-completions.tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,33 +276,6 @@ tap.test('OpenAI instrumentation - chat completions', (t) => {
})
}

t.test('should spread metadata across events if present on agent.llm.metadata', (test) => {
const { client, agent } = t.context
const api = helper.getAgentApi()
helper.runInTransaction(agent, async (tx) => {
const meta = { key: 'value', extended: true, vendor: 'overwriteMe', id: 'bogus' }
api.setLlmMetadata(meta)

await client.chat.completions.create({
messages: [{ role: 'user', content: 'You are a mathematician.' }]
})

const events = agent.customEventAggregator.events.toArray()
events.forEach(([, testEvent]) => {
test.equal(testEvent.key, 'value')
test.equal(testEvent.extended, true)
test.equal(
testEvent.vendor,
'openAI',
'should not override properties of message with metadata'
)
test.not(testEvent.id, 'bogus', 'should not override properties of message with metadata')
})
tx.end()
test.end()
})
})

t.test('should not create llm events when not in a transaction', async (test) => {
const { client, agent } = t.context
await client.chat.completions.create({
Expand Down
27 changes: 0 additions & 27 deletions test/versioned/openai/embeddings.tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,33 +114,6 @@ tap.test('OpenAI instrumentation - embedding', (t) => {
})
})

t.test('should spread metadata across events if present on agent.llm.metadata', (test) => {
const { client, agent } = t.context
const api = helper.getAgentApi()
helper.runInTransaction(agent, async (tx) => {
const meta = { key: 'value', extended: true, vendor: 'overwriteMe', id: 'bogus' }
api.setLlmMetadata(meta)

await client.embeddings.create({
input: 'This is an embedding test.',
model: 'text-embedding-ada-002'
})

const events = agent.customEventAggregator.events.toArray()
const [[, testEvent]] = events
test.equal(testEvent.key, 'value')
test.equal(testEvent.extended, true)
test.equal(
testEvent.vendor,
'openAI',
'should not override properties of message with metadata'
)
test.not(testEvent.id, 'bogus', 'should not override properties of message with metadata')
tx.end()
test.end()
})
})

t.test('embedding invalid payload errors should be tracked', (test) => {
const { client, agent } = t.context
helper.runInTransaction(agent, async (tx) => {
Expand Down