Skip to content

feat: auto-propagate test trace context through IHttpClientFactory#5603

Merged
thomhurst merged 2 commits intomainfrom
feat/auto-propagate-httpclientfactory
Apr 17, 2026
Merged

feat: auto-propagate test trace context through IHttpClientFactory#5603
thomhurst merged 2 commits intomainfrom
feat/auto-propagate-httpclientfactory

Conversation

@thomhurst
Copy link
Copy Markdown
Owner

Summary

  • Registers TUnitHttpClientFilter : IHttpMessageHandlerBuilderFilter in TestWebApplicationFactory.GetIsolatedFactory so every IHttpClientFactory pipeline built inside the SUT (AddHttpClient<T>(), named clients, typed clients) auto-prepends ActivityPropagationHandler + TUnitTestIdHandler. Outbound HTTP from the SUT to downstream services now carries traceparent, baggage, and X-TUnit-TestId without the user having to call .AddHttpMessageHandler<>() per registration.
  • New opt-out: WebApplicationTestOptions.AutoPropagateHttpClientFactory (default true). Set false when the SUT already instruments outbound HTTP (e.g. OpenTelemetry HttpClient instrumentation).
  • De-duplicates the shared handler-prepending logic across TestWebApplicationFactory.CreateDefaultClient, TracedWebApplicationFactory.CreateClient/CreateDefaultClient, and the new filter via TUnitHttpClientFilter.PrependPropagationHandlers.
  • Defensive guard in TUnitTestIdHandler.SendAsync so the X-TUnit-TestId header is never added twice when an outer handler already set it (matches the pre-existing traceparent guard).
  • Docs: removes the manual .AddHttpMessageHandler<ActivityPropagationHandler>() recipe from distributed-tracing.md + opentelemetry.md, and lists the new auto-behavior (plus opt-out) in aspnet.md Quick Start.

Closes #5590.

Test plan

  • New IHttpClientFactoryPropagationTests asserts outbound call via SUT's named IHttpClientFactory client carries traceparent, baggage, X-TUnit-TestId, and the current test's TraceId.
  • New IHttpClientFactoryPropagationOptOutTests asserts no propagation headers on the downstream hop when AutoPropagateHttpClientFactory = false.
  • Full TUnit.AspNetCore.Tests suite green (23/23 on net10.0) — covers existing ActivityPropagationHandler, baggage correlation, hosted-service flow suppression, minimal API, and HTTP exchange capture tests.
  • CI to confirm on net8.0 + net9.0.

Registers an IHttpMessageHandlerBuilderFilter in TestWebApplicationFactory so that
every IHttpClientFactory pipeline built inside the SUT (AddHttpClient<T>(), named
clients, typed clients) automatically carries the test's traceparent, baggage, and
X-TUnit-TestId headers on outbound calls. Opt out per-test via
WebApplicationTestOptions.AutoPropagateHttpClientFactory = false.

Closes #5590.
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Summary

This is a well-designed feature. Using IHttpMessageHandlerBuilderFilter is the correct ASP.NET Core extension point for intercepting every IHttpClientFactory pipeline, TryAddEnumerable is the right registration call, and the PrependPropagationHandlers static helper nicely removes the duplicated array-construction code that existed in three places. Tests cover both the happy-path and the opt-out case. Good work overall.


Issues to address

1. Handler instances in the filter are shared across concurrent requests

// TUnitHttpClientFilter.cs
builder.AdditionalHandlers.Insert(0, new ActivityPropagationHandler());
builder.AdditionalHandlers.Insert(1, new TUnitTestIdHandler());

IHttpClientFactory caches handler pipelines per named client — the same handler objects are reused for every request on that client until the pipeline expires. If multiple tests run in parallel and each makes an outbound HTTP call through the same named client, the same TUnitTestIdHandler instance handles all of them concurrently.

TUnitTestIdHandler.SendAsync reads from TestContext.Current (ambient/async-local), which is per-async-flow, so that specific access is fine. However, both handler classes should be documented as intentionally stateless and thread-safe — a future contributor adding instance state to either handler would break parallel test runs silently. A brief comment in TUnitHttpClientFilter noting this expectation would prevent that footgun.

2. Handler insertion order is subtly fragile

next(builder);  // may leave AdditionalHandlers non-empty
builder.AdditionalHandlers.Insert(0, new ActivityPropagationHandler());
builder.AdditionalHandlers.Insert(1, new TUnitTestIdHandler());

After next(builder) runs, AdditionalHandlers could already contain SUT-registered handlers. Insert(0) then Insert(1) correctly places TUnit handlers at the front (outermost position), but the intent is non-obvious. A comment would prevent future refactors from accidentally reversing the order:

next(builder);
// Insert at outermost positions so TUnit headers are written before
// any SUT-configured handlers execute.
builder.AdditionalHandlers.Insert(0, new ActivityPropagationHandler());
builder.AdditionalHandlers.Insert(1, new TUnitTestIdHandler());

3. Opt-out test does not assert absence of baggage

// IHttpClientFactoryPropagationOptOutTests.cs
await Assert.That(echoed).DoesNotContain(TUnitTestIdHandler.HeaderName);
await Assert.That(echoed).DoesNotContain("traceparent:");
// baggage is not checked

