Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r/application_insights: support for the Workspace model #7667

Closed
ghost opened this issue Jul 9, 2020 · 46 comments
Closed

r/application_insights: support for the Workspace model #7667

ghost opened this issue Jul 9, 2020 · 46 comments
Labels
enhancement preview sdk/requires-newer-api-version This requires upgrading the version of the API being used service/application-insights upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR
Milestone

Comments

@ghost
Copy link

ghost commented Jul 9, 2020

This issue was originally opened by @klainn as hashicorp/terraform#25533. It was migrated here as a result of the provider split. The original body of the issue is below.


Current Terraform Version

Terraform v0.12.25
+ provider.azurerm v2.17.0

Use-cases

I am attempting to bind a new application insights resource to a pre-existing log analytics workspace for log consolidation and query.

Attempted Solutions

I attempted to look through the Terraform documentation for how to change Resource mode from classic to workspace-based (which is in tech preview) but there appears to currently be no such option.

Proposal

Extend the azurerm_application_insights resource to allow for a resource_mode directive which would allow for a no options classic mode (today) and a workspace-based option which would take in the current subscription and the id of the log analytics workspace.

References

@reidblomquist-kr
Copy link

From what I can tell this isn't available even for creating new log workspaces? Am I missing something?

@jwshive
Copy link

jwshive commented Aug 12, 2020

It's not available in terraform, but you can select workspace based resource mode in the Azure Portal. It is still in Preview phase though.

@khaliddermoumi
Copy link
Contributor

khaliddermoumi commented Sep 9, 2020

Dear Terraform team, @mybayern1974,
I would be willing to create a pull request for this feature.
Please let me know if:

  • you currently develop the feature yourself, so a pull request wouldn't make sense
  • you would not welcome a pull request for this feature (for whatever reasons)

Thanks! :)

@scharan31

This comment has been minimized.

@khaliddermoumi
Copy link
Contributor

Today I found the time to start implementing this feature.

Unfortunately it turns out that there's no straightforward path to implementing this currently. The reason is that the Azure SDK for Go does not support recent API versions yet, the API version which would have to be required is "2020-02-02-preview".
I made a feature request for it: appinsights: support API version "2020-02-02-preview" #13012.

@SureshBettadapur

This comment has been minimized.

@timja

This comment has been minimized.

@SureshBettadapur

This comment has been minimized.

@haflidif

This comment has been minimized.

@kenia-cloud

This comment has been minimized.

@kibnelbachyr

This comment has been minimized.

@ricohomewood
Copy link

Microsoft have just announced that they will be retiring the classic implementation of Application Insights as of 29th Feb 2024 as per here: https://azure.microsoft.com/en-us/updates/we-re-retiring-classic-application-insights-on-29-february-2024/ and as such workspace based Application Insights resources will be the default for all newly created AI resources going forward.

@sebader
Copy link
Contributor

sebader commented Feb 24, 2021

and as such workspace based Application Insights resources will be the default for all newly created AI resources going forward.

I don't think that's the case when creating via the APIs/SDKs. As the Go SDK doesn't yet allow you to specify a Workspace ID, it cannot yet create a workspace-attached AI

@mybayern1974
Copy link
Collaborator

@neil-yechenwei for awareness

@ricohomewood
Copy link

and as such workspace based Application Insights resources will be the default for all newly created AI resources going forward.

I don't think that's the case when creating via the APIs/SDKs. As the Go SDK doesn't yet allow you to specify a Workspace ID, it cannot yet create a workspace-attached AI

I can see it is in the Azure API 2020-02-02-preview but that may be sitting with MSFT to release still as you can see in this request to add it to the Azure SDK for GO: Azure/azure-sdk-for-go#13012

@tombuildsstuff tombuildsstuff changed the title Please add the ability to change the resource mode and apply a log analytics workspace ID to azurerm_application_insights r/application_insights: support for the Workspace model Feb 25, 2021
@tombuildsstuff tombuildsstuff added sdk/not-yet-supported Support for this does not exist in the upstream SDK at this time sdk/requires-newer-api-version This requires upgrading the version of the API being used labels Feb 25, 2021
@scuderig

This comment has been minimized.

@ricohomewood
Copy link

ricohomewood commented Mar 26, 2021

This was posted 3 days ago from the service team that manages the Azure SDK for Go

