From 8913c7e2148480398409e3f3a4f0762695dbf6ca Mon Sep 17 00:00:00 2001 From: afscrome Date: Sun, 14 Sep 2025 10:18:44 +0100 Subject: [PATCH 1/4] Fix Otel Collector dev cert path calculation to work when host is windows. --- .../OpenTelemetryCollectorExtensions.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs b/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs index ecdcdd62e..93dd95d4b 100644 --- a/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs +++ b/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs @@ -53,8 +53,11 @@ public static IResourceBuilder AddOpenTelemetryC if (!settings.ForceNonSecureReceiver && isHttpsEnabled && builder.ExecutionContext.IsRunMode && builder.Environment.IsDevelopment()) { resourceBuilder.RunWithHttpsDevCertificate(); - var certFilePath = Path.Combine(DevCertHostingExtensions.DEV_CERT_BIND_MOUNT_DEST_DIR, DevCertHostingExtensions.CERT_FILE_NAME); - var certKeyPath = Path.Combine(DevCertHostingExtensions.DEV_CERT_BIND_MOUNT_DEST_DIR, DevCertHostingExtensions.CERT_KEY_FILE_NAME); + + // Not using `Path.Combine` as we MUST use unix style paths in the container + var certFilePath = $"{DevCertHostingExtensions.DEV_CERT_BIND_MOUNT_DEST_DIR}/{DevCertHostingExtensions.CERT_FILE_NAME}"; + var certKeyPath = $"{DevCertHostingExtensions.DEV_CERT_BIND_MOUNT_DEST_DIR}/{DevCertHostingExtensions.CERT_KEY_FILE_NAME}"; + if (settings.EnableHttpEndpoint) { resourceBuilder.WithArgs( From 925e24f8980ff9d742e741135d0a6cc0e944cb9f Mon Sep 17 00:00:00 2001 From: afscrome Date: Sun, 14 Sep 2025 10:21:35 +0100 Subject: [PATCH 2/4] Use `HostUrl` to contextually resolve the aspire dashboard url within a container. --- .../OpenTelemetryCollectorExtensions.cs | 13 +------------ .../ResourceCreationTests.cs | 4 +++- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs b/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs index 93dd95d4b..a4ad606dd 100644 --- a/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs +++ b/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs @@ -36,12 +36,10 @@ public static IResourceBuilder AddOpenTelemetryC var isHttpsEnabled = !settings.ForceNonSecureReceiver && url.StartsWith("https", StringComparison.OrdinalIgnoreCase); - var dashboardOtlpEndpoint = ReplaceLocalhostWithContainerHost(url, builder.Configuration); - var resource = new OpenTelemetryCollectorResource(name); var resourceBuilder = builder.AddResource(resource) .WithImage(settings.CollectorImage, settings.CollectorTag) - .WithEnvironment("ASPIRE_ENDPOINT", dashboardOtlpEndpoint) + .WithEnvironment("ASPIRE_ENDPOINT", new HostUrl(url)) .WithEnvironment("ASPIRE_API_KEY", builder.Configuration[DashboardOtlpApiKeyVariableName]); if (settings.EnableGrpcEndpoint) @@ -87,15 +85,6 @@ public static IResourceBuilder WithAppForwarding return builder; } - private static string ReplaceLocalhostWithContainerHost(string value, IConfiguration configuration) - { - var hostName = configuration["AppHost:ContainerHostname"] ?? "host.docker.internal"; - - return value.Replace("localhost", hostName, StringComparison.OrdinalIgnoreCase) - .Replace("127.0.0.1", hostName) - .Replace("[::1]", hostName); - } - /// /// Adds a config file to the collector /// diff --git a/tests/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector.Tests/ResourceCreationTests.cs b/tests/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector.Tests/ResourceCreationTests.cs index ee24c94ca..ed447a215 100644 --- a/tests/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector.Tests/ResourceCreationTests.cs +++ b/tests/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector.Tests/ResourceCreationTests.cs @@ -173,7 +173,9 @@ public void ContainerHasAspireEnvironmentVariables() Assert.Contains("ASPIRE_ENDPOINT", context.EnvironmentVariables.Keys); Assert.Contains("ASPIRE_API_KEY", context.EnvironmentVariables.Keys); - Assert.Equal("http://host.docker.internal:18889", context.EnvironmentVariables["ASPIRE_ENDPOINT"]); + + var url = Assert.IsType(context.EnvironmentVariables["ASPIRE_ENDPOINT"]); + Assert.Equal("http://localhost:18889", url.Url); Assert.NotNull(context.EnvironmentVariables["ASPIRE_API_KEY"]); } From 5eac39d750371de90fe9eb542b23e4985b3af6ac Mon Sep 17 00:00:00 2001 From: afscrome Date: Sun, 14 Sep 2025 10:53:38 +0100 Subject: [PATCH 3/4] Refactor `WithAppForwarding` 1. Rewrite all resources with the `OtlpExporterAnnotation`, not just project resources 2. Call into `WithOpenTelemetryCollectorRouting` to ensure logic remains consistent between the two approaches of wiring up telemetry --- .../OpenTelemetryCollectorExtensions.cs | 86 ++++--------------- 1 file changed, 15 insertions(+), 71 deletions(-) diff --git a/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs b/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs index a4ad606dd..077a48b0e 100644 --- a/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs +++ b/src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs @@ -79,8 +79,21 @@ public static IResourceBuilder AddOpenTelemetryC /// public static IResourceBuilder WithAppForwarding(this IResourceBuilder builder) { - builder.AddEnvironmentVariablesEventHook() - .WithFirstStartup(); + builder.ApplicationBuilder.Eventing.Subscribe((evt, ct) => + { + var logger = evt.Services.GetRequiredService().GetLogger(builder.Resource); + var otelSenders = evt.Model.Resources + .OfType() + .Where(x => x.HasAnnotationOfType()); + + foreach (var otelSender in otelSenders) + { + var otelSenderBuilder = builder.ApplicationBuilder.CreateResourceBuilder(otelSender); + otelSenderBuilder.WithOpenTelemetryCollectorRouting(builder); + } + + return Task.CompletedTask; + }); return builder; } @@ -98,73 +111,4 @@ public static IResourceBuilder WithConfig(this I .WithArgs($"--config=/config/{configFileInfo.Name}"); } - /// - /// Sets up the OnBeforeResourceStarted event to add a wait annotation to all resources that have the OtlpExporterAnnotation - /// - /// - /// - private static IResourceBuilder WithFirstStartup(this IResourceBuilder builder) - { - builder.OnBeforeResourceStarted((collectorResource, beforeStartedEvent, cancellationToken) => - { - var logger = beforeStartedEvent.Services.GetRequiredService().GetLogger(collectorResource); - var appModel = beforeStartedEvent.Services.GetRequiredService(); - var resources = appModel.GetProjectResources(); - - foreach (var resourceItem in resources.Where(r => r.HasAnnotationOfType())) - { - resourceItem.Annotations.Add(new WaitAnnotation(collectorResource, WaitType.WaitUntilHealthy)); - } - return Task.CompletedTask; - }); - return builder; - } - - /// - /// Sets up the OnResourceEndpointsAllocated event to add/update the OTLP environment variables for the collector to the various resources - /// - /// - private static IResourceBuilder AddEnvironmentVariablesEventHook(this IResourceBuilder builder) - { - builder.OnResourceEndpointsAllocated((collectorResource, allocatedEvent, cancellationToken) => - { - var logger = allocatedEvent.Services.GetRequiredService().GetLogger(collectorResource); - var appModel = allocatedEvent.Services.GetRequiredService(); - var resources = appModel.GetProjectResources(); - - var grpcEndpoint = collectorResource.GetEndpoint(collectorResource.GrpcEndpoint.EndpointName); - var httpEndpoint = collectorResource.GetEndpoint(collectorResource.HttpEndpoint.EndpointName); - - if (!resources.Any()) - { - logger.LogInformation("No resources to add Environment Variables to"); - } - - foreach (var resourceItem in resources.Where(r => r.HasAnnotationOfType())) - { - logger.LogDebug("Forwarding Telemetry for {name} to the collector", resourceItem.Name); - if (resourceItem is null) continue; - - resourceItem.Annotations.Add(new EnvironmentCallbackAnnotation(context => - { - var protocol = context.EnvironmentVariables.GetValueOrDefault("OTEL_EXPORTER_OTLP_PROTOCOL", ""); - var endpoint = protocol.ToString() == "http/protobuf" ? httpEndpoint : grpcEndpoint; - - if (endpoint is null) - { - logger.LogWarning("No {protocol} endpoint on the collector for {resourceName} to use", - protocol, resourceItem.Name); - return; - } - - context.EnvironmentVariables.Remove("OTEL_EXPORTER_OTLP_ENDPOINT"); - context.EnvironmentVariables.Add("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.Url); - })); - } - - return Task.CompletedTask; - }); - - return builder; - } } \ No newline at end of file From f07086bfb9c3c46099d9168f028d80d3e55c81e5 Mon Sep 17 00:00:00 2001 From: afscrome Date: Sun, 14 Sep 2025 18:34:09 +0100 Subject: [PATCH 4/4] Update `ContainerHasAspireEnvironmentVariables` to evaluate final env vars. --- .../ResourceCreationTests.cs | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector.Tests/ResourceCreationTests.cs b/tests/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector.Tests/ResourceCreationTests.cs index ed447a215..06b2d15eb 100644 --- a/tests/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector.Tests/ResourceCreationTests.cs +++ b/tests/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector.Tests/ResourceCreationTests.cs @@ -1,16 +1,16 @@ +using Aspire.Components.Common.Tests; using Aspire.Hosting; using Aspire.Hosting.Utils; -using Microsoft.Extensions.FileProviders; -using Microsoft.Extensions.Hosting; +using Xunit.Abstractions; namespace CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector.Tests; -public class ResourceCreationTests +public class ResourceCreationTests(ITestOutputHelper testOutputHelper) { [Fact] public void CanCreateTheCollectorResource() { - var builder = DistributedApplication.CreateBuilder(); + var builder = TestDistributedApplicationBuilder.Create(); builder.AddOpenTelemetryCollector("collector") .WithConfig("./config.yaml") @@ -150,33 +150,33 @@ public void CanDisableBothEndpoints() } [Fact] - public void ContainerHasAspireEnvironmentVariables() + [RequiresDocker] + public async Task ContainerHasAspireEnvironmentVariables() { - var builder = DistributedApplication.CreateBuilder(); + using var builder = TestDistributedApplicationBuilder.Create() + .WithTestAndResourceLogging(testOutputHelper); + builder.Configuration["APPHOST:ContainerHostname"] = "what.ever"; - builder.AddOpenTelemetryCollector("collector") + var collector = builder.AddOpenTelemetryCollector("collector") .WithAppForwarding(); using var app = builder.Build(); var appModel = app.Services.GetRequiredService(); - var collectorResource = appModel.Resources.OfType().SingleOrDefault(); - Assert.NotNull(collectorResource); - var envs = collectorResource.Annotations.OfType().ToList(); - Assert.NotEmpty(envs); + var resourceNotificationService = app.Services.GetRequiredService(); - var context = new EnvironmentCallbackContext(new DistributedApplicationExecutionContext(new DistributedApplicationExecutionContextOptions(DistributedApplicationOperation.Run))); - foreach (var env in envs) - { - env.Callback(context); - } + await app.StartAsync(); + await resourceNotificationService.WaitForResourceHealthyAsync(collector.Resource.Name); + + Assert.True(resourceNotificationService.TryGetCurrentState(collector.Resource.Name, out var resourceEvent)); + + var envVars = resourceEvent.Snapshot.EnvironmentVariables.ToDictionary(k => k.Name, v => v.Value); - Assert.Contains("ASPIRE_ENDPOINT", context.EnvironmentVariables.Keys); - Assert.Contains("ASPIRE_API_KEY", context.EnvironmentVariables.Keys); + var endpoint = Assert.Contains("ASPIRE_ENDPOINT", envVars); + var apiKey = Assert.Contains("ASPIRE_API_KEY", envVars); - var url = Assert.IsType(context.EnvironmentVariables["ASPIRE_ENDPOINT"]); - Assert.Equal("http://localhost:18889", url.Url); - Assert.NotNull(context.EnvironmentVariables["ASPIRE_API_KEY"]); + Assert.Equal($"http://what.ever:18889", endpoint); + Assert.NotNull(apiKey); } [Fact]