Skip to content

Commit 11f941e

Browse files
afscromeafscrome
authored
Fix Otel Collector on Windows (#853)
* Fix Otel Collector dev cert path calculation to work when host is windows. * Use `HostUrl` to contextually resolve the aspire dashboard url within a container. * 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 * Update `ContainerHasAspireEnvironmentVariables` to evaluate final env vars.
1 parent b6c7480 commit 11f941e

File tree

2 files changed

+42
-104
lines changed

2 files changed

+42
-104
lines changed

‎src/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector/OpenTelemetryCollectorExtensions.cs‎

Lines changed: 20 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,10 @@ public static IResourceBuilder<OpenTelemetryCollectorResource> AddOpenTelemetryC
3636

3737
var isHttpsEnabled = !settings.ForceNonSecureReceiver && url.StartsWith("https", StringComparison.OrdinalIgnoreCase);
3838

39-
var dashboardOtlpEndpoint = ReplaceLocalhostWithContainerHost(url, builder.Configuration);
40-
4139
var resource = new OpenTelemetryCollectorResource(name);
4240
var resourceBuilder = builder.AddResource(resource)
4341
.WithImage(settings.CollectorImage, settings.CollectorTag)
44-
.WithEnvironment("ASPIRE_ENDPOINT", dashboardOtlpEndpoint)
42+
.WithEnvironment("ASPIRE_ENDPOINT", new HostUrl(url))
4543
.WithEnvironment("ASPIRE_API_KEY", builder.Configuration[DashboardOtlpApiKeyVariableName]);
4644

4745
if (settings.EnableGrpcEndpoint)
@@ -53,8 +51,11 @@ public static IResourceBuilder<OpenTelemetryCollectorResource> AddOpenTelemetryC
5351
if (!settings.ForceNonSecureReceiver && isHttpsEnabled && builder.ExecutionContext.IsRunMode && builder.Environment.IsDevelopment())
5452
{
5553
resourceBuilder.RunWithHttpsDevCertificate();
56-
var certFilePath = Path.Combine(DevCertHostingExtensions.DEV_CERT_BIND_MOUNT_DEST_DIR, DevCertHostingExtensions.CERT_FILE_NAME);
57-
var certKeyPath = Path.Combine(DevCertHostingExtensions.DEV_CERT_BIND_MOUNT_DEST_DIR, DevCertHostingExtensions.CERT_KEY_FILE_NAME);
54+
55+
// Not using `Path.Combine` as we MUST use unix style paths in the container
56+
var certFilePath = $"{DevCertHostingExtensions.DEV_CERT_BIND_MOUNT_DEST_DIR}/{DevCertHostingExtensions.CERT_FILE_NAME}";
57+
var certKeyPath = $"{DevCertHostingExtensions.DEV_CERT_BIND_MOUNT_DEST_DIR}/{DevCertHostingExtensions.CERT_KEY_FILE_NAME}";
58+
5859
if (settings.EnableHttpEndpoint)
5960
{
6061
resourceBuilder.WithArgs(
@@ -78,19 +79,23 @@ public static IResourceBuilder<OpenTelemetryCollectorResource> AddOpenTelemetryC
7879
/// <returns></returns>
7980
public static IResourceBuilder<OpenTelemetryCollectorResource> WithAppForwarding(this IResourceBuilder<OpenTelemetryCollectorResource> builder)
8081
{
81-
builder.AddEnvironmentVariablesEventHook()
82-
.WithFirstStartup();
82+
builder.ApplicationBuilder.Eventing.Subscribe<BeforeStartEvent>((evt, ct) =>
83+
{
84+
var logger = evt.Services.GetRequiredService<ResourceLoggerService>().GetLogger(builder.Resource);
85+
var otelSenders = evt.Model.Resources
86+
.OfType<IResourceWithEnvironment>()
87+
.Where(x => x.HasAnnotationOfType<OtlpExporterAnnotation>());
8388

84-
return builder;
85-
}
89+
foreach (var otelSender in otelSenders)
90+
{
91+
var otelSenderBuilder = builder.ApplicationBuilder.CreateResourceBuilder(otelSender);
92+
otelSenderBuilder.WithOpenTelemetryCollectorRouting(builder);
93+
}
8694

87-
private static string ReplaceLocalhostWithContainerHost(string value, IConfiguration configuration)
88-
{
89-
var hostName = configuration["AppHost:ContainerHostname"] ?? "host.docker.internal";
95+
return Task.CompletedTask;
96+
});
9097

91-
return value.Replace("localhost", hostName, StringComparison.OrdinalIgnoreCase)
92-
.Replace("127.0.0.1", hostName)
93-
.Replace("[::1]", hostName);
98+
return builder;
9499
}
95100

96101
/// <summary>
@@ -106,73 +111,4 @@ public static IResourceBuilder<OpenTelemetryCollectorResource> WithConfig(this I
106111
.WithArgs($"--config=/config/{configFileInfo.Name}");
107112
}
108113

109-
/// <summary>
110-
/// Sets up the OnBeforeResourceStarted event to add a wait annotation to all resources that have the OtlpExporterAnnotation
111-
/// </summary>
112-
/// <param name="builder"></param>
113-
/// <returns></returns>
114-
private static IResourceBuilder<OpenTelemetryCollectorResource> WithFirstStartup(this IResourceBuilder<OpenTelemetryCollectorResource> builder)
115-
{
116-
builder.OnBeforeResourceStarted((collectorResource, beforeStartedEvent, cancellationToken) =>
117-
{
118-
var logger = beforeStartedEvent.Services.GetRequiredService<ResourceLoggerService>().GetLogger(collectorResource);
119-
var appModel = beforeStartedEvent.Services.GetRequiredService<DistributedApplicationModel>();
120-
var resources = appModel.GetProjectResources();
121-
122-
foreach (var resourceItem in resources.Where(r => r.HasAnnotationOfType<OtlpExporterAnnotation>()))
123-
{
124-
resourceItem.Annotations.Add(new WaitAnnotation(collectorResource, WaitType.WaitUntilHealthy));
125-
}
126-
return Task.CompletedTask;
127-
});
128-
return builder;
129-
}
130-
131-
/// <summary>
132-
/// Sets up the OnResourceEndpointsAllocated event to add/update the OTLP environment variables for the collector to the various resources
133-
/// </summary>
134-
/// <param name="builder"></param>
135-
private static IResourceBuilder<OpenTelemetryCollectorResource> AddEnvironmentVariablesEventHook(this IResourceBuilder<OpenTelemetryCollectorResource> builder)
136-
{
137-
builder.OnResourceEndpointsAllocated((collectorResource, allocatedEvent, cancellationToken) =>
138-
{
139-
var logger = allocatedEvent.Services.GetRequiredService<ResourceLoggerService>().GetLogger(collectorResource);
140-
var appModel = allocatedEvent.Services.GetRequiredService<DistributedApplicationModel>();
141-
var resources = appModel.GetProjectResources();
142-
143-
var grpcEndpoint = collectorResource.GetEndpoint(collectorResource.GrpcEndpoint.EndpointName);
144-
var httpEndpoint = collectorResource.GetEndpoint(collectorResource.HttpEndpoint.EndpointName);
145-
146-
if (!resources.Any())
147-
{
148-
logger.LogInformation("No resources to add Environment Variables to");
149-
}
150-
151-
foreach (var resourceItem in resources.Where(r => r.HasAnnotationOfType<OtlpExporterAnnotation>()))
152-
{
153-
logger.LogDebug("Forwarding Telemetry for {name} to the collector", resourceItem.Name);
154-
if (resourceItem is null) continue;
155-
156-
resourceItem.Annotations.Add(new EnvironmentCallbackAnnotation(context =>
157-
{
158-
var protocol = context.EnvironmentVariables.GetValueOrDefault("OTEL_EXPORTER_OTLP_PROTOCOL", "");
159-
var endpoint = protocol.ToString() == "http/protobuf" ? httpEndpoint : grpcEndpoint;
160-
161-
if (endpoint is null)
162-
{
163-
logger.LogWarning("No {protocol} endpoint on the collector for {resourceName} to use",
164-
protocol, resourceItem.Name);
165-
return;
166-
}
167-
168-
context.EnvironmentVariables.Remove("OTEL_EXPORTER_OTLP_ENDPOINT");
169-
context.EnvironmentVariables.Add("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.Url);
170-
}));
171-
}
172-
173-
return Task.CompletedTask;
174-
});
175-
176-
return builder;
177-
}
178114
}

‎tests/CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector.Tests/ResourceCreationTests.cs‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
1+
using Aspire.Components.Common.Tests;
12
using Aspire.Hosting;
23
using Aspire.Hosting.Utils;
3-
using Microsoft.Extensions.FileProviders;
4-
using Microsoft.Extensions.Hosting;
4+
using Xunit.Abstractions;
55

66
namespace CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector.Tests;
77

8-
public class ResourceCreationTests
8+
public class ResourceCreationTests(ITestOutputHelper testOutputHelper)
99
{
1010
[Fact]
1111
public void CanCreateTheCollectorResource()
1212
{
13-
var builder = DistributedApplication.CreateBuilder();
13+
var builder = TestDistributedApplicationBuilder.Create();
1414

1515
builder.AddOpenTelemetryCollector("collector")
1616
.WithConfig("./config.yaml")
@@ -150,31 +150,33 @@ public void CanDisableBothEndpoints()
150150
}
151151

152152
[Fact]
153-
public void ContainerHasAspireEnvironmentVariables()
153+
[RequiresDocker]
154+
public async Task ContainerHasAspireEnvironmentVariables()
154155
{
155-
var builder = DistributedApplication.CreateBuilder();
156+
using var builder = TestDistributedApplicationBuilder.Create()
157+
.WithTestAndResourceLogging(testOutputHelper);
158+
builder.Configuration["APPHOST:ContainerHostname"] = "what.ever";
156159

157-
builder.AddOpenTelemetryCollector("collector")
160+
var collector = builder.AddOpenTelemetryCollector("collector")
158161
.WithAppForwarding();
159162

160163
using var app = builder.Build();
161164
var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();
162-
var collectorResource = appModel.Resources.OfType<OpenTelemetryCollectorResource>().SingleOrDefault();
163-
Assert.NotNull(collectorResource);
164165

165-
var envs = collectorResource.Annotations.OfType<EnvironmentCallbackAnnotation>().ToList();
166-
Assert.NotEmpty(envs);
166+
var resourceNotificationService = app.Services.GetRequiredService<ResourceNotificationService>();
167167

168-
var context = new EnvironmentCallbackContext(new DistributedApplicationExecutionContext(new DistributedApplicationExecutionContextOptions(DistributedApplicationOperation.Run)));
169-
foreach (var env in envs)
170-
{
171-
env.Callback(context);
172-
}
168+
await app.StartAsync();
169+
await resourceNotificationService.WaitForResourceHealthyAsync(collector.Resource.Name);
170+
171+
Assert.True(resourceNotificationService.TryGetCurrentState(collector.Resource.Name, out var resourceEvent));
172+
173+
var envVars = resourceEvent.Snapshot.EnvironmentVariables.ToDictionary(k => k.Name, v => v.Value);
174+
175+
var endpoint = Assert.Contains("ASPIRE_ENDPOINT", envVars);
176+
var apiKey = Assert.Contains("ASPIRE_API_KEY", envVars);
173177

174-
Assert.Contains("ASPIRE_ENDPOINT", context.EnvironmentVariables.Keys);
175-
Assert.Contains("ASPIRE_API_KEY", context.EnvironmentVariables.Keys);
176-
Assert.Equal("http://host.docker.internal:18889", context.EnvironmentVariables["ASPIRE_ENDPOINT"]);
177-
Assert.NotNull(context.EnvironmentVariables["ASPIRE_API_KEY"]);
178+
Assert.Equal($"http://what.ever:18889", endpoint);
179+
Assert.NotNull(apiKey);
178180
}
179181

180182
[Fact]

0 commit comments

Comments
 (0)