refactor: separate test and lifecycle tracing#5572
Conversation
Keep traceparent and baggage propagation working when the synthetic TUnit.AspNetCore.Http span is not created. Also align the ASP.NET Core and OpenTelemetry docs with the current root-per-test trace model and CreateClient() guidance.
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 1 medium |
🟢 Metrics 74 complexity
Metric Results Complexity 74
TIP This summary will be updated as you push new changes. Give us feedback
Keep exported traces focused on per-test parent-child trees so backends and log correlation work naturally. Move session/discovery/suite/shared setup spans to TUnit.Lifecycle and finish the test span after per-test cleanup.
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
Keep trace propagation anchored in the ambient test activity while preserving the synthetic client span when it exists. This fixes net8 WebApplicationFactory requests dropping baggage from the outgoing context.
Avoid phantom parent span IDs when Aspire forwards trace context without a client span. Backend UIs like Seq can then render the server span under the exported test body span.
TestServer and Aspire bypass the built-in HTTP diagnostics pipeline, so TUnit has to propagate correlation itself. .NET 8 still defaults to the legacy Correlation-Context baggage behavior, so emit the W3C baggage header explicitly while keeping traceparent/tracestate propagation on the ambient activity.
There was a problem hiding this comment.
Code Review: Separate Test and Lifecycle Tracing
This is a well-motivated architectural change that solves a real problem — per-test traces were polluted by session/class/assembly spans, making log correlation and backend rendering awkward. The refactor is largely clean and well-documented. Here are the issues I found, from most to least significant.
Issue 1: Double-Pass Baggage Injection in ActivityPropagationHandler — May Conflict with Registered Propagators
File: TUnit.AspNetCore.Core/Http/ActivityPropagationHandler.cs
InjectTraceContext(propagationActivity, request.Headers);
InjectBaggage(propagationActivity, request.Headers);InjectTraceContext calls DistributedContextPropagator.Current.Inject(...). On .NET 8+ with OTel SDK configured, the current propagator is typically a composite of TraceContextPropagator + BaggagePropagator, so it already emits a W3C baggage header. Then InjectBaggage does headers.Remove("baggage") and re-adds a manually built version. This creates two problems:
- It silently overrides the user's propagator configuration. If someone registers a custom
BaggagePropagator(e.g., to add extra fields or use a different encoding),InjectBaggagewill replace its output. - The two outputs can diverge.
DistributedContextPropagator.Current.Injectmay encode entries not directly onactivity.Baggage(e.g., fromBaggage.Current), while the manual builder only iteratesactivity.Baggage.
The comment says "Older target frameworks still default to Correlation-Context for baggage" — but this should be solved by always installing a known-good propagator at setup time (e.g., OpenTelemetry.Context.Propagation.TraceContextPropagator + BaggagePropagator) rather than patching the output after the fact.
Suggested approach: Either use TextMapPropagator directly to control exactly which propagators run, or check whether the baggage header is already present (as TUnitBaggagePropagationHandler does with !httpRequest.Headers.Contains(key)) before emitting the fallback. This would avoid silently overriding OTel SDK configuration.
Issue 2: ActivityPropagationHandlerTests Uses Reflection Magic Strings
File: TUnit.AspNetCore.Tests/ActivityPropagationHandlerTests.cs
var activityPropagationHandlerType = typeof(TUnitTestIdHandler).Assembly
.GetType("TUnit.AspNetCore.ActivityPropagationHandler", throwOnError: true)!;This uses a hardcoded fully-qualified type name to reflect into an internal sealed class. A rename or namespace change would compile fine but fail at runtime. Even with throwOnError: true, that's still a runtime breakage with an obscure error message.
Why this matters: The internal ActivityPropagationHandler(Func<HttpRequestMessage, Activity?> startActivity) constructor already exists purely for testing, so the seam is there. The missing piece is access.
Suggested approach: Add [assembly: InternalsVisibleTo("TUnit.AspNetCore.Tests")] to the TUnit.AspNetCore.Core project and reference the type directly. This preserves the internal visibility contract but makes the test refactor-safe.
Issue 3: Inconsistent Header Overwrite Behavior Between Handlers
ActivityPropagationHandler.InjectTraceContext always overwrites:
h.Remove(key);
h.TryAddWithoutValidation(key, value);TUnitBaggagePropagationHandler (new code) never overwrites:
if (carrier is HttpRequestMessage httpRequest && key is not null && !httpRequest.Headers.Contains(key))
{
httpRequest.Headers.TryAddWithoutValidation(key, value);
}If a test author pre-sets a custom traceparent header on a request (a valid integration test pattern), ActivityPropagationHandler will silently discard it, while TUnitBaggagePropagationHandler would respect it. This asymmetry could produce hard-to-diagnose tracing issues when both handlers are in the pipeline. The two should agree on one policy.
Issue 4: FinishTestActivity Signature Change — Cancelled/Timeout Exception Visibility
File: TUnit.Engine/TestExecutor.cs
Previously FinishTestActivity(executableTest, capturedException) received the exception directly from the catch block. Now:
else if (result?.Exception is { } exception)
{
TUnitActivitySource.RecordException(activity, exception);
}The PR correctly adds:
TestState.Timeout => "fail",
TestState.Cancelled => "fail",But result?.Exception may be null for cancellation (which sometimes surfaces as OperationCanceledException that isn't stored in the result). If the exception goes unrecorded, the span will show status=fail but have no error.type or exception.* attributes — backends like Seq/Jaeger will flag the test as failed with no trace of why.
Worth adding a test that verifies a cancelled test's activity has error.type set, or documenting that cancel exceptions aren't attributed.
Issue 5: Activity.Current = null in TestCoordinator Is an Implicit Contract
File: TUnit.Engine/Services/TestExecution/TestCoordinator.cs
// Per-test cleanup has completed...
System.Diagnostics.Activity.Current = null;
// ...
var hookExceptions = await _testExecutor.ExecuteAfterClassAssemblyHooks(...);
// ...
TestExecutor.FinishTestActivity(test);Setting Activity.Current = null mid-method before calling async cleanup is an implicit convention that future contributors need to understand. The comment is good, but this creates a subtle invariant: the test activity must never be set as Activity.Current anywhere between cleanup and FinishTestActivity, or it'll be leaked into class/session cleanup.
Suggested approach: Extract the null-setting into a small method with a clear name like DetachTestActivityFromAmbientContext(), or restructure so the post-cleanup logic runs in a new async context (fresh AsyncLocal snapshot). This makes the contract explicit rather than implicit via comment.
Minor: GetSourceForSharedType Moves Shared Fixture Spans to Opt-In Source
File: TUnit.Core/TUnitActivitySource.cs
internal static ActivitySource GetSourceForSharedType(SharedType? sharedType) =>
sharedType is not null and not SharedType.None
? LifecycleSource
: Source;Shared fixture setup/teardown now emits to TUnit.Lifecycle. Users who subscribe only to "TUnit" (common pattern from existing docs) will silently lose shared fixture spans from their traces and HTML report. Since shared fixture failures often explain test failures, this could make debugging harder.
The docs do mention adding "TUnit.Lifecycle", but the migration path for existing users who only see "TUnit" should be clearer — perhaps a note in the HTML report or a diagnostic message when lifecycle spans are present but unsubscribed.
Positive notes
- Removing
ActivityLinkfrom test case to class activity is the right call — links are poorly supported in most backends and added complexity without clear benefit. - The
CopyBaggagefallback for WebApplicationFactory is a pragmatic fix with a good explanatory comment. - Adding
tunit.session.id,tunit.assembly.name, andtunit.class.namespacedirectly to the test case span is excellent for backends that need to correlate without navigating the lifecycle source. - The
TestState.Timeout/Cancelled => "fail"fix is a genuine correctness improvement. - Backend validation with real Seq screenshots is a great addition to the PR description.
Overall this is solid work. The main concerns are the double-pass baggage injection potentially conflicting with OTel SDK propagators, and the reflection-based test instantiation pattern. The others are smaller design points worth addressing.
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.33.0 to 1.35.2. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.35.2 <!-- Release notes generated using configuration in .github/release.yml at v1.35.2 --> ## What's Changed ### Other Changes * fix: restore SourceLink and deterministic builds in published packages by @thomhurst in thomhurst/TUnit#5579 ### Dependencies * chore(deps): update tunit to 1.35.0 by @thomhurst in thomhurst/TUnit#5578 **Full Changelog**: thomhurst/TUnit@v1.35.0...v1.35.2 ## 1.35.0 <!-- Release notes generated using configuration in .github/release.yml at v1.35.0 --> ## What's Changed ### Other Changes * fix: support open generic transitive auto-mocks by @thomhurst in thomhurst/TUnit#5568 * refactor: separate test and lifecycle tracing by @thomhurst in thomhurst/TUnit#5572 * fix: expand nested And/Or expectations in failure messages (#5573) by @thomhurst in thomhurst/TUnit#5577 ### Dependencies * chore(deps): update tunit to 1.34.5 by @thomhurst in thomhurst/TUnit#5566 * chore(deps): bump follow-redirects from 1.15.11 to 1.16.0 in /docs by @dependabot[bot] in thomhurst/TUnit#5538 * chore(deps): update verify to 31.16.0 by @thomhurst in thomhurst/TUnit#5570 * chore(deps): update verify to 31.16.1 by @thomhurst in thomhurst/TUnit#5574 * chore(deps): update gittools/actions action to v4 by @thomhurst in thomhurst/TUnit#5575 **Full Changelog**: thomhurst/TUnit@v1.34.5...v1.35.0 ## 1.34.5 <!-- Release notes generated using configuration in .github/release.yml at v1.34.5 --> ## What's Changed ### Other Changes * fix: cap test output at 1M chars to prevent OOM by @thomhurst in thomhurst/TUnit#5561 * fix: handle explicit interface impl with different return types in mock generator by @thomhurst in thomhurst/TUnit#5564 * fix: include XML documentation files in NuGet packages by @thomhurst in thomhurst/TUnit#5565 ### Dependencies * chore(deps): update tunit to 1.34.0 by @thomhurst in thomhurst/TUnit#5562 **Full Changelog**: thomhurst/TUnit@v1.34.0...v1.34.5 ## 1.34.0 <!-- Release notes generated using configuration in .github/release.yml at v1.34.0 --> ## What's Changed ### Other Changes * refactor: move CorrelatedTUnitLogger to TUnit.Logging.Microsoft and auto-inject handlers by @thomhurst in thomhurst/TUnit#5532 * feat: add Dev Drive setup for Windows in CI workflow by @thomhurst in thomhurst/TUnit#5544 * fix: start session activity before discovery so discovery spans parent correctly by @thomhurst in thomhurst/TUnit#5534 * feat: cross-process test log correlation via OTLP receiver by @thomhurst in thomhurst/TUnit#5533 * refactor: use natural OTEL trace propagation instead of synthetic TraceIds by @thomhurst in thomhurst/TUnit#5557 * fix: route ITestOutput writes through synchronized ConcurrentStringWriter by @thomhurst in thomhurst/TUnit#5558 ### Dependencies * chore(deps): update tunit to 1.33.0 by @thomhurst in thomhurst/TUnit#5527 * chore(deps): update dependency dompurify to v3.4.0 by @thomhurst in thomhurst/TUnit#5537 * chore(deps): update dependency docusaurus-plugin-llms to ^0.3.1 by @thomhurst in thomhurst/TUnit#5541 * chore(deps): update dependency microsoft.sourcelink.github to 10.0.202 by @thomhurst in thomhurst/TUnit#5543 * chore(deps): update dependency microsoft.entityframeworkcore to 10.0.6 by @thomhurst in thomhurst/TUnit#5542 * chore(deps): update dependency microsoft.templateengine.authoring.templateverifier to 10.0.202 by @thomhurst in thomhurst/TUnit#5546 * chore(deps): update dependency microsoft.templateengine.authoring.cli to v10.0.202 by @thomhurst in thomhurst/TUnit#5545 * chore(deps): update dependency system.commandline to 2.0.6 by @thomhurst in thomhurst/TUnit#5547 * chore(deps): update microsoft.aspnetcore to 10.0.6 by @thomhurst in thomhurst/TUnit#5548 * chore(deps): update dependency nuget.protocol to 7.3.1 by @thomhurst in thomhurst/TUnit#5549 * chore(deps): update microsoft.extensions to 10.0.6 by @thomhurst in thomhurst/TUnit#5550 * chore(deps): update dependency dotnet-sdk to v10.0.202 by @thomhurst in thomhurst/TUnit#5551 * chore(deps): update opentelemetry by @thomhurst in thomhurst/TUnit#5552 * chore(deps): update microsoft.extensions to 10.5.0 by @thomhurst in thomhurst/TUnit#5554 **Full Changelog**: thomhurst/TUnit@v1.33.0...v1.34.0 Commits viewable in [compare view](thomhurst/TUnit@v1.33.0...v1.35.2). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Summary
TUnittest spans fromTUnit.Aspireto the same OTLP backend used by the Aspire dashboard whenDOTNET_DASHBOARD_OTLP_ENDPOINT_URLis present.../v1/tracesso path-based OTLP backends like Seq also work correctlyOut-of-Box Backend Validation
Local Jaeger validation on April 16, 2026 using only
DOTNET_DASHBOARD_OTLP_ENDPOINT_URL=http://127.0.0.1:4318in the test runner and a realOtlpCorrelationIntegrationTests/TraceDemo_Request_ProducesNestedCorrelatedLogsrun. No manual tracer provider was added to the test project.Jaeger search/timeline view showing one trace with 10 spans across both services (
TUnit.Aspire.Testsandapi-service).Jaeger trace waterfall showing the full request tree:
test case -> test body -> GET /trace-demo -> internal spans -> nested HTTP spans.Testing
dotnet test TUnit.Aspire.Tests\TUnit.Aspire.Tests.csproj --framework net10.0 -- --treenode-filter "/*/*/TestTraceExporterTests/*"dotnet test TUnit.Aspire.Tests\TUnit.Aspire.Tests.csproj --framework net10.0 -- --treenode-filter "/*/*/BaggagePropagationHandlerTests/*"dotnet test TUnit.Aspire.Tests\TUnit.Aspire.Tests.csproj --framework net10.0 -- --treenode-filter "/*/*/OtlpReceiverTests/*"dotnet test TUnit.Aspire.Tests\TUnit.Aspire.Tests.csproj --framework net10.0 -- --treenode-filter "/*/*/OtlpCorrelationIntegrationTests/TraceDemo_Request_ProducesNestedCorrelatedLogs"DOTNET_DASHBOARD_OTLP_ENDPOINT_URL=http://127.0.0.1:4318 dotnet test TUnit.Aspire.Tests\TUnit.Aspire.Tests.csproj --framework net10.0 -- --treenode-filter "/*/*/OtlpCorrelationIntegrationTests/TraceDemo_Request_ProducesNestedCorrelatedLogs"dotnet build TUnit.Aspire\TUnit.Aspire.csproj -c Debug