feat: auto-align DistributedContextPropagator to W3C

[thomhurst]

Summary

Closes #5592.

• Add [ModuleInitializer] in TUnit.Core/PropagatorAlignment.cs that swaps .NET's default LegacyPropagator (emits Correlation-Context) for DistributedContextPropagator.CreateW3CPropagator() (emits W3C baggage). Any user-customised propagator is left alone. Opt out via TUNIT_KEEP_LEGACY_PROPAGATOR=1.
• TestWebApplicationFactory.ConfigureWebHost re-applies alignment so SUT startup code cannot accidentally revert it.
• Docs refreshed — manual Sdk.SetDefaultTextMapPropagator(...) snippet retained only for out-of-process SUTs that don't reference TUnit.Core.

Why

The OpenTelemetry SDK's BaggagePropagator reads W3C baggage; .NET's default writes Correlation-Context. tunit.test.id silently drops between processes, so cross-process correlation (real HTTP servers, Aspire hosts, out-of-proc SUTs) never resolves the originating test.

Test plan

• PropagatorAlignmentTests — 3 tests: module init replaces default Legacy; AlignIfDefault preserves user custom propagator; AlignIfDefault leaves an existing W3C propagator reference-equal.
• ActivityBaggageCorrelationTests (7) + ActivityPropagationHandlerTests (3) still pass — existing propagation paths unaffected.
• TUnit.PublicAPI (3) still pass — no public API surface change (PropagatorAlignment is internal).
• Builds clean on net10.0 for TUnit.Core, TUnit.AspNetCore.Core, TUnit.UnitTests.
• Verify on a real Aspire/WebApplicationFactory scenario that tunit.test.id now reaches SUT spans without manual Sdk.SetDefaultTextMapPropagator wiring.

[claude]

Code Review

Overall this is a clean, well-motivated change. The [ModuleInitializer] approach is the right tool for auto-alignment, the opt-out mechanism is correct, and the tests are solid. A few architectural observations worth considering:

---

Concern: AlignIfDefault() in ConfigureWebHost may fire too early

In TestWebApplicationFactory.ConfigureWebHost, the alignment is called before SUT services are configured:

protected override void ConfigureWebHost(IWebHostBuilder builder)
{
    base.ConfigureWebHost(builder);
    PropagatorAlignment.AlignIfDefault();   // ← fires here
    builder.ConfigureServices(services => { ... });
}

The SUT's Program.cs / Startup.cs (including any OpenTelemetry SDK Sdk.SetDefaultTextMapPropagator(...) calls) executes during the host's Build() phase — after all ConfigureWebHost callbacks have registered. So if user startup code resets DistributedContextPropagator.Current, this call won't counter it.

A more robust placement would be an IStartupFilter registered via builder.ConfigureServices:

builder.ConfigureServices(services =>
{
    services.AddSingleton<IStartupFilter, PropagatorAlignmentStartupFilter>();
    services.AddCorrelatedTUnitLogging();
});

// In a separate internal class:
internal sealed class PropagatorAlignmentStartupFilter : IStartupFilter
{
    public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
        => app => { PropagatorAlignment.AlignIfDefault(); next(app); };
}

