feat(playwright): propagate W3C trace context into browser contexts

[thomhurst]

Summary

• BrowserTest.NewContext now seeds each IBrowserContext with W3C traceparent/baggage headers derived from the per-test Activity started by the engine, so backend spans/logs correlate to the originating test (mirrors the TUnit.AspNetCore and TUnit.Aspire propagation handlers).
• New public virtual bool PropagateTraceContext => true on BrowserTest lets users opt out per-class (e.g. when hitting third-party domains where leaking a trace id is undesirable).
• User-supplied ExtraHTTPHeaders always win on key conflict; options are cloned (not mutated) only when we actually inject headers.
• No-op on netstandard2.0 (engine activity plumbing is #if NET only).

Test plan

• TUnit.Playwright builds clean on net8.0/net9.0/net10.0/netstandard2.0
• TUnit.PublicAPI Playwright_Library_Has_No_API_Changes snapshot regenerated and passes on all four TFMs
• Manual: run a test hitting a locally-instrumented backend, confirm server spans parent under the test activity in the collector

[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

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

[claude]

Code Review

Good implementation overall — the design follows the established TUnit.AspNetCore and TUnit.Aspire propagation patterns closely and handles edge cases correctly.

Strengths

• User headers correctly win on conflict: seeding merged with user headers using = (overwrite), then injecting trace headers via TryAdd (no-overwrite), is the right precedence.
• Clone-on-write: BrowserNewContextOptions is only cloned when headers are actually added. The before/after count trick efficiently avoids an allocation when there is nothing to inject.
• Correct #if NET guards throughout — netstandard2.0 is a clean no-op.
• InternalsVisibleTo addition is consistent with the existing TUnit.Aspire and TUnit.OpenTelemetry pattern.

---

Issues

1. Unnecessary defensive cast in the propagator callback

In PlaywrightActivityPropagator.cs lines 22-28, the carrier is IDictionary<string, string> dict pattern match will always succeed — the carrier is the exact Dictionary<string, string> we passed in. The check adds a type-test per injected header for no reason:

DistributedContextPropagator.Current.Inject(activity, headers, static (carrier, key, value) =>
{
    if (key is not null && value is not null)
        ((IDictionary<string, string>)carrier!).TryAdd(key, value);
});

This is more honest about the contract and marginally faster when many baggage entries are present.

2. Redundant TryBuildBaggageHeader call on .NET 8/9

PlaywrightActivityPropagator.InjectInto calls:

1. DistributedContextPropagator.Current.Inject(...) — on .NET 8/9 this resolves to PropagatorAlignment.W3CBaggagePropagator, which already calls TUnitActivitySource.TryBuildBaggageHeader internally and injects the baggage header.
2. Then TUnitActivitySource.TryBuildBaggageHeader(activity) is called again explicitly.

The second call is a no-op thanks to TryAdd, but it is redundant on .NET 8/9 (it IS needed on .NET 10+ where CreateW3CPropagator() handles only standard activity baggage).

Compare with ActivityPropagationHandler.InjectBaggage which guards with headers.Contains(BaggageHeader) before calling TryBuildBaggageHeader — that pattern makes the guard explicit. Either approach is correct, but a comment explaining why the explicit call is needed for .NET 10+ would help future maintainers avoid "cleaning up" what looks like duplication.

3. PropagateTraceContext docstring omits the netstandard2.0 caveat

The property is visible on all TFMs including netstandard2.0, but has no effect there (the #if NET guard in MergeTelemetryHeaders means it is silently ignored). Worth adding a <remarks> note that this property has no effect on netstandard2.0 targets.

---

Summary

The design is correct and the core logic is solid. The three points above are minor — (1) is a style/clarity issue, (2) is harmless redundancy that would benefit from a comment, (3) is a docs gap. None are blocking.

[claude]

Code Review (Follow-up)

Previous review raised three issues. Checking against the current HEAD (548fe7b0):

Previous Issues — Status

Issue #1 (Unnecessary defensive cast) — Resolved. The callback now uses the direct cast ((IDictionary<string, string>)carrier!).TryAdd(key, value) as recommended.

Issue #2 (Redundant TryBuildBaggageHeader call) — Addressed with a comment. The belt-and-braces comment clearly explains the TUNIT_KEEP_LEGACY_PROPAGATOR=1 scenario where DistributedContextPropagator.Current emits only Correlation-Context and the explicit W3C baggage emission is still needed. This is the right approach.

Issue #3 (Missing netstandard2.0 caveat in docs) — Resolved. The <remarks> block is present on PropagateTraceContext.

---

Fresh Look at Current Code

Strengths:

• Header precedence is correct: user headers seeded with = (overwrite), trace headers injected with TryAdd (no-overwrite). User values always win.
• Clone-on-write is efficient: the count diff trick (before/after) avoids allocating a new BrowserNewContextOptions when no trace headers are injected (e.g. user already had a traceparent).
• Activity.Current guard in both MergeTelemetryHeaders and InjectInto handles the race correctly.
• InternalsVisibleTo pattern is consistent with TUnit.Aspire and TUnit.OpenTelemetry.

One minor note (non-blocking):

Trace headers are only injected through the NewContext(BrowserNewContextOptions) overload. Callers who bypass BrowserTest.NewContext and call Browser.NewContextAsync() directly won't get propagation. This is arguably fine — it's not a regression and the doc comment on NewContext makes the contract clear — but worth a note in user-facing docs if you have any.

---

Summary

All previous issues have been addressed. The implementation is clean, consistent with the existing AspNetCore/Aspire propagation patterns, and handles edge cases correctly.

[claude]

Follow-up Review

All three issues raised in the previous review have been addressed in the follow-up commits.

Previous Feedback — Status

Issue 1 — Defensive is-pattern in propagator callback
Resolved. The pattern match was replaced with a direct cast ((IDictionary<string, string>)carrier!), which is correct since the caller always passes the concrete Dictionary<string, string>.

Issue 2 — Redundant TryBuildBaggageHeader call
Resolved. A comment was added explaining the intent: this is a belt-and-braces guard for the TUNIT_KEEP_LEGACY_PROPAGATOR=1 code path where the ambient propagator only emits Correlation-Context, not W3C baggage. The TryAdd makes it a no-op when W3C baggage was already injected.

Issue 3 — PropagateTraceContext docstring missing netstandard2.0 caveat
Resolved. A <remarks> block now explicitly states the property has no effect on netstandard2.0 targets.

---

New Observations

Logic correctness: The merged.Count == before early-return in MergeTelemetryHeaders handles all cases correctly:

• No activity → returned early before before is even assigned.
• Activity present, user headers conflict with trace headers → TryAdd silences the duplicate, count unchanged, original options returned untouched (user wins, no allocation).
• Activity present, new trace headers added → clone issued with merged dict.

Parallelism: Activity.Current flows with the async execution context, so concurrent tests each capture their own activity correctly when NewContext is called.

---

Summary

Clean, well-structured implementation that mirrors the TUnit.AspNetCore and TUnit.Aspire propagation patterns faithfully. All prior feedback addressed. No new concerns. LGTM.