From d2d1d214e2b0619b9448870c754b9c7b4d353854 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Fri, 13 Jan 2017 15:06:54 -0800 Subject: [PATCH] Removed dependency on @google/cloud-diagnostics-common (#215) PR-URL: #215 --- package.json | 3 +- src/agent/debuglet.js | 56 ++++++++++-- src/agent/v8debugapi.js | 15 +++- test/fixtures/fib.js | 5 ++ test/nocks.js | 9 +- test/standalone/test-duplicate-expressions.js | 4 +- .../test-duplicate-nested-expressions.js | 4 +- test/standalone/test-fat-arrow.js | 4 +- test/standalone/test-max-data-size.js | 4 +- test/standalone/test-this-context.js | 4 +- test/standalone/test-try-catch.js | 4 +- test/standalone/test-v8debugapi.js | 4 +- test/test-debuglet.js | 85 ++++++++++++++++--- test/test-options-credentials.js | 25 ------ 14 files changed, 161 insertions(+), 65 deletions(-) diff --git a/package.json b/package.json index 7e3c55a2..6d47e9dc 100644 --- a/package.json +++ b/package.json @@ -32,8 +32,7 @@ "proxyquire": "^1.4.0" }, "dependencies": { - "@google-cloud/common": "^0.9.0", - "@google/cloud-diagnostics-common": "0.3.0", + "@google-cloud/common": "^0.11.0", "acorn": "^4.0.3", "async": "^2.1.2", "coffee-script": "^1.9.3", diff --git a/src/agent/debuglet.js b/src/agent/debuglet.js index 037224b3..95579563 100644 --- a/src/agent/debuglet.js +++ b/src/agent/debuglet.js @@ -25,14 +25,13 @@ var util = require('util'); var semver = require('semver'); var _ = require('lodash'); var metadata = require('gcp-metadata'); +var common = require('@google-cloud/common'); var v8debugapi = require('./v8debugapi.js'); var Debuggee = require('../debuggee.js'); var DebugletApi = require('../controller.js'); var defaultConfig = require('./config.js'); var scanner = require('./scanner.js'); -var common = require('@google/cloud-diagnostics-common'); -var Logger = common.logger; var StatusMessage = require('../status-message.js'); var SourceMapper = require('./sourcemapper.js'); var pjson = require('../../package.json'); @@ -44,6 +43,43 @@ var NODE_VERSION_MESSAGE = 'Node.js version not supported. Node.js 5.2.0 and ' + var BREAKPOINT_ACTION_MESSAGE = 'The only currently supported breakpoint actions' + ' are CAPTURE and LOG.'; +/** + * Formats a breakpoint object prefixed with a provided message as a string + * intended for logging. + * @param {string} msg The message that prefixes the formatted breakpoint. + * @param {Breakpoint} breakpoint The breakpoint to format. + * @return {string} A formatted string. + */ +var formatBreakpoint = function(msg, breakpoint) { + var text = msg + util.format('breakpoint id: %s,\n\tlocation: %s', + breakpoint.id, util.inspect(breakpoint.location)); + if (breakpoint.createdTime) { + var unixTime = parseInt(breakpoint.createdTime.seconds, 10); + var date = new Date(unixTime * 1000); // to milliseconds. + text += '\n\tcreatedTime: ' + date.toString(); + } + if (breakpoint.condition) { + text += '\n\tcondition: ' + util.inspect(breakpoint.condition); + } + if (breakpoint.expressions) { + text += '\n\texpressions: ' + util.inspect(breakpoint.expressions); + } + return text; +}; + +/** + * Formats a map of breakpoint objects prefixed with a provided message as a + * string intended for logging. + * @param {string} msg The message that prefixes the formatted breakpoint. + * @param {Object.} breakpoints A map of breakpoints. + * @return {string} A formatted string. + */ +var formatBreakpoints = function(msg, breakpoints) { + return msg + Object.keys(breakpoints).map(function (b) { + formatBreakpoint('', b); + }).join('\n'); +}; + module.exports = Debuglet; /** @@ -80,8 +116,11 @@ function Debuglet(debug, config) { /** @private {boolean} */ this.fetcherActive_ = false; - /** @private {Logger} */ - this.logger_ = Logger.create(this.config_.logLevel, '@google-cloud/debug'); + /** @private {common.logger} */ + this.logger_ = new common.logger({ + level: common.logger.LEVELS[this.config_.logLevel], + tag: '@google-cloud/debug' + }); /** @private {DebugletApi} */ this.debugletApi_ = new DebugletApi(this.debug_); @@ -274,7 +313,7 @@ Debuglet.prototype.getProjectId_ = function(callback) { // to access the metadata service as a test for that. // TODO: change this to getProjectId in the future. metadata.project( - 'numeric-project-id', function(err, response, metadataProject) { + 'project-id', function(err, response, metadataProject) { // We should get an error if we are not on GCP. var onGCP = !err; @@ -407,8 +446,8 @@ Debuglet.prototype.scheduleBreakpointFetch_ = function(seconds) { }); that.updateActiveBreakpoints_(bps); if (Object.keys(that.activeBreakpointMap_).length) { - that.logger_.breakpoints(Logger.INFO, 'Active Breakpoints:', - that.activeBreakpointMap_); + that.logger_.info(formatBreakpoint('Active Breakpoints: ', + that.activeBreakpointMap_)); } that.scheduleBreakpointFetch_(that.config_.breakpointUpdateIntervalSec); return; @@ -427,7 +466,8 @@ Debuglet.prototype.updateActiveBreakpoints_ = function(breakpoints) { var updatedBreakpointMap = this.convertBreakpointListToMap_(breakpoints); if (breakpoints.length) { - that.logger_.breakpoints(Logger.INFO, 'Server breakpoints:', updatedBreakpointMap); + that.logger_.info(formatBreakpoints('Server breakpoints: ', + updatedBreakpointMap)); } breakpoints.forEach(function(breakpoint) { diff --git a/src/agent/v8debugapi.js b/src/agent/v8debugapi.js index cd29495d..08780411 100644 --- a/src/agent/v8debugapi.js +++ b/src/agent/v8debugapi.js @@ -22,7 +22,6 @@ /** @const */ var semver = require('semver'); /** @const */ var state = require('./state.js'); -/** @const */ var logModule = require('@google/cloud-diagnostics-common').logger; /** @const */ var StatusMessage = require('../status-message.js'); /** @const */ var messages = { @@ -46,6 +45,18 @@ /** @const */ var MODULE_WRAP_PREFIX_LENGTH = require('module').wrap('☃') .indexOf('☃'); +/** + * Formats a provided message and a high-resolution interval of the format + * [seconds, nanoseconds] (for example, from process.hrtime()) prefixed with a + * provided message as a string intended for logging. + * @param {string} msg The mesage that prefixes the formatted interval. + * @param {number[]} interval The interval to format. + * @return {string} A formatted string. + */ +var formatInterval = function(msg, interval) { + return msg + (interval[0] * 1000 + interval[1] / 1000000) + 'ms'; +}; + var singleton; module.exports.create = function(logger_, config_, jsFiles_, sourcemapper_) { if (singleton) { @@ -468,7 +479,7 @@ module.exports.create = function(logger_, config_, jsFiles_, sourcemapper_) { messages.CAPTURE_BREAKPOINT_DATA + err); } var end = process.hrtime(start); - logger.interval(logModule.INFO, 'capture time', end); + logger.info(formatInterval('capture time: ', end)); callback(null); } diff --git a/test/fixtures/fib.js b/test/fixtures/fib.js index 510917d0..65947cd0 100644 --- a/test/fixtures/fib.js +++ b/test/fixtures/fib.js @@ -29,6 +29,11 @@ debug.startAgent({ breakpointUpdateIntervalSec: 1 }); +// Make troubleshooting easier if run by itself +if (!process.send) { + process.send = console.log; +} + var timedOut = false; var registrationTimeout = setTimeout(function() { timedOut = true; diff --git a/test/nocks.js b/test/nocks.js index 65f34d75..8cb1244b 100644 --- a/test/nocks.js +++ b/test/nocks.js @@ -26,6 +26,7 @@ function nockOAuth2(validator) { validator = validator || accept; return nock('https://accounts.google.com') .post('/o/oauth2/token', validator) + .once() .reply(200, { refresh_token: 'hello', access_token: 'goodbye', @@ -37,17 +38,19 @@ function nockRegister(validator) { validator = validator || accept; return nock('https://clouddebugger.googleapis.com') .post('/v2/controller/debuggees/register', validator) + .once() .reply(200); } -function nockNumericProjectId(reply) { +function nockProjectId(reply) { return nock('http://metadata.google.internal') - .get('/computeMetadata/v1/project/numeric-project-id') + .get('/computeMetadata/v1/project/project-id') + .once() .reply(reply); } module.exports = { oauth2: nockOAuth2, - numericProjectId: nockNumericProjectId, + projectId: nockProjectId, register: nockRegister }; diff --git a/test/standalone/test-duplicate-expressions.js b/test/standalone/test-duplicate-expressions.js index bf9c6372..d66f8cfb 100644 --- a/test/standalone/test-duplicate-expressions.js +++ b/test/standalone/test-duplicate-expressions.js @@ -29,7 +29,7 @@ var breakpointInFoo = { var assert = require('assert'); var extend = require('extend'); var v8debugapi = require('../../src/agent/v8debugapi.js'); -var logModule = require('@google/cloud-diagnostics-common').logger; +var common = require('@google-cloud/common'); var defaultConfig = require('../../src/agent/config.js'); var SourceMapper = require('../../src/agent/sourcemapper.js'); var scanner = require('../../src/agent/scanner.js'); @@ -47,7 +47,7 @@ describe('v8debugapi', function() { var config = extend({}, defaultConfig, { workingDirectory: path.join(process.cwd(), 'test', 'standalone') }); - var logger = logModule.create(config.logLevel); + var logger = common.logger({ logLevel: config.logLevel }); var api = null; beforeEach(function(done) { diff --git a/test/standalone/test-duplicate-nested-expressions.js b/test/standalone/test-duplicate-nested-expressions.js index ba51a5b0..01fe8b1b 100644 --- a/test/standalone/test-duplicate-nested-expressions.js +++ b/test/standalone/test-duplicate-nested-expressions.js @@ -27,7 +27,7 @@ var assert = require('assert'); var extend = require('extend'); var v8debugapi = require('../../src/agent/v8debugapi.js'); -var logModule = require('@google/cloud-diagnostics-common').logger; +var common = require('@google-cloud/common'); var defaultConfig = require('../../src/agent/config.js'); var SourceMapper = require('../../src/agent/sourcemapper.js'); var scanner = require('../../src/agent/scanner.js'); @@ -46,7 +46,7 @@ describe('v8debugapi', function() { var config = extend({}, defaultConfig, { workingDirectory: path.join(process.cwd(), 'test', 'standalone') }); - var logger = logModule.create(config.logLevel); + var logger = common.logger({ logLevel: config.logLevel }); var api = null; beforeEach(function(done) { diff --git a/test/standalone/test-fat-arrow.js b/test/standalone/test-fat-arrow.js index 7c6bb4a2..bffa7638 100644 --- a/test/standalone/test-fat-arrow.js +++ b/test/standalone/test-fat-arrow.js @@ -18,7 +18,7 @@ var assert = require('assert'); var extend = require('extend'); var v8debugapi = require('../../src/agent/v8debugapi.js'); -var logModule = require('@google/cloud-diagnostics-common').logger; +var common = require('@google-cloud/common'); var defaultConfig = require('../../src/agent/config.js'); var SourceMapper = require('../../src/agent/sourcemapper.js'); var scanner = require('../../src/agent/scanner.js'); @@ -39,7 +39,7 @@ describe('v8debugapi', function() { var config = extend({}, defaultConfig, { workingDirectory: path.join(process.cwd(), 'test') }); - var logger = logModule.create(config.logLevel); + var logger = common.logger({ logLevel: config.logLevel }); var api = null; var foo; before(function () { diff --git a/test/standalone/test-max-data-size.js b/test/standalone/test-max-data-size.js index aef25041..fdeb5060 100644 --- a/test/standalone/test-max-data-size.js +++ b/test/standalone/test-max-data-size.js @@ -23,7 +23,7 @@ process.env.GCLOUD_DIAGNOSTICS_CONFIG = 'test/fixtures/test-config.js'; var assert = require('assert'); var extend = require('extend'); -var logModule = require('@google/cloud-diagnostics-common').logger; +var common = require('@google-cloud/common'); var v8debugapi = require('../../src/agent/v8debugapi.js'); var SourceMapper = require('../../src/agent/sourcemapper.js'); var scanner = require('../../src/agent/scanner.js'); @@ -40,7 +40,7 @@ describe('maxDataSize', function() { before(function(done) { if (!api) { - var logger = logModule.create(config.logLevel); + var logger = common.logger({ logLevel: config.logLevel }); scanner.scan(true, config.workingDirectory, /.js$/, function(err, fileStats, hash) { assert(!err); diff --git a/test/standalone/test-this-context.js b/test/standalone/test-this-context.js index b2e0dfc7..ebc641d9 100644 --- a/test/standalone/test-this-context.js +++ b/test/standalone/test-this-context.js @@ -27,7 +27,7 @@ var assert = require('assert'); var extend = require('extend'); var v8debugapi = require('../../src/agent/v8debugapi.js'); -var logModule = require('@google/cloud-diagnostics-common').logger; +var common = require('@google-cloud/common'); var defaultConfig = require('../../src/agent/config.js'); var SourceMapper = require('../../src/agent/sourcemapper.js'); var scanner = require('../../src/agent/scanner.js'); @@ -46,7 +46,7 @@ describe('v8debugapi', function() { var config = extend({}, defaultConfig, { workingDirectory: path.join(process.cwd(), 'test', 'standalone') }); - var logger = logModule.create(config.logLevel); + var logger = common.logger({ logLevel: config.logLevel }); var api = null; beforeEach(function(done) { diff --git a/test/standalone/test-try-catch.js b/test/standalone/test-try-catch.js index 8e2649ca..7e84a7ce 100644 --- a/test/standalone/test-try-catch.js +++ b/test/standalone/test-try-catch.js @@ -27,7 +27,7 @@ var assert = require('assert'); var extend = require('extend'); var v8debugapi = require('../../src/agent/v8debugapi.js'); -var logModule = require('@google/cloud-diagnostics-common').logger; +var common = require('@google-cloud/common'); var defaultConfig = require('../../src/agent/config.js'); var SourceMapper = require('../../src/agent/sourcemapper.js'); var scanner = require('../../src/agent/scanner.js'); @@ -46,7 +46,7 @@ describe('v8debugapi', function() { var config = extend({}, defaultConfig, { workingDirectory: path.join(process.cwd(), 'test', 'standalone') }); - var logger = logModule.create(config.logLevel); + var logger = common.logger({ logLevel: config.logLevel }); var api = null; beforeEach(function(done) { diff --git a/test/standalone/test-v8debugapi.js b/test/standalone/test-v8debugapi.js index 6c1f7387..64b52384 100644 --- a/test/standalone/test-v8debugapi.js +++ b/test/standalone/test-v8debugapi.js @@ -34,7 +34,7 @@ var MAX_INT = 2147483647; // Max signed int32. var assert = require('assert'); var extend = require('extend'); var v8debugapi = require('../../src/agent/v8debugapi.js'); -var logModule = require('@google/cloud-diagnostics-common').logger; +var common = require('@google-cloud/common'); var defaultConfig = require('../../src/agent/config.js'); var StatusMessage = require('../../src/status-message.js'); var scanner = require('../../src/agent/scanner.js'); @@ -115,7 +115,7 @@ describe('v8debugapi', function() { var config = extend({}, defaultConfig, { workingDirectory: path.join(process.cwd(), 'test') }); - var logger = logModule.create(config.logLevel); + var logger = common.logger({ logLevel: config.logLevel }); var api = null; beforeEach(function(done) { diff --git a/test/test-debuglet.js b/test/test-debuglet.js index 2fe62e0c..e27bed73 100644 --- a/test/test-debuglet.js +++ b/test/test-debuglet.js @@ -31,6 +31,8 @@ var nock = require('nock'); var nocks = require('./nocks.js'); nock.disableNetConnect(); +var oldGP; + var bp = { id: 'test', action: 'CAPTURE', @@ -44,6 +46,14 @@ var errorBp = { describe('Debuglet', function() { describe('setup', function () { + before(function() { + oldGP = process.env.GCLOUD_PROJECT; + }); + + after(function() { + process.env.GCLOUD_PROJECT = oldGP; + }); + beforeEach(function() { delete process.env.GCLOUD_PROJECT; nocks.oauth2(); @@ -53,10 +63,6 @@ describe('Debuglet', function() { nock.cleanAll(); }); - after(function() { - delete process.env.GCLOUD_PROJECT; - }); - it('should not start when projectId is not available', function(done) { this.timeout(8000); var debug = require('..')(); @@ -64,7 +70,7 @@ describe('Debuglet', function() { // The following mock is neccessary for the case when the test is running // on GCP. In that case we will get the projectId from the metadata service. - var scope = nocks.numericProjectId(404); + var scope = nocks.projectId(404); debuglet.once('initError', function(err) { assert.ok(err); @@ -85,7 +91,7 @@ describe('Debuglet', function() { // The following mock is neccessary for the case when the test is running // on GCP. In that case we will get the projectId from the metadata service. - var scope = nocks.numericProjectId(404); + var scope = nocks.projectId(404); debuglet.once('started', function() { assert.fail(); @@ -97,9 +103,35 @@ describe('Debuglet', function() { debuglet.start(); }); - it('should accept non-numeric GCLOUD_PROJECT', function(done) { + it('should use config.projectId', function(done) { + var projectId = '11020304f2934-a'; + var debug = require('..')( + {projectId: projectId, credentials: fakeCredentials}); + var debuglet = new Debuglet(debug, defaultConfig); + + var scope = nock(API) + .post(REGISTER_PATH) + .reply(200, { + debuggee: { + id: DEBUGGEE_ID + } + }); + + debuglet.once('registered', function(id) { + assert.equal(id, DEBUGGEE_ID); + assert.equal(debuglet.debuggee_.project, projectId); + debuglet.stop(); + scope.done(); + done(); + }); + + debuglet.start(); + }); + + it('should use GCLOUD_PROJECT in lieu of config.projectId', function(done) { + process.env.GCLOUD_PROJECT = '11020304f2934-b'; var debug = require('..')( - {projectId: '11020304f2934', credentials: fakeCredentials}); + {credentials: fakeCredentials}); var debuglet = new Debuglet(debug, defaultConfig); var scope = nock(API) @@ -112,6 +144,7 @@ describe('Debuglet', function() { debuglet.once('registered', function(id) { assert.equal(id, DEBUGGEE_ID); + assert.equal(debuglet.debuggee_.project, process.env.GCLOUD_PROJECT); debuglet.stop(); scope.done(); done(); @@ -120,6 +153,36 @@ describe('Debuglet', function() { debuglet.start(); }); + it('should use options.projectId in preference to the environment variable', + function(done) { + process.env.GCLOUD_PROJECT = 'should-not-be-used'; + var debug = require('..')({ + projectId: 'project-via-options', + credentials: fakeCredentials + }); + var debuglet = new Debuglet(debug, defaultConfig); + + // TODO: also make sure we don't request the project from metadata + // service. + var scope = nock(API) + .post(REGISTER_PATH) + .reply(200, { + debuggee: { + id: DEBUGGEE_ID + } + }); + + debuglet.once('registered', function(id) { + assert.equal(id, DEBUGGEE_ID); + assert.equal(debuglet.debuggee_.project, 'project-via-options'); + debuglet.stop(); + scope.done(); + done(); + }); + + debuglet.start(); + }); + it('should respect GCLOUD_DEBUG_LOGLEVEL', function(done) { process.env.GCLOUD_PROJECT='11020304f2934'; process.env.GCLOUD_DEBUG_LOGLEVEL = 3; @@ -139,11 +202,11 @@ describe('Debuglet', function() { var STRING1 = 'jjjjjjjjjjjjjjjjjfjfjfjf'; var STRING2 = 'kkkkkkkfkfkfkfkfkkffkkkk'; - var buffer = []; + var buffer = ''; var oldLog = console.log; - console.log = function () { - buffer = buffer.concat([].slice.call(arguments)); + console.log = function (str) { + buffer += str; }; logger.info(STRING1); logger.debug(STRING2); diff --git a/test/test-options-credentials.js b/test/test-options-credentials.js index ed0f3589..97113b56 100644 --- a/test/test-options-credentials.js +++ b/test/test-options-credentials.js @@ -42,31 +42,6 @@ describe('test-options-credentials', function() { process.env.GCLOUD_PROJECT = envProject; }); - it('should use options.projectId in preference to the environment variable', - function(done) { - process.env.GCLOUD_PROJECT = 'should-not-be-used'; - - var options = extend({}, { - projectId: 'project-via-options', - credentials: require('./fixtures/gcloud-credentials.json') - }); - var debug = require('..')(options); - - // TODO: also make sure we don't request the project from metadata - // service. - var scope = nocks.oauth2(); - nocks.register(function(body) { - assert.ok(body.debuggee); - assert.equal(body.debuggee.project, 'project-via-options'); - scope.done(); - setImmediate(done); - return true; - }); - - debuglet = new Debuglet(debug, config); - debuglet.start(); - }); - it('should use the keyFilename field of the options object', function(done) { var credentials = require('./fixtures/gcloud-credentials.json'); var options = extend({}, {