This runs after all ConfigureServices callbacks (including the SUT's) so alignment wins even when SUT startup code has its own propagator assignment.

---

Minor: String-based internal type detection

private const string LegacyPropagatorTypeName = "System.Diagnostics.LegacyPropagator";

if (DistributedContextPropagator.Current.GetType().FullName == LegacyPropagatorTypeName)

LegacyPropagator is an internal .NET type with no public accessor, so this is the only realistic approach — it's pragmatic. Worth noting in a code comment that this is intentional (the System.Diagnostics namespace makes an internal rename very unlikely, but a reader unfamiliar with .NET internals will wonder why a magic string is used). The existing XML doc on the class partially covers this, but a brief inline note at the constant would make it obvious at the point of use.

---

Tests look good

The test class is well-structured:

• [NotInParallel] correctly guards against global-state races.
• try/finally restores DistributedContextPropagator.Current in each test.
• The three cases (module-init ran, custom propagator untouched, W3C idempotent) cover the key invariants.

One minor note: ModuleInitializer_Replaces_Default_Legacy_Propagator implicitly depends on the TUnit.Core module having been loaded. Because the test project references TUnit.Core types, this will always be true in practice — but a short comment explaining the assumption would help future readers.

---

Documentation

The docs are clear and accurate. The one thing to address: "TUnit 0.x (issue #5592)" in opentelemetry.md contains a placeholder version number (0.x) that should be updated to the actual release version before merging, or replaced with a format like "since [version]" once the release version is known.

---

Summary

The core implementation is correct and the approach is sound. The main actionable item is the ConfigureWebHost placement — the IStartupFilter pattern would make the ASP.NET Core side robust against SUT startup code that resets the propagator. Everything else is minor polish.

[claude]

Test review post

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 27 complexity

Metric	Results
Complexity	27

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

[claude]

Follow-up Review (after commit 6da2043)

The new commit adds the W3CBaggagePropagator fallback for .NET 8/9 — good catch, since DistributedContextPropagator.CreateW3CPropagator() only exists on .NET 10+. The implementation is correct: LegacyPropagator (used via CreateDefaultPropagator()) does inject traceparent for W3C-format activities in .NET 8/9, so delegating trace-context to it and handling baggage separately is sound.

Open from previous review

ConfigureWebHost timing — the IStartupFilter suggestion was not addressed. If user Program.cs/Startup.cs code calls Sdk.SetDefaultTextMapPropagator(...) after host configuration, the current AlignIfDefault() call (placed before builder.ConfigureServices) won't win. The prior review explains this with a code example; it's still worth considering for robustness.

"TUnit 0.x" placeholder in docs/docs/examples/opentelemetry.md still reads "TUnit 0.x (issue #5592)" — needs the real version number before shipping.

New code analysis

W3CBaggagePropagator — implementation looks correct. The wrapping callback that strips Correlation-Context from DefaultPropagator.Inject is a clean approach. One minor note: FieldNames is typed as IReadOnlyCollection<string> but initialized with new[] (array). This works fine, but new[] { ... } creates a mutable array that the caller could cast and mutate. Using Array.AsReadOnly(new[] { ... }) or (IReadOnlyCollection<string>)new[] { ... } is inconsequential here but the former is more expressive of the intent.

DefaultPropagator static capture — CreateDefaultPropagator() is called at class initialisation, capturing a fresh LegacyPropagator rather than snapshotting Current. This is deliberate and correct: it avoids circular references regardless of propagator swap ordering.

Test update — changing the test to use PropagatorAlignment.CreateW3CPropagator() instead of DistributedContextPropagator.CreateW3CPropagator() directly is the right fix; it means the test exercises the same code path as the production flow on all target frameworks.

Summary

The new commit is solid and the .NET 8/9 fallback is well-implemented. Two items from the previous review remain unaddressed: the ConfigureWebHost timing concern (architectural) and the version-placeholder in docs (cosmetic). Otherwise this is ready.

[claude]

Follow-up Review (after commit 6c1eff9)

The latest refactor commit is solid — extracting BaggageHeader to TUnitActivitySource, reusing TryBuildBaggageHeader inside W3CBaggagePropagator, renaming CreateAlignedPropagator to avoid shadowing the BCL static, and the span-walker baggage parser all reflect good discipline. The allocation saving on the parse path aligns well with TUnit's performance focus.

---

Still open from previous reviews

1. ConfigureWebHost timing (architectural — raised twice, still unaddressed)

protected override void ConfigureWebHost(IWebHostBuilder builder)
{
    base.ConfigureWebHost(builder);
    PropagatorAlignment.AlignIfDefault();   // runs before SUT services are registered
    builder.ConfigureServices(services => { ... });
}

Any SUT that configures OpenTelemetry via Sdk.SetDefaultTextMapPropagator(...) in ConfigureServices (which runs during Build()) will silently revert the alignment, leaving tunit.test.id dropped again. The IStartupFilter pattern suggested in the first review runs after Build(), so it wins in the typical OTel-SDK setup:

builder.ConfigureServices(services =>
{
    services.Insert(0, ServiceDescriptor.Transient<IStartupFilter, PropagatorAlignmentStartupFilter>());
    services.AddCorrelatedTUnitLogging();
});

internal sealed class PropagatorAlignmentStartupFilter : IStartupFilter
{
    public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
        => app => { PropagatorAlignment.AlignIfDefault(); next(app); };
}

The Insert(0, ...) ensures TUnit's filter runs first in the pipeline, after all ConfigureServices callbacks have completed. This is the correct fix and it is worthwhile — without it the WebApplicationFactory alignment is unreliable.

2. "TUnit 0.x" version placeholder (cosmetic, but would ship as-is)

docs/docs/examples/opentelemetry.md still reads:

Since TUnit 0.x (issue #5592), the test process auto-aligns...

This needs the real version number (or a link-only reference) before the PR merges.

---

Minor: FieldNames backed by mutable array

private static readonly IReadOnlyCollection<string> FieldNames = new[] { "traceparent", "tracestate", TUnitActivitySource.BaggageHeader };

A caller who casts to string[] can mutate it. Non-exploitable here, but using Array.AsReadOnly(new[] { ... }) or a FrozenSet makes the intent unambiguous and prevents the cast.

---

Summary

The refactoring work in the latest commit is clean and well-targeted. Two carry-over items remain before this is fully ready: the IStartupFilter fix for ConfigureWebHost (which would make the ASP.NET Core alignment actually reliable) and the version placeholder in the docs.

[claude]

Follow-up Review (after commit 3a18a4b)

All two carry-over items from the previous reviews are now resolved. This PR is in good shape.

---

RESOLVED: ConfigureWebHost timing (architectural concern raised 3× previously)

The direct AlignIfDefault() call has been replaced with PropagatorAlignmentStartupFilter registered via services.AddSingleton<IStartupFilter, PropagatorAlignmentStartupFilter>().

One thing worth noting about the AddSingleton choice: ASP.NET Core processes startup filters in reverse registration order when building the middleware chain (see WebHost.BuildApplication() → startupFilters.Reverse()). This means the last-registered filter becomes the innermost wrapper, so its pre-next(app) code runs last among all startup filters — right before the actual pipeline is built. That's exactly what we want: TUnit's alignment wins over any SUT IStartupFilter that might set the propagator. The Insert(0, ...) pattern suggested in prior reviews would have been wrong here (it would have run TUnit's filter first/outermost, then SUT filters could revert it). AddSingleton is the correct choice.

RESOLVED: "TUnit 0.x" version placeholder

The docs no longer contain a version reference — the text reads generically "TUnit auto-aligns...", which is correct and won't go stale.

---

Still present (minor): FieldNames backed by mutable array

// TUnit.Core/PropagatorAlignment.cs:74
private static readonly IReadOnlyCollection<string> FieldNames = new[] { "traceparent", "tracestate", TUnitActivitySource.BaggageHeader };

A caller can cast Fields to string[] and mutate it. Non-exploitable and low-risk since the type is private sealed, but using Array.AsReadOnly(new[] { ... }) would make the intent unambiguous and prevent any accidental mutation:

private static readonly IReadOnlyCollection<string> FieldNames =
    Array.AsReadOnly(new[] { "traceparent", "tracestate", TUnitActivitySource.BaggageHeader });

This is genuinely minor — not a blocker.

---

Summary

Both significant items (IStartupFilter timing, version placeholder) are addressed. The AddSingleton placement is correct and better than what the prior reviews suggested. The only remaining note is the cosmetic FieldNames mutable-array issue. This PR is ready to merge.