feat: auto-configure OpenTelemetry in TestWebApplicationFactory SUT#5607
feat: auto-configure OpenTelemetry in TestWebApplicationFactory SUT#5607
Conversation
…5594) TestWebApplicationFactory<T> now augments the SUT's TracerProvider with TUnit's AspNetCore activity source, TUnitTestCorrelationProcessor, and ASP.NET Core + HttpClient instrumentation. Spans emitted inside the SUT stay queryable per-test (tunit.test.id tag) even when third-party libs break the parent chain. Opt out per-test with WebApplicationTestOptions.AutoConfigureOpenTelemetry = false. TUnitTestCorrelationProcessor moves from TUnit.OpenTelemetry to TUnit.Core (NET-only) so the AspNetCore wrapper can reference it without pulling in the zero-config package. Public namespace (TUnit.OpenTelemetry) is unchanged.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 5 |
TIP This summary will be updated as you push new changes. Give us feedback
There was a problem hiding this comment.
Code Review
Overall this is a well-executed feature — the opt-out pattern mirrors AutoPropagateHttpClientFactory, the docs are thorough, the tests are well-annotated, and the OnStart guard in TUnitTestCorrelationProcessor correctly prevents double-tagging. No prior review comments to address.
One significant architectural concern and two smaller issues below.
Major: TUnit.Core should not own an OpenTelemetry dependency
Moving TUnitTestCorrelationProcessor into TUnit.Core and adding OpenTelemetry as a conditional PackageReference there means every TUnit user — including those who never touch ASP.NET Core or OpenTelemetry — now gets OTel pulled into their transitive dependency graph on net8+. This creates unnecessary version-conflict surface for users who already manage their own OTel version.
The motivation is clear (avoid a circular dep between TUnit.AspNetCore.Core and TUnit.OpenTelemetry), but there are cleaner exits:
Option A — TUnit.AspNetCore.Core references TUnit.OpenTelemetry directly.
Since TUnit.OpenTelemetry is part of the same test-infrastructure family, a direct dep is conceptually clean. Users of TUnit.AspNetCore.Core are already in "heavy test infra" territory.
Option B — Extract a thin TUnit.OpenTelemetry.Abstractions package containing only TUnitTestCorrelationProcessor (and any other shared primitives). Both TUnit.OpenTelemetry and TUnit.AspNetCore.Core depend on it; TUnit.Core stays free of OTel.
Either way TUnit.Core remains OTel-agnostic, which is its current contract.
Moderate: Binary-breaking change for TUnit.OpenTelemetry users
TUnitTestCorrelationProcessor keeps the TUnit.OpenTelemetry namespace but now lives in TUnit.Core.dll. That's a binary-breaking change: any user who loaded the type by assembly-qualified name, or who uses IL weaving / source generators that embed the assembly name, will break at runtime even though their source compiles. The namespace preservation is good, but the assembly move deserves a CHANGELOG note and a semantic-version bump in TUnit.OpenTelemetry.
Minor: Implicit double-processor when TUnit.OpenTelemetry auto-start is active
When a test suite uses the TUnit.OpenTelemetry zero-config package and TestWebApplicationFactory with AutoConfigureOpenTelemetry = true, two separate TracerProvider instances each register a TUnitTestCorrelationProcessor — one on the test-runner side, one inside the SUT. The OnStart guard (if (activity.GetTagItem(...) is not null) return;) prevents double-tagging, so functionally it's fine. But the interaction is invisible. A short note in AddTUnitOpenTelemetry's XML doc or in the distributed-tracing guide ("safe to use alongside TUnit.OpenTelemetry; the processor de-duplicates tags") would help future maintainers.
The feature itself fills a real gap (SUT spans losing correlation when parent chains break), the test coverage is solid, and the docs give users everything they need to opt out. Addressing the TUnit.Core dependency scope would make this the right shape for the long term.
…NetCore.Core Addresses PR review: TUnit.Core should not own an OpenTelemetry dependency (every TUnit user would get OTel transitively). Revert the processor move: TUnitTestCorrelationProcessor stays in TUnit.OpenTelemetry, and TUnit.AspNetCore.Core references that project directly. Also document zero-config + SUT double-processor safety on the helper.
There was a problem hiding this comment.
Code Review — Follow-up
Previous review raised three concerns. Here's the status:
✅ Major (resolved): TUnit.Core stays OTel-free
The implementation correctly has TUnit.AspNetCore.Core add a direct <ProjectReference> to TUnit.OpenTelemetry — exactly Option A from the prior review. TUnit.Core.csproj has no new OTel dependency; the only TUnit.Core change is adding AspNetCoreHttpSourceName as a plain string const, which is fine.
However, the PR description says "TUnitTestCorrelationProcessor moves from TUnit.OpenTelemetry to TUnit.Core" — that does not match the actual diff. TUnitTestCorrelationProcessor stays in TUnit.OpenTelemetry; TUnit.AspNetCore.Core just references that package directly. Please update the PR description to avoid confusion for anyone reading the changelog.
✅ Moderate (resolved): No binary break
Because TUnitTestCorrelationProcessor was not moved to TUnit.Core.dll, there is no assembly relocation and no binary-breaking change for existing users. The concern is moot.
✅ Minor (resolved): Double-processor safety documented
The XML doc on AddTUnitOpenTelemetry now explicitly states that combining with the TUnit.OpenTelemetry zero-config package is safe because the processor's idempotent OnStart guard prevents double-tagging. Well done.
Remaining note: AddAspNetCoreInstrumentation() double-call claim
The XML doc says "Safe to call even if the SUT already registers these — OpenTelemetry de-duplicates them." This is true in practice (the OTel SDK merges multiple WithTracing() callbacks into one TracerProvider, and extension methods like AddAspNetCoreInstrumentation use TryAdd-style DI registrations). But it is a subtler guarantee than it looks — if a future OTel version changes that behaviour, double-registration would produce duplicate spans. A brief clarifying note such as "relies on OTel SDK's singleton DI registration for instrumentation libraries" would harden the comment against future reader confusion. Not a blocker.
Overall
The implementation is clean: TUnit.Core stays dependency-free, no binary break, docs are solid, test coverage is good with correct [NotInParallel] isolation rationale. The only action item is updating the PR description to match what was actually implemented.
Adds the newly introduced public const on TUnitActivitySource to the verified PublicAPI snapshots for net8.0, net9.0, and net10.0.
There was a problem hiding this comment.
Code Review — Third Pass
Previous reviews raised three concerns; all were addressed. One action item remained open from the second review. Here is the current status.
✅ New in this pass: Public API snapshot files
The latest commit adds to the verified PublicAPI snapshots for all three .NET targets (net8.0, net9.0, net10.0). That is the correct process, and the snapshots match the constant added to TUnitActivitySource.
⚠️ Still open: PR description does not match the implementation
The second review explicitly called this out as the only remaining action item. The PR description still reads:
TUnitTestCorrelationProcessormoves fromTUnit.OpenTelemetrytoTUnit.Core(NET-only, guarded with#if NET) so the AspNetCore wrapper can reference it directly without pulling in the zero-config package.
The actual implementation is the opposite: TUnitTestCorrelationProcessor stays in TUnit.OpenTelemetry, and TUnit.AspNetCore.Core gains a direct <ProjectReference> to TUnit.OpenTelemetry. TUnit.Core gained nothing except the string constant. The description as written will mislead anyone reading the changelog or trying to track down where the type lives.
Please update the summary bullet to something like:
TUnit.AspNetCore.Corenow referencesTUnit.OpenTelemetrydirectly, soTestWebApplicationFactorycan useTUnitTestCorrelationProcessorwithout requiring users to install the zero-config package.TUnitTestCorrelationProcessorstays inTUnit.OpenTelemetry; only a single string constant was added toTUnit.Core.
Non-blocking reminder (unchanged from review 2)
The XML doc on AddTUnitOpenTelemetry says "OpenTelemetry de-duplicates them" when the SUT already registers these. This is accurate today but relies on OTel SDK's TryAdd-style DI registration for instrumentation libraries — worth naming that assumption explicitly. Still not a blocker.
Overall
Implementation is clean and all substantive concerns are resolved. The only blocker is the PR description correction above.
There was a problem hiding this comment.
Code Review — Third Pass
Previous reviews raised three concerns; all were addressed. One action item remained open from the second review. Here is the current status.
New in this pass: Public API snapshot files
The latest commit adds AspNetCoreHttpSourceName to the verified PublicAPI snapshots for all three .NET targets (net8.0, net9.0, net10.0). That is the correct process, and the snapshots match the constant added to TUnitActivitySource.
Still open: PR description does not match the implementation
The second review explicitly called this out as the only remaining action item. The PR description still reads:
TUnitTestCorrelationProcessormoves fromTUnit.OpenTelemetrytoTUnit.Core(NET-only, guarded with#if NET) so the AspNetCore wrapper can reference it directly without pulling in the zero-config package.
The actual implementation is the opposite: TUnitTestCorrelationProcessor stays in TUnit.OpenTelemetry, and TUnit.AspNetCore.Core gains a direct <ProjectReference> to TUnit.OpenTelemetry. TUnit.Core gained nothing except the string constant. The description as written will mislead anyone reading the changelog or trying to track down where the type lives.
Please update the summary bullet to something like:
TUnit.AspNetCore.Corenow referencesTUnit.OpenTelemetrydirectly, soTestWebApplicationFactorycan useTUnitTestCorrelationProcessorwithout requiring users to install the zero-config package.TUnitTestCorrelationProcessorstays inTUnit.OpenTelemetry; only a single string constant was added toTUnit.Core.
Non-blocking reminder (unchanged from review 2)
The XML doc on AddTUnitOpenTelemetry says "OpenTelemetry de-duplicates them" when the SUT already registers these. This is accurate today but relies on OTel SDK's TryAdd-style DI registration for instrumentation libraries — worth naming that assumption explicitly. Still not a blocker.
Overall
Implementation is clean and all substantive concerns are resolved. The only remaining item is the PR description correction above.
Closes #5594.
Summary
TestWebApplicationFactory<T>now auto-augments the SUT'sTracerProviderwith TUnit'sAspNetCore.Httpsource,TUnitTestCorrelationProcessor, and ASP.NET Core + HttpClient instrumentation — so spans emitted inside the SUT keep thetunit.test.idtag even when third-party libs break the parent chain.WebApplicationTestOptions.AutoConfigureOpenTelemetry = false(mirrors theAutoPropagateHttpClientFactorytoggle).TUnitTestCorrelationProcessormoves fromTUnit.OpenTelemetrytoTUnit.Core(NET-only, guarded with#if NET) so the AspNetCore wrapper can reference it directly without pulling in the zero-config package. Public namespace (TUnit.OpenTelemetry) is preserved for API compatibility.Test plan
AutoConfigureOpenTelemetryTests— asserts an ASP.NET Core span emitted from the SUT carriestunit.test.idwhen auto-wire is on.AutoConfigureOpenTelemetryOptOutTests— asserts notunit.test.idtag is applied when the option is disabled.[NotInParallel]because OTel'sActivityListeneris process-global — without serialization, one factory's correlation processor can tag the other factory's spans.dotnet buildclean; 2/2 tests pass on net10.0.