The positive test asserts all three headers (traceparent, baggage, X-TUnit-TestId). The opt-out test should mirror that by asserting absence of baggage: as well — unless ActivityPropagationHandler intentionally omits baggage when there is no ambient activity. If so, add a comment explaining the asymmetry so future reviewers don't flag it as an oversight.


Minor observations

  • WebApplicationTestOptions is a record — mutating properties in ConfigureTestOptions is a bit surprising for a record type. Not a new issue introduced by this PR; the new property follows the existing pattern correctly.
  • IHttpMessageHandlerBuilderFilter is not deprecated across net8.0/net9.0/net10.0, so the extension point is sound.
  • The docs updates are accurate and removing the stale "tracking automation: #5590" note is appreciated.

Verdict

The feature solves a real pain point using the right ASP.NET Core primitives. The handler-sharing concern (point 1) is the most important thing to verify — confirm both handlers are safe under concurrent invocations, then document that expectation in TUnitHttpClientFilter so it stays true.

@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Apr 17, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric Results
Complexity 0

View in Codacy

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

- Document that both handler types inserted by TUnitHttpClientFilter must
  remain stateless/thread-safe because IHttpClientFactory caches pipelines
  and shares handler instances across concurrent parallel-test requests.
- Explain the outermost-insert intent so a future refactor doesn't reverse
  the order.
- Opt-out test now also asserts the downstream hop does not carry baggage,
  mirroring the positive test.
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review — Follow-up

All three issues raised in the previous review have been addressed. Here's the verification:

Previous issues — resolved

1. Handler-sharing / thread-safety documentation
The TUnitHttpClientFilter XML doc now explicitly states:

"Both handler types must remain stateless and thread-safe … do not add instance fields capturing per-request state to either handler."

This is exactly the right place for the warning — it's on the type itself so future contributors will see it before touching the handlers.

2. Insertion-order comment

// Insert at outermost positions so TUnit headers are emitted before any
// SUT-registered handler can run. Order must stay ActivityPropagationHandler
// first (writes traceparent/baggage) then TUnitTestIdHandler (writes X-TUnit-TestId).
builder.AdditionalHandlers.Insert(0, new ActivityPropagationHandler());
builder.AdditionalHandlers.Insert(1, new TUnitTestIdHandler());

Intent is now clear.

3. Opt-out test missing baggage: assertion

await Assert.That(echoed).DoesNotContain(TUnitTestIdHandler.HeaderName);
await Assert.That(echoed).DoesNotContain("traceparent:");
await Assert.That(echoed).DoesNotContain("baggage:");

All three headers are now asserted absent in the opt-out path, matching the positive-path test.


Fresh observations

Design: two-sided instrumentation is correctly separated. CreateDefaultClient / TracedWebApplicationFactory instrument the test→SUT side; the IHttpMessageHandlerBuilderFilter instruments the SUT→downstream side. These are orthogonal, non-overlapping, and the AutoPropagateHttpClientFactory flag correctly scopes only the latter. No conflict.

TryAddEnumerable with ServiceDescriptor.Singleton — correct choice. Prevents double-registration if GetIsolatedFactory is called more than once or if user registers their own filter.

HeaderEchoHandler in TestWebApp is a clean, minimal test double. The explicit Activity start in SutHttpClientFactory_Propagates_TestContextHeaders ensures Activity.Current is non-null for the propagation handlers — this is the right setup, not a test smell.

TracedWebApplicationFactory always prepends via PrependPropagationHandlers regardless of AutoPropagateHttpClientFactory. This is correct since TracedWebApplicationFactory predates and is independent of the WebApplicationTestOptions lifecycle.


Verdict

The implementation is clean, well-documented, and the tests are thorough. Nothing new to block on. Ready to merge pending CI on net8.0 + net9.0 (as noted in the test plan).

@thomhurst thomhurst enabled auto-merge (squash) April 17, 2026 16:08
@thomhurst thomhurst disabled auto-merge April 17, 2026 16:47
@thomhurst thomhurst merged commit b8f9bb5 into main Apr 17, 2026
13 of 15 checks passed
@thomhurst thomhurst deleted the feat/auto-propagate-httpclientfactory branch April 17, 2026 16:47
This was referenced Apr 22, 2026
BenjaminMichaelis pushed a commit to IntelliTect/EssentialCSharp.Web that referenced this pull request Apr 25, 2026
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.35.2 to
1.37.10.

<details>
<summary>Release notes</summary>

