Skip to content

Commit 95cc708

Browse files
committed
Address PR comments
1 parent acbd6b1 commit 95cc708

File tree

6 files changed

+77
-45
lines changed

6 files changed

+77
-45
lines changed

lib/subscribers/google-genai/base.js

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class GoogleGenAISubscriber extends Subscriber {
2222
}
2323

2424
get enabled() {
25-
return super.enabled && this.config?.ai_monitoring?.enabled
25+
return super.enabled && this.agent?.config?.ai_monitoring?.enabled
2626
}
2727

2828
/**
@@ -58,16 +58,11 @@ class GoogleGenAISubscriber extends Subscriber {
5858
recordChatCompletionMessages({
5959
segment,
6060
request,
61-
response,
61+
response = {},
6262
err,
6363
transaction
6464
}) {
6565
const agent = this.agent
66-
if (!response) {
67-
// If we get an error, it is possible that `response = null`.
68-
// In that case, we define it to be an empty object.
69-
response = {}
70-
}
7166

7267
// Explicitly end segment to provide consistent duration
7368
// for both LLM events and the segment
@@ -87,20 +82,21 @@ class GoogleGenAISubscriber extends Subscriber {
8782
const inputMessages = Array.isArray(request.contents) ? request.contents : [request.contents]
8883
const responseMessage = response?.candidates?.[0]?.content
8984
const messages = responseMessage !== undefined ? [...inputMessages, responseMessage] : inputMessages
90-
messages.forEach((message, index) => {
85+
for (let i = 0; i < messages.length; i++) {
86+
const message = messages[i]
9187
const completionMsg = new LlmChatCompletionMessage({
9288
agent,
9389
segment,
9490
transaction,
9591
request,
9692
response,
97-
index,
93+
index: i,
9894
completionId: completionSummary.id,
9995
message
10096
})
10197

10298
this.recordEvent({ type: 'LlmChatCompletionMessage', msg: completionMsg })
103-
})
99+
}
104100

105101
this.recordEvent({ type: 'LlmChatCompletionSummary', msg: completionSummary })
106102

lib/subscribers/google-genai/config.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const generateContentInternal = {
88
instrumentations: [
99
{
1010
channelName: 'nr_generateContentInternal',
11-
module: { name: '@google/genai', versionRange: '>=1.1.0 <1.5.0 || >=1.5.1', filePath: 'dist/node/index.cjs' },
11+
module: { name: '@google/genai', versionRange: '>=1.1.0', filePath: 'dist/node/index.cjs' },
1212
functionQuery: {
1313
className: 'Models',
1414
methodName: 'generateContentInternal',
@@ -17,7 +17,7 @@ const generateContentInternal = {
1717
},
1818
{
1919
channelName: 'nr_generateContentInternal',
20-
module: { name: '@google/genai', versionRange: '>=1.1.0 <1.5.0 || >=1.5.1', filePath: 'dist/node/index.mjs' },
20+
module: { name: '@google/genai', versionRange: '>=1.1.0', filePath: 'dist/node/index.mjs' },
2121
functionQuery: {
2222
className: 'Models',
2323
methodName: 'generateContentInternal',
@@ -32,7 +32,7 @@ const generateContentStreamInternal = {
3232
instrumentations: [
3333
{
3434
channelName: 'nr_generateContentStreamInternal',
35-
module: { name: '@google/genai', versionRange: '>=1.1.0 <1.5.0 || >=1.5.1', filePath: 'dist/node/index.cjs' },
35+
module: { name: '@google/genai', versionRange: '>=1.1.0', filePath: 'dist/node/index.cjs' },
3636
functionQuery: {
3737
className: 'Models',
3838
methodName: 'generateContentStreamInternal',
@@ -41,7 +41,7 @@ const generateContentStreamInternal = {
4141
},
4242
{
4343
channelName: 'nr_generateContentStreamInternal',
44-
module: { name: '@google/genai', versionRange: '>=1.1.0 <1.5.0 || >=1.5.1', filePath: 'dist/node/index.mjs' },
44+
module: { name: '@google/genai', versionRange: '>=1.1.0', filePath: 'dist/node/index.mjs' },
4545
functionQuery: {
4646
className: 'Models',
4747
methodName: 'generateContentStreamInternal',
@@ -56,7 +56,7 @@ const embedContent = {
5656
instrumentations: [
5757
{
5858
channelName: 'nr_embedContent',
59-
module: { name: '@google/genai', versionRange: '>=1.1.0 <1.5.0 || >=1.5.1', filePath: 'dist/node/index.cjs' },
59+
module: { name: '@google/genai', versionRange: '>=1.1.0', filePath: 'dist/node/index.cjs' },
6060
functionQuery: {
6161
className: 'Models',
6262
methodName: 'embedContent',
@@ -65,7 +65,7 @@ const embedContent = {
6565
},
6666
{
6767
channelName: 'nr_embedContent',
68-
module: { name: '@google/genai', versionRange: '>=1.1.0 <1.5.0 || >=1.5.1', filePath: 'dist/node/index.mjs' },
68+
module: { name: '@google/genai', versionRange: '>=1.1.0', filePath: 'dist/node/index.mjs' },
6969
functionQuery: {
7070
className: 'Models',
7171
methodName: 'embedContent',

lib/subscribers/google-genai/embed-content.js

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ const {
1212
} = require('../../../lib/llm-events/google-genai')
1313

1414
class GoogleGenAIEmbedContentSubscriber extends GoogleGenAISubscriber {
15-
constructor ({ agent, logger, channelName = 'nr_embedContent' }) {
16-
super({ agent, logger, channelName })
15+
constructor ({ agent, logger }) {
16+
super({ agent, logger, channelName: 'nr_embedContent' })
1717
}
1818

1919
handler(data, ctx) {
@@ -32,15 +32,11 @@ class GoogleGenAIEmbedContentSubscriber extends GoogleGenAISubscriber {
3232
return
3333
}
3434
this.addLlmMeta({ transaction: ctx.transaction, version: data.moduleVersion })
35-
let { result: response, arguments: args, error: err } = data
35+
// If we get an error, it is possible that `response = null`.
36+
// In that case, we define it to be an empty object.
37+
const { result: response = {}, arguments: args, error: err } = data
3638
const [request] = args
3739

38-
if (!response) {
39-
// If we get an error, it is possible that `response = null`.
40-
// In that case, we define it to be an empty object.
41-
response = {}
42-
}
43-
4440
// Explicitly end segment to get consistent duration
4541
// for both LLM events and the segment
4642
ctx.segment.end()

lib/subscribers/google-genai/generate-content-stream.js

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,16 @@ const GoogleGenAIGenerateContentSubscriber = require('./generate-content')
77
const { AI } = require('../../../lib/metrics/names')
88

99
class GoogleGenAIGenerateContentStreamSubscriber extends GoogleGenAIGenerateContentSubscriber {
10-
constructor({ agent, logger, channelName = 'nr_generateContentStreamInternal' }) {
11-
super({ agent, logger, channelName })
10+
constructor({ agent, logger }) {
11+
super({ agent, logger, channelName: 'nr_generateContentStreamInternal' })
12+
}
13+
14+
handler(data, ctx) {
15+
if (!this.agent?.config?.ai_monitoring?.streaming?.enabled) {
16+
return ctx
17+
}
18+
19+
return super.handler(data, ctx)
1220
}
1321

1422
/**
@@ -19,17 +27,7 @@ class GoogleGenAIGenerateContentStreamSubscriber extends GoogleGenAIGenerateCont
1927
* @param {TraceSegment} params.segment the active trace segment
2028
* @param {Transaction} params.transaction the active transaction
2129
*/
22-
async instrumentStream({ request, response, segment, transaction }) {
23-
const { agent, logger } = this
24-
25-
if (!agent.config.ai_monitoring.streaming.enabled) {
26-
logger.warn(
27-
'`ai_monitoring.streaming.enabled` is set to `false`, stream will not be instrumented.'
28-
)
29-
agent.metrics.getOrCreateMetric(AI.STREAMING_DISABLED).incrementCallCount()
30-
return
31-
}
32-
30+
instrumentStream({ request, response, segment, transaction }) {
3331
const originalNext = response.next
3432
let err
3533
let content
@@ -41,8 +39,8 @@ class GoogleGenAIGenerateContentStreamSubscriber extends GoogleGenAIGenerateCont
4139
try {
4240
result = await originalNext.apply(response, nextArgs)
4341
if (result?.value?.text) {
44-
modelVersion = result.value.modelVersion
45-
content = result.value.candidates[0].content
42+
modelVersion = result?.value?.modelVersion
43+
content = result?.value?.candidates?.[0]?.content
4644
entireMessage += result.value.text // readonly variable that equates to result.value.candidates[0].content.parts[0].text
4745
}
4846
if (result?.value?.candidates?.[0]?.finishReason) {
@@ -88,19 +86,26 @@ class GoogleGenAIGenerateContentStreamSubscriber extends GoogleGenAIGenerateCont
8886
}
8987

9088
asyncEnd(data) {
89+
if (!this.agent?.config?.ai_monitoring?.streaming?.enabled) {
90+
this.logger.warn(
91+
'`ai_monitoring.streaming.enabled` is set to `false`, stream will not be instrumented.'
92+
)
93+
this.agent.metrics.getOrCreateMetric(AI.STREAMING_DISABLED).incrementCallCount()
94+
95+
return
96+
}
9197
const ctx = this.agent.tracer.getContext()
9298
if (!ctx?.segment || !ctx?.transaction) {
9399
return
94100
}
95-
const { result: response, arguments: args, error: err } = data
101+
const { result: response, arguments: args } = data
96102
const [request] = args
103+
97104
this.instrumentStream({
98105
request,
99-
headers: ctx.extras?.headers,
100106
response,
101107
segment: ctx.segment,
102108
transaction: ctx.transaction,
103-
err
104109
})
105110

106111
this.addLlmMeta({

lib/subscribers/google-genai/generate-content.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ class GoogleGenAIGenerateContentSubscriber extends GoogleGenAISubscriber {
1313
}
1414

1515
handler(data, ctx) {
16+
if (!this.agent?.config?.ai_monitoring?.enabled) {
17+
return ctx
18+
}
19+
1620
const segment = this.agent.tracer.createSegment({
1721
name: GEMINI.COMPLETION,
1822
parent: ctx.segment,
@@ -22,6 +26,9 @@ class GoogleGenAIGenerateContentSubscriber extends GoogleGenAISubscriber {
2226
}
2327

2428
asyncEnd(data) {
29+
if (!this.agent?.config?.ai_monitoring?.enabled) {
30+
return
31+
}
2532
const ctx = this.agent.tracer.getContext()
2633
if (!ctx?.segment || !ctx?.transaction) {
2734
return
@@ -34,7 +41,6 @@ class GoogleGenAIGenerateContentSubscriber extends GoogleGenAISubscriber {
3441
transaction: ctx.transaction,
3542
request,
3643
response,
37-
headers: ctx.extras?.headers,
3844
err
3945
})
4046

test/versioned/google-genai/chat-completions.test.js

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,30 @@ test('handles error in stream', (t, end) => {
309309
})
310310

311311
// Other tests
312+
test('should not create llm events when ai_monitoring.enabled is false', (t, end) => {
313+
const { client, agent } = t.nr
314+
agent.config.ai_monitoring.enabled = false
315+
helper.runInTransaction(agent, async (tx) => {
316+
const model = 'gemini-2.0-flash'
317+
const result = await client.models.generateContent({
318+
model,
319+
contents: 'You are a mathematician.'
320+
})
321+
assert.ok(result)
322+
323+
const events = agent.customEventAggregator.events.toArray()
324+
assert.equal(events.length, 0, 'should not create llm events')
325+
326+
const activeSeg = agent.tracer.getSegment()
327+
assert.equal(activeSeg?.isRoot, true)
328+
const children = tx.trace.getChildren(activeSeg.id)
329+
assert.notEqual(children?.[0]?.name, GEMINI.COMPLETION)
330+
331+
tx.end()
332+
end()
333+
})
334+
})
335+
312336
test('should not create llm events when ai_monitoring.streaming.enabled is false', (t, end) => {
313337
const { client, agent } = t.nr
314338
agent.config.ai_monitoring.streaming.enabled = false
@@ -335,12 +359,17 @@ test('should not create llm events when ai_monitoring.streaming.enabled is false
335359
assert.equal(res, expectedRes.body.candidates[0].content.parts[0].text)
336360

337361
const events = agent.customEventAggregator.events.toArray()
338-
assert.equal(events.length, 0, 'should not llm events when streaming is disabled')
362+
assert.equal(events.length, 0, 'should not create llm events when streaming is disabled')
339363
const streamingDisabled = agent.metrics.getOrCreateMetric(
340364
'Supportability/Nodejs/ML/Streaming/Disabled'
341365
)
342366
assert.equal(streamingDisabled.callCount > 0, true)
343367

368+
const activeSeg = agent.tracer.getSegment()
369+
assert.equal(activeSeg?.isRoot, true)
370+
const children = tx.trace.getChildren(activeSeg.id)
371+
assert.notEqual(children?.[0]?.name, GEMINI.COMPLETION)
372+
344373
tx.end()
345374
end()
346375
})

0 commit comments

Comments
 (0)