You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(foundry): azd enable app insights in projects by default - #10743
Automatically configure Application Insights for newly created Foundry projects through the normal provisioning flow, with no new command or flag required.
Summary
New projects: monitoring is synthesized into the main infrastructure graph and is on by default. It creates a workspace-based Application Insights component, its Log Analytics workspace, and a project-scoped AppInsights connection (ApiKey, credentials resolved internally). This works for native embedded ARM, editable Bicep, and Terraform. There is no separate imperative monitoring deployment.
Existing (BYO) projects: nothing is created and nothing changes unless the user sets monitoring.applicationInsightsResourceId. That connects an existing App Insights component (including cross-resource-group) to the project, preserving any existing compatible connection. monitoring: {enabled: true} without a resource ID fails with a clear error. Automatic creation of App Insights for existing projects is out of scope.
Opt-out:monitoring.enabled: false always wins, and opting out does not delete existing monitoring.
azure.yaml stays authoritative until ejection. After ejection, the exported Bicep/parameters or Terraform/tfvars are authoritative.
Terraform: uses ephemeral reads and write-only credentials, so the instrumentation key is never persisted in state or generated files. Monitoring-enabled exports require Terraform 1.11+ and AzAPI 2.7+; other exports keep their existing requirements.
Safety: a What-If preflight and a create-graph check protect new-project creation, and a template-export retry handles transient failures. Monitoring failures fail provisioning under normal provisioning semantics.
Shared synthesis module: synthesis now lives in the nested Go module azure.ai.projects/synthesis, shared by the Projects and Agents extensions via require + replace. The Projects ci-test.ps1 also runs its tests.
Validation
Live e2e (westus3) passed on native, Bicep and Terraform: new project with default monitoring, opt-out, existing project with a cross-RG BYO component, and the existing-project enabled: true error. For new projects, prompt and hosted agent invocations produced correlated traces in Application Insights.
Terraform: the instrumentation key is absent from state and generated files.
Unit tests, vet and format pass for the synthesis, Projects and Agents modules. Dependency alignment passes, and both ci-build.ps1 scripts succeed. Synthesis coverage is 84%.
The branch was rebased onto main after the live run (which included the upstream BYO AKS/ACR changes). Offline tests and builds pass, but the live e2e was not re-run on the rebased commit.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Project-target validation and monitoring configuration handling have unresolved defects that can misdirect credentials or create unintended billed resources.
This PR adds optional Application Insights tracing across the Foundry Projects, Agents, and Connections extensions, separating monitoring setup from required agent infrastructure and deployment.
Changes:
Enable monitoring when authoring new agent projects, with configuration, opt-out guidance, and cost notices.
Add isolated workspace-backed Application Insights provisioning and predeploy connection setup.
Add setup-status outputs, tests, documentation, and scaffold assertions.
The predeploy tests replace h.connect with a stub, so none exercises these authentication/network policy checks or verifies that rejected components cause no connection PUT. Add transport-backed tests through connectMonitoring for disabled local authentication, disabled public ingestion, equivalent inherited/Entra connection reuse, and credential-bearing create errors. These are safety guarantees of default tracing, but the current tests would remain green if their enforcement or error sanitization were removed.
Initialize reuse-only provider subscription before credential validation
For a freshly initialized reuse-only provider, p.subID is empty, so ensureCredential fails the host's subscription validation. Consequently, azd env refresh clears valid monitoring identifiers and changes ready to not-configured. Initialize the subscription from projectID before requesting credentials, and add a regression test through the reuse-only initialization and State path.
Preserve project hash when truncating deployment names
For long deployment names, this truncation removes the project-path hash, so distinct projects with the same environment name get the same monitoring deployment name. If they share a resource group, their deployment records collide and concurrent setup can fail. Trim from the beginning instead to preserve the identity-bearing suffix within the 64-character limit.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
An unset subscription causes fresh-provider refreshes to clear valid monitoring identifiers and block connection setup, while authentication and network-policy guards lack transport-backed regression coverage.
No open issues 🎉
2 previously missed issues
In code that hasn't changed since last review
Add transport-backed tests for monitoring connection safety guards
The current predeploy tests replace connect with a stub, so they never exercise these authentication/network-policy guards in connectMonitoring. Add transport-backed cases for DisableLocalAuth=true and disabled public ingestion that assert no connection PUT is sent. These safety checks could currently be removed or misread from the ARM response without failing any regression test.
Initialize subscription before credential lookup for reused projects
For a reuse-only existing project, a fresh provider reaches this call with p.subID empty, so LookupTenant rejects the credential lookup. As a result, azd env refresh clears valid monitoring identifiers and saves not-configured, blocking subsequent connection setup until provisioning runs again. Initialize the subscription from projectID before requesting credentials, as provisionMonitoring does, and add a fresh-provider State regression test.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Reported live validation predates the rebase, so a maintainer must verify final-commit provisioning, credential non-persistence, and teardown for default-on billable infrastructure across ARM, Bicep, and Terraform.
Existing-project creation is no longer supported, so the opt-in test now
renders the new-project graph and maps its resource references to the test locals.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c1d0003f-40c8-481d-92a5-ecb8d7afb491
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
A maintainer should validate billable-resource ownership, create/retry/cleanup behavior, and Terraform credential-state safety on the rebased commit because the reported live validation predates it.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed for a critical compatibility regression that causes existing private-network projects to fail provisioning even without monitoring opt-in.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for responding to the architectural feedback. The design is much cleaner and better aligned with ACR now! I still have a few questions about the implementation. Could you help clarify?
Projects with a network: block that omit monitoring: no longer fail
synthesis; explicitly enabling monitoring there is still rejected.
Fix stale existing-project wording in the README.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c1d0003f-40c8-481d-92a5-ecb8d7afb491
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Live Azure validation predates the rebase, so a maintainer must confirm ownership, connection preservation, and credential safety on the final ARM, Bicep, and Terraform deployment paths.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because an unpublished critical finding shows that project-ID tags can exceed Azure’s length limit and cause default-on provisioning to fail for valid inputs.
Port upstream BYO AKS supporting-resource changes to the relocated
azure.ai.projects/synthesis module and adapt new tests to monitoring.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c1d0003f-40c8-481d-92a5-ecb8d7afb491
Resolve conflicts with phase telemetry: keep the top-level synthesis package and add phasetelemetry to project_deployment.go and the provisioning provider.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c1d0003f-40c8-481d-92a5-ecb8d7afb491
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The project ID used in monitoring tags can exceed Azure’s 256-character limit, causing otherwise valid projects to fail provisioning with default monitoring enabled.
The raw project resource ID can exceed Azure's 256-character tag value limit and fail provisioning. Tag the workspace and component with a UUIDv5 of the lowercased project ID, using the same namespace in Bicep and Terraform.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c1d0003f-40c8-481d-92a5-ecb8d7afb491
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Live ARM, Bicep, and Terraform checks predate the BYO AKS/ACR rebase, so credential non-persistence and resource-ownership behavior need maintainer validation on the rebased commit.
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
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.
microsoft-github-policy-service
Bot
added
the
customer-reported
Kayla Seager (kseager)
added
ext-agents
Zhijie Huang (hund030)
left a comment
Richard Park (richardpark-msft)
left a comment
Motivation
Automatically configure Application Insights for newly created Foundry projects through the normal provisioning flow, with no new command or flag required.
Summary
AppInsightsconnection (ApiKey, credentials resolved internally). This works for native embedded ARM, editable Bicep, and Terraform. There is no separate imperative monitoring deployment.monitoring.applicationInsightsResourceId. That connects an existing App Insights component (including cross-resource-group) to the project, preserving any existing compatible connection.monitoring: {enabled: true}without a resource ID fails with a clear error. Automatic creation of App Insights for existing projects is out of scope.monitoring.enabled: falsealways wins, and opting out does not delete existing monitoring.azure.yamlstays authoritative until ejection. After ejection, the exported Bicep/parameters or Terraform/tfvars are authoritative.azure.ai.projects/synthesis, shared by the Projects and Agents extensions viarequire+replace. The Projectsci-test.ps1also runs its tests.Validation
enabled: trueerror. For new projects, prompt and hosted agent invocations produced correlated traces in Application Insights.ci-build.ps1scripts succeed. Synthesis coverage is 84%.Extra
Fixes #9355