From 8e90de5660f320a98f4c14bf3a6e3a40e89cfaea Mon Sep 17 00:00:00 2001 From: rochdev Date: Wed, 24 Jan 2018 16:54:16 -0500 Subject: [PATCH 1/4] add http request + test utilities --- index.js | 9 ++-- package.json | 13 +++-- src/platform/index.js | 9 ++++ src/platform/node/index.js | 45 +++++++++++++++++ test/.eslintrc.json | 17 +++++++ test/mocha.opts | 1 + test/platform/node/index.spec.js | 84 ++++++++++++++++++++++++++++++++ test/setup.js | 15 ++++++ 8 files changed, 186 insertions(+), 7 deletions(-) create mode 100644 src/platform/index.js create mode 100644 src/platform/node/index.js create mode 100644 test/.eslintrc.json create mode 100644 test/platform/node/index.spec.js create mode 100644 test/setup.js diff --git a/index.js b/index.js index 9742505876..d6bcaf7ab2 100644 --- a/index.js +++ b/index.js @@ -1,7 +1,8 @@ 'use strict' -var DatadogTracer = require('./src/opentracing/tracer') +var platform = require('./src/platform') +var node = require('./src/platform/node') -module.exports = { - Tracer: DatadogTracer -} +platform.use(node) + +module.exports = {} diff --git a/package.json b/package.json index ecf4784c8a..d6f1987e30 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "bench": "node benchmark", "lint": "eslint .", "tdd": "mocha --watch", - "test": "mocha" + "test": "nyc mocha" }, "repository": { "type": "git", @@ -27,7 +27,9 @@ "homepage": "https://github.com/DataDog/dd-trace-js#readme", "dependencies": { "inherits": "^2.0.3", - "opentracing": "^0.14.1" + "lodash.assign": "^4.2.0", + "opentracing": "^0.14.1", + "safe-buffer": "^5.1.1" }, "devDependencies": { "benchmark": "^2.1.4", @@ -38,6 +40,11 @@ "eslint-plugin-node": "^5.2.1", "eslint-plugin-promise": "^3.6.0", "eslint-plugin-standard": "^3.0.1", - "mocha": "^2.4.5" + "mocha": "^2.4.5", + "nock": "^8.2.2", + "nyc": "^10.3.2", + "proxyquire": "^1.8.0", + "sinon": "^3.3.0", + "sinon-chai": "^2.14.0" } } diff --git a/src/platform/index.js b/src/platform/index.js new file mode 100644 index 0000000000..208fcd853c --- /dev/null +++ b/src/platform/index.js @@ -0,0 +1,9 @@ +'use strict' + +var assign = require('lodash.assign') + +module.exports = { + use: function (impl) { + assign(this, impl) + } +} diff --git a/src/platform/node/index.js b/src/platform/node/index.js new file mode 100644 index 0000000000..c4dd303496 --- /dev/null +++ b/src/platform/node/index.js @@ -0,0 +1,45 @@ +'use strict' + +var http = require('http') +var assign = require('lodash.assign') + +module.exports = { + request: function (options, callback) { + options = assign({ + headers: {}, + timeout: 5000 + }, options) + + options.headers['Content-Length'] = byteLength(options.data) + + var req = http.request(options, function (res) { + res.on('data', function (chunk) {}) + + res.on('end', function () { + if (res.statusCode >= 200 && res.statusCode <= 299) { + callback(null) + } else { + var error = new Error(http.STATUS_CODES[res.statusCode]) + error.status = res.statusCode + + callback(error) + } + }) + }) + + req.setTimeout(options.timeout, function () { + req.abort() + }) + + req.on('error', function (e) { + callback(e) + }) + + req.write(options.data) + req.end() + } +} + +function byteLength (data) { + return data ? data.length : 0 +} diff --git a/test/.eslintrc.json b/test/.eslintrc.json new file mode 100644 index 0000000000..2d58d58ac6 --- /dev/null +++ b/test/.eslintrc.json @@ -0,0 +1,17 @@ +{ + "extends": [ + "../.eslintrc.json" + ], + "env": { + "mocha": true + }, + "globals": { + "expect": true, + "sinon": true, + "proxyquire": true, + "nock": true + }, + "rules": { + "no-unused-expressions": 0 + } +} diff --git a/test/mocha.opts b/test/mocha.opts index 58d8fe55a1..5d3fa10625 100644 --- a/test/mocha.opts +++ b/test/mocha.opts @@ -1 +1,2 @@ +--require test/setup.js test/**/*.spec.js diff --git a/test/platform/node/index.spec.js b/test/platform/node/index.spec.js new file mode 100644 index 0000000000..3fd7c46f0f --- /dev/null +++ b/test/platform/node/index.spec.js @@ -0,0 +1,84 @@ +'use strict' + +var Buffer = require('safe-buffer').Buffer + +describe('Node Platform', function () { + var platform + + beforeEach(function () { + platform = require('../../../src/platform/node') + }) + + describe('request', function () { + it('should send an http request with a buffer', function (done) { + nock('http://test:123', { + reqheaders: { + 'content-type': 'application/octet-stream', + 'content-length': '13' + } + }) + .put('/path', { foo: 'bar' }) + .reply(200) + + platform.request({ + protocol: 'http:', + hostname: 'test', + port: 123, + path: '/path', + method: 'PUT', + headers: { + 'Content-Type': 'application/octet-stream' + }, + data: Buffer.from(JSON.stringify({ foo: 'bar' })) + }, done) + }) + + it('should handle an http error', function (done) { + nock('http://localhost:80') + .put('/path') + .reply(400) + + return platform.request({ + path: '/path', + method: 'PUT' + }, function (err) { + expect(err).to.be.instanceof(Error) + expect(err.status).to.equal(400) + done() + }) + }) + + it('should timeout after 5 seconds by default', function (done) { + nock('http://localhost:80') + .put('/path') + .socketDelay(5001) + .reply(200) + + return platform.request({ + path: '/path', + method: 'PUT' + }, function (err) { + expect(err).to.be.instanceof(Error) + expect(err.code).to.equal('ECONNRESET') + done() + }) + }) + + it('should have a configurable timeout', function (done) { + nock('http://localhost:80') + .put('/path') + .socketDelay(2001) + .reply(200) + + return platform.request({ + path: '/path', + method: 'PUT', + timeout: 2000 + }, function (err) { + expect(err).to.be.instanceof(Error) + expect(err.code).to.equal('ECONNRESET') + done() + }) + }) + }) +}) diff --git a/test/setup.js b/test/setup.js new file mode 100644 index 0000000000..a3865b90ee --- /dev/null +++ b/test/setup.js @@ -0,0 +1,15 @@ +'use strict' + +var sinon = require('sinon') +var chai = require('chai') +var sinonChai = require('sinon-chai') +var proxyquire = require('proxyquire') +var nock = require('nock') + +chai.use(sinonChai) +nock.disableNetConnect() + +global.sinon = sinon +global.expect = chai.expect +global.proxyquire = proxyquire.noCallThru() +global.nock = nock From 375d79f6a8f6b83041f3ee192bf136aaf5e35cc0 Mon Sep 17 00:00:00 2001 From: rochdev Date: Thu, 25 Jan 2018 16:47:47 -0500 Subject: [PATCH 2/4] refactor for node >=4 + reduce default timeout --- .circleci/config.yml | 17 ++----- .eslintignore | 1 + .eslintrc.json | 2 + .nvmrc | 2 +- README.md | 7 --- benchmark/index.js | 12 ++--- index.js | 4 +- package.json | 15 +++--- src/opentracing/tracer.js | 9 +--- src/platform/index.js | 6 +-- src/platform/node/index.js | 47 +++++++++---------- test/opentracing/api.spec.js | 8 ++-- test/platform/node/index.spec.js | 78 +++++++++++++++++--------------- test/setup.js | 10 ++-- 14 files changed, 100 insertions(+), 118 deletions(-) create mode 100644 .eslintignore diff --git a/.circleci/config.yml b/.circleci/config.yml index 81d77a0574..fda2d8a73b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -2,7 +2,7 @@ version: 2 jobs: lint: docker: - - image: mhart/alpine-node + - image: node:alpine working_directory: ~/dd-trace-js steps: - checkout @@ -17,7 +17,7 @@ jobs: command: npm run lint build-node-base: &node-base docker: - - image: mhart/alpine-node + - image: node:alpine working_directory: ~/dd-trace-js steps: - checkout @@ -33,14 +33,6 @@ jobs: - run: name: Benchmark command: npm run bench - build-node-0.10: - <<: *node-base - docker: - - image: mhart/alpine-node:0.10 - build-node-0.12: - <<: *node-base - docker: - - image: mhart/alpine-node:0.12 build-node-4: <<: *node-base docker: @@ -53,14 +45,15 @@ jobs: <<: *node-base docker: - image: mhart/alpine-node:8 + build-node-latest: + <<: *node-base workflows: version: 2 build: jobs: - lint - - build-node-0.10 - - build-node-0.12 - build-node-4 - build-node-6 - build-node-8 + - build-node-latest diff --git a/.eslintignore b/.eslintignore new file mode 100644 index 0000000000..4ebc8aea50 --- /dev/null +++ b/.eslintignore @@ -0,0 +1 @@ +coverage diff --git a/.eslintrc.json b/.eslintrc.json index 1450db15e9..312ba67ddd 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -11,7 +11,9 @@ }, "rules": { "max-len": [2, 120, 2], + "no-var": 2, "no-console": 2, + "prefer-const": 2, "object-curly-spacing": [2, "always"] } } diff --git a/.nvmrc b/.nvmrc index 68c123cf10..b8626c4cff 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -0.10 +4 diff --git a/README.md b/README.md index 53e1c1722a..3077e241f7 100644 --- a/README.md +++ b/README.md @@ -39,11 +39,6 @@ To run the linter, use: $ npm run lint ``` -**Note:** ESLint only supports Node >= 4, so make sure you are using -a compatible Node version before running this command. We also -recommend using a plugin on your editor instead of constantly running -this command. - ### Continuous Integration We rely on CircleCI 2.0 for our tests. If you want to test how the CI behaves @@ -54,8 +49,6 @@ After installing the `circleci` CLI, simply run one of the following: ```sh $ circleci build --job lint -$ circleci build --job build-node-0.10 -$ circleci build --job build-node-0.12 $ circleci build --job build-node-4 $ circleci build --job build-node-6 $ circleci build --job build-node-8 diff --git a/benchmark/index.js b/benchmark/index.js index 09e3c862e7..abf53d29a3 100644 --- a/benchmark/index.js +++ b/benchmark/index.js @@ -1,16 +1,16 @@ 'use strict' -var Benchmark = require('benchmark') -var DatadogTracer = require('../src/opentracing/tracer') +const Benchmark = require('benchmark') +const DatadogTracer = require('../src/opentracing/tracer') -var suite = new Benchmark.Suite() -var tracer = new DatadogTracer() +const suite = new Benchmark.Suite() +const tracer = new DatadogTracer() suite - .add('DatadogTracer#startSpan', function () { + .add('DatadogTracer#startSpan', () => { tracer.startSpan() }) - .on('cycle', function (event) { + .on('cycle', event => { console.log(String(event.target)) // eslint-disable-line no-console }) .run({ 'async': true }) diff --git a/index.js b/index.js index d6bcaf7ab2..b995ba2499 100644 --- a/index.js +++ b/index.js @@ -1,7 +1,7 @@ 'use strict' -var platform = require('./src/platform') -var node = require('./src/platform/node') +const platform = require('./src/platform') +const node = require('./src/platform/node') platform.use(node) diff --git a/package.json b/package.json index d6f1987e30..d2b9e0af5b 100644 --- a/package.json +++ b/package.json @@ -25,26 +25,27 @@ "url": "https://github.com/DataDog/dd-trace-js/issues" }, "homepage": "https://github.com/DataDog/dd-trace-js#readme", + "engines": { + "node": ">=4" + }, "dependencies": { - "inherits": "^2.0.3", - "lodash.assign": "^4.2.0", "opentracing": "^0.14.1", "safe-buffer": "^5.1.1" }, "devDependencies": { "benchmark": "^2.1.4", - "chai": "^3.4.1", + "chai": "^4.1.2", "eslint": "^4.15.0", "eslint-config-standard": "^11.0.0-beta.0", "eslint-plugin-import": "^2.8.0", "eslint-plugin-node": "^5.2.1", "eslint-plugin-promise": "^3.6.0", "eslint-plugin-standard": "^3.0.1", - "mocha": "^2.4.5", - "nock": "^8.2.2", - "nyc": "^10.3.2", + "mocha": "^5.0.0", + "nock": "^9.1.6", + "nyc": "^11.4.1", "proxyquire": "^1.8.0", - "sinon": "^3.3.0", + "sinon": "^4.2.1", "sinon-chai": "^2.14.0" } } diff --git a/src/opentracing/tracer.js b/src/opentracing/tracer.js index e0f0e66787..3551fd954b 100644 --- a/src/opentracing/tracer.js +++ b/src/opentracing/tracer.js @@ -1,12 +1,7 @@ 'use strict' -var inherits = require('inherits') -var Tracer = require('opentracing').Tracer +const Tracer = require('opentracing').Tracer -inherits(DatadogTracer, Tracer) - -function DatadogTracer () { - Tracer.call(this) -} +class DatadogTracer extends Tracer {} module.exports = DatadogTracer diff --git a/src/platform/index.js b/src/platform/index.js index 208fcd853c..61e61b7a12 100644 --- a/src/platform/index.js +++ b/src/platform/index.js @@ -1,9 +1,7 @@ 'use strict' -var assign = require('lodash.assign') - module.exports = { - use: function (impl) { - assign(this, impl) + use (impl) { + Object.assign(this, impl) } } diff --git a/src/platform/node/index.js b/src/platform/node/index.js index c4dd303496..49479b5bb7 100644 --- a/src/platform/node/index.js +++ b/src/platform/node/index.js @@ -1,42 +1,37 @@ 'use strict' -var http = require('http') -var assign = require('lodash.assign') +const http = require('http') module.exports = { - request: function (options, callback) { - options = assign({ + request (options, callback) { + options = Object.assign({ headers: {}, - timeout: 5000 + timeout: 2000 }, options) options.headers['Content-Length'] = byteLength(options.data) - var req = http.request(options, function (res) { - res.on('data', function (chunk) {}) - - res.on('end', function () { - if (res.statusCode >= 200 && res.statusCode <= 299) { - callback(null) - } else { - var error = new Error(http.STATUS_CODES[res.statusCode]) - error.status = res.statusCode - - callback(error) - } + return new Promise((resolve, reject) => { + const req = http.request(options, res => { + res.on('data', chunk => {}) + res.on('end', () => { + if (res.statusCode >= 200 && res.statusCode <= 299) { + resolve() + } else { + const error = new Error(http.STATUS_CODES[res.statusCode]) + error.status = res.statusCode + + reject(error) + } + }) }) - }) - req.setTimeout(options.timeout, function () { - req.abort() - }) + req.setTimeout(options.timeout, req.abort) + req.on('error', reject) - req.on('error', function (e) { - callback(e) + req.write(options.data) + req.end() }) - - req.write(options.data) - req.end() } } diff --git a/test/opentracing/api.spec.js b/test/opentracing/api.spec.js index fbee423b53..aec23b9153 100644 --- a/test/opentracing/api.spec.js +++ b/test/opentracing/api.spec.js @@ -1,8 +1,6 @@ 'use strict' -var apiCompatibilityChecks = require('opentracing/lib/test/api_compatibility').default -var DatadogTracer = require('../../src/opentracing/tracer') +const apiCompatibilityChecks = require('opentracing/lib/test/api_compatibility').default +const DatadogTracer = require('../../src/opentracing/tracer') -apiCompatibilityChecks(function () { - return new DatadogTracer({ service: 'test' }) -}) +apiCompatibilityChecks(() => new DatadogTracer({ service: 'test' })) diff --git a/test/platform/node/index.spec.js b/test/platform/node/index.spec.js index 3fd7c46f0f..8c77461dc4 100644 --- a/test/platform/node/index.spec.js +++ b/test/platform/node/index.spec.js @@ -1,16 +1,16 @@ 'use strict' -var Buffer = require('safe-buffer').Buffer +const Buffer = require('safe-buffer').Buffer -describe('Node Platform', function () { - var platform +describe('Node Platform', () => { + let platform - beforeEach(function () { + beforeEach(() => { platform = require('../../../src/platform/node') }) - describe('request', function () { - it('should send an http request with a buffer', function (done) { + describe('request', () => { + it('should send an http request with a buffer', () => { nock('http://test:123', { reqheaders: { 'content-type': 'application/octet-stream', @@ -20,7 +20,7 @@ describe('Node Platform', function () { .put('/path', { foo: 'bar' }) .reply(200) - platform.request({ + return platform.request({ protocol: 'http:', hostname: 'test', port: 123, @@ -30,55 +30,61 @@ describe('Node Platform', function () { 'Content-Type': 'application/octet-stream' }, data: Buffer.from(JSON.stringify({ foo: 'bar' })) - }, done) + }) }) - it('should handle an http error', function (done) { + it('should handle an http error', done => { nock('http://localhost:80') .put('/path') .reply(400) - return platform.request({ - path: '/path', - method: 'PUT' - }, function (err) { - expect(err).to.be.instanceof(Error) - expect(err.status).to.equal(400) - done() - }) + platform + .request({ + path: '/path', + method: 'PUT' + }) + .catch(err => { + expect(err).to.be.instanceof(Error) + expect(err.status).to.equal(400) + done() + }) }) - it('should timeout after 5 seconds by default', function (done) { + it('should timeout after 5 seconds by default', done => { nock('http://localhost:80') .put('/path') .socketDelay(5001) .reply(200) - return platform.request({ - path: '/path', - method: 'PUT' - }, function (err) { - expect(err).to.be.instanceof(Error) - expect(err.code).to.equal('ECONNRESET') - done() - }) + platform + .request({ + path: '/path', + method: 'PUT' + }) + .catch(err => { + expect(err).to.be.instanceof(Error) + expect(err.code).to.equal('ECONNRESET') + done() + }) }) - it('should have a configurable timeout', function (done) { + it('should have a configurable timeout', done => { nock('http://localhost:80') .put('/path') .socketDelay(2001) .reply(200) - return platform.request({ - path: '/path', - method: 'PUT', - timeout: 2000 - }, function (err) { - expect(err).to.be.instanceof(Error) - expect(err.code).to.equal('ECONNRESET') - done() - }) + platform + .request({ + path: '/path', + method: 'PUT', + timeout: 2000 + }) + .catch(err => { + expect(err).to.be.instanceof(Error) + expect(err.code).to.equal('ECONNRESET') + done() + }) }) }) }) diff --git a/test/setup.js b/test/setup.js index a3865b90ee..91f68cd662 100644 --- a/test/setup.js +++ b/test/setup.js @@ -1,10 +1,10 @@ 'use strict' -var sinon = require('sinon') -var chai = require('chai') -var sinonChai = require('sinon-chai') -var proxyquire = require('proxyquire') -var nock = require('nock') +const sinon = require('sinon') +const chai = require('chai') +const sinonChai = require('sinon-chai') +const proxyquire = require('proxyquire') +const nock = require('nock') chai.use(sinonChai) nock.disableNetConnect() From 1f99df637835a84adc8326cfa893ff02cfb10997 Mon Sep 17 00:00:00 2001 From: rochdev Date: Thu, 25 Jan 2018 16:50:25 -0500 Subject: [PATCH 3/4] change node docker images to the offical ones --- .circleci/config.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index fda2d8a73b..43781a8e11 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -36,15 +36,15 @@ jobs: build-node-4: <<: *node-base docker: - - image: mhart/alpine-node:4 + - image: node:4-alpine build-node-6: <<: *node-base docker: - - image: mhart/alpine-node:6 + - image: node:6-alpine build-node-8: <<: *node-base docker: - - image: mhart/alpine-node:8 + - image: node:8-alpine build-node-latest: <<: *node-base From 696383359937576fb0dfe4eefbb73796cb644304 Mon Sep 17 00:00:00 2001 From: rochdev Date: Thu, 25 Jan 2018 16:51:23 -0500 Subject: [PATCH 4/4] improve documentation --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 3077e241f7..405e8c063d 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,7 @@ $ circleci build --job lint $ circleci build --job build-node-4 $ circleci build --job build-node-6 $ circleci build --job build-node-8 +$ circleci build --job build-node-latest ``` ### Benchmarks