Repository: microsoft/ApplicationInsights-node.js
Pull request #589 (2020-02-07 01:09:48)

refactor: agent tests
- Agent: stub `sdkAlreadyExists` in tests
- Agent: Fix issue with recursive folder creation
- Agent: Make `StatusLogger` no-op by default


Patch:
diff --git a/AutoCollection/Exceptions.ts b/AutoCollection/Exceptions.ts
index 30ce41f..f4091b5 100644
--- a/AutoCollection/Exceptions.ts
+++ b/AutoCollection/Exceptions.ts
@@ -51,7 +51,7 @@ class AutoCollectExceptions {
                     this._client.flush({ isAppCrashing: true });
                     // only rethrow when we are the only listener
                     if (reThrow && name && process.listeners(name).length === 1) {
-                        Logging.warn(AutoCollectExceptions._RETHROW_EXIT_MESSAGE, name, error);
+                        console.error(error);
                         process.exit(1);
                     }
                 };
diff --git a/Bootstrap/ConsoleStatusLogger.ts b/Bootstrap/ConsoleStatusLogger.ts
new file mode 100644
index 0000000..99b241e
--- /dev/null
+++ b/Bootstrap/ConsoleStatusLogger.ts
@@ -0,0 +1,29 @@
+import { StatusLogger, StatusContract } from "./StatusLogger";
+import { AgentLogger } from "./DataModel";
+
+export class ConsoleStatusLogger implements StatusLogger {
+    public FULL_PATH: string;
+
+    constructor(private _logger: AgentLogger = console) {
+    }
+
+    public isNodeVersionCompatible(): boolean {
+        return true;
+    }
+
+    public makeStatusDirs(filepath?: string): string {
+        // no op
+        return null;
+    }
+
+    public writeFile(data: StatusContract, cb?: Function): void {
+        this._logger.log(data);
+        if (typeof cb === "function") {
+            cb();
+        }
+    }
+
+    public addCloseHandler(): void {
+        // no op
+    }
+}
diff --git a/Bootstrap/DataModel.ts b/Bootstrap/DataModel.ts
new file mode 100644
index 0000000..bb03196
--- /dev/null
+++ b/Bootstrap/DataModel.ts
@@ -0,0 +1,4 @@
+export interface AgentLogger {
+    log(message?: any, ...optional: any[]): void;
+    error(message?: any, ...optional: any[]): void;
+}
diff --git a/Bootstrap/Default.ts b/Bootstrap/Default.ts
index 76a1d79..db82d2c 100644
--- a/Bootstrap/Default.ts
+++ b/Bootstrap/Default.ts
@@ -1,10 +1,14 @@
 import * as types from "../applicationinsights";
-import * as StatusLogger from "./StatusLogger";
+import * as Helpers from "./Helpers";
+import * as DataModel from "./DataModel";
+import { StatusLogger, StatusContract } from "./StatusLogger";
+import { ConsoleStatusLogger } from "./ConsoleStatusLogger";
 
 // Private configuration vars
 let _appInsights: typeof types | null;
-let _logger: AgentLogger = console;
+let _logger: DataModel.AgentLogger = console;
 let _prefix = "ad_"; // App Services, Default
+let _statusLogger: StatusLogger = new ConsoleStatusLogger();
 
 // Env var local constants
 const ENV_extensionVersion = "APPLICATIONINSIGHTS_EXTENSION_VERSION";
@@ -12,41 +16,16 @@ const _setupString = process.env.APPLICATIONINSIGHTS_CONNECTION_STRING || proces
 const _extensionEnabled = process.env[ENV_extensionVersion] && process.env[ENV_extensionVersion] !== "disabled";
 
 // Other local constants
