chore: Don't create OpenAI segments if ai_monitoring is disabled#3625
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3625 +/- ##
==========================================
+ Coverage 89.39% 97.71% +8.32%
==========================================
Files 437 438 +1
Lines 57537 57568 +31
Branches 1 1
==========================================
+ Hits 51435 56254 +4819
+ Misses 6102 1314 -4788
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I suspect there's a better way of handling this than to replicate checks in multiple methods across multiple implementations. Perhaps something like:
class Subscriber {
enable() {
if (this.shouldEnable === false) {
this.logger.trace(
{ packageName: this.packageName },
'Not subscribing to channel events. Instrumentation is disabled.'
)
return
}
// do the real stuff
}
}There was a problem hiding this comment.
Ideally, this would be enough. However, because collect_ai / ai_monitoring can be changed serverside, we have to check the config values after the subscribers have been setup/enabled.
Also, I wanted to maintain parity with the other AIM subscribers, and I'll likely refactor this ai_monitoring check logic in #3487
Description
OpenAI instrumentation should not create segments if
ai_monitoring.enabled === false. This PR adds this missing logic and tests to assert it.How to Test
Related Issues
Closes #3622