Hi all, the service team is working on the swagger correctness issues now, once they finish their work, we could get a release on the new api-version.

@martinfletcher
Copy link

martinfletcher commented Apr 14, 2021

Until this is supported natively by terraform, you can use the following to set the workspace for the application insights instance:

monitoring.tf

resource "azurerm_resource_group" "monitoring" {
  name     = "rg-monitoring"
  location = "uksouth"
}

resource "azurerm_log_analytics_workspace" "monitoring" {
  name                            = "la-monitoring"
  location                        = azurerm_resource_group.monitoring.location
  resource_group_name = azurerm_resource_group.monitoring.name
}

resource "azurerm_application_insights" "monitoring" {
  name                            = "ai-monitoring"
  location                        = azurerm_resource_group.monitoring.location
  resource_group_name = azurerm_resource_group.monitoring.name
  application_type           = "other"
}

resource "null_resource" "monitoring_configure_application_insights" {
  triggers = {
    resource_group_name                        = azurerm_resource_group.monitoring.name
    application_insights_name                  = azurerm_application_insights.monitoring.name
    log_analytics_workspace_resource_id = azurerm_log_analytics_workspace.monitoring.id
  }

  provisioner "local-exec" {
    interpreter    = ["pwsh", "-command"]
    command     = file("${path.module}/Configure-ApplicationInsights.ps1")
    environment = {
      resource_group_name                         = self.triggers.resource_group_name
      application_insights_name                  = self.triggers.application_insights_name
      log_analytics_workspace_resource_id = self.triggers.log_analytics_workspace_resource_id
    }
  }
}

Configure-ApplicationInsights.ps1

if(0 -eq (az extension list | ConvertFrom-Json | Where-Object { "application-insights" -eq $_.name }).Length)
{
    az extension add --name application-insights
}

az monitor app-insights component update --resource-group $env:RESOURCE_GROUP_NAME `
                                      --app $env:APPLICATION_INSIGHTS_NAME `
                                      --workspace $env:LOG_ANALYTICS_WORKSPACE_RESOURCE_ID

@BHoggs
Copy link
Contributor

BHoggs commented May 5, 2021

Excuse my ignorance - but given this provider is frequently held back by updates to the Go SDK, wouldn't it be better if terraform just leveraged the ARM API directly instead of leveraging the SDK? After all... TF is kind of an SDK in itself.
Obviously it would be too much re-work at this point, but I'm just curious as to why the SDK is used in the first place.

@khaliddermoumi
Copy link
Contributor

BHoggs is absolutely correct.
My impression is that MS is working suspiciously slowly on the Go SDK - even though Go is a major platform nowadays.
This leads directly to their own framework "bicep", which they are pushing a lot nowadays, and which will be a major competitor for Terraform on Azure.
Terraform on Azure would be much better off directly using the ARM Rest API. If Terraform wants to stay relevant on Azure, this shift would be a major step.

@mybayern1974
Copy link
Collaborator

Hi @BHoggs , @khaliddermoumi , I'd like to chime in to share my understanding of the reason of implementation of this feature being not fast enough. I believe having Terraform directly rely on ARM API is definitely an interesting and open topic to discuss, though settling down this strategy in Terraform might be a relatively long journey.

However, I also get the impression that being lack of GO SDK is not the cause of the progress for this issue being slow, but its rest api is not ready yet. Per the linked request that files Azure GO SDK for this feature, a maintainer of the Azure GO SDK repo has explained that currently the service rest api still does not meet some Azure internal bar for rest api yet. Once that bar could be met so GO SDK cannot be made, Azure GO SDK, which is auto-generated from rest api, is expected to come into being soon.

With noticing this is a highly voted up feature request, I'm also keeping an eye on it with periodically ping Azure service team to get their possible timeline of having the rest api ready.

I also noticed it's mentioned this required feature has been there on Portal. AFAIK portal does not directly calls service rest api to interact with service backend (maybe some services do while some not. I'm not the expert for that), if my assumption is right, then portal-has-but-rest-api-not-ready is not that strange any more.

@CrHasher
Copy link

CrHasher commented Jul 6, 2021

Until this is supported natively by terraform, you can use the following to set the workspace for the application insights instance:

monitoring.tf

resource "azurerm_resource_group" "monitoring" {
  name     = "rg-monitoring"
  location = "uksouth"
}