-const defaultStatus: StatusLogger.StatusContract = {
+const defaultStatus: StatusContract = {
     ...StatusLogger.DEFAULT_STATUS,
     Ikey: _setupString,
 };
 
-export interface AgentLogger {
-    log(message?: any, ...optional: any[]): void;
-    error(message?: any, ...optional: any[]): void;
-}
-
-function sdkAlreadyExists(): boolean {
-    try {
-        // appInstance should either resolve to user SDK or crash. If it resolves to attach SDK, user probably modified their NODE_PATH
-        const appInstance = require.resolve("applicationinsights"); // assumes that the cwd is near user's package.json
-        const attachInstance = require.resolve("../applicationinsights");
-        if (appInstance !== attachInstance) {
-            _logger.log(
-                "applicationinsights module is already installed in this application; not re-attaching. Installed SDK location:",
-                appInstance
-            );
-            return true;
-        }
-        // User probably modified their NODE_PATH to resolve to this instance. Attach appinsights
-        return false;
-    } catch (e) {
-        // crashed while trying to resolve "applicationinsights", so SDK does not exist. Attach appinsights
-        return false;
-    }
-}
-
 /**
  * Sets the attach-time logger
  * @param logger logger which implements the `AgentLogger` interface
  */
-export function setLogger(logger: AgentLogger) {
+export function setLogger(logger: DataModel.AgentLogger) {
     return _logger = logger;
 }
 
@@ -58,15 +37,20 @@ export function setUsagePrefix(prefix: string) {
     _prefix = prefix;
 }
 
+export function setStatusLogger(statusLogger: StatusLogger) {
+    _statusLogger = statusLogger;
+}
+
 /**
  * Try to setup and start this app insights instance if attach is enabled.
  * @param setupString connection string or instrumentation key
  */
 export function setupAndStart(setupString = _setupString): typeof types | null {
-    StatusLogger.addCloseHandler();
+    _statusLogger.makeStatusDirs();
+    _statusLogger.addCloseHandler();
 
     if (!_extensionEnabled) {
-        StatusLogger.writeFile({
+        _statusLogger.writeFile({
             ...defaultStatus,
             AgentInitializedSuccessfully: false,
             Reason: `Extension is not enabled. env.${ENV_extensionVersion}=${process.env[ENV_extensionVersion]}`
@@ -75,12 +59,12 @@ export function setupAndStart(setupString = _setupString): typeof types | null {
     }
 
     // If app already contains SDK, skip agent attach
-    if (sdkAlreadyExists()) {
-        StatusLogger.writeFile({
+    if (Helpers.sdkAlreadyExists(_logger)) {
+        _statusLogger.writeFile({
             ...defaultStatus,
             AgentInitializedSuccessfully: false,
             SDKPresent: true,
-            Reason: "SDK already exists. Instrumenting using Application Insights SDK"
+            Reason: "SDK already exists"
         })
         return null;
     }
@@ -88,7 +72,7 @@ export function setupAndStart(setupString = _setupString): typeof types | null {
     if (!setupString) {
         const message = "Application Insights wanted to be started, but no Connection String or Instrumentation Key was provided";
         _logger.error(message, setupString);
-        StatusLogger.writeFile({
+        _statusLogger.writeFile({
             ...defaultStatus,
             AgentInitializedSuccessfully: false,
             Reason: message,
@@ -115,13 +99,13 @@ export function setupAndStart(setupString = _setupString): typeof types | null {
 
         // Agent successfully instrumented the SDK
         _logger.log("Application Insights was started with setupString", setupString, _extensionEnabled);
-        StatusLogger.writeFile({
+        _statusLogger.writeFile({
             ...defaultStatus,
             AgentInitializedSuccessfully: true
         });
     } catch (e) {
         _logger.error("Error setting up Application Insights", e);
-        StatusLogger.writeFile({
+        _statusLogger.writeFile({
             ...defaultStatus,
             AgentInitializedSuccessfully: false,
             Reason: `Error setting up Application Insights: ${e && e.message}`
diff --git a/Bootstrap/Helpers.ts b/Bootstrap/Helpers.ts
new file mode 100644
index 0000000..dcc53fd
--- /dev/null
+++ b/Bootstrap/Helpers.ts
@@ -0,0 +1,21 @@
+import { AgentLogger } from "./DataModel";
+
+export function sdkAlreadyExists(_logger: AgentLogger = console): boolean {
+    try {
+        // appInstance should either resolve to user SDK or crash. If it resolves to attach SDK, user probably modified their NODE_PATH
+        const appInstance = require.resolve("applicationinsights"); // assumes that the cwd is near user's package.json
+        const attachInstance = require.resolve("../applicationinsights");
+        if (appInstance !== attachInstance) {
+            _logger.log(
+                "applicationinsights module is already installed in this application; not re-attaching. Installed SDK location:",
+                appInstance
+            );
+            return true;
+        }
+        // User probably modified their NODE_PATH to resolve to this instance. Attach appinsights
+        return false;
+    } catch (e) {
+        // crashed while trying to resolve "applicationinsights", so SDK does not exist. Attach appinsights
+        return false;
+    }
+}
diff --git a/Bootstrap/StatusLogger.ts b/Bootstrap/StatusLogger.ts
index 71f6211..73e4c23 100644
--- a/Bootstrap/StatusLogger.ts
+++ b/Bootstrap/StatusLogger.ts
@@ -13,12 +13,6 @@ export interface StatusContract {
     Ikey: string;
 }
 
-const _APP_TYPE = "node.js";
-const _HOME_DIR = os.homedir ? os.homedir() :( process.env[(process.platform == 'win32') ? 'USERPROFILE' : 'HOME']);
-const _FILE_PATH = `${_HOME_DIR}/LogFiles/ApplicationInsights/status/`;
-const _FILE_NAME = `status_${os.hostname()}_${process.pid}.json`;
-export const FULL_PATH = _FILE_PATH + _FILE_NAME;
-
 function readPackageVersion() {
     let packageJsonPath = path.resolve(__dirname, "../package.json");
     try {
@@ -33,70 +27,87 @@ function readPackageVersion() {
 /**
  * Zero dependencies: recursive mkdir
  */
-function mkDirByPathSync(targetDir: string, { isRelativeToScript = false } = {}) {
+function mkDirByPathSync(HOME_DIR: string, targetDir: string, { isRelativeToScript = false } = {}) {
     const sep = path.sep;
-    const initDir = path.isAbsolute(targetDir) ? sep : '';
-    const baseDir = isRelativeToScript ? __dirname : '.';
+    const initDir = path.isAbsolute(targetDir) ? sep : "";
+    const baseDir = isRelativeToScript ? __dirname : ".";
 
     return targetDir.split(sep).reduce((parentDir, childDir) => {
-      const curDir = path.resolve(baseDir, parentDir, childDir);
-      try {
-        fs.mkdirSync(curDir);
-      } catch (err) {
-        if (err.code === 'EEXIST') { // curDir already exists!
-          return curDir;
-        }
+        const curDir = path.resolve(baseDir, parentDir, childDir);
+        try {
+            // Don't try to recreate homedir
+            if (HOME_DIR.indexOf(curDir) === -1) {
+                fs.mkdirSync(curDir);
+            }
+        } catch (err) {
+            if (err.code === "EEXIST") { // curDir already exists!
+                return curDir;
+            }
 
-        // To avoid `EISDIR` error on Mac and `EACCES`-->`ENOENT` and `EPERM` on Windows.
-        if (err.code === 'ENOENT') { // Throw the original parentDir error on curDir `ENOENT` failure.
-          throw new Error(`EACCES: permission denied, mkdir '${parentDir}'`);
-        }
+            // To avoid `EISDIR` error on Mac and `EACCES`-->`ENOENT` and `EPERM` on Windows.
+            if (err.code === "ENOENT") { // Throw the original parentDir error on curDir `ENOENT` failure.
+                throw new Error(`EACCES: permission denied, mkdir "${parentDir}"`);
+            }
 
-        const caughtErr = ['EACCES', 'EPERM', 'EISDIR'].indexOf(err.code) > -1;
-        if (!caughtErr || caughtErr && curDir === path.resolve(targetDir)) {
-          throw err; // Throw if it's just the last created dir.
+            const caughtErr = ["EACCES", "EPERM", "EISDIR"].indexOf(err.code) > -1;
+            if (!caughtErr || caughtErr && curDir === path.resolve(targetDir)) {
+                throw err; // Throw if it"s just the last created dir.
+            }
         }
-      }
+        return curDir;
 
-      return curDir;
     }, initDir);
 }
 
-export function isNodeVersionCompatible() {
-  const majVer = process.versions.node.split(".")[0];
-  return parseInt(majVer) >= 1;
-}
+export class StatusLogger {
+    private _HOME_DIR?: string = os.homedir ? os.homedir() :( process.env[(process.platform == "win32") ? "USERPROFILE" : "HOME"]);
+    private _FILE_PATH?: string = `${this._HOME_DIR}/LogFiles/ApplicationInsights/status/`;
+    private _FILE_NAME?: string = `status_${os.hostname()}_${process.pid}.json`;
 
-export const DEFAULT_STATUS: StatusContract = {
-    AgentInitializedSuccessfully: false,
-    SDKPresent: false,
-    Ikey: null,
-    AppType: _APP_TYPE,
-    SdkVersion: readPackageVersion(),
-    MachineName: os.hostname(),
-    PID: String(process.pid)
-}
+    private static _APP_TYPE?: string = "node.js";
+    public static DEFAULT_STATUS?: StatusContract = {
+        AgentInitializedSuccessfully: false,
+        SDKPresent: false,
+        Ikey: null,
+        AppType: StatusLogger._APP_TYPE,
+        SdkVersion: readPackageVersion(),
+        MachineName: os.hostname(),
+        PID: String(process.pid)
+    }
 
-export function makeStatusDirs(path = _FILE_PATH) {
-    return mkDirByPathSync(path);
-}
+    public FULL_PATH = this._FILE_PATH + this._FILE_NAME;
 
-export function writeFile(data: StatusContract, cb?: Function) {
-    fs.open(FULL_PATH, "w", (err, fid) => {
-        if (err) return;
-        console.log(FULL_PATH);
-        fs.writeFile(FULL_PATH, JSON.stringify(data, null, 2), { encoding: "utf8" }, (err) => {
-            if (err) {
-                console.error("Error writing Application Insights status file", err);
-            } else if (cb && typeof cb === "function") {
-                cb(err);
-            }
+    public isNodeVersionCompatible() {
+      const majVer = process.versions.node.split(".")[0];
+      return parseInt(majVer) >= 1;
+    }
+
+
+    public makeStatusDirs(filepath = this._FILE_PATH) {
+        try {
+            return mkDirByPathSync(this._HOME_DIR, filepath.replace(/\\/g, path.sep).replace(/\//g, path.sep));
+        } catch (e) {
+            console.error("Error creating Application Insights status folder", e);
+        }
+    }
+
+    public writeFile(data: StatusContract, cb?: Function) {
+        fs.open(this.FULL_PATH, "w", (err, fid) => {
+            if (err) return;
+            fs.writeFile(this.FULL_PATH, JSON.stringify(data, null, 2), { encoding: "utf8" }, (err) => {
+                if (err) {
+                    console.error("Error writing Application Insights status file", err);
+                }
+                if (cb && typeof cb === "function") {
+                    cb(err);
+                }
+            });
         });
-    });
-}
+    }
 
-export function addCloseHandler() {
-    process.on("exit", () => { try {
-        fs.unlinkSync(FULL_PATH);
-    } catch (err) { }});
+    public addCloseHandler() {
+        process.on("exit", () => { try {
+            fs.unlinkSync(this.FULL_PATH);
+        } catch (err) { }});
+    }
 }
diff --git a/Library/Logging.ts b/Library/Logging.ts
index 127731f..dc6c3fe 100644
--- a/Library/Logging.ts
+++ b/Library/Logging.ts
@@ -1,6 +1,7 @@
 class Logging {
     public static enableDebug = false;
     public static disableWarnings = false;
+    public static disableErrors = false;
 
     private static TAG = "ApplicationInsights:";
 


Test patch:
diff --git a/Tests/Bootstrap/Default.spec.ts b/Tests/Bootstrap/Default.spec.ts
index b0939df..c9bd13f 100644
--- a/Tests/Bootstrap/Default.spec.ts
+++ b/Tests/Bootstrap/Default.spec.ts
@@ -1,10 +1,12 @@
 import assert = require("assert");
 import sinon = require("sinon");
-import * as types from "../../Bootstrap/Default";
+import * as DataModel from "../../Bootstrap/DataModel";
+import * as Helpers from "../../Bootstrap/Helpers";
+import * as DefaultTypes from "../../Bootstrap/Default";
 
 const appInsights = require("../../applicationinsights");
 
-class LoggerSpy implements types.AgentLogger {
+class LoggerSpy implements DataModel.AgentLogger {
     public logCount = 0
     public errorCount = 0;
 
@@ -37,14 +39,16 @@ describe("#setupAndStart()", () => {
         const logger = new LoggerSpy();
         const origEnv = process.env.APPLICATIONINSIGHTS_EXTENSION_VERSION;
         process.env.APPLICATIONINSIGHTS_EXTENSION_VERSION = "~2";
+        const alreadyExistsStub = sinon.stub(Helpers, "sdkAlreadyExists", () => false);
 
         // Test
-        const Default = require("../../Bootstrap/Default") as typeof types;
+        const Default = require("../../Bootstrap/Default") as typeof DefaultTypes;
         Default.setLogger(logger);
         const instance = Default.setupAndStart("abc");
         assert.deepEqual(instance, appInsights);
 
         // Cleanup
+        alreadyExistsStub.restore();
         instance.dispose();
         process.env.APPLICATIONINSIGHTS_EXTENSION_VERSION = origEnv;
 
@@ -61,14 +65,16 @@ describe("#setupAndStart()", () => {
         const logger = new LoggerSpy();
         const origEnv = process.env.APPLICATIONINSIGHTS_EXTENSION_VERSION;
         process.env.APPLICATIONINSIGHTS_EXTENSION_VERSION = "disabled";
+        const alreadyExistsStub = sinon.stub(Helpers, "sdkAlreadyExists", () => false);
 
         // Test
-        const Default = require("../../Bootstrap/Default") as typeof types;
+        const Default = require("../../Bootstrap/Default") as typeof DefaultTypes;
         Default.setLogger(logger);
         const instance = Default.setupAndStart("abc");
         assert.equal(instance, null);
 
         // Cleanup
+        alreadyExistsStub.restore();
         process.env.APPLICATIONINSIGHTS_EXTENSION_VERSION = origEnv;
 
         // start was never called
@@ -88,14 +94,16 @@ describe("#setupAndStart()", () => {
         process.env.APPLICATIONINSIGHTS_EXTENSION_VERSION = "~2";
         delete process.env.APPINSIGHTS_INSTRUMENTATION_KEY;
         delete process.env.APPLICATIONINSIGHTS_CONNECTION_STRING;
+        const alreadyExistsStub = sinon.stub(Helpers, "sdkAlreadyExists", () => false);
 
         // Test
-        const Default = require("../../Bootstrap/Default") as typeof types;
+        const Default = require("../../Bootstrap/Default") as typeof DefaultTypes;
         Default.setLogger(logger);
         const instance = Default.setupAndStart();
         assert.equal(instance, null);
 
         // Cleanup
+        alreadyExistsStub.restore();
         process.env.APPLICATIONINSIGHTS_EXTENSION_VERSION = origEnv;
 
         // start was never called
diff --git a/Tests/Bootstrap/StatusLogger.spec.ts b/Tests/Bootstrap/StatusLogger.spec.ts
index a271ab4..bcea0cc 100644
--- a/Tests/Bootstrap/StatusLogger.spec.ts
+++ b/Tests/Bootstrap/StatusLogger.spec.ts
@@ -1,16 +1,17 @@
 import * as fs from "fs";
 import assert = require("assert");
 import sinon = require("sinon");
-import * as StatusLogger from "../../Bootstrap/StatusLogger";
+import { StatusLogger } from "../../Bootstrap/StatusLogger";
 
 describe("#writeFile()", () => {
     it("should write a status file to disk", (done) => {
-        if (!StatusLogger.isNodeVersionCompatible()) {
+        const statusLogger = new StatusLogger();
+        if (!statusLogger.isNodeVersionCompatible()) {
             done();
         } else {
-            StatusLogger.makeStatusDirs();
-            StatusLogger.writeFile(StatusLogger.DEFAULT_STATUS, () => {
-                const buffer = JSON.parse(fs.readFileSync(StatusLogger.FULL_PATH, "utf8"));
+            statusLogger.makeStatusDirs();
+            statusLogger.writeFile(StatusLogger.DEFAULT_STATUS, () => {
+                const buffer = JSON.parse(fs.readFileSync(statusLogger.FULL_PATH, "utf8"));
                 assert.deepEqual(buffer, StatusLogger.DEFAULT_STATUS);
                 done();
             });
@@ -20,11 +21,12 @@ describe("#writeFile()", () => {
 
 describe("#addCloseHandler()", () => {
     it("should add a process exit handler", () => {
-        if (StatusLogger.isNodeVersionCompatible()) {
+        const statusLogger = new StatusLogger();
+        if (statusLogger.isNodeVersionCompatible()) {
             const processSpy = sinon.spy(process, "on");
             assert.ok(processSpy.notCalled);
 
-            StatusLogger.addCloseHandler();
+            statusLogger.addCloseHandler();
             assert.equal(processSpy.callCount, 1);
             assert.equal(processSpy.args[0][0], "exit");