Support for workspace mode in application_insights_resource

[dylanmorley]

As per issue #7667, updating the application_insights_resource to enable support of workspace mode, available with upgrade of the Go SDK at v55.5 via PR #12435.

• Updated the insights API from 2015-05-01 to 2020-02-02-preview
• Fixed upgrade issues (enum changes )
• Supporting workspace ID as an optional parameter when creating AI resources
• Added workspace_id to data source resource
• Using ResourceClient to perform update of retention_in_days, if option is specified - see details in ticket r/application_insights: support for the Workspace model #7667 for reasons.
• Documentation updates

[dylanmorley]

Anyone picking up a review on this - I'd appreciate some eyes on this commit

109f25c

RetentionInDays became a read only property in this version of the insights API, so we needed a way of setting the value after creating the resource

Am using the ResourceClient against hardcoded API Value - better way to do this, or is this acceptable?

[manicminer]

Hi @dylanmorley, thanks for working on this long awaited feature. It is unfortunate that the RetentionInDays field became read-only - despite the clever workaround you have using ResourcesClient, there is no precedent for this elsewhere in the provider (that I'm aware of), so I am hesitant to approve that approach.

However, at first glance everything else looks good if you can take a look at the failed checks? I'll get back to you on the retention workaround.

[manicminer]

@dylanmorley Unfortunately hacking around this with the resources client isn't an approach that we're comfortable with. I understand this feature has been outstanding for a long time but it's up to the API team to maintain compatibility and not break it in this way. Can you raise an issue upstream on https://github.com/Azure/azure-rest-api-specs and link it here? Once that's resolved, this should be in a position to be mergeable. Thanks!

Until this is resolved by the service team, I'm going to close this to help us focus on active unblocked PRs. Once the retention property is fixed and made writable again, we can reopen it with a view to merging.

[dylanmorley]

OK thanks @manicminer - wasn't that comfortable with it either, but wanted an opinion as data retention as an option is going to deprecate when Microsoft make Workspace mode the default and also deprecate 'classic mode'. Is there any general guidance on handling breaking changes / deprecations in the Terraform Azure provider?

It was removed as an option on the ARM API which is why I presume it became read-only here

I'm interested to understand the things that need to change in order to use a standard approach (Azure API / Go SDK?). I'll fix failed checks in the meantime.

[manicminer]

@dylanmorley Another option might be to implement this in a new resource / data source, for example azurerm_application_insights_workspace_mode (just an initial suggestion). The new resource could potentially use the newer API and only support workspace mode, e.g. doesn't have the retention_in_days or other deprecated properties. This may or may not be feasible depending on how much dependency mess happens as a result.

At the same time, the existing (classic) resource azurem_application_insights should probably be deprecated. We should also test whether newly migrated workspaces (in the portal) can be imported to the new resource, and potentially whether importing a classic resource into the new resource will essentially migrate it via Terraform.

WDYT?

[dylanmorley]

@manicminer upstream dependencies fixed & I've updated this branch with the latest - can we reopen this PR pls?

[timja]

@manicminer upstream dependencies fixed & I've updated this branch with the latest - can we reopen this PR pls?

if you can't re-open probably best to just create a new one

[dylanmorley]

Opened #12818

[github-actions]

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