re-add workspace default app insights, fix project ARM template (#36325) (#36336)

* re-add workspace default app insights, fix project ARM template

* move cl lines to right place

* cl wording

Author: MilesHolland

sdk/ml/azure-ai-ml/CHANGELOG.md

@@ -9,6 +9,8 @@ Expose `public_ip_address` in `AmlComputeNodeInfo`, to get the public ip address
 ### Bugs Fixed
 - InputTypes exported in constants module
 - WorkspaceConnection tags are now listed as deprecated, and the erroneously-deprecated metadata field has been un-deprecated and added as a initialization field. These two fields still point to the same underlying object property, and actual API usage of this value is unchanged.
+- Workspace Create operation works without an application insights being provided, and creates a default appIn resource for normal workspaces in that case.
+- Project create operations works in general.
 
 ### Other Changes
 - WorkspaceConnections are officially GA'd and no longer experimental. But its much newer subclasses remain experimental.

sdk/ml/azure-ai-ml/azure/ai/ml/_arm_deployments/arm_templates/workspace_base.json

@@ -119,10 +119,11 @@
         },
         "applicationInsightsOption": {
             "type": "string",
-            "defaultValue": "none",
+            "defaultValue": "new",
             "allowedValues": [
-                "none",
-                "existing"
+                "new",
+                "existing",
+                "none"
             ],
             "metadata": {
                 "description": "Determines whether or not new ApplicationInsights should be provisioned."
@@ -795,6 +796,34 @@
                 "networkAcls": "[if(equals(parameters('keyVaultBehindVNet'), 'true'), variables('networkRuleSetBehindVNet'), json('null'))]"
             }
         },
+        {
+            "condition": "[and(variables('enablePE'), equals(parameters('applicationInsightsOption'), 'new'))]",
+            "type": "Microsoft.OperationalInsights/workspaces",
+            "tags": "[parameters('tagValues')]",
+            "apiVersion": "2020-08-01",
+            "name": "[parameters('logAnalyticsName')]",
+            "location": "[if(or(equals(toLower(parameters('applicationInsightsLocation')),'westcentralus'), equals(toLower(parameters('applicationInsightsLocation')),'eastus2euap'), equals(toLower(parameters('applicationInsightsLocation')),'centraluseuap')),'southcentralus', parameters('applicationInsightsLocation'))]",
+            "kind": "web",
+            "properties": {
+                "Application_Type": "web"
+            }
+        },
+        {
+            "condition": "[and(variables('enablePE'), equals(parameters('applicationInsightsOption'), 'new'))]",
+            "type": "Microsoft.Insights/components",
+            "tags": "[parameters('tagValues')]",
+            "apiVersion": "2020-02-02-preview",
+            "name": "[parameters('applicationInsightsName')]",
+            "location": "[if(or(equals(toLower(parameters('applicationInsightsLocation')),'westcentralus'), equals(toLower(parameters('applicationInsightsLocation')),'eastus2euap'), equals(toLower(parameters('applicationInsightsLocation')),'centraluseuap')),'southcentralus', parameters('applicationInsightsLocation'))]",
+            "kind": "web",
+            "dependsOn": [
+                "[resourceId('Microsoft.OperationalInsights/workspaces', parameters('logAnalyticsName'))]"
+            ],
+            "properties": {
+                "Application_Type": "web",
+                "WorkspaceResourceId": "[parameters('logAnalyticsArmId')]"
+            }
+        },
         {
             "condition": "[variables('enablePE')]",
             "type": "Microsoft.MachineLearningServices/workspaces",
@@ -805,7 +834,8 @@
             "location": "[parameters('location')]",
             "dependsOn": [
                 "[resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName'))]",
-                "[resourceId('Microsoft.KeyVault/vaults', parameters('keyVaultName'))]"
+                "[resourceId('Microsoft.KeyVault/vaults', parameters('keyVaultName'))]",
+                "[resourceId('Microsoft.Insights/components', parameters('applicationInsightsName'))]"
             ],
             "identity": "[parameters('identity')]",
             "properties": {

sdk/ml/azure-ai-ml/azure/ai/ml/_arm_deployments/arm_templates/workspace_param.json

@@ -39,7 +39,7 @@
         "value": ""
     },
     "applicationInsightsOption": {
-        "value": "none"
+        "value": "new"
     },
     "applicationInsightsName": {
         "value": ""

sdk/ml/azure-ai-ml/azure/ai/ml/_arm_deployments/arm_templates/workspace_project.json

@@ -119,10 +119,11 @@
         },
         "applicationInsightsOption": {
             "type": "string",
-            "defaultValue": "none",
+            "defaultValue": "new",
             "allowedValues": [
-                "none",
-                "existing"
+                "new",
+                "existing",
+                "none"
             ],
             "metadata": {
                 "description": "Determines whether or not new ApplicationInsights should be provisioned."
@@ -161,7 +162,7 @@
             "type": "string",
             "defaultValue": "none",
             "allowedValues": [
-                "existing",
+                "new",
                 "none"
             ],
             "metadata": {
@@ -622,6 +623,25 @@
             "metadata": {
                 "description": "Whether to grant materialization identity permissions"
             }
+        },
+        "allowRoleAssignmentOnRG": {
+            "type": "string",
+            "defaultValue": "true",
+            "allowedValues": [
+                "true",
+                "false"
+            ],
+            "metadata": {
+                "description": "Unused for projects, but required due to interdependency of normal and project ARM templates."
+            }
+        },
+        
+        "systemDatastoresAuthMode": {
+            "type": "string",
+            "defaultValue": "",
+            "metadata": {
+                "description": "Unused for projects, but required due to interdependency of normal and project ARM templates"
+            }
         }
     },
     "variables": {

sdk/ml/azure-ai-ml/azure/ai/ml/operations/_workspace_operations_base.py

@@ -589,6 +589,14 @@ def _populate_arm_parameters(self, workspace: Workspace, **kwargs: Any) -> Tuple
                 param["applicationInsightsResourceGroupName"],
                 group_name,
             )
+        elif workspace._kind and workspace._kind.lower() in {WorkspaceKind.HUB, WorkspaceKind.PROJECT}:
+            _set_val(param["applicationInsightsOption"], "none")
+            # Set empty values because arm templates whine over unset values.
+            _set_val(param["applicationInsightsName"], "ignoredButCantBeEmpty")
+            _set_val(
+                param["applicationInsightsResourceGroupName"],
+                "ignoredButCantBeEmpty",
+            )
         else:
             log_analytics = _generate_log_analytics(workspace.name, resources_being_deployed)
             _set_val(param["logAnalyticsName"], log_analytics)

sdk/ml/azure-ai-ml/tests/workspace/ai_workspaces/e2etests/test_ai_workspace_operations.py

@@ -64,6 +64,7 @@ def test_ai_workspace_operations(self, client: MLClient, randstr: Callable[[], s
             )
             created_hub = client.workspaces.begin_create(workspace=local_hub).result()
             assert created_hub.associated_workspaces == []
+            assert created_hub.application_insights is None
 
             local_project1 = Project(
                 name=f"test_proj_1_{randstr('project_1_name')}",
@@ -73,6 +74,7 @@ def test_ai_workspace_operations(self, client: MLClient, randstr: Callable[[], s
                 # don't set project location, make sure it gets copied via value injection
             )
             created_project1 = client.workspaces.begin_create(workspace=local_project1).result()
+            assert created_project1.application_insights is None
 
             local_project2 = Project(
                 name=f"test_proj_2_{randstr('project_2_name')}",
@@ -81,6 +83,7 @@ def test_ai_workspace_operations(self, client: MLClient, randstr: Callable[[], s
                 display_name="project2 display name",
             )
             created_project2 = client.workspaces.begin_create(workspace=local_project2).result()
+            assert created_project2.application_insights is None
 
             project_ids = [created_project1.id, created_project2.id]
 
@@ -144,6 +147,14 @@ def test_ai_workspace_operations(self, client: MLClient, randstr: Callable[[], s
                 poller.wait()  # Need to wait for projects to be cleaned before deleting parent hub.
 
             if created_hub is not None:
+                if is_live():
+                    from time import sleep
+
+                    sleep(60)
                 poller = client.workspaces.begin_delete(created_hub.name, delete_dependent_resources=True)
                 assert poller
                 assert isinstance(poller, LROPoller)
+            # Double check that we didn't fail out of main code block before creating resources.
+            assert created_hub is not None
+            assert created_project1 is not None
+            assert created_project2 is not None

sdk/ml/azure-ai-ml/tests/workspace/e2etests/test_workspace.py

@@ -125,6 +125,7 @@ def workspace_validation(wps):
         workspace = client.workspaces.get(wps_name)
         assert isinstance(workspace, Workspace)
         assert workspace.name == wps_name
+        assert workspace.application_insights is not None
 
         param_image_build_compute = "compute"
         param_display_name = "Test display name"