Adding sanitization of properties to avoid Breeze errors

AutoCollection/Exceptions.ts

@@ -1,10 +1,6 @@
-import http = require("http");
-
 import Contracts = require("../Declarations/Contracts");
 import TelemetryClient = require("../Library/TelemetryClient");
-import Sender = require("../Library/Sender");
-import Queue = require("../Library/Channel");
-import Util = require("../Library/Util");
+
 
 class AutoCollectExceptions {
 

AutoCollection/Statsbeat.ts

@@ -8,6 +8,7 @@ import Vm = require("../Library/AzureVirtualMachine");
 import Config = require("../Library/Config");
 import Context = require("../Library/Context");
 import Network = require("./NetworkStatsbeat");
+import Util = require("../Library/Util");
 
 const STATSBEAT_LANGUAGE = "node";
 
@@ -63,24 +64,15 @@ class Statsbeat {
         if (isEnabled) {
             if (!this._handle) {
                 this._handle = setInterval(() => {
-                    this.trackShortIntervalStatsbeats().catch((error) => {
-                        // Failed to send Statsbeat
-                        Logging.info(Statsbeat.TAG, error);
-                    });
+                    this.trackShortIntervalStatsbeats();
                 }, Statsbeat.STATS_COLLECTION_SHORT_INTERVAL);
                 this._handle.unref(); // Allow the app to terminate even while this loop is going on
             }
             if (!this._longHandle) {
                 // On first enablement
-                this.trackLongIntervalStatsbeats().catch((error) => {
-                    // Failed to send Statsbeat
-                    Logging.info(Statsbeat.TAG, error);
-                });
+                this.trackLongIntervalStatsbeats();
                 this._longHandle = setInterval(() => {
-                    this.trackLongIntervalStatsbeats().catch((error) => {
-                        // Failed to send Statsbeat
-                        Logging.info(Statsbeat.TAG, error);
-                    });
+                    this.trackLongIntervalStatsbeats();
                 }, Statsbeat.STATS_COLLECTION_LONG_INTERVAL);
                 this._longHandle.unref(); // Allow the app to terminate even while this loop is going on
             }
@@ -124,11 +116,11 @@ class Statsbeat {
         this._instrumentation &= ~instrumentation;
     }
 
-    public countRequest(category: number, endpoint: string, duration: number, success: boolean) {
+    public countRequest(endpoint: number, host: string, duration: number, success: boolean) {
         if (!this.isEnabled()) {
             return;
         }
-        let counter: Network.NetworkStatsbeat = this._getNetworkStatsbeatCounter(category, endpoint);
+        let counter: Network.NetworkStatsbeat = this._getNetworkStatsbeatCounter(endpoint, host);
         counter.totalRequestCount++;
         counter.intervalRequestExecutionTime += duration;
         if (success === false) {
@@ -137,73 +129,82 @@ class Statsbeat {
         else {
             counter.totalSuccesfulRequestCount++;
         }
-
     }
 
-    public countException(category: number, endpoint: string) {
+    public countException(endpoint: number, host: string) {
         if (!this.isEnabled()) {
             return;
         }
-        let counter: Network.NetworkStatsbeat = this._getNetworkStatsbeatCounter(category, endpoint);
+        let counter: Network.NetworkStatsbeat = this._getNetworkStatsbeatCounter(endpoint, host);
         counter.exceptionCount++;
     }
 
-    public countThrottle(category: number, endpoint: string) {
+    public countThrottle(endpoint: number, host: string) {
         if (!this.isEnabled()) {
             return;
         }
-        let counter: Network.NetworkStatsbeat = this._getNetworkStatsbeatCounter(category, endpoint);
+        let counter: Network.NetworkStatsbeat = this._getNetworkStatsbeatCounter(endpoint, host);
         counter.throttleCount++;
     }
 
-    public countRetry(category: number, endpoint: string) {
+    public countRetry(endpoint: number, host: string) {
         if (!this.isEnabled()) {
             return;
         }
-        let counter: Network.NetworkStatsbeat = this._getNetworkStatsbeatCounter(category, endpoint);
+        let counter: Network.NetworkStatsbeat = this._getNetworkStatsbeatCounter(endpoint, host);
         counter.retryCount++;
     }
 
     public async trackShortIntervalStatsbeats() {
-        await this._getResourceProvider();
-        let networkProperties = {
-            "os": this._os,
-            "rp": this._resourceProvider,
-            "cikey": this._cikey,
-            "runtimeVersion": this._runtimeVersion,
-            "language": this._language,
-            "version": this._sdkVersion,
-            "attach": this._attach,
+        try {
+            await this._getResourceProvider();
+            let networkProperties = {
+                "os": this._os,
+                "rp": this._resourceProvider,
+                "cikey": this._cikey,
+                "runtimeVersion": this._runtimeVersion,
+                "language": this._language,
+                "version": this._sdkVersion,
+                "attach": this._attach,
+            }
+            this._trackRequestDuration(networkProperties);
+            this._trackRequestsCount(networkProperties);
+            await this._sendStatsbeats();
+        }
+        catch (error) {
+            Logging.info(Statsbeat.TAG, "Failed to send Statsbeat metrics: " + Util.dumpObj(error));
         }
-        this._trackRequestDuration(networkProperties);
-        this._trackRequestsCount(networkProperties);
-        await this._sendStatsbeats();
     }
 
     public async trackLongIntervalStatsbeats() {
-        await this._getResourceProvider();
-        let commonProperties = {
-            "os": this._os,
-            "rp": this._resourceProvider,
-            "cikey": this._cikey,
-            "runtimeVersion": this._runtimeVersion,
-            "language": this._language,
-            "version": this._sdkVersion,
-            "attach": this._attach,
-        };
-        let attachProperties = Object.assign({
-            "rpId": this._resourceIdentifier,
-        }, commonProperties);
-        this._statbeatMetrics.push({ name: Constants.StatsbeatCounter.ATTACH, value: 1, properties: attachProperties });
-        if (this._instrumentation != Constants.StatsbeatInstrumentation.NONE) {// Only send if there are some instrumentations enabled
-            let instrumentationProperties = Object.assign({ "feature": this._instrumentation, "type": Constants.StatsbeatFeatureType.Instrumentation }, commonProperties);
-            this._statbeatMetrics.push({ name: Constants.StatsbeatCounter.FEATURE, value: 1, properties: instrumentationProperties });
+        try {
+            await this._getResourceProvider();
+            let commonProperties = {
+                "os": this._os,
+                "rp": this._resourceProvider,
+                "cikey": this._cikey,
+                "runtimeVersion": this._runtimeVersion,
+                "language": this._language,
+                "version": this._sdkVersion,
+                "attach": this._attach,
+            };
+            let attachProperties = Object.assign({
+                "rpId": this._resourceIdentifier,
+            }, commonProperties);
+            this._statbeatMetrics.push({ name: Constants.StatsbeatCounter.ATTACH, value: 1, properties: attachProperties });
+            if (this._instrumentation != Constants.StatsbeatInstrumentation.NONE) {// Only send if there are some instrumentations enabled
+                let instrumentationProperties = Object.assign({ "feature": this._instrumentation, "type": Constants.StatsbeatFeatureType.Instrumentation }, commonProperties);
+                this._statbeatMetrics.push({ name: Constants.StatsbeatCounter.FEATURE, value: 1, properties: instrumentationProperties });
+            }
+            if (this._feature != Constants.StatsbeatFeature.NONE) {// Only send if there are some features enabled
+                let featureProperties = Object.assign({ "feature": this._feature, "type": Constants.StatsbeatFeatureType.Feature }, commonProperties);
+                this._statbeatMetrics.push({ name: Constants.StatsbeatCounter.FEATURE, value: 1, properties: featureProperties });
+            }
+            await this._sendStatsbeats();
         }
-        if (this._feature != Constants.StatsbeatFeature.NONE) {// Only send if there are some features enabled
-            let featureProperties = Object.assign({ "feature": this._feature, "type": Constants.StatsbeatFeatureType.Feature }, commonProperties);
-            this._statbeatMetrics.push({ name: Constants.StatsbeatCounter.FEATURE, value: 1, properties: featureProperties });
+        catch (error) {
+            Logging.info(Statsbeat.TAG, "Failed to send Statsbeat metrics: " + Util.dumpObj(error));
         }
-        await this._sendStatsbeats();
     }
 
     private _getNetworkStatsbeatCounter(endpoint: number, host: string): Network.NetworkStatsbeat {

Library/AuthorizationHandler.ts

@@ -25,7 +25,6 @@ class AuthorizationHandler {
     public async addAuthorizationHeader(requestOptions: http.RequestOptions | https.RequestOptions): Promise<void> {
         let authHeaderName = azureCore.Constants.HeaderConstants.AUTHORIZATION;
         let webResource = new azureCore.WebResource("https://");
-        this
         await this._azureTokenPolicy.sendRequest(webResource);
         requestOptions.headers[authHeaderName] = webResource.headers.get(authHeaderName);
     }

Library/EnvelopeFactory.ts

@@ -56,24 +56,27 @@ class EnvelopeFactory {
                 break;
         }
 
-        if (commonProperties && Contracts.domainSupportsProperties(data.baseData)) { // Do instanceof check. TS will automatically cast and allow the properties property
-            if (data && data.baseData) {
-                // if no properties are specified just add the common ones
-                if (!data.baseData.properties) {
-                    data.baseData.properties = commonProperties;
-                } else {
-                    // otherwise, check each of the common ones
-                    for (var name in commonProperties) {
-                        // only override if the property `name` has not been set on this item
-                        if (!data.baseData.properties[name]) {
-                            data.baseData.properties[name] = commonProperties[name];
+        if (data && data.baseData) {
+            if (Contracts.domainSupportsProperties(data.baseData)) { // Do instanceof check. TS will automatically cast and allow the properties property
+                if (commonProperties) {
+                    // if no properties are specified just add the common ones
+                    if (!data.baseData.properties) {
+                        data.baseData.properties = commonProperties;
+                    } else {
+                        // otherwise, check each of the common ones
+                        for (var name in commonProperties) {
+                            // only override if the property `name` has not been set on this item
+                            if (!data.baseData.properties[name]) {
+                                data.baseData.properties[name] = commonProperties[name];
+                            }
                         }
                     }
                 }
+                if (data.baseData.properties) {
+                    // sanitize properties
+                    data.baseData.properties = Util.validateStringMap(data.baseData.properties);
+                }
             }
-
-            // sanitize properties
-            data.baseData.properties = Util.validateStringMap(data.baseData.properties);
         }
 
         var iKey = config ? config.instrumentationKey || "" : "";

Library/Sender.ts

@@ -124,14 +124,18 @@ class Sender {
                     await authHandler.addAuthorizationHeader(options);
                 }
                 catch (authError) {
-                    let errorMsg = "Failed to get AAD bearer token for the Application. Error:" + authError.toString();
-                    // If AAD auth fails do not send to Breeze
+                    let errorMsg = "Failed to get AAD bearer token for the Application.";
+                    if (this._enableDiskRetryMode) {
+                        errorMsg += "This batch of telemetry items will be retried. ";
+                        this._storeToDisk(envelopes);
+                    }
+                    errorMsg += "Error:" + authError.toString();
+                    Logging.warn(Sender.TAG, errorMsg);
+
                     if (typeof callback === "function") {
                         callback(errorMsg);
                     }
-                    this._storeToDisk(envelopes);
-                    Logging.warn(Sender.TAG, errorMsg);
-                    return;
+                    return; // If AAD auth fails do not send to Breeze
                 }
             }
 

Tests/Library/EnvelopeFactoryTests.ts

@@ -57,6 +57,27 @@ describe("Library/EnvelopeFactory", () => {
             var envelope = EnvelopeFactory.createEnvelope(<Contracts.EventTelemetry>{ name: "name" }, Contracts.TelemetryType.Event, commonproperties, client.context, client.config);
             assert.equal(envelope.name, "Microsoft.ApplicationInsights.key.Event");
         });
+
+        it("should sanitize properties", () => {
+            var client1 = new Client("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
+            let commonProps = {
+                "commonProperty": 123,
+            };
+            var eventTelemetry = <Contracts.EventTelemetry>{ name: "name" };
+            eventTelemetry.properties = {
+                "prop1": false,
+                "prop2": 123,
+                "prop3": { "subProp1": "someValue" }
+            };
+            var env = EnvelopeFactory.createEnvelope(eventTelemetry, Contracts.TelemetryType.Event, (<any>commonProps), client1.context, client1.config);
+            var envData: Contracts.Data<Contracts.EventData> = <Contracts.Data<Contracts.EventData>>env.data;
+
+            // check properties
+            assert.equal(envData.baseData.properties.commonProperty, "123");
+            assert.equal(envData.baseData.properties.prop1, "false");
+            assert.equal(envData.baseData.properties.prop2, "123");
+            assert.equal(envData.baseData.properties.prop3, "{\"subProp1\":\"someValue\"}");
+        });
     });
 
     describe("#createDependencyData()", () => {

Tests/Library/Sender.tests.ts

@@ -354,6 +354,7 @@ describe("Library/Sender", () => {
             var addHeaderStub = sandbox.stub(handler, "addAuthorizationHeader", () => { throw new Error(); });
 
             var sender = new Sender(config, getAuthorizationHandler);
+            sender["_enableDiskRetryMode"] = true;
             var storeToDiskStub = sandbox.stub(sender, "_storeToDisk");
             let envelope = new Contracts.Envelope();
             envelope.name = "TestEnvelope";