_Sourced from [TUnit's
releases](https://github.com/thomhurst/TUnit/releases)._

## 1.37.10

<!-- Release notes generated using configuration in .github/release.yml
at v1.37.10 -->

## What's Changed
### Other Changes
* docs(test-filters): add migration callout for --filter →
--treenode-filter by @​johnkattenhorn in
thomhurst/TUnit#5628
* fix: re-enable RPC tests and modernize harness (#​5540) by @​thomhurst
in thomhurst/TUnit#5632
* fix(mocks): propagate [Obsolete] and null-forgiving raise dispatch
(#​5626) by @​JohnVerheij in
thomhurst/TUnit#5631
* ci: use setup-dotnet built-in NuGet cache by @​thomhurst in
thomhurst/TUnit#5635
* feat(playwright): propagate W3C trace context into browser contexts by
@​thomhurst in thomhurst/TUnit#5636
### Dependencies
* chore(deps): update tunit to 1.37.0 by @​thomhurst in
thomhurst/TUnit#5625

## New Contributors
* @​johnkattenhorn made their first contribution in
thomhurst/TUnit#5628
* @​JohnVerheij made their first contribution in
thomhurst/TUnit#5631

**Full Changelog**:
thomhurst/TUnit@v1.37.0...v1.37.10

## 1.37.0

<!-- Release notes generated using configuration in .github/release.yml
at v1.37.0 -->

## What's Changed
### Other Changes
* fix: stabilize flaky tests across analyzer, OTel, and engine suites by
@​thomhurst in thomhurst/TUnit#5609
* perf: engine hot-path allocation wins (#​5528 B) by @​thomhurst in
thomhurst/TUnit#5610
* feat(analyzers): detect collection IsEqualTo reference equality
(TUnitAssertions0016) by @​thomhurst in
thomhurst/TUnit#5615
* perf: consolidate test dedup + hook register guards (#​5528 A) by
@​thomhurst in thomhurst/TUnit#5612
* perf: engine discovery/init path cleanup (#​5528 C) by @​thomhurst in
thomhurst/TUnit#5611
* fix(assertions): render collection contents in IsEqualTo failure
messages (#​5613 B) by @​thomhurst in
thomhurst/TUnit#5619
* feat(analyzers): code-fix for TUnit0015 to insert CancellationToken
(#​5613 D) by @​thomhurst in
thomhurst/TUnit#5621
* fix(assertions): add Task reference forwarders on
AsyncDelegateAssertion by @​thomhurst in
thomhurst/TUnit#5618
* test(asp-net): fix race in FactoryMethodOrderTests by @​thomhurst in
thomhurst/TUnit#5623
* feat(analyzers): code-fix for TUnit0049 to insert [MatrixDataSource]
(#​5613 C) by @​thomhurst in
thomhurst/TUnit#5620
* fix(pipeline): isolate AOT publish outputs to stop clobbering pack
DLLs (#​5622) by @​thomhurst in
thomhurst/TUnit#5624
### Dependencies
* chore(deps): update tunit to 1.36.0 by @​thomhurst in
thomhurst/TUnit#5608
* chore(deps): update modularpipelines to 3.2.8 by @​thomhurst in
thomhurst/TUnit#5614


**Full Changelog**:
thomhurst/TUnit@v1.36.0...v1.37.0

## 1.36.0

<!-- Release notes generated using configuration in .github/release.yml
at v1.36.0 -->

## What's Changed
### Other Changes
* fix: don't render test's own trace as Linked Trace in HTML report by
@​thomhurst in thomhurst/TUnit#5580
* fix(docs): benchmark index links 404 by @​thomhurst in
thomhurst/TUnit#5587
* docs: replace repeated benchmark link suffix with per-test
descriptions by @​thomhurst in
thomhurst/TUnit#5588
* docs: clearer distributed tracing setup and troubleshooting by
@​thomhurst in thomhurst/TUnit#5597
* fix: auto-suppress ExecutionContext flow for hosted services (#​5589)
by @​thomhurst in thomhurst/TUnit#5598
* feat: auto-align DistributedContextPropagator to W3C by @​thomhurst in
thomhurst/TUnit#5599
* feat: TUnit0064 analyzer + code fix for direct WebApplicationFactory
inheritance by @​thomhurst in
thomhurst/TUnit#5601
* feat: auto-propagate test trace context through IHttpClientFactory by
@​thomhurst in thomhurst/TUnit#5603
* feat: TUnit.OpenTelemetry zero-config tracing package by @​thomhurst
in thomhurst/TUnit#5602
* fix: restore [Obsolete] members removed in v1.27 (#​5539) by
@​thomhurst in thomhurst/TUnit#5605
* feat: generalize OTLP receiver for use outside TUnit.Aspire by
@​thomhurst in thomhurst/TUnit#5606
* feat: auto-configure OpenTelemetry in TestWebApplicationFactory SUT by
@​thomhurst in thomhurst/TUnit#5607
### Dependencies
* chore(deps): update tunit to 1.35.2 by @​thomhurst in
thomhurst/TUnit#5581
* chore(deps): update dependency typescript to ~6.0.3 by @​thomhurst in
thomhurst/TUnit#5582
* chore(deps): update dependency coverlet.collector to v10 by
@​thomhurst in thomhurst/TUnit#5600


**Full Changelog**:
thomhurst/TUnit@v1.35.2...v1.36.0

Commits viewable in [compare
view](thomhurst/TUnit@v1.35.2...v1.37.10).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=TUnit&package-manager=nuget&previous-version=1.35.2&new-version=1.37.10)](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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auto-register IHttpMessageHandlerBuilderFilter for IHttpClientFactory in TUnit.AspNetCore

1 participant