openai: make services entirely optional when initializing (#3420)

Author: tlhunter

.github/workflows/plugins.yml

@@ -786,6 +786,23 @@ jobs:
         uses: ./.github/actions/testagent/logs
       - uses: codecov/codecov-action@v2
 
+  openai:
+    runs-on: ubuntu-latest
+    env:
+      PLUGINS: openai
+    steps:
+      - uses: actions/checkout@v2
+      - uses: ./.github/actions/testagent/start
+      - uses: ./.github/actions/node/setup
+      - run: yarn install
+      - uses: ./.github/actions/node/oldest
+      - run: yarn test:plugins:ci
+      - uses: ./.github/actions/node/latest
+      - run: yarn test:plugins:ci
+      - if: always()
+        uses: ./.github/actions/testagent/logs
+      - uses: codecov/codecov-action@v2
+
   opensearch:
     runs-on: ubuntu-latest
     services:

packages/datadog-plugin-openai/src/index.js

@@ -26,7 +26,9 @@ class OpenApiPlugin extends TracingPlugin {
     this.sampler = new Sampler(0.1) // default 10% log sampling
 
     // hoist the max length env var to avoid making all of these functions a class method
-    MAX_TEXT_LEN = this._tracerConfig.openaiSpanCharLimit
+    if (this._tracerConfig) {
+      MAX_TEXT_LEN = this._tracerConfig.openaiSpanCharLimit
+    }
   }
 
   configure (config) {
@@ -547,7 +549,7 @@ function usageExtraction (tags, body) {
 }
 
 function truncateApiKey (apiKey) {
-  return `sk-...${apiKey.substr(apiKey.length - 4)}`
+  return apiKey && `sk-...${apiKey.substr(apiKey.length - 4)}`
 }
 
 /**

packages/datadog-plugin-openai/src/services.js

@@ -1,7 +1,7 @@
 'use strict'
 
 const { DogStatsDClient, NoopDogStatsDClient } = require('../../dd-trace/src/dogstatsd')
-const ExternalLogger = require('../../dd-trace/src/external-logger/src')
+const { ExternalLogger, NoopExternalLogger } = require('../../dd-trace/src/external-logger/src')
 
 const FLUSH_INTERVAL = 10 * 1000
 
@@ -10,7 +10,7 @@ let logger = null
 let interval = null
 
 module.exports.init = function (tracerConfig) {
-  if (tracerConfig.dogstatsd) {
+  if (tracerConfig && tracerConfig.dogstatsd) {
     metrics = new DogStatsDClient({
       host: tracerConfig.dogstatsd.hostname,
       port: tracerConfig.dogstatsd.port,
@@ -24,13 +24,17 @@ module.exports.init = function (tracerConfig) {
     metrics = new NoopDogStatsDClient()
   }
 
-  logger = new ExternalLogger({
-    ddsource: 'openai',
-    hostname: tracerConfig.hostname,
-    service: tracerConfig.service,
-    apiKey: tracerConfig.apiKey,
-    interval: FLUSH_INTERVAL
-  })
+  if (tracerConfig && tracerConfig.apiKey) {
+    logger = new ExternalLogger({
+      ddsource: 'openai',
+      hostname: tracerConfig.hostname,
+      service: tracerConfig.service,
+      apiKey: tracerConfig.apiKey,
+      interval: FLUSH_INTERVAL
+    })
+  } else {
+    logger = new NoopExternalLogger()
+  }
 
   interval = setInterval(() => {
     metrics.flush()

packages/datadog-plugin-openai/test/index.spec.js

@@ -6,12 +6,15 @@ const { expect } = require('chai')
 const semver = require('semver')
 const nock = require('nock')
 const sinon = require('sinon')
+const { spawn } = require('child_process')
 
 const agent = require('../../dd-trace/test/plugins/agent')
 const { DogStatsDClient } = require('../../dd-trace/src/dogstatsd')
-const ExternalLogger = require('../../dd-trace/src/external-logger/src')
+const { NoopExternalLogger } = require('../../dd-trace/src/external-logger/src')
 const Sampler = require('../../dd-trace/src/sampler')
 
+const tracerRequirePath = '../../dd-trace'
+
 describe('Plugin', () => {
   let openai
   let clock
@@ -20,8 +23,10 @@ describe('Plugin', () => {
 
   describe('openai', () => {
     withVersions('openai', 'openai', version => {
+      const moduleRequirePath = `../../../versions/openai@${version}`
+
       beforeEach(() => {
-        require('../../dd-trace')
+        require(tracerRequirePath)
       })
 
       before(() => {
@@ -34,7 +39,7 @@ describe('Plugin', () => {
 
       beforeEach(() => {
         clock = sinon.useFakeTimers()
-        const { Configuration, OpenAIApi } = require(`../../../versions/openai@${version}`).get()
+        const { Configuration, OpenAIApi } = require(moduleRequirePath).get()
 
         const configuration = new Configuration({
           apiKey: 'sk-DATADOG-ACCEPTANCE-TESTS'
@@ -43,7 +48,7 @@ describe('Plugin', () => {
         openai = new OpenAIApi(configuration)
 
         metricStub = sinon.stub(DogStatsDClient.prototype, '_add')
-        externalLoggerStub = sinon.stub(ExternalLogger.prototype, 'log')
+        externalLoggerStub = sinon.stub(NoopExternalLogger.prototype, 'log')
         sinon.stub(Sampler.prototype, 'isSampled').returns(true)
       })
 
@@ -52,6 +57,20 @@ describe('Plugin', () => {
         sinon.restore()
       })
 
+      describe('without initialization', () => {
+        it('should not error', (done) => {
+          spawn('node', ['no-init'], {
+            cwd: __dirname,
+            stdio: 'inherit',
+            env: {
+              ...process.env,
+              PATH_TO_DDTRACE: tracerRequirePath,
+              PATH_TO_OPENAI: moduleRequirePath
+            }
+          }).on('exit', done) // non-zero exit status fails test
+        })
+      })
+
       describe('createCompletion()', () => {
         let scope
 

packages/datadog-plugin-openai/test/no-init.js

@@ -0,0 +1,11 @@
+#!/usr/bin/env node
+
+/**
+ * Due to the complexity of the service initialization required by openai
+ * there was a bug where when requiring dd-trace followed by openai
+ * would result in an error if dd-trace wasn't first initialized.
+ *
+ * @see https://github.com/DataDog/dd-trace-js/issues/3357
+ */
+require(process.env.PATH_TO_DDTRACE)
+require(process.env.PATH_TO_OPENAI).get()

packages/datadog-plugin-openai/test/services.spec.js

@@ -9,7 +9,7 @@ describe('Plugin', () => {
         services.shutdown()
       })
 
-      it('dogstatsd does not throw', () => {
+      it('dogstatsd does not throw when missing .dogstatsd', () => {
         const service = services.init({
           hostname: 'foo',
           service: 'bar',
@@ -30,6 +30,13 @@ describe('Plugin', () => {
 
         service.logger.log('hello')
       })
+
+      it('logger does not throw when passing in null', () => {
+        const service = services.init(null)
+
+        service.metrics.increment('mykey')
+        service.logger.log('hello')
+      })
     })
   })
 })

packages/dd-trace/src/external-logger/src/index.js

@@ -127,4 +127,12 @@ class ExternalLogger {
   }
 }
 
-module.exports = ExternalLogger
+class NoopExternalLogger {
+  log () { }
+  enqueue () { }
+  shutdown () { }
+  flush () { }
+}
+
+module.exports.ExternalLogger = ExternalLogger
+module.exports.NoopExternalLogger = NoopExternalLogger

packages/dd-trace/src/external-logger/test/index.spec.js

@@ -15,7 +15,7 @@ describe('External Logger', () => {
   beforeEach(() => {
     errorLog = sinon.spy(tracerLogger, 'error')
 
-    const ExternalLogger = proxyquire('../src', {
+    const { ExternalLogger } = proxyquire('../src', {
       '../../log': {
         error: errorLog
       }