Conversation
... to distinguish between "regular" and "tunnel" services
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14557Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14557" |
There was a problem hiding this comment.
Pull request overview
Enables Aspire’s container tunnel by default to provide consistent container-to-host connectivity across orchestrators, while updating OTLP/dashboard endpoint modeling and refactoring DCP startup work for better parallelism and correctness.
Changes:
- Default-enable the container tunnel and refactor DCP object creation into finer-grained parallel tasks.
- Add a multi-resource dependency computation API and use it to determine tunnel-dependent resources.
- Update OTLP/dashboard endpoint naming and add/adjust tests (including a helper to reduce port-collision flakiness).
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/perf/Measure-StartupPerformance.ps1 | Updates perf script launch profile handling and output behavior. |
| tests/Aspire.Hosting.Tests/ResourceDependencyTests.cs | Adds coverage for new multi-resource dependency resolution behavior. |
| tests/Aspire.Hosting.Tests/Helpers/Network.cs | Adds helper for selecting an available TCP port to reduce test flakiness. |
| tests/Aspire.Hosting.Tests/DistributedApplicationTests.cs | Uses dynamic ports in proxyless endpoint/container tests. |
| tests/Aspire.Hosting.Tests/Dashboard/DashboardResourceTests.cs | Switches OTLP endpoint name usage to centralized constants. |
| tests/Aspire.Hosting.Tests/Dashboard/DashboardLifecycleHookTests.cs | Switches OTLP endpoint name usage to centralized constants. |
| tests/Aspire.Hosting.Tests/ContainerTunnelTests.cs | Adds tunnel coverage for proxyless endpoints and adjusts YARP test behavior/timeouts. |
| tests/Aspire.Hosting.Tests/Aspire.Hosting.Tests.csproj | Adds Polly.Core dependency for new test helper. |
| src/Shared/OtlpEndpointResolver.cs | Extends OTLP resolution logic and adds helper to resolve scheme/port. |
| src/Shared/KnownEndpointNames.cs | Introduces shared OTLP endpoint-name constants. |
| src/Aspire.Hosting/OtlpConfigurationExtensions.cs | Centralizes OTLP env var key and uses resolver directly for endpoint/protocol. |
| src/Aspire.Hosting/Dcp/OtlpEndpointReferenceGatherer.cs | Adds DCP gatherer to replace OTLP endpoint env var with dashboard endpoint reference for containers. |
| src/Aspire.Hosting/Dcp/DcpOptions.cs | Enables container tunnel by default. |
| src/Aspire.Hosting/Dcp/DcpExecutor.cs | Refactors startup into interdependent tasks; adds tunnel-dependent container classification and OTLP gatherer hookup. |
| src/Aspire.Hosting/Dashboard/DashboardEventHandlers.cs | Uses shared OTLP endpoint-name constants and ensures endpoints are annotated. |
| src/Aspire.Hosting/AspireEventSource.cs | Adds events for service-object preparation timing. |
| src/Aspire.Hosting/Aspire.Hosting.csproj | Links new shared KnownEndpointNames into hosting assembly. |
| src/Aspire.Hosting/ApplicationModel/ResourceExtensions.cs | Adds efficient multi-resource dependency computation API and exposes direct-dependency gatherer internally. |
| src/Aspire.Hosting.Maui/Aspire.Hosting.Maui.csproj | Links shared KnownResourceNames/KnownEndpointNames into MAUI package. |
| playground/yarp/Yarp.AppHost/Properties/launchSettings.json | Removes explicit tunnel enablement since it’s now default. |
Comments suppressed due to low confidence (3)
src/Shared/KnownEndpointNames.cs:9
- These endpoint name fields are mutable static strings. They should be
const(or at leaststatic readonly) to prevent accidental mutation and to satisfy analyzers that flag non-constant static fields.
internal static class KnownEndpointNames
{
public static string OtlpGrpcEndpointName = "otlp-grpc";
public static string OtlpHttpEndpointName = "otlp-http";
src/Aspire.Hosting/Dcp/DcpExecutor.cs:3012
- Typo in exception message: 'follwing' should be 'following'.
var containerNames = persistentTunnelDependent.Select(td => td.ModelResource.Name).Aggregate(string.Empty, (acc, next) => acc + " '" + next + "'");
throw new InvalidOperationException($"The follwing containers are marked as persistent and rely on resources on the host network:{containerNames}. This is not supported.");
}
tests/Aspire.Hosting.Tests/ResourceDependencyTests.cs:779
- There is a stray extra semicolon here (
...WithHttpEndpoint(...); ;) which results in an empty statement. Please remove it to keep the test code clean.
// A -> B -> C -> A (circular), plus D as external dependency
var d = builder.AddContainer("d", "alpine")
.WithHttpEndpoint(8083, 8083, "http"); ;
var a = builder.AddContainer("a", "alpine")
| using System.Diagnostics; | ||
| using Aspire.Hosting.ApplicationModel; | ||
| using Aspire.Hosting.Dashboard; | ||
| using Microsoft.AspNetCore.Builder; | ||
| using Microsoft.Extensions.Configuration; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using ModelContextProtocol.Protocol; |
There was a problem hiding this comment.
This file has multiple unused using directives (e.g., System.Diagnostics, Aspire.Hosting.ApplicationModel, Aspire.Hosting.Dashboard, Microsoft.AspNetCore.Builder, Microsoft.Extensions.DependencyInjection, ModelContextProtocol.Protocol). With TreatWarningsAsErrors=true, this will fail the build; please remove the unused imports.
| using System.Diagnostics; | |
| using Aspire.Hosting.ApplicationModel; | |
| using Aspire.Hosting.Dashboard; | |
| using Microsoft.AspNetCore.Builder; | |
| using Microsoft.Extensions.Configuration; | |
| using Microsoft.Extensions.DependencyInjection; | |
| using ModelContextProtocol.Protocol; | |
| using Microsoft.Extensions.Configuration; |
| if (!resource.IsContainer() || !resource.TryGetAnnotationsOfType<OtlpExporterAnnotation>(out _)) | ||
| { | ||
| // This gatherer is only relevant for container resources that emit OTEL telemetry. | ||
| return; | ||
| } | ||
|
|
||
| if (!context.EnvironmentVariables.TryGetValue(OtlpConfigurationExtensions.OtlpEndpointEnvironmentVariableName, out _)) | ||
| { | ||
| // If the OTLP endpoint is not set, do not try to set it. | ||
| return; | ||
| } | ||
|
|
||
| var model = executionContext.ServiceProvider.GetRequiredService<DistributedApplicationModel>(); | ||
| var dashboardResource = model.Resources.SingleOrDefault(r => StringComparers.ResourceName.Equals(r.Name, KnownResourceNames.AspireDashboard)) as IResourceWithEndpoints; | ||
| if (dashboardResource == null) | ||
| { | ||
| // Most test runs do not include the dashboard, and that's ok. If the dashboard is not present, do not try to set the OTLP endpoint. | ||
| return; | ||
| } | ||
|
|
||
| if (!dashboardResource.TryGetEndpoints(out var dashboardEndpoints)) | ||
| { | ||
| Debug.Fail("Dashboard does not have any endpoints??"); | ||
| return; | ||
| } | ||
|
|
||
| var grpcEndpoint = dashboardEndpoints.FirstOrDefault(e => e.Name == KnownEndpointNames.OtlpGrpcEndpointName); | ||
| var httpEndpoint = dashboardEndpoints.FirstOrDefault(e => e.Name == KnownEndpointNames.OtlpHttpEndpointName); | ||
| var resourceNetwork = resource.GetDefaultResourceNetwork(); | ||
|
|
||
| var endpointReference = (grpcEndpoint, httpEndpoint) switch | ||
| { | ||
| (not null, _) => new EndpointReference(dashboardResource, grpcEndpoint, resourceNetwork), | ||
| (_, not null) => new EndpointReference(dashboardResource, httpEndpoint, resourceNetwork), | ||
| _ => null |
There was a problem hiding this comment.
The endpoint selection here always prefers gRPC when present, but OtlpExporterAnnotation.RequiredProtocol (and the existing OTEL_EXPORTER_OTLP_PROTOCOL value set by OtlpConfigurationExtensions) may require HTTP. This can result in an endpoint/protocol mismatch after this gatherer overwrites only OTEL_EXPORTER_OTLP_ENDPOINT. Please select the dashboard endpoint based on the required protocol (or the protocol env var) and keep endpoint/protocol consistent.
| var createTunnel = Task.Run(async () => | ||
| { | ||
| if (allocatedEndpointsAdvertised.Add(resource.ModelResource.Name)) | ||
| if (!tunnelDependent.Any()) | ||
| { | ||
| await _distributedApplicationEventing.PublishAsync( | ||
| new ResourceEndpointsAllocatedEvent(resource.ModelResource, _executionContext.ServiceProvider), | ||
| EventDispatchBehavior.NonBlockingConcurrent, | ||
| cancellationToken | ||
| ).ConfigureAwait(false); | ||
| return; // No tunnel-dependent containers, nothing to do. | ||
| } | ||
| } | ||
|
|
||
| await Task.WhenAll([getProxyAddresses, createContainerNetworks]).ConfigureAwait(false); | ||
|
|
||
| await CreateAllDcpObjectsAsync<ContainerNetworkTunnelProxy>(ct).ConfigureAwait(false); | ||
| await EnsureContainerServiceAddressInfo(ct).ConfigureAwait(false); | ||
|
|
||
| AddAllocatedEndpointInfo(executables, AllocatedEndpointsMode.ContainerTunnel); | ||
| }, ct); | ||
|
|
||
| var createTunnelDependentContainers = Task.Run(async () => | ||
| { | ||
| if (!tunnelDependent.Any()) | ||
| { | ||
| return; // No tunnel-dependent containers, nothing to do. | ||
| } | ||
|
|
||
| await Task.WhenAll([getProxyAddresses, createContainerNetworks, createExecutableEndpoints]).ConfigureAwait(false); | ||
|
|
||
| AddAllocatedEndpointInfo(tunnelDependent, AllocatedEndpointsMode.Workload); | ||
| await PublishEndpointAllocatedEventAsync(endpointsAdvertised, tunnelDependent, ct).ConfigureAwait(false); | ||
|
|
||
| await Task.WhenAll( | ||
| CreateContainersAsync(tunnelDependent, ct), | ||
| CreateContainerExecutablesAsync(tunnelDependentContainerExes, ct) | ||
| ).WaitAsync(ct).ConfigureAwait(false); |
There was a problem hiding this comment.
createTunnelDependentContainers doesn't await createTunnel. Since createTunnel is what creates the tunnel proxy and calls EnsureContainerServiceAddressInfo/adds tunnel-specific allocated endpoints, tunnel-dependent containers can race and start being created before the tunnel is ready. Consider making createTunnelDependentContainers (and endpoint publication for affected host resources) depend on createTunnel completion to avoid nondeterministic startup failures.
This PR enables Aspire container tunnel feature by default. This allows containers to consume resources on the host network. Without the tunnel (current default) it only works with Docker Desktop, because other container orchestrators lack the necessary bridging functionality.
To make it happen I made several changes and bug fixes to app model APIs,
DcpExecutor, and tests. Most importantly:ASPIRE_ENABLE_CONTAINER_TUNNEL#12998DistributedApplicationTeststo use it--it should help us get those out of quarantined status.CAVEAT: with this change, containers that use host resources will commence their startup after a delay (measured on my devbox VM as roughly 3-4 seconds in "warm" case). An example of such container is our Yarp integration, which pushes OTEL telemetry to the dashboard. This is because some of the endpoints they use are now provided by the tunnel, specifically the tunnel proxy container, that needs to start up and allocate these endpoints. Previously these contaienrs were leveraging Docker Desktop host connectivity, which required only textual replacement of the endpoint address
localhost-->host.docker.internal. This drawback has to be weighted against the fact that the tunnel provides uniform behavior regardless of the container orchestrator and that we need to get more feedback about it from the users, which we won't get if the tunnel remains off by default.Making the tunnel as fast as Docker Desktop-based solution is possible, but the plan for that requires making the tunnel itself persistent, which in turn requires persistent Executables, which is tracked here: microsoft/dcp#13