Repository navigation
Link App Insights after its private endpoint and wait for endpoint updates - #5281
Merged
Jay W (JayDoubleu) merged 5 commits intoOct 29, 2026
Merged
Jay W (JayDoubleu) merged 5 commits into
Jay W (JayDoubleu) merged 5 commits into
Conversation
App Insights in core and in each workspace received no telemetry. Public ingestion is disabled, and ingestion through the Azure Monitor private endpoint returned HTTP 403: "Component private link id does not match incoming link and has publicNetworkAccessForIngestion=Disabled". Terraform added App Insights to the scope before it created the private endpoint. A scratch reproduction in switzerlandnorth showed: - scoped resource, then endpoint: 15 of 15 probes returned 403 - endpoint, then scoped resource: 15 of 15 probes returned 200 - deleting and creating the scoped resource after the endpoint: 200 The scoped resource now depends on the private endpoint. The new name makes Terraform replace the link in existing deployments, which repairs them on upgrade. Contract tests check both files. Part of microsoft#5280.
A link or unlink in the workspace Azure Monitor private link scope makes Azure update the scope private endpoint. The activity log shows 13 to 16 seconds for a link. For the two unlinks on delete, Azure ran one update of up to 132 seconds, or one update of 80 to 89 seconds for each unlink, one after the other. During an update, Azure rejects a DNS zone group create or delete with 409 AnotherOperationInProgress. Terraform changed the Log Analytics link at the same time as the endpoint, so some workspace deploys and deletes failed and left the workspace resource group behind. Both scoped services now depend on a terraform_data resource that depends on the endpoint. On create, the scope changes after the endpoint and its zone group exist. On destroy, Terraform removes the links, waits 5 minutes, then deletes the endpoint. In the measured deletes, the endpoint stayed busy for 136 to 177 seconds. The workspace versions stay at the bump of the previous commit. Part of microsoft#3193.
The previous commit made the core App Insights link depend on the private endpoint. The core Log Analytics link had no order, so on a new core it could change the scope while Terraform created the endpoint and its DNS zone group. A scope change makes Azure update the endpoint, and during the update Azure rejects a zone group create with 409 AnotherOperationInProgress. Both core links now depend on the endpoint, as in the workspace. A depends_on change does not replace the link, so an existing core gets no change. The contract test now checks both links in both files. It ignores comment lines, so a depends_on in a comment does not count. It ends a block at the closing brace in the first column, and it follows only names that have a resource block in the same file. The test that the App Insights link has a new name is removed. The comment on the name explains the replacement. Part of microsoft#3193.
test_azure_monitor_contract.py reads the Azure Monitor Terraform of the core and of the base workspace. The E2E helper tests ran only for changes in e2e_tests, porter.yaml files and the workflow, so a change to these Terraform files alone did not run the contract test. The path filter now includes the two azure-monitor folders. The README of the unit tests names the test. Part of microsoft#5280 and microsoft#3193.
A later run measured endpoint updates of 91 and 92 seconds for each unlink, one after the other, and the endpoint stayed busy for up to 203 seconds after the last unlink. The comments now give about 100 seconds for each unlink and up to 3 minutes and 23 seconds in total. The contract test now requires a wait longer than 100 seconds for each unlink. The 5-minute wait does not change. Part of microsoft#3193.
Unit Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 0c9a192. |
Collaborator
Author
|
/test-extended 0c9a192 |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/40749502784 (in response to this comment from Jay W (@JayDoubleu)) |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The upgrade from 2.12.2 has only Terraform-plan validation, so a maintainer needs to verify telemetry restoration and subsequent deletion in Azure after the AMPLS dependency and scoped-service changes.
No open issues 🎉
What changed in this PR
This PR addresses missing Application Insights telemetry (#5280) and workspace deletion conflicts (#3193) by changing Azure Monitor private-link lifecycle ordering in the core and workspace infrastructure.
Changes:
- Create scoped-service links after the private endpoint, and rename Application Insights links to replace them on upgrade.
- Add a five-minute workspace destroy wait between removing links and deleting the endpoint.
- Add ordering and wait-duration contract tests, expand CI triggers, and update component versions and release notes.
| File | Description |
|---|---|
templates/workspaces/unrestricted/porter.yaml |
Bumps the bundle version to 0.15.2. |
templates/workspaces/base/terraform/azure-monitor/azure-monitor.tf |
Reorders links, renames the Application Insights link, and adds the destroy wait. |
templates/workspaces/base/porter.yaml |
Bumps the base bundle version to 2.12.3. |
templates/workspaces/airlock-import-review/porter.yaml |
Bumps the bundle version to 0.18.2. |
e2e_tests/unit_tests/test_azure_monitor_contract.py |
Adds dependency-order and destroy-wait contract tests. |
e2e_tests/unit_tests/README.md |
Documents the new Terraform contract tests. |
core/version.txt |
Bumps the core version to 0.18.16. |
core/terraform/azure-monitor/azure-monitor.tf |
Makes both links depend on the endpoint and renames the Application Insights link. |
CHANGELOG.md |
Records both fixes and affected versions. |
.github/workflows/e2e_helper_tests.yml |
Runs helper tests when either Azure Monitor Terraform directory changes. |
Review effort: Balanced
Jay W (JayDoubleu)
marked this pull request as ready for review
October 28, 2026 18:56
Sven Aelterman (SvenAelterman)
approved these changes
Oct 28, 2026
Sven Aelterman (SvenAelterman)
left a comment
Collaborator
There was a problem hiding this comment.
LGTM
Chris Chapman (ChrisChapman-gh)
approved these changes
Oct 29, 2026
Max Martin (maxmartin-cgi)
approved these changes
Oct 29, 2026
Jay W (JayDoubleu)
added a commit
to JayDoubleu/AzureTRE
that referenced
this pull request
Nov 2, 2026
…soft#5202) An upstream SQL run got HTTP 500 while it polled a workspace delete. The cause is unknown. Two gaps hid it: - The poll diagnostics log the request-id, x-request-id and traceparent headers of a failed response. The API sends none of them, so the test log has no ID to find the server logs. - In those environments, App Insights got no API telemetry, because Terraform linked App Insights to the private link scope before its private endpoint existed. microsoft#5281 fixed that. A small ASGI middleware now adds `Request-Id: |<trace ID>.<span ID>.` to each HTTP response. It wraps the generic error middleware, so a response for an unhandled exception also gets the header. App Insights stores the trace ID as OperationId. The E2E docs give the query. In another fork environment, App Insights has API telemetry since 2026-10-21 21:30 UTC. It shows no HTTP 5xx in about 31,000 API requests, which include many workspace deletes. So the fault did not occur here, and this change does not fix it. It makes a recurrence traceable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #5280, resolves #3193
What is being addressed
The Azure Monitor private link scope (AMPLS) code in the core and in the base workspace has two defects. Both come from the order of the scope links and the private endpoint of the scope.
Component private link id does not match incoming link and has publicNetworkAccessForIngestion=Disabled.AnotherOperationInProgresson the DNS zone group of the endpoint (Workspace deletion fails due to AnotherOperationInProgress pe-ampls-* #3193). A link or an unlink in the scope makes Azure update the endpoint. During the update, Azure rejects a create or a delete of the zone group. Terraform changed the Log Analytics link at the same time as the endpoint.The Activity Log shows 13 to 16 seconds of endpoint update for a link. For the two unlinks on delete, Azure ran one update of up to 132 seconds. In other deletes, Azure ran one update of 80 to 92 seconds for each unlink, one after the other.
How is this addressed
core/terraform/azure-monitor/azure-monitor.tf, both links (App Insights and Log Analytics) depend on the private endpoint.templates/workspaces/base/terraform/azure-monitor/azure-monitor.tf,terraform_data.ampls_endpoint_update_waitdepends on the endpoint, and both links depend on it:ampls-app-insights-servicetoampls-app-insights. Thus the next upgrade replaces the link and repairs an existing deployment.make tre-destroydeletes the core resource groups withaz group delete, not with Terraform.e2e_tests/unit_tests/test_azure_monitor_contract.pymakes sure of the order in both files. It also makes sure that the wait is longer than 100 seconds for each unlink.azure-monitorTerraform folders change, because the contract test reads them.core0.18.16,tre-workspace-base2.12.3,tre-workspace-unrestricted0.15.2 andtre-workspace-airlock-import-review0.18.2.CHANGELOG.mdhas two entries with these versions.The wait adds 5 minutes to each workspace delete.
Relation to other work on issue 3193
azapi_resource. They retry its delete on the 409 error.How this was tested
The live runs ran in another fork. The
/test-extendedrun used this branch, deployed a new environment and passed. The earlier runs used a test branch in the same fork, before the order of the core Log Analytics link was part of the change. Each row says which code ran.test_azure_monitor_contract.pymain. It also fails on 3 edited copies: one with thedepends_onin a comment, one without the order of the core Log Analytics link, and one with a shorter wait. It passes on this branch.terraform fmt,yamllint, Markdown lintterraform validate(core and base workspace)/test-extendedrun/test-extendedrun/test-extendedrun/test-extendedrunmain, which has the defecttre-workspace-base2.12.2