Move initConfig logic to debuglet (#202)

googleapis · Dec 20, 2016 · 416031f · 416031f
1 parent 30bd528
commit 416031f
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 73 deletions.
diff --git a/src/agent/config.js b/src/agent/config.js
@@ -20,13 +20,6 @@
  */
 
 module.exports = {
-  /**
-   * @property {boolean} Whether the debug agent should be started.
-   * @memberof DebugAgentConfig
-   * @default
-   */
-  enabled: true,
-
   // FIXME(ofrobots): presently this is dependent what cwd() is at the time this
   // file is first required. We should make the default config static.
   /**

diff --git a/src/agent/debuglet.js b/src/agent/debuglet.js
@@ -19,12 +19,14 @@
 var fs = require('fs');
 var path = require('path');
 var EventEmitter = require('events').EventEmitter;
+var extend = require('extend');
 var util = require('util');
 var semver = require('semver');
 
 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;
@@ -41,20 +43,21 @@ var BREAKPOINT_ACTION_MESSAGE = 'The only currently supported breakpoint actions
 module.exports = Debuglet;
 
 /**
- * @param {Object} config The option parameters for the Debuglet.
+ * @param {Debug} debug - A Debug instance.
+ * @param {object=} config - The option parameters for the Debuglet.
  * @event 'error' on startup errors
  * @event 'started' once the startup tasks are completed
  * @event 'registered' once successfully registered to the debug api
  * @event 'stopped' if the agent stops due to a fatal error after starting
  * @constructor
  */
-function Debuglet(debug, config, logger) {
+function Debuglet(debug, config) {
+  /** @private {object} */
+  this.config_ = this.normalizeConfig_(config);
+
   /** @private {Debug} */
   this.debug_ = debug;
 
-  /** @private {object} */
-  this.config_ = config || {};
-
   /**
    * @private {object} V8 Debug API. This can be null if the Node.js version
    *     is out of date.
@@ -71,7 +74,7 @@ function Debuglet(debug, config, logger) {
   this.fetcherActive_ = false;
 
   /** @private {Logger} */
-  this.logger_ = logger;
+  this.logger_ = Logger.create(this.config_.logLevel, '@google-cloud/debug');
 
   /** @private {DebugletApi} */
   this.debugletApi_ = new DebugletApi(this.debug_);
@@ -90,6 +93,28 @@ function Debuglet(debug, config, logger) {
 
 util.inherits(Debuglet, EventEmitter);
 
+Debuglet.prototype.normalizeConfig_ = function(config) {
+  config = extend({}, defaultConfig, config);
+
+  if (config.keyFilename || config.credentials || config.projectId) {
+    throw new Error('keyFilename, projectId or credentials should be provided' + 
+                    ' to the Debug module constructor rather than startAgent');
+  }
+
+  if (process.env.hasOwnProperty('GCLOUD_DEBUG_LOGLEVEL')) {
+    config.logLevel = process.env.GCLOUD_DEBUG_LOGLEVEL;
+  }
+  if (process.env.hasOwnProperty('GAE_MODULE_NAME')) {
+    config.serviceContext = config.serviceContext || {};
+    config.serviceContext.service = process.env.GAE_MODULE_NAME;
+  }
+  if (process.env.hasOwnProperty('GAE_MODULE_VERSION')) {
+    config.serviceContext = config.serviceContext || {};
+    config.serviceContext.version = process.env.GAE_MODULE_VERSION;
+  }
+  return config;
+};
+
 /**
  * Starts the Debuglet. It is important that this is as quick as possible
  * as it is on the critical path of application startup.

diff --git a/src/index.js b/src/index.js
@@ -16,11 +16,9 @@
 
 'use strict';
 
-var logger = require('@google/cloud-diagnostics-common').logger;
 var common = require('@google-cloud/common');
 var Debuglet = require('./agent/debuglet.js');
 var util = require('util');
-var _ = require('lodash');
 
 /**
  * <p class="notice">
@@ -73,30 +71,6 @@ function Debug(options) {
 }
 util.inherits(Debug, common.Service);
 
-var initConfig = function(config_) {
-  var config = config_ || {};
-
-  if (config.keyFilename || config.credentials || config.projectId) {
-    throw new Error('keyFilename, projectId or credentials should be provided' + 
-                    ' to the Debug module constructor rather than startAgent');
-  }
-
-  var defaults = require('./agent/config.js');
-  _.defaultsDeep(config, defaults);
-  if (process.env.hasOwnProperty('GCLOUD_DEBUG_LOGLEVEL')) {
-    config.logLevel = process.env.GCLOUD_DEBUG_LOGLEVEL;
-  }
-  if (process.env.hasOwnProperty('GAE_MODULE_NAME')) {
-    config.serviceContext = config.serviceContext || {};
-    config.serviceContext.service = process.env.GAE_MODULE_NAME;
-  }
-  if (process.env.hasOwnProperty('GAE_MODULE_VERSION')) {
-    config.serviceContext = config.serviceContext || {};
-    config.serviceContext.version = process.env.GAE_MODULE_VERSION;
-  }
-  return config;
-};
-
 var debuglet;
 
 /**
@@ -118,14 +92,9 @@ Debug.prototype.startAgent = function(config) {
     throw new Error('Debug Agent has already been started');
   }
 
-  // FIXME(ofrobots): the initConfig logic belongs in the agent/ directory.
-  config = initConfig(config);
-  if (config.enabled) {
-    debuglet = new Debuglet(
-        this, config, logger.create(config.logLevel, '@google-cloud/debug'));
-    debuglet.start();
-    this.private_ = debuglet;
-  }
+  debuglet = new Debuglet(this, config);
+  debuglet.start();
+  this.private_ = debuglet;
 };
 
 module.exports = Debug;
diff --git a/test/e2e/test-capture-time.js b/test/e2e/test-capture-time.js
@@ -17,7 +17,6 @@
 
 var assert = require('assert');
 var request = require('request');
-var logger = require('@google/cloud-diagnostics-common').logger;
 var config = require('../../src/agent/config.js');
 var semver = require('semver');
 var Debuglet = require('../../src/debuglet.js');
@@ -35,8 +34,7 @@ var debuglet;
 describe(__filename, function(){
   beforeEach(function() {
     process.env.GCLOUD_PROJECT = 0;
-    debuglet = new Debuglet(
-      config, logger.create(config.logLevel, '@google/cloud-debug'));
+    debuglet = new Debuglet(config);
     debuglet.once('started', function() {
       debuglet.debugletApi_.request_ = request; // Avoid authing.
     });

diff --git a/test/standalone/test-debuglet.js b/test/standalone/test-debuglet.js
@@ -16,7 +16,6 @@
 'use strict';
 
 var assert = require('assert');
-var loggerModule = require('@google/cloud-diagnostics-common').logger;
 var defaultConfig = require('../../src/agent/config.js');
 var Debuglet = require('../../src/agent/debuglet.js');
 var extend = require('extend');
@@ -32,8 +31,6 @@ var nock = require('nock');
 var nocks = require('../nocks.js');
 nock.disableNetConnect();
 
-var logger = loggerModule.create(process.env.GCLOUD_DEBUG_LOGLEVEL || 0, 'test');
-
 var bp = {
   id: 'test',
   action: 'CAPTURE',
@@ -59,7 +56,7 @@ describe(__filename, function(){
 
   it('should not start when projectId is not available', function(done) {
     var debug = require('../..')();
-    debuglet = new Debuglet(debug, defaultConfig, logger);
+    debuglet = new Debuglet(debug, defaultConfig);
 
     // 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.
@@ -80,7 +77,7 @@ describe(__filename, function(){
 
   it('should not crash without project num', function(done) {
     var debug = require('../..')();
-    debuglet = new Debuglet(debug, defaultConfig, logger);
+    debuglet = new Debuglet(debug, defaultConfig);
 
     // 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.
@@ -101,7 +98,7 @@ describe(__filename, function(){
   it('should accept non-numeric GCLOUD_PROJECT', function(done) {
     var debug = require('../..')(
         {projectId: '11020304f2934', credentials: fakeCredentials});
-    debuglet = new Debuglet(debug, defaultConfig, logger);
+    debuglet = new Debuglet(debug, defaultConfig);
 
     nocks.oauth2();
     var scope = nock(API)
@@ -125,7 +122,7 @@ describe(__filename, function(){
     this.timeout(10000);
     process.env.GCLOUD_PROJECT='11020304f2934';
     var debug = require('../..')({credentials: fakeCredentials});
-    debuglet = new Debuglet(debug, defaultConfig, logger);
+    debuglet = new Debuglet(debug, defaultConfig);
 
     nocks.oauth2();
     var scope = nock(API)
@@ -153,7 +150,7 @@ describe(__filename, function(){
     var debug = require('../..')(
         {projectId: 'fake-project', credentials: fakeCredentials});
     var config = extend({}, defaultConfig, {workingDirectory: __dirname});
-    debuglet = new Debuglet(debug, config, logger);
+    debuglet = new Debuglet(debug, config);
 
     nocks.oauth2();
     debuglet.once('initError', function(err) {
@@ -167,7 +164,7 @@ describe(__filename, function(){
   it('should register successfully otherwise', function(done) {
     var debug = require('../..')(
         {projectId: 'fake-project', credentials: fakeCredentials});
-    debuglet = new Debuglet(debug, defaultConfig, logger);
+    debuglet = new Debuglet(debug, defaultConfig);
 
     nocks.oauth2();
     var scope =
@@ -188,7 +185,7 @@ describe(__filename, function(){
 
     var debug = require('../..')(
         {projectId: 'fake-project', credentials: fakeCredentials});
-    debuglet = new Debuglet(debug, defaultConfig, logger);
+    debuglet = new Debuglet(debug, defaultConfig);
 
     nocks.oauth2();
     var scope = nock(API)
@@ -216,7 +213,7 @@ describe(__filename, function(){
     this.timeout(4000);
     var debug = require('../..')(
         {projectId: 'fake-project', credentials: fakeCredentials});
-    debuglet = new Debuglet(debug, defaultConfig, logger);
+    debuglet = new Debuglet(debug, defaultConfig);
 
     nocks.oauth2();
     var scope = nock(API)
@@ -253,7 +250,7 @@ describe(__filename, function(){
   it('should re-register when registration expires', function(done) {
     var debug = require('../..')(
         {projectId: 'fake-project', credentials: fakeCredentials});
-    debuglet = new Debuglet(debug, defaultConfig, logger);
+    debuglet = new Debuglet(debug, defaultConfig);
 
     nocks.oauth2();
     var scope = nock(API)
@@ -288,7 +285,7 @@ describe(__filename, function(){
     this.timeout(2000);
     var debug = require('../..')(
         {projectId: 'fake-project', credentials: fakeCredentials});
-    debuglet = new Debuglet(debug, defaultConfig, logger);
+    debuglet = new Debuglet(debug, defaultConfig);
 
     nocks.oauth2();
     var scope = nock(API)
@@ -320,7 +317,7 @@ describe(__filename, function(){
 
     var debug = require('../..')(
         {projectId: 'fake-project', credentials: fakeCredentials});
-    debuglet = new Debuglet(debug, defaultConfig, logger);
+    debuglet = new Debuglet(debug, defaultConfig);
 
     nocks.oauth2();
     var scope = nock(API)
@@ -390,7 +387,7 @@ describe(__filename, function(){
       })
       .reply(200);
 
-    debuglet = new Debuglet(debug, config, logger);
+    debuglet = new Debuglet(debug, config);
     debuglet.once('registered', function(id) {
       assert.equal(id, DEBUGGEE_ID);
       setTimeout(function() {
@@ -444,7 +441,7 @@ describe(__filename, function(){
         breakpoints: [bp]
       });
 
-    debuglet = new Debuglet(debug, config, logger);
+    debuglet = new Debuglet(debug, config);
     debuglet.once('registered', function(id) {
       assert.equal(id, DEBUGGEE_ID);
       setTimeout(function() {

diff --git a/test/standalone/test-options-credentials.js b/test/standalone/test-options-credentials.js
@@ -20,7 +20,6 @@ var assert = require('assert');
 var nock = require('nock');
 var nocks = require('../nocks.js');
 var extend = require('extend');
-var logger = require('@google/cloud-diagnostics-common').logger;
 var defaultOptions = {};
 var config = require('../../src/agent/config.js');
 var Debuglet = require('../../src/agent/debuglet.js');
@@ -66,8 +65,7 @@ describe('test-options-credentials', function() {
           return true;
         });
 
-       debuglet =
-           new Debuglet(debug, config, logger.create(logger.WARN, 'testing'));
+        debuglet = new Debuglet(debug, config);
        debuglet.start();
      });
 
@@ -90,8 +88,7 @@ describe('test-options-credentials', function() {
       setImmediate(done);
       return true;
     });
-    debuglet =
-        new Debuglet(debug, config, logger.create(logger.WARN, 'testing'));
+    debuglet = new Debuglet(debug, config);
     debuglet.start();
   });
 
@@ -113,7 +110,7 @@ describe('test-options-credentials', function() {
       setImmediate(done);
       return true;
     });
-    debuglet = new Debuglet(debug, config, logger.create(undefined, 'testing'));
+    debuglet = new Debuglet(debug, config);
     debuglet.start();
   });
 
@@ -148,7 +145,7 @@ describe('test-options-credentials', function() {
       assert(options.credentials.hasOwnProperty(field));
       assert.notEqual(options.credentials[field], fileCredentials[field]);
     });
-    debuglet = new Debuglet(debug, config, logger.create(undefined, 'testing'));
+    debuglet = new Debuglet(debug, config);
     debuglet.start();
   });
 });