resource "azurerm_log_analytics_workspace" "monitoring" {
  name                            = "la-monitoring"
  location                        = azurerm_resource_group.monitoring.location
  resource_group_name = azurerm_resource_group.monitoring.name
}

resource "azurerm_application_insights" "monitoring" {
  name                            = "ai-monitoring"
  location                        = azurerm_resource_group.monitoring.location
  resource_group_name = azurerm_resource_group.monitoring.name
  application_type           = "other"
}

resource "null_resource" "monitoring_configure_application_insights" {
  triggers = {
    resource_group_name                        = azurerm_resource_group.monitoring.name
    application_insights_name                  = azurerm_application_insights.monitoring.name
    log_analytics_workspace_resource_id = azurerm_log_analytics_workspace.monitoring.id
  }

  provisioner "local-exec" {
    interpreter    = ["pwsh", "-command"]
    command     = file("${path.module}/Configure-ApplicationInsights.ps1")
    environment = {
      resource_group_name                         = self.triggers.resource_group_name
      application_insights_name                  = self.triggers.application_insights_name
      log_analytics_workspace_resource_id = self.triggers.log_analytics_workspace_resource_id
    }
  }
}

Configure-ApplicationInsights.ps1

if(0 -eq (az extension list | ConvertFrom-Json | Where-Object { "application-insights" -eq $_.name }).Length)
{
    az extension add --name application-insights
}

az monitor app-insights component update --resource-group $env:RESOURCE_GROUP_NAME `
                                      --app $env:APPLICATION_INSIGHTS_NAME `
                                      --workspace $env:LOG_ANALYTICS_WORKSPACE_RESOURCE_ID

For some reason local-exec is not executed at all, know any reason why this might happen?

Not even a simple echo works in my case:

resource "null_resource" "monitoring_configure" {
  triggers = {
    subscription_id                     = var.devsubid
    resource_group_name                 = azurerm_resource_group.resource_group.name
    application_insights_name           = azurerm_application_insights.application_insights.name
    log_analytics_workspace_resource_id = azurerm_log_analytics_workspace.log_analytics_workspace.id
  }

  provisioner "local-exec" {
    #interpreter = ["pwsh", "-command"]
    command     = "echo This is the local exec!"
    # command     = file("${path.module}/Configure-ApplicationInsights.ps1")
    on_failure  = fail
    #environment = {
    #  subscription_id                     = self.triggers.subscription_id
    #  resource_group_name                 = self.triggers.resource_group_name
    #  application_insights_name           = self.triggers.application_insights_name
    #  log_analytics_workspace_resource_id = self.triggers.log_analytics_workspace_resource_id
    #}
  }
}

When I do terraform apply there is no such output for null resource like ex.:

 # null_resource.monitoring_configure will be created

And I also tried to add a simple echo to a resource that is actually created but local-exec is still not executed ...

@CrHasher
Copy link

CrHasher commented Jul 7, 2021

#7667 (comment)
Note: To long to quote

The fix was to just copy all files *.tf and scripts (in my case *.ps1) to a new folder and do a terraform apply there, terraform state was corrupted for some odd reason.

@khaliddermoumi
Copy link
Contributor

I agree this should be implemented in the provider. The workarounds aren't a good solution.
But I personally will not be able to do this soon.
If anybody would like to implement it, please drop a note here. I will too, as soon as I start working on this.

@muthu329024
Copy link

Until this is supported natively by terraform, you can use the following to set the workspace for the application insights instance:
Note: This is will make state of the resource to be maintained within terraform.

ApplicationInsights.tf

resource "azurerm_log_analytics_workspace" "log_analytics" {
  name                = replace(var.resource_group, "RGP", "LAW")
  location            = "australiasoutheast"
  resource_group_name = var.resource_group
  sku                 = "PerGB2018"
  retention_in_days   = 30
}

resource "azurerm_application_insights" "ApplicationInsights" {
    name = replace(var.resource_group, "RGP", "AppInsight")
    location = "australiasoutheast"
    resource_group_name = var.resource_group
    retention_in_days = 30
    application_type = "web"
}

