-
Notifications
You must be signed in to change notification settings - Fork 372
feat(ai, llmobs): properly support ToolLoopAgent via existing patching #7571
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
base: master
Are you sure you want to change the base?
Changes from all commits
28597e9
b8f0de9
25b5993
d06b7ef
ad24959
e752e0c
2871d69
f2cf595
a9ab279
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. before, it seems we didn't support different ranges in the instrumentation definition file. i think i could have also just put the instrumentation definitions in reverse order ( happy to do this however, though |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| 'use strict' | ||
|
|
||
| module.exports = [ | ||
| // getTracer - for patching tracer | ||
| { | ||
| module: { | ||
| name: 'ai', | ||
| versionRange: '>=4.0.0', | ||
| filePath: 'dist/index.js', | ||
| }, | ||
| functionQuery: { | ||
| functionName: 'getTracer', | ||
| kind: 'Sync', | ||
| }, | ||
| channelName: 'getTracer', | ||
| }, | ||
| { | ||
| module: { | ||
| name: 'ai', | ||
| versionRange: '>=4.0.0', | ||
| filePath: 'dist/index.mjs', | ||
| }, | ||
| functionQuery: { | ||
| functionName: 'getTracer', | ||
| kind: 'Sync', | ||
| }, | ||
| channelName: 'getTracer', | ||
| }, | ||
| // selectTelemetryAttributes - makes sure we set isEnabled properly | ||
| { | ||
| module: { | ||
| name: 'ai', | ||
| versionRange: '>=4.0.0 <6.0.0', | ||
| filePath: 'dist/index.js', | ||
| }, | ||
| functionQuery: { | ||
| functionName: 'selectTelemetryAttributes', | ||
| kind: 'Sync', | ||
| }, | ||
| channelName: 'selectTelemetryAttributes', | ||
| }, | ||
| { | ||
| module: { | ||
| name: 'ai', | ||
| versionRange: '>=4.0.0 <6.0.0', | ||
| filePath: 'dist/index.mjs', | ||
| }, | ||
| functionQuery: { | ||
| functionName: 'selectTelemetryAttributes', | ||
| kind: 'Sync', | ||
| }, | ||
| channelName: 'selectTelemetryAttributes', | ||
| }, | ||
| { | ||
| module: { | ||
| name: 'ai', | ||
| versionRange: '>=6.0.0', | ||
| filePath: 'dist/index.js', | ||
| }, | ||
| functionQuery: { | ||
| functionName: 'selectTelemetryAttributes', | ||
| kind: 'Async', | ||
| }, | ||
| channelName: 'selectTelemetryAttributes', | ||
| }, | ||
| { | ||
| module: { | ||
| name: 'ai', | ||
| versionRange: '>=6.0.0', | ||
| filePath: 'dist/index.mjs', | ||
| }, | ||
| functionQuery: { | ||
| functionName: 'selectTelemetryAttributes', | ||
| kind: 'Async', | ||
| }, | ||
| channelName: 'selectTelemetryAttributes', | ||
| }, | ||
| // tool | ||
| { | ||
| module: { | ||
| name: 'ai', | ||
| versionRange: '>=4.0.0', | ||
| filePath: 'dist/index.js', | ||
| }, | ||
| functionQuery: { | ||
| functionName: 'tool', | ||
| kind: 'Sync', | ||
| }, | ||
| channelName: 'tool', | ||
| }, | ||
| { | ||
| module: { | ||
| name: 'ai', | ||
| versionRange: '>=4.0.0', | ||
| filePath: 'dist/index.mjs', | ||
| }, | ||
| functionQuery: { | ||
| functionName: 'tool', | ||
| kind: 'Sync', | ||
| }, | ||
| channelName: 'tool', | ||
| }, | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| 'use strict' | ||
|
|
||
| module.exports = [ | ||
| ...require('./langchain'), | ||
| ...require('./ai'), | ||
| ...require('./bullmq'), | ||
| ...require('./langchain'), | ||
| ] |
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. assertions removed from this file are due to not actually adding a tracer/modifying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see note here - not sure if we'd really leak here since we should be 1:1 with already-created spans that will be GC'd anyways, so this should be as well.
i also thought about trying to detect the no-op span via a WeakSet, potentially with just one entry, as the only time we should see the same span reference here is that case, and then do this cloning if so. Otherwise, if from a custom or real OpenTelemetry tracer, all spans should be unique.