fix(aspire): preserve user-supplied OTLP endpoint (#4818)

[thomhurst]

Summary

Fixes #4818 — TUnit.Aspire was silently dropping SUT traces from a user-hosted Aspire dashboard whenever OTEL_EXPORTER_OTLP_ENDPOINT was set via WithEnvironment in ConfigureBuilder.

• AspireFixture.ConfigureOtlpEndpoints now captures any pre-existing user OTEL_EXPORTER_OTLP_ENDPOINT value as the receiver's upstream forwarding target before overriding it with the local TUnit receiver URL. Project resources still export to TUnit (for log/test correlation), and the receiver now forwards everything to the user's dashboard.
• OtlpReceiver.UpstreamEndpoint made settable post-construction (via lock-free Volatile.Read/Write); a single static readonly HttpClient handles forwarding so there's no instance lock on the per-export hot path.
• New OtlpEndpointEnvironment.CaptureAndOverride helper isolates the capture-then-override logic so it's unit-testable without spinning up a full Aspire host.

Why it broke

Environment callbacks resolve at resource startup, not at StartOtlpReceiver time. Before this fix, the receiver only knew about DOTNET_DASHBOARD_OTLP_ENDPOINT_URL (set when Aspire's built-in dashboard is enabled). Users disabling the built-in dashboard (options.DisableDashboard = true) and pointing project resources at their own dashboard had no way to inform the receiver — so spans went into the receiver and never came out the other side.

ContainerResource-typed resources (e.g. WireMock) kept working because TUnit only annotates ProjectResources, leaving container env vars untouched. That asymmetry is what made the regression so confusing in the issue report.

Test plan

• New unit tests for OtlpEndpointEnvironment.CaptureAndOverride (capture / no-key / non-string-value cases) — TUnit.Aspire.Tests/OtlpEndpointEnvironmentTests.cs
• New OtlpReceiver test that sets UpstreamEndpoint post-construction and verifies forwarding still works — TUnit.Aspire.Tests/OtlpReceiverTests.cs
• All existing OtlpReceiverTests still pass (14/14)
• End-to-end verification with redoz's repro repo (https://github.com/redoz/tunit-aspire-wiremock-example) — recommend running before release

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 5 complexity

Metric	Results
Complexity	5

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes. Give us feedback}

[claude]

Code Review: fix(aspire): preserve user-supplied OTLP endpoint (#4818)

This is a well-structured fix for a genuinely tricky timing problem. The root cause diagnosis is correct — environment callbacks resolve at resource startup rather than receiver construction time — and the capture-then-override pattern is the right architectural response. The Volatile.Read/Write usage and the static readonly HttpClient refactor are both sound.

A few things worth discussing before this ships:

---

1. Architectural: CaptureAndOverride couples env-var logic to receiver configuration

public static string? CaptureAndOverride(
    IDictionary<string, object> environmentVariables,
    OtlpReceiver receiver,          // ← receiver as param
    string overrideEndpoint)

The method mixes two concerns: (a) capturing/overriding an env-var entry, and (b) wiring up the receiver's upstream. The PR description justifies the helper as "unit-testable without spinning up a full Aspire host" — but the helper still takes OtlpReceiver directly, which means a test still needs a live OtlpReceiver.

A cleaner split would be to return the captured value and let the caller configure the receiver:

// OtlpEndpointEnvironment — pure env-var manipulation
internal static string? CaptureAndOverride(
    IDictionary<string, object> environmentVariables,
    string overrideEndpoint)
{
    string? userEndpoint = null;
    if (environmentVariables.TryGetValue(OtelExporterEndpointEnvVar, out var existing)
        && existing is string s && !string.IsNullOrWhiteSpace(s))
    {
        userEndpoint = s;
    }
    environmentVariables[OtelExporterEndpointEnvVar] = overrideEndpoint;
    return userEndpoint;
}

// AspireFixture callback
var userEndpoint = OtlpEndpointEnvironment.CaptureAndOverride(
    context.EnvironmentVariables, otlpEndpoint);
if (userEndpoint is not null)
    receiver.UpstreamEndpoint = userEndpoint;

This makes OtlpEndpointEnvironment a true pure-logic helper (testable with just a plain Dictionary), and keeps receiver wiring in the one place that owns the receiver.

---

2. Test coverage gap: non-string case doesn't assert the env-var override still happens

[Test]
public async Task Capture_IgnoresNonStringExistingValue()
{
    // ...
    await Assert.That(captured).IsNull();
    await Assert.That(receiver.UpstreamEndpoint).IsNull();
    // Missing: env["OTEL_EXPORTER_OTLP_ENDPOINT"] is still overridden to overrideEndpoint
}

The code does unconditionally override the env var (the assignment is outside the if), but the test doesn't verify it. Given that "TUnit silently drops the user's non-string endpoint reference" is exactly the scenario that could bite someone, the assertion is worth adding:

await Assert.That(env["OTEL_EXPORTER_OTLP_ENDPOINT"] as string).IsEqualTo("http://127.0.0.1:5000");

Also worth calling out in a comment: EndpointReference/ParameterResource-typed values are forwarded by Aspire at process launch — they resolve to real URLs before the SUT sees them. By replacing them with a static string, TUnit loses the ability to capture that resolved URL. The current behavior (silently not-forwarding from non-string entries) should be documented with a // TODO or a known-limitation comment so a future contributor doesn't wonder why.

---

3. Minor: public method on an internal class is misleading

internal static class OtlpEndpointEnvironment
{
    public static string? CaptureAndOverride(...) // effectively internal

Since the class is internal, public on its members has no effect. This should be internal static for clarity. Same note applies to OtlpReceiver.UpstreamEndpoint (though that one needs to remain accessible from the test project, so is probably internal or uses InternalsVisibleTo already).

---

4. Minor: read-check-write in UpstreamEndpoint setter adds complexity without a real benefit

set
{
    var normalized = value?.TrimEnd('/');
    if (normalized == Volatile.Read(ref _upstreamEndpoint))  // TOCTOU window
    {
        return;
    }
    Volatile.Write(ref _upstreamEndpoint, normalized);
}

The check is a micro-optimisation but introduces a non-atomic read-check-write. In the real scenario (AspireFixture setup, single writer), this is harmless. But a Volatile.Write on a string field is cheap enough that the check isn't justified. Removing it makes the setter simpler and correct by inspection:

set => Volatile.Write(ref _upstreamEndpoint, value?.TrimEnd('/'));

---

What's good

• Capture-then-override is exactly the right pattern for the timing constraint described in the PR.
• Volatile.Read/Write is the correct concurrency primitive for this single-writer, many-reader string field.
• Moving from per-instance HttpClient to static readonly follows .NET best practices and avoids socket exhaustion under load.
• The new tests are well-described and cover the three meaningful cases.
• The ContainerResource asymmetry explanation in the PR description is helpful for future readers.

The end-to-end verification item (redoz's repro repo) is the right call — recommend completing that before release.

[claude]

Follow-up Review: refactor(aspire): address PR review feedback

All four issues raised in the previous review have been resolved in the follow-up commit. Tracking them here for completeness:

#	Issue	Status
1	CaptureAndOverride coupled receiver wiring to env-var logic	✅ Fixed — method is now a pure string?-returning helper; caller owns receiver wiring
2	Non-string test missing assertion that env var is still overridden	✅ Fixed — Capture_IgnoresNonStringExistingValue_ButStillOverrides now asserts env[EnvVar] equals OverrideEndpoint
3	public method on internal class	✅ Fixed — OtlpEndpointEnvironment.CaptureAndOverride is now internal
4	Redundant read-check-write in UpstreamEndpoint setter	✅ Fixed — setter is now set => Volatile.Write(ref _upstreamEndpoint, value?.TrimEnd('/'))

---

One remaining minor point

OtlpReceiver.UpstreamEndpoint is still declared public on an internal sealed class. As noted before, this is effectively internal given the class visibility, so it's not a correctness issue — but internal would more clearly communicate intent. Not a blocker.

---

One new edge case to be aware of (not a blocker)

In ConfigureOtlpEndpoints, the loop registers a callback for every ProjectResource. If a multi-project Aspire setup has two project resources each independently configured with different OTEL_EXPORTER_OTLP_ENDPOINT values, the last callback to fire wins and silently overwrites the first. In practice this is unlikely (users set one upstream dashboard for all resources), but it's worth a // TODO comment or a note in the known-limitations doc if anyone ever reports oddities with multi-project configurations.

---

What's good about the refactor

• var receiver = _otlpReceiver local capture in ConfigureOtlpEndpoints is the correct pattern — it pins the specific receiver instance into the closure rather than closing over the mutable field, preventing any potential issue if _otlpReceiver were ever reassigned.
• OtlpEndpointEnvironment is now a true pure-logic helper, testable with a plain Dictionary and no live infrastructure.
• ForwardAsync being static after the refactor is the right consequence of removing all instance state from it.

The PR is in good shape. The end-to-end verification against the repro repo remains the right gate before release.