Alert Monitor Alert - Move Rest Client to Common #7738

vincent1173 · 2018-07-16T09:37:41Z

Move Rest Client to Common
L0 Tests
E2E Tests

kmkumaran · 2018-07-19T04:29:02Z

Tasks/AzureMonitorAlertsV0/npm-shrinkwrap.json

      "requires": {
        "q": "1.4.1",
        "typed-rest-client": "0.12.0",
-        "vsts-task-lib": "2.0.1-preview"
+        "vsts-task-lib": "2.0.5"


Any new nested modules coming in part of this?

Task brings the same modules with different version, but all the new versions are IP scanned.

kmkumaran · 2018-07-19T04:36:18Z

Tasks/AzureMonitorAlertsV0/azuremonitoralerts.ts

@@ -1,13 +1,9 @@
 import * as tl from "vsts-task-lib/task";
 import * as path from "path";
+import { AzureRMEndpoint } from 'azure-arm-rest/azure-arm-endpoint';


@asranja - Can you review this PR.

asranja · 2018-07-20T08:30:36Z

Tasks/AzureMonitorAlertsV0/operations/AzureMonitorAlertsUtility.ts

+
+		tl.debug(`Getting AzureRm resource details - '${resourceName}' in resource group '${resourceGroupName}'`);
+
+        let resources: Resources = new Resources(this._azureEndpoint);


Indentation

asranja · 2018-07-20T08:30:59Z

Tasks/AzureMonitorAlertsV0/task.json

@@ -13,7 +13,7 @@
 	"version": {
 		"Major": 0,
 		"Minor": 1,


asranja · 2018-07-20T08:33:27Z

Tasks/AzureMonitorAlertsV0/task.json

@@ -130,6 +130,7 @@
 		"UpdatedRule": "Updated rule : '%s'",
 		"Couldnotfetchaccesstoken": "Could not fetch access token for Azure. Status Code: %s (%s) %s.",
 		"SPNExpiredCheck": "Check if the SPN is valid and not expired.",
-		"MSINotSupported": "Managed Service Identity(MSI) authentication is not supported for this task."
+		"FailedToGetApplicationInsightsResourceAlerts": "Failed to get Application Insights Alert rule: %s. Error: %s",
+        "FailedToUpdateApplicationInsightsResourceAlerts": "Failed to update Application Insights Alert rule '%s' Resource. Error: %s"


Indentation

asranja · 2018-07-20T08:33:40Z

Tasks/AzureMonitorAlertsV0/operations/AzureMonitorAlertsUtility.ts

+	let buildUri = tl.getVariable("Build.BuildUri");
+	let releaseWebUrl = tl.getVariable("Release.ReleaseWebUrl");
+	let collectionUrl = tl.getVariable('System.TeamFoundationCollectionUri'); 
+    let teamProject = tl.getVariable('System.TeamProjectId');


indentation

asranja · 2018-07-23T10:11:01Z

Tasks/AzureMonitorAlertsV0/operations/AzureMonitorAlertsUtility.ts

+
+        let resourceId: string = `/subscriptions/${this._azureEndpoint.subscriptionID}/resourceGroups/${this._resourceGroupName}/providers/${this._resourceType}/${this._resourceName}`;
+
+        let azureApplicationInsightsAlerts :AzureMonitorAlerts = new AzureMonitorAlerts(this._azureEndpoint, this._resourceGroupName);


rename azureApplicationInsightsAlerts to azureMetricAlerts or AzureMontiorAlerts

asranja · 2018-07-23T10:11:37Z

Tasks/AzureMonitorAlertsV0/operations/AzureMonitorAlertsUtility.ts

+
+
+    private async _getRequestBodyForAddingAlertRule(
+        azureApplicationInsightsAlerts :AzureMonitorAlerts,


rename azureApplicationInsightsAlerts.

asranja · 2018-07-23T10:11:53Z

Tasks/AzureMonitorAlertsV0/operations/AzureMonitorAlertsUtility.ts

+			console.log(tl.loc("AlertRuleExists", rule.alertName, resourceGroupName));
+		}
+		catch (error) {
+            console.log(error);


indentation

asranja · 2018-07-23T10:12:22Z

Tasks/AzureMonitorAlertsV0/operations/AzureMonitorAlertsUtility.ts

+}
+
+
+


Too many empty lines.

asranja · 2018-07-23T10:12:56Z

Tasks/Common/azure-arm-rest/azure-arm-appinsigths-alerts.ts

+        {
+            '{resourceGroupName}': this._resourceGroupName,
+            '{resourceName}': alertRuleName,
+        }, null, APIVersions.azure_arm_appinsights_alerts);


rename azure_arm_appinsights_alerts.

asranja · 2018-07-23T10:13:24Z

Tasks/Common/azure-arm-rest/constants.ts

@@ -9,7 +9,8 @@ export const productionSlot: string = "production";
 export const mysqlApiVersion: string = '2017-12-01';

 export const APIVersions = {
-    azure_arm_appinsights: '2015-05-01'
+    azure_arm_appinsights: '2015-05-01',
+    azure_arm_appinsights_alerts: '2016-03-01'


rename azure_arm_appinsights_alerts

asranja · 2018-07-23T10:13:39Z

Tasks/Common/azure-arm-rest/module.json

@@ -135,6 +135,8 @@
        "FailedToGetDeploymentLogs": "Failed to get deployment logs. Error: %s",
        "ARG_RedirectResponseInvalidStatusCode": "The HTTP response code: '%s' is not a valid redirect status code.",
        "ARG_RedirectResponseLocationHeaderIsNull": "Location header is null for HTTP response with status code: %s",
-        "ASE_SSLIssueRecommendation": "To use a certificate in App Service, the certificate must be signed by a trusted certificate authority. If your web app gives you certificate validation errors, you're probably using a self-signed certificate and to resolve them you need to set a variable named VSTS_ARM_REST_IGNORE_SSL_ERRORS to the value true in the build or release definition"
+        "ASE_SSLIssueRecommendation": "To use a certificate in App Service, the certificate must be signed by a trusted certificate authority. If your web app gives you certificate validation errors, you're probably using a self-signed certificate and to resolve them you need to set a variable named VSTS_ARM_REST_IGNORE_SSL_ERRORS to the value true in the build or release definition",
+        "FailedToGetApplicationInsightsResourceAlerts": "Failed to get Application Insights Alert rule: %s. Error: %s",


rename FailedToGetApplicationInsightsResourceAlerts.

asranja · 2018-07-23T10:13:52Z

Tasks/Common/azure-arm-rest/module.json

-        "ASE_SSLIssueRecommendation": "To use a certificate in App Service, the certificate must be signed by a trusted certificate authority. If your web app gives you certificate validation errors, you're probably using a self-signed certificate and to resolve them you need to set a variable named VSTS_ARM_REST_IGNORE_SSL_ERRORS to the value true in the build or release definition"
+        "ASE_SSLIssueRecommendation": "To use a certificate in App Service, the certificate must be signed by a trusted certificate authority. If your web app gives you certificate validation errors, you're probably using a self-signed certificate and to resolve them you need to set a variable named VSTS_ARM_REST_IGNORE_SSL_ERRORS to the value true in the build or release definition",
+        "FailedToGetApplicationInsightsResourceAlerts": "Failed to get Application Insights Alert rule: %s. Error: %s",
+        "FailedToUpdateApplicationInsightsResourceAlerts": "Failed to update Application Insights Alert rule '%s' Resource. Error: %s"


Rename FailedToUpdateApplicationInsightsResourceAlerts

asranja

Please take care of the renaming.

Alert Monitor Alert - Move Rest Client to Common

b24f69d

vincent1173 added Area: Release Area: AzureAppService Label to monitor Azure App Service issues labels Jul 16, 2018

vincent1173 assigned kmkumaran Jul 16, 2018

vincent1173 requested review from kmkumaran and asranja July 16, 2018 09:37

modified L0 Tests

44de7ba

kmkumaran reviewed Jul 19, 2018

View reviewed changes

loc strings updated

5388954

asranja reviewed Jul 20, 2018

View reviewed changes

Tasks/AzureMonitorAlertsV0/task.json Outdated

@@ -13,7 +13,7 @@

"version": {

"Major": 0,

"Minor": 1,

Copy link

Contributor

asranja Jul 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.2.0

asranja reviewed Jul 20, 2018

View reviewed changes

vincent1173 added 3 commits July 20, 2018 17:04

indentation fix

2e5e125

addressed review comments

9f3cf44

indentation fix

20f9186

asranja reviewed Jul 23, 2018

View reviewed changes

asranja approved these changes Jul 23, 2018

View reviewed changes

addressed review comments

0c451bd

vincent1173 merged commit 2a6e2da into master Jul 24, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alert Monitor Alert - Move Rest Client to Common #7738

Alert Monitor Alert - Move Rest Client to Common #7738

vincent1173 commented Jul 16, 2018 •

edited

Loading

kmkumaran Jul 19, 2018

vincent1173 Jul 19, 2018

kmkumaran Jul 19, 2018

asranja Jul 20, 2018

asranja Jul 20, 2018

asranja Jul 20, 2018

asranja Jul 20, 2018

asranja Jul 23, 2018

vincent1173 Jul 24, 2018

asranja Jul 23, 2018

vincent1173 Jul 24, 2018

asranja Jul 23, 2018

vincent1173 Jul 24, 2018

asranja Jul 23, 2018

vincent1173 Jul 24, 2018

asranja Jul 23, 2018

vincent1173 Jul 24, 2018

asranja Jul 23, 2018

vincent1173 Jul 24, 2018

asranja Jul 23, 2018

vincent1173 Jul 24, 2018

asranja Jul 23, 2018

vincent1173 Jul 24, 2018

asranja left a comment


		tl.debug(`Getting AzureRm resource details - '${resourceName}' in resource group '${resourceGroupName}'`);

		let resources: Resources = new Resources(this._azureEndpoint);


		let resourceId: string = `/subscriptions/${this._azureEndpoint.subscriptionID}/resourceGroups/${this._resourceGroupName}/providers/${this._resourceType}/${this._resourceName}`;

		let azureApplicationInsightsAlerts :AzureMonitorAlerts = new AzureMonitorAlerts(this._azureEndpoint, this._resourceGroupName);



		private async _getRequestBodyForAddingAlertRule(
		azureApplicationInsightsAlerts :AzureMonitorAlerts,

Alert Monitor Alert - Move Rest Client to Common #7738

Alert Monitor Alert - Move Rest Client to Common #7738

Conversation

vincent1173 commented Jul 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asranja left a comment

Choose a reason for hiding this comment

vincent1173 commented Jul 16, 2018 •

edited

Loading