Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .github/workflows/plugins.yml
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,23 @@ jobs:
uses: ./.github/actions/testagent/logs
- uses: codecov/codecov-action@v2

openai:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests weren't running in CI at all! I never noticed since I only pushed code if tests passed locally.

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:
Expand Down
6 changes: 4 additions & 2 deletions packages/datadog-plugin-openai/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)}`
}

/**
Expand Down
22 changes: 13 additions & 9 deletions packages/datadog-plugin-openai/src/services.js
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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,
Expand All @@ -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()
Expand Down
27 changes: 23 additions & 4 deletions packages/datadog-plugin-openai/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,8 +23,10 @@ describe('Plugin', () => {

describe('openai', () => {
withVersions('openai', 'openai', version => {
const moduleRequirePath = `../../../versions/openai@${version}`

beforeEach(() => {
require('../../dd-trace')
require(tracerRequirePath)
})

before(() => {
Expand All @@ -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'
Expand All @@ -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)
})

Expand All @@ -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

Expand Down
11 changes: 11 additions & 0 deletions packages/datadog-plugin-openai/test/no-init.js
Original file line number Diff line number Diff line change
@@ -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()
9 changes: 8 additions & 1 deletion packages/datadog-plugin-openai/test/services.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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')
})
})
})
})
10 changes: 9 additions & 1 deletion packages/dd-trace/src/external-logger/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,12 @@ class ExternalLogger {
}
}

module.exports = ExternalLogger
class NoopExternalLogger {
log () { }
enqueue () { }
shutdown () { }
flush () { }
}

module.exports.ExternalLogger = ExternalLogger
module.exports.NoopExternalLogger = NoopExternalLogger
2 changes: 1 addition & 1 deletion packages/dd-trace/src/external-logger/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('External Logger', () => {
beforeEach(() => {
errorLog = sinon.spy(tracerLogger, 'error')

const ExternalLogger = proxyquire('../src', {
const { ExternalLogger } = proxyquire('../src', {
'../../log': {
error: errorLog
}
Expand Down