Merge pull request #355 from Microsoft/develop

Merge develop to master for 1.0.1
microsoft · Nov 29, 2017 · 001cc4f · 001cc4f
2 parents 96ddd67 + 7b52f9a
commit 001cc4f
Show file tree

Hide file tree

Showing 14 changed files with 625 additions and 312 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -4,6 +4,7 @@ services:
   - docker
 node_js:
   - "node"
+  - "9"
   - "8"
   - "7"
   - "6"
@@ -20,6 +21,7 @@ before_script:
 script:
   - npm test
   # Only run funcs for LTS releases
+  - if [ "$TRAVIS_NODE_VERSION" = "8" ]; then npm run functionaltest; fi
   - if [ "$TRAVIS_NODE_VERSION" = "6" ]; then npm run functionaltest; fi
   - if [ "$TRAVIS_NODE_VERSION" = "4" ]; then npm run functionaltest; fi
   # Only run backcompat tests once

diff --git a/AutoCollection/HttpDependencies.ts b/AutoCollection/HttpDependencies.ts
@@ -22,6 +22,7 @@ class AutoCollectHttpDependencies {
     public static INSTANCE: AutoCollectHttpDependencies;
 
     private static requestNumber = 1;
+    private static alreadyAutoCollectedFlag = '_appInsightsAutoCollected';
 
     private _client: TelemetryClient;
     private _isEnabled: boolean;
@@ -54,30 +55,51 @@ class AutoCollectHttpDependencies {
     private _initialize() {
         this._isInitialized = true;
 
+        const originalGet = http.get;
         const originalRequest = http.request;
-        http.request = (options, ...requestArgs: any[]) => {
-            const request: http.ClientRequest = originalRequest.call(
-                http, options, ...requestArgs);
-            if (request && options && !(<any>options)[AutoCollectHttpDependencies.disableCollectionRequestOption]) {
+        const originalHttpsRequest = https.request;
+
+        const clientRequestPatch = (request: http.ClientRequest, options: http.RequestOptions | https.RequestOptions) => {
+            var shouldCollect = !(<any>options)[AutoCollectHttpDependencies.disableCollectionRequestOption] &&
+                !(<any>request)[AutoCollectHttpDependencies.alreadyAutoCollectedFlag];
+
+            (<any>request)[AutoCollectHttpDependencies.alreadyAutoCollectedFlag] = true;
+
+            if (request && options && shouldCollect) {
                 AutoCollectHttpDependencies.trackRequest(this._client, options, request);
             }
+        };
+
+        // On node >= v0.11.12 and < 9.0 (excluding 8.9.0) https.request just calls http.request (with additional options).
+        // On node < 0.11.12, 8.9.0, and 9.0 > https.request is handled separately
+        // Patch both and leave a flag to not double-count on versions that just call through
+        // We add the flag to both http and https to protect against strange double collection in other scenarios
+        http.request = (options, ...requestArgs: any[]) => {
+            const request: http.ClientRequest = originalRequest.call(http, options, ...requestArgs);
+            clientRequestPatch(request, options);
             return request;
         };
 
-        // On node >= v0.11.12, https.request just calls http.request (with additional options).
-        // But on older versions, https.request needs to be patched also.
-        // The regex matches versions < 0.11.12 (avoiding a semver package dependency).
-        if (/^0\.([0-9]\.)|(10\.)|(11\.([0-9]|10|11)$)/.test(process.versions.node)) {
-            const originalHttpsRequest = https.request;
-            https.request = (options, ...requestArgs: any[]) => {
-                const request: http.ClientRequest = originalHttpsRequest.call(
-                    https, options, ...requestArgs);
-                if (request && options && !(<any>options)[AutoCollectHttpDependencies.disableCollectionRequestOption]) {
-                    AutoCollectHttpDependencies.trackRequest(this._client, options, request);
-                }
-                return request;
-            };
-        }
+        https.request = (options, ...requestArgs: any[]) => {
+            const request: http.ClientRequest = originalHttpsRequest.call(https, options, ...requestArgs);
+            clientRequestPatch(request, options);
+            return request;
+        };
+
+        // Node 8 calls http.request from http.get using a local reference!
+        // We have to patch .get manually in this case and can't just assume request is enough
+        // We have to replace the entire method in this case. We can't call the original.
+        // This is because calling the original will give us no chance to set headers as it internally does .end().
+        http.get = (options, ...requestArgs: any[]) => {
+            const request: http.ClientRequest = http.request.call(http, options, ...requestArgs);
+            request.end();
+            return request;
+        };
+        https.get = (options, ...requestArgs: any[]) => {
+            const request: http.ClientRequest = https.request.call(https, options, ...requestArgs);
+            request.end();
+            return request;
+        };
     }
 
     /**

diff --git a/Library/Config.ts b/Library/Config.ts
@@ -18,7 +18,7 @@ class Config {
     public endpointUrl: string;
     /** The maximum number of telemetry items to include in a payload to the ingestion endpoint (Default 250) */
     public maxBatchSize: number;
-    /** The maximum amount of time to wait for a payload to reach maxBatchSize (Default 1500) */
+    /** The maximum amount of time to wait for a payload to reach maxBatchSize (Default 15000) */
     public maxBatchIntervalMs: number;
     /** A flag indicating if telemetry transmission is disabled (Default false) */
     public disableAppInsights: boolean;

diff --git a/Library/Context.ts b/Library/Context.ts
@@ -25,7 +25,7 @@ class Context {
 
         try {
             // note: this should return the host package.json
-            var packageJson = require(packageJsonPath || "../../../package.json");
+            var packageJson = require(packageJsonPath || "../../../../package.json");
             if(packageJson) {
                 if (typeof packageJson.version === "string") {
                     version = packageJson.version;

diff --git a/Library/EnvelopeFactory.ts b/Library/EnvelopeFactory.ts
@@ -83,6 +83,12 @@ class EnvelopeFactory {
         envelope.time = (new Date()).toISOString();
         envelope.ver = 1;
         envelope.sampleRate = config ? config.samplingPercentage : 100;
+
+        // Exclude metrics from sampling by default
+        if (telemetryType === Contracts.TelemetryType.Metric) {
+            envelope.sampleRate = 100;
+        }
+
         return envelope;
     }
 
@@ -111,6 +117,7 @@ class EnvelopeFactory {
         remoteDependency.success = telemetry.success;
         remoteDependency.type = telemetry.dependencyTypeName;
         remoteDependency.properties = telemetry.properties;
+        remoteDependency.resultCode = telemetry.resultCode;
 
         if (telemetry.id) {
             remoteDependency.id = telemetry.id;

diff --git a/README.md b/README.md
@@ -176,6 +176,37 @@ http.createServer( (req, res) => {
 });
 ```
 
+An example utility using `trackMetric` to measure how long event loop scheduling takes:
+
+```javascript
+function startMeasuringEventLoop() {
+  var startTime = process.hrtime();
+  var sampleSum = 0;
+  var sampleCount = 0;
+
+  // Measure event loop scheduling delay
+  setInterval(() => {
+    var elapsed = process.hrtime(startTime);
+    sampleSum += elapsed[0] * 1e9 + elapsed[1];
+    sampleCount++;
+  }, 0);
+
+  // Report custom metric every second
+  setInterval(() => {
+    var samples = sampleSum;
+    var count = sampleCount;
+    sampleSum = 0;
+    sampleCount = 0;
+
+    if (count > 0) {
+      var avgNs = samples / count;
+      var avgMs = Math.round(avgNs / 1e6);
+      client.trackMetric({name: "Event Loop Delay", value: avgMs});
+    }
+  }, 1000);
+}
+```
+
 ## Preprocess data with Telemetry Processors
 
 ```javascript
@@ -354,8 +385,11 @@ separately from clients created with `new appInsights.TelemetryClient()`.
     ```
 3. Run tests
     ```bash
-    npm test
+    npm run test
+    npm run backcompattest
+    npm run functionaltest
     ```
+    _Note: Functional tests require Docker_
 
 ---
 

diff --git a/Tests/EndToEnd.tests.ts b/Tests/EndToEnd.tests.ts
@@ -8,6 +8,7 @@ import sinon = require("sinon");
 import events = require("events");
 import AppInsights = require("../applicationinsights");
 import Sender = require("../Library/Sender");
+import { EventEmitter } from "events";
 
 /**
  * A fake response class that passes by default
@@ -20,25 +21,29 @@ class fakeResponse {
     constructor(private passImmediately: boolean = true) { }
 
     public on(event: string, callback: () => void) {
-        this.callbacks[event] = callback;
-        if (event == "end" && this.passImmediately) {
-            this.pass();
+        if (!this.callbacks[event]) {
+            this.callbacks[event] = callback;
+        } else {
+            var lastCallback = this.callbacks[event];
+            this.callbacks[event] = () => {
+                callback();
+                lastCallback();
+            };
         }
-    }
 
-    public once(event: string, callback: () => void) {
-        this.callbacks[event] = callback;
         if (event == "end" && this.passImmediately) {
-            this.pass();
+            this.pass(true);
         }
     }
 
-    public pass(): void {
-        this.statusCode = 200;
+    public pass(test = false): void {
         this.callbacks["data"] ? this.callbacks["data"]("data") : null;
         this.callbacks["end"] ? this.callbacks["end"]() : null;
         this.callbacks["finish"] ? this.callbacks["finish"]() : null;
     }
+
+    public end = this.pass;
+    public once = this.on;
 }
 
 /**
@@ -85,7 +90,6 @@ class fakeHttpServer extends events.EventEmitter {
         var response = new fakeResponse(false);
         this.emit("request", request, response);
         request.end();
-        response.pass();
     }
 }
 
@@ -141,6 +145,7 @@ describe("EndToEnd", () => {
             client.flush({
                 callback: (response) => {
                     assert.ok(response, "response should not be empty");
+                    assert.ok(response !== "no data to send", "response should have data");
                     done();
                 }
             });
@@ -152,62 +157,92 @@ describe("EndToEnd", () => {
                 fakeHttpSrv.setCallback(callback);
                 return fakeHttpSrv;
             });
-
-            sandbox.stub(http, 'get', (uri: string, callback: any) => {
-                fakeHttpSrv.emitRequest(uri);
-            });
-
+
             AppInsights
                 .setup("ikey")
+                .setAutoCollectRequests(true)
                 .start();
 
-            var server = http.createServer((req: http.ServerRequest, res: http.ServerResponse) => {
-                setTimeout(() => {
-                    AppInsights.defaultClient.flush({
-                        callback: (response) => {
-                            assert.ok(response, "response should not be empty");
-                            done();
-                        }
-                    });
-                }, 10);
+            var track = sandbox.stub(AppInsights.defaultClient, 'track');
+            http.createServer((req, res) => {
+                assert.equal(track.callCount, 0);
+                res.end();
+                assert.equal(track.callCount, 1);
+                done();
             });
 
-            server.on("listening", () => {
-                http.get("http://localhost:0/test", (response: http.ClientResponse) => { });
-            });
-            server.listen(0, "::");
+            fakeHttpSrv.emitRequest("http://localhost:0/test");
         });
 
         it("should collect https request telemetry", (done) => {
-            var fakeHttpsSrv = new fakeHttpsServer();
+            var fakeHttpSrv = new fakeHttpServer();
             sandbox.stub(https, 'createServer', (options: any, callback: (req: http.ServerRequest, res: http.ServerResponse) => void) => {
-                fakeHttpsSrv.setCallback(callback);
-                return fakeHttpsSrv;
+                fakeHttpSrv.setCallback(callback);
+                return fakeHttpSrv;
             });
+
+            AppInsights
+                .setup("ikey")
+                .setAutoCollectRequests(true)
+                .start();
 
-            sandbox.stub(https, 'get', (uri: string, callback: any) => {
-                fakeHttpsSrv.emitRequest(uri);
+            var track = sandbox.stub(AppInsights.defaultClient, 'track');
+            https.createServer(null, (req: http.ServerRequest, res: http.ServerResponse) => {
+                assert.equal(track.callCount, 0);
+                res.end();
+                assert.equal(track.callCount, 1);
+                done();
+            });
+
+            fakeHttpSrv.emitRequest("http://localhost:0/test");
+        });
+
+        it("should collect http dependency telemetry", (done) => {
+            this.request.restore();
+            var eventEmitter = new EventEmitter();
+            (<any>eventEmitter).method = "GET";
+            sandbox.stub(http, 'request', (url: string, c: Function) => {
+                process.nextTick(c);
+                return eventEmitter;
             });
 
             AppInsights
                 .setup("ikey")
+                .setAutoCollectDependencies(true)
                 .start();
 
-            var server = https.createServer(null, (req: http.ServerRequest, res: http.ServerResponse) => {
-                setTimeout(() => {
-                    AppInsights.defaultClient.flush({
-                        callback: (response) => {
-                            assert.ok(response, "response should not be empty");
-                            done();
-                        }
-                    });
-                }, 10);
+            var track = sandbox.stub(AppInsights.defaultClient, 'track');
+
+            http.request(<any>'http://test.com', (c) => {
+                assert.equal(track.callCount, 0);
+                eventEmitter.emit("response", {});
+                assert.equal(track.callCount, 1);
+                done();
+            });
+        });
+
+        it("should collect https dependency telemetry", (done) => {
+            this.request.restore();
+            var eventEmitter = new EventEmitter();
+            (<any>eventEmitter).method = "GET";
+            sandbox.stub(https, 'request', (url: string, c: Function) => {
+                process.nextTick(c);
+                return eventEmitter;
             });
 
-            server.on("listening", () => {
-                https.get(<any>"https://localhost:0/test", (response: http.ClientResponse) => { });
+            AppInsights
+                .setup("ikey")
+                .setAutoCollectDependencies(true)
+                .start();
+
+            var track = sandbox.stub(AppInsights.defaultClient, 'track');
+
+            https.request(<any>'https://test.com', (c) => {
+                assert.equal(track.callCount, 0);
+                eventEmitter.emit("response", {});
+                assert.equal(track.callCount, 1);
+                done();
             });
-            server.listen(0, "::");
         });
     });
 

diff --git a/Tests/FunctionalTests/Runner/TaskExpectations.js b/Tests/FunctionalTests/Runner/TaskExpectations.js
@@ -168,7 +168,8 @@ module.exports = {
     "ConsoleAssert": outputContract(
         "MessageData",
         (telemetry) => {
-            return telemetry.data.baseData.message.indexOf("AssertionError: Test console.assert") === 0 &&
+            return telemetry.data.baseData.message.indexOf("AssertionError") === 0 &&
+            telemetry.data.baseData.message.indexOf("Test console.assert") > 0 &&
             telemetry.data.baseData.severityLevel === 2;
         }
     ),
@@ -177,7 +178,7 @@ module.exports = {
         (telemetry) => {
             return telemetry.data.baseData.name === "SELECT * FROM 'test_table'" &&
             telemetry.data.baseData.data === "SELECT * FROM 'test_table'" &&
-            telemetry.data.baseData.target.indexOf(":3306") > -1 &&
+            telemetry.data.baseData.target.indexOf(":33060") > -1 &&
             telemetry.data.baseData.type == "mysql";
         }
     ),