/*
  The azurerm_application_insights resource type doesn't allow you to specify workspace
  ! Very limiting. Luckily, we can use the TF resource to *create* by this resource state can be maintained
*/
resource "azurerm_template_deployment" "AppInsight_workspace" {
  depends_on          = [azurerm_log_analytics_workspace.log_analytics,azurerm_application_insights.ApplicationInsights]
  name                = replace(var.resource_group, "RGP", "AppInsight-workspace")
  resource_group_name = var.resource_group
  deployment_mode     = "Incremental"

  template_body = file("${path.module}/ApplicationInsights_WorkspaceResource.template.json")

  parameters = {
    AppInsightName = azurerm_application_insights.ApplicationInsights.name
    ApplicationType = azurerm_application_insights.ApplicationInsights.application_type
    LawWorkspaceID = azurerm_log_analytics_workspace.log_analytics.id
    ApplicationLocation = azurerm_application_insights.ApplicationInsights.location
  }
}

ApplicationInsights_WorkspaceResource.template.json

{
  "$schema": "http://schema.management.azure.com/schemas/2014-04-01-preview/deploymentTemplate.json#",
  "contentVersion": "1.0.0.0",
  "parameters": {
    "AppInsightName": {
      "type": "string"
    },
    "LawWorkspaceID": {
      "type": "string"
    },
    "ApplicationType": {
      "type": "string"
    },
    "ApplicationLocation": {
      "type": "string"
    }
  },
  "resources": [
    {
      "apiVersion": "2020-02-02-preview",
      "type": "microsoft.insights/components",
      "name": "[parameters('AppInsightName')]",
      "location": "[parameters('ApplicationLocation')]",
      "properties": {
          "ApplicationId": "[parameters('AppInsightName')]",
          "Application_Type": "[parameters('ApplicationType')]",
          "Flow_Type": "Redfield",
          "Request_Source": "Custom",
          "WorkspaceResourceId": "[parameters('LawWorkspaceID')]"
      }
    }
  ]
}

@CrHasher
Copy link

CrHasher commented Jul 12, 2021

@muthu329024 Thanks for the suggestion for using azurerm_template_deployment it actually works and its better than using az command. Tags are the only thing not created this way. Any idea how to add tags?

So far I have this:

resource "azurerm_log_analytics_workspace" "log_analytics_workspace" {
  name                = lower("log-${var.game}-dev-${var.id}-${var.location}")
  location            = azurerm_resource_group.resource_group.location
  resource_group_name = azurerm_resource_group.resource_group.name
  sku                 = "PerGB2018"
  retention_in_days   = 30
  tags                = { environment = "development" }
}

resource "azurerm_application_insights" "application_insights" {
  name                = lower("appi-${var.game}-dev-${var.id}-${var.location}")
  location            = azurerm_resource_group.resource_group.location
  resource_group_name = azurerm_resource_group.resource_group.name
  application_type    = "web"
  retention_in_days   = 30
  tags                = { environment = "development" }
}

output "instrumentation_key" {
  sensitive = true
  value     = azurerm_application_insights.application_insights.instrumentation_key
}

output "app_id" {
  value = azurerm_application_insights.application_insights.app_id
}

resource "azurerm_template_deployment" "application_insights_workspace" {
  depends_on          = [azurerm_log_analytics_workspace.log_analytics_workspace, azurerm_application_insights.application_insights]
  name                = lower("appi-wksp-${var.game}-dev-${var.id}-${var.location}")
  resource_group_name = azurerm_resource_group.resource_group.name
  deployment_mode     = "Incremental"

  template_body = file("${path.module}/ApplicationInsights_WorkspaceResource.template.json")

  parameters = {
    AppInsightName      = azurerm_application_insights.application_insights.name
    ApplicationType     = azurerm_application_insights.application_insights.application_type
    LawWorkspaceID      = azurerm_log_analytics_workspace.log_analytics_workspace.id
    ApplicationLocation = azurerm_application_insights.application_insights.location
  }
}

@dylanmorley
Copy link
Contributor

I've picked this up, shall be raising a PR in a little while - Will link to this ticket when I do.

@dylanmorley
Copy link
Contributor

Initial work is done - PR will look like master...dylanmorley:master

A couple of bits finish - one failing test to investigate - then a bit of polish up / linting / documentation.

@dylanmorley
Copy link
Contributor

dylanmorley commented Jul 13, 2021

Failing test is due to what looks like a breaking change with the API bump around data retention (retention_in_days)

