-
Notifications
You must be signed in to change notification settings - Fork 745
Added support for enabling Application Insights for App Service #11998
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
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 11998Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 11998" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for enabling Application Insights in Azure App Service environments and surfaces the resulting App Service URL during deployment.
- Introduces environment properties and extension methods to enable and configure Application Insights (string or parameter-based location).
- Generates Bicep outputs and infrastructure resources (Log Analytics + Application Insights) and adds related app settings.
- Updates deployment utilities to construct and print the App Service URL.
Reviewed Changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| AzureAppServiceTests.AzureAppServiceEnvironmentCanReferenceExistingAppServicePlan.verified.bicep | Adds unique string output used for App Service hostname construction. |
| AzureAppServiceTests.AddContainerAppEnvironmentAddsEnvironmentResource.verified.bicep | Same unique string output added for hostname logic. |
| AzureAppServiceTests.AddAppServiceWithApplicationInsightsLocationParam.verified.json | Adds manifest params including Application Insights location parameter. |
| AzureAppServiceTests.AddAppServiceWithApplicationInsightsLocationParam.verified.bicep | Generates resources (Web App, Log Analytics, App Insights) using parameter-based location. |
| AzureAppServiceTests.AddAppServiceWithApplicationInsightsLocation.verified.json | Adds manifest without explicit App Insights param (fixed location). |
| AzureAppServiceTests.AddAppServiceWithApplicationInsightsLocation.verified.bicep | Generates resources with explicit string location for App Insights. |
| AzureAppServiceTests.AddAppServiceWithApplicationInsightsDefaultLocation.verified.json | Manifest for default (resource group) location scenario. |
| AzureAppServiceTests.AddAppServiceWithApplicationInsightsDefaultLocation.verified.bicep | Generates resources with App Insights in resource group location. |
| AzureAppServiceTests.AddAppServiceEnvironmentWithoutDashboardAddsEnvironmentResource.verified.bicep | Adds unique string output for hostname when dashboard disabled. |
| AzureAppServiceTests.cs | Adds three new tests covering string, default, and parameter-based App Insights location. |
| AzureEnvironmentResource.cs | Adds logic to derive and print App Service endpoint URL using unique string output. |
| AzureAppServiceWebsiteContext.cs | Adds creation of Log Analytics workspace, Application Insights component, and related app settings. |
| AzureAppServiceEnvironmentResource.cs | Adds properties controlling Application Insights enablement and location configuration. |
| AzureAppServiceEnvironmentExtensions.cs | Adds two public extension methods to enable Application Insights (string or parameter). |
| Aspire.Hosting.Azure.AppService.csproj | Adds project reference to Application Insights provisioning assembly. |
src/Aspire.Hosting.Azure.AppService/AzureAppServiceWebsiteContext.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure.AppService/AzureAppServiceEnvironmentResource.cs
Outdated
Show resolved
Hide resolved
| /// <summary> | ||
| /// Configures whether Azure Application Insights should be enabled for the Azure App Service. | ||
| /// </summary> | ||
| /// <param name="builder">The AzureAppServiceEnvironmentResource to configure.</param> | ||
| /// <param name="applicationInsightsLocation">The location for Application Insights.</param> | ||
| /// <returns><see cref="IResourceBuilder{T}"/></returns> | ||
| public static IResourceBuilder<AzureAppServiceEnvironmentResource> WithAzureApplicationInsights(this IResourceBuilder<AzureAppServiceEnvironmentResource> builder, string? applicationInsightsLocation = null) | ||
| { |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These public extension methods lack remarks and usage examples required by the XML documentation standards; add a section explaining default behavior (resource group location when null), precedence between string and parameter overloads, and an showing enabling Application Insights with explicit and parameter-based locations.
| /// <summary> | ||
| /// Configures whether Azure Application Insights should be enabled for the Azure App Service. | ||
| /// </summary> | ||
| /// <param name="builder">The AzureAppServiceEnvironmentResource to configure.</param> | ||
| /// <param name="applicationInsightsLocation">The location parameter for Application Insights.</param> | ||
| /// <returns><see cref="IResourceBuilder{T}"/></returns> | ||
| public static IResourceBuilder<AzureAppServiceEnvironmentResource> WithAzureApplicationInsights(this IResourceBuilder<AzureAppServiceEnvironmentResource> builder, IResourceBuilder<ParameterResource> applicationInsightsLocation) | ||
| { |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These public extension methods lack remarks and usage examples required by the XML documentation standards; add a section explaining default behavior (resource group location when null), precedence between string and parameter overloads, and an showing enabling Application Insights with explicit and parameter-based locations.
|
|
||
| await ExecuteBeforeStartHooksAsync(app, default); | ||
|
|
||
| var model = app.Services.GetRequiredService<DistributedApplicationModel>(); |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable model is retrieved but never used in these three test methods; remove the unused assignments to keep tests concise.
src/Aspire.Hosting.Azure.AppService/AzureAppServiceEnvironmentResource.cs
Outdated
Show resolved
Hide resolved
…Resource.cs Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…Resource.cs Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
src/Aspire.Hosting.Azure.AppService/AzureAppServiceEnvironmentExtensions.cs
Outdated
Show resolved
Hide resolved
|
You need to be able to customize this. |
src/Aspire.Hosting.Azure.AppService/AzureAppServiceComputeResourceExtensions.cs
Outdated
Show resolved
Hide resolved
Added support for that using ConfigureInfrastructure - AppInsights is now provisioned as an infra resource |
src/Aspire.Hosting.Azure.ApplicationInsights/AzureApplicationInsightsReferenceAnnotation.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure.ApplicationInsights/AzureApplicationInsightsReferenceAnnotation.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure.AppService/AzureAppServiceEnvironmentResource.cs
Outdated
Show resolved
Hide resolved
| var hostName = $"{computeResource.Name.ToLowerInvariant()}.{webSiteSuffix}"; | ||
| if (hostName.Length > 60) | ||
| { | ||
| hostName = hostName.Substring(0, 60); | ||
| } | ||
| var endpoint = $"https://{hostName}.azurewebsites.net"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 things here.
- This needs a
-in between, not a.. - In a follow up, we could use this method.
aspire/src/Aspire.Hosting.Azure.AppService/AzureAppServiceEnvironmentResource.cs
Lines 67 to 71 in d9c3266
ReferenceExpression IComputeEnvironmentResource.GetHostAddressExpression(EndpointReference endpointReference) { var resource = endpointReference.Resource; return ReferenceExpression.Create($"{resource.Name.ToLowerInvariant()}-{WebSiteSuffix}.azurewebsites.net"); }
This scenario is exactly what that method is intended for - given an EndpointReference to a compute resource, get its HostAddress for the given environment.
Co-authored-by: Eric Erhardt <[email protected]>
eerhardt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
## Description
The changes in this PR is for the following change:
#### Adding support to enable Application Insights for App Service
- Added properties to AzureAppServiceEnvironmentResource to track if App Insights is enabled (default is false) and to track location for provisioning App Insights.
- This is needed because the available regions for Application Insights resource is very different from the available regions for App Service.
- Added 3 overloads for `WithAzureApplicationInsights`
- `WithAzureApplicationInsights (string? location)`
- `location` is optional, if not specified we provision app insights resource in ResourceGroup location
- `WithAzureApplicationInsights (IResouceBuilder<ParameterResource>)`
- This is to allow customers to choose App Insights location during deployment.
- `WithAzureApplicationInsights (IResouceBuilder<AzureApplicationInsightsResource>)`
- This is to allow customers to choose an existing App Insights resource.
- Also added support WithAzureApplicationInsights for app service
- This allows an app service to disable application insights
* Enabled Application Insights by default
* Build fixes
* Added override for App Insights location
* Printing app service uri
* Added resource parameter for app insights location and tests
* Fixed ParameterResource for app insights location
* Changed method signatures for enabling app insights
* Update src/Aspire.Hosting.Azure.AppService/AzureAppServiceEnvironmentResource.cs
Co-authored-by: Copilot <[email protected]>
* Update tests/Aspire.Hosting.Azure.Tests/AzureAppServiceTests.cs
Co-authored-by: Copilot <[email protected]>
* Apply suggestion from @Copilot
Co-authored-by: Copilot <[email protected]>
* Update src/Aspire.Hosting.Azure.AppService/AzureAppServiceEnvironmentResource.cs
Co-authored-by: Copilot <[email protected]>
* Update tests/Aspire.Hosting.Azure.Tests/AzureAppServiceTests.cs
Co-authored-by: Copilot <[email protected]>
* Reverting the change to print app service urls
* Moved AI resource to environment
* Printing endpoints of compute resources
* Added support to add existing application insights
* Added tests
* Remove support to disable AppInsights at website level
* Switched to use ApplicationInsightsResource directly in AzureAppServiceEnvironment
* nit fix
* Update src/Aspire.Hosting.Azure/AzureEnvironmentResource.cs
Co-authored-by: Eric Erhardt <[email protected]>
---------
Co-authored-by: Shilpi Rachna <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Eric Erhardt <[email protected]>
Description
The changes in this PR is for the following change:
Adding support to enable Application Insights for App Service
Added properties to AzureAppServiceEnvironmentResource to track if App Insights is enabled (default is false) and to track location for provisioning App Insights.
Added 3 overloads for
WithAzureApplicationInsightsWithAzureApplicationInsights (string? location)locationis optional, if not specified we provision app insights resource in ResourceGroup locationWithAzureApplicationInsights (IResouceBuilder<ParameterResource>)WithAzureApplicationInsights (IResouceBuilder<AzureApplicationInsightsResource>)Also added support WithAzureApplicationInsights for app service
Testing
Ran azd up and aspire deploy for the 3 scenarios:
Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplatebreaking-changetemplatediagnostictemplate