refactor(opentelemetry): depend on TUnit.Core instead of umbrella TUnit

[thomhurst]

Summary

• TUnit.OpenTelemetry now references TUnit.Core directly instead of the umbrella TUnit package, dropping the transitive TUnit.Engine + TUnit.Assertions + Microsoft.Testing.Extensions footprint from its dependency chain.
• Cross-assembly contract for external span ingestion is now expressed via a small ExternalSpanSink hook in TUnit.Core (dependency inversion). ActivityCollector (Engine) registers itself as the sink at session start; OtlpReceiver (OpenTelemetry) pushes through the sink without taking a hard dependency on Engine.
• Moved 4 internal POCOs (SpanData, SpanEvent, SpanLink, ReportKeyValue) from TUnit.Engine/Reporters/Html/HtmlReportDataModel.cs to a new TUnit.Core/SpanData.cs so both ends of the contract share the same shape.

Test plan

• TUnit.Engine builds clean (all TFMs).
• TUnit.OpenTelemetry builds clean against TUnit.Core only.
• TUnit.OpenTelemetry.Tests 30/30 pass.
• TUnit.Aspire.Tests 92/92 pass.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric	Results
Complexity	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Code Review

This is a well-motivated refactor. Inverting the dependency so TUnit.OpenTelemetry references TUnit.Core instead of the umbrella package is the right architectural direction — it removes a transitive blob of Engine + Assertions from the OpenTelemetry package's footprint, which is the correct application of the Dependency Inversion Principle here.

A few observations:

---

1. ExternalSpanSink.Current is not a volatile read

Interlocked.CompareExchange provides a full memory barrier at write-time, but reading _sink via the plain property Current does not get that guarantee:

public static Action<SpanData>? Current => _sink;  // plain read

On ARM or with aggressive JIT reordering, a thread calling ProcessTraces could see a stale (cached) null even after Register has already stored a non-null delegate. The fix is trivially cheap:

public static Action<SpanData>? Current => Volatile.Read(ref _sink);

ActivityCollector._current has the same non-volatile read pattern — both should be consistent.

---

2. ExternalSpanSink is internal but lives in TUnit.Core — that boundary is leaky

Both ExternalSpanSink and the four data types (SpanData, SpanEvent, SpanLink, ReportKeyValue) are internal and bridged via InternalsVisibleTo. That works, but it exposes TUnit.Core's InternalsVisibleTo list to what are essentially engine-level concerns. Any future third-party package that wants to do the same thing cannot.

A forward-looking alternative is to make the two small files — ExternalSpanSink and the data model — public but [clearly documented as infrastructure-level]. The data classes are pure POCOs with no behaviour; making them public has negligible risk and removes the InternalsVisibleTo coupling. The ExternalSpanSink static itself could stay internal while the delegate signature (Action<SpanData>) becomes public through a named delegate type if desired.

This is not a blocker for the PR as-is; the InternalsVisibleTo approach is well-understood and the set of trusted assemblies is small and stable.

---

3. ReportKeyValue is a reporting artefact — its name is misleading in TUnit.Core

ReportKeyValue is a JSON serialisation DTO whose name implies it belongs to the HTML reporter. Now that it lives in TUnit.Core and is shared with the OpenTelemetry package, a name like SpanAttribute or KeyValueData would communicate its purpose more clearly at the Core layer.

This is a cosmetic concern and entirely optional.

---

4. SpanLink is not registered in HtmlReportJsonContext

HtmlReportJsonContext explicitly registers SpanEvent and ReportKeyValue but not SpanLink:

[JsonSerializable(typeof(ReportKeyValue))]
[JsonSerializable(typeof(SpanData))]
[JsonSerializable(typeof(SpanEvent))]

SpanLink is reachable through SpanData.Links so the source-generated context would include it transitively via SpanData, but an explicit registration is consistent with how the other peer types are listed. Worth adding for clarity.

---

5. OtlpReceiverIngestionTests.Receiver_ParsedTrace_ReachesActivityCollector still references ActivityCollector directly

The stated goal of the refactor is that TUnit.OpenTelemetry no longer depends on TUnit.Engine. But TUnit.OpenTelemetry.Tests still references ActivityCollector.Current directly (line 17 of OtlpReceiverIngestionTests.cs). Tests are not the shipped package, so this doesn't break the stated goal, but it does mean the test file imports TUnit.Engine.Reporters.Html and therefore the test project still carries the Engine dependency.

A cleaner test would assert the outcome through ExternalSpanSink or would use ActivityCollector only for setup, which is arguably what ActivityCollectorIngestionTests already does well. Not a blocker.

---

Summary

The design — ExternalSpanSink as a single-slot, first-wins, Interlocked-guarded process-wide hook — is simple, low-allocation, and appropriate for the single-session lifecycle. The POCO movement and InternalsVisibleTo approach are pragmatic. The main actionable item is the missing Volatile.Read on the Current property (point 1); everything else is quality-of-life.

[claude]

Follow-up Review (after commit ba7baec)

Both actionable items from the previous review have been addressed — thank you for the quick turnaround.

✅ Fixed: Volatile.Read on ExternalSpanSink.Current

The missing memory-ordering guarantee is now correct:

public static Action<SpanData>? Current => Volatile.Read(ref _sink);

This pairs properly with the full fence from Interlocked.CompareExchange on the write side and is safe on ARM/weak-memory-model architectures.

✅ Fixed: SpanLink registered in HtmlReportJsonContext

[JsonSerializable(typeof(SpanLink))]

Now explicitly listed alongside the other peer types — consistent and correct for source-generated JSON.

---

Remaining non-blocking items (unchanged by design, acknowledged as acceptable)

• ExternalSpanSink internals / InternalsVisibleTo — The internal + InternalsVisibleTo approach is pragmatic and the trusted-assembly set is small and stable. Acceptable for the current scope.
• ReportKeyValue naming — Still a cosmetic mismatch with its new home in TUnit.Core, but low risk. Could be a follow-up rename if desired.
• OtlpReceiverIngestionTests references ActivityCollector directly — Tests still carry the Engine dependency, but the shipped package (TUnit.OpenTelemetry) does not. The stated goal of the refactor is achieved.

---

Overall

The design is clean — a single-slot, first-wins, Interlocked-guarded sink is exactly the right shape for this single-session lifecycle, and the POCO migration gives both sides of the contract a shared type without any public API surface exposure. With the two fixes in place, this PR is in good shape. LGTM.