In the 2015-05-01/insights models, this value is read/write

https://github.com/terraform-providers/terraform-provider-azurerm/blob/bf0bcf39b9258514bbf56363da22773cef963aec/vendor/github.com/Azure/azure-sdk-for-go/services/appinsights/mgmt/2015-05-01/insights/models.go#L798

In the 2020-02-02-preview, this has become read only

https://github.com/dylanmorley/terraform-provider-azurerm/blob/89e51e3c91a45be908c5a090bdb61c602c7a93a7/vendor/github.com/Azure/azure-sdk-for-go/services/preview/appinsights/mgmt/2020-02-02-preview/insights/models.go#L814

'Complete' test is failing as it obviously hasn't set this property as expected

image

Going to need to dig into this a bit further to understand the change - will slow me down a bit. Any extra eyes on this appreciated.

Edit/
Removed from ARM in this version : https://docs.microsoft.com/en-us/azure/templates/microsoft.insights/2020-02-02-preview/components?tabs=json

Change makes sense from the POV that, when configured as a workspace resource, data retention is controlled by the log analytics workspace and not the app insights resource. However, you can still create 'legacy / not workspace connected' resources where the setting would be valid.

Can we use resources client to perform an update, similar to how az cli would do, e.g. https://stackoverflow.com/a/61810033/1538039

@freeman
Copy link
Contributor

freeman commented Jul 13, 2021

It seems this API version requires workspace based approach. See https://github.com/Azure/azure-sdk-for-go/blob/main/services/preview/appinsights/mgmt/2020-02-02-preview/insights/models.go#L819

I don’t know if we can use multiple version of the API at the same time in go ?

@dylanmorley
Copy link
Contributor

dylanmorley commented Jul 16, 2021

As mentioned in the closed PR - I worked around the problem of the removed retention in days option by using the ResourcesClient to do an update to the created 'classic mode' AI resource. Whilst this worked, would rather not workaround the API problem in this way as there's no other place in the provider that does this.

Therefore, a suggestion is to create a new resource for workspace mode resource, azurerm_application_insights_workspace_mode (something along those lines) and mark the azurerm_application_insights as superseded in the documentation. See #12583 for a bit of context

Will cause a bit of duplication but is a clean enough approach - any thoughts or opinions?

@csorzi
Copy link

csorzi commented Jul 20, 2021

@dylanmorley When do you think this can be implemented? I am currently working on a TF implementation for a client and this is the only missing piece. I would rather wait a bit for this than use some workaround with ARM templates.

@neil-yechenwei
Copy link
Contributor

For the issue of the RetentionInDays property, seems it's better to be fixed from API side. Once the fix is ready, TF could support LA feature easily.

@dylanmorley
Copy link
Contributor

Issue for Rest API here Azure/azure-rest-api-specs#14801.

@tombuildsstuff tombuildsstuff added upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR and removed sdk/not-yet-supported Support for this does not exist in the upstream SDK at this time labels Jul 22, 2021
@neil-yechenwei
Copy link
Contributor

@dylan-asos
Copy link
Contributor

Great thanks @neil-yechenwei looks like 15247 resolves the issue so I'll amend my PR with the latest SDK - should be some minor changes so I'll get this finished in the next day or so.

katbyte pushed a commit that referenced this issue Aug 5, 2021
As per issue #7667, updating the application_insights_resource to enable support of workspace mode, available with updated package 2020-02-02 in the Go SDK at v56.0.

Updated the insights API from 2015-05-01 to 2020-02-02
Fixed upgrade issues (enum changes )
Supporting workspace ID as an optional parameter when creating AI resources
Added workspace_id to data source resource
Documentation updates
@katbyte katbyte added this to the v2.71.0 milestone Aug 5, 2021
@katbyte
Copy link
Collaborator

katbyte commented Aug 5, 2021

closed by #12818 i believe

@katbyte katbyte closed this as completed Aug 5, 2021
@github-actions
Copy link

github-actions bot commented Aug 6, 2021

This functionality has been released in v2.71.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@aristosvo
Copy link
Collaborator

@dylan-asos Thanks a lot for adding this, I've been waiting for this to long, great work! 🥳 🎉

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement preview sdk/requires-newer-api-version This requires upgrading the version of the API being used service/application-insights upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR
Projects
None yet
Development

No branches or pull requests