[thomhurst]

Summary

Shared data-source objects get an initialize <Type> span, but their disposal had none (see the TODO in ObjectTracker.UntrackObject). This PR wraps that disposal in a matching dispose <Type> span:

• Emitted on the TUnit.Lifecycle source. It applies only to disposable objects registered in TraceScopeRegistry with a shared scope (PerClass, PerAssembly, PerTestSession, Keyed), and only when that source has a listener. Per-test objects and runs with no listener take the old path unchanged.
• Parented by the same rules as the init span: per-class objects under the class activity, the other scopes under the assembly activity. The parent-context helper moved from ObjectLifecycleService to TUnitActivitySource.GetSharedObjectParentContext, so init and dispose share one copy.
• Tagged with tunit.trace.scope and tunit.test.class. A failed dispose records the error on the span and still propagates to the caller.
• The test context now passes through UntrackObjects, so the span can find its parent. UntrackObjectAsync (static properties) has no context, so its span is a root span carrying only the scope tag.

HtmlReportGenerator already treats the dispose prefix as a lifecycle span, so the report shows init and dispose cost side by side per fixture. I also added a short note on these spans to docs/docs/examples/opentelemetry.md.

Execution after metadata collection is shared, so source-generated and reflection modes get the same behaviour.

Validation

• New tests/TUnit.UnitTests/SharedObjectDisposeSpanTests.cs (4 tests): per-class span parent, scope tag and class tag; session span under the assembly activity; a failing dispose gives an error-status span and still returns the exception; per-test objects get no span.
• TUnit.UnitTests net10.0: 689 passed, 4 skipped, 0 failed.
• TUnit.OpenTelemetry.Tests net10.0: 42 passed.
• TUnit.Engine Release build, all TFMs: 0 warnings.
• No public API change: every new member is internal.

Closes #7287

Summary by CodeRabbit

• New Features

  • Shared data-source initialization and disposal spans now include lifetime-specific context, including scope and type information.
  • Spans for class-scoped objects appear under the test-suite span; session-, assembly-, and keyed-scoped objects appear under the test-assembly span.
  • Disposal failures are recorded as errors in the span.

• Documentation

  • Updated the OpenTelemetry guide with shared-object lifecycle span details.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 0e5ae16c-9ab3-4873-9780-5fc035ab8f82

Commits

Reviewing files that changed from the base of the PR and between 13ee2f3 and b18141d.

Files selected for processing: 5

• docs/docs/examples/opentelemetry.md
• src/TUnit.Core/TUnitActivitySource.cs
• src/TUnit.Core/Tracking/ObjectTracker.cs
• src/TUnit.Engine/Services/ObjectLifecycleService.cs
• tests/TUnit.UnitTests/SharedObjectDisposeSpanTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review.

Walkthrough

The change adds disposal spans for shared data-source objects. It uses shared-object parent-context rules for both initialization and disposal spans, adds disposal-span tests, and documents span placement and scope tags.

Changes: Shared-object lifecycle tracing

Layer / File(s)	Summary
Shared-object activity parent context src/TUnit.Core/TUnitActivitySource.cs, src/TUnit.Engine/Services/ObjectLifecycleService.cs	TUnitActivitySource.GetSharedObjectParentContext selects the parent context for shared-object lifecycle spans. Initialization now uses this method instead of a local helper.
Disposal span instrumentation and validation src/TUnit.Core/Tracking/ObjectTracker.cs, tests/TUnit.UnitTests/SharedObjectDisposeSpanTests.cs, docs/docs/examples/opentelemetry.md	ObjectTracker passes test context through disposal and emits a tagged disposal span for registered shared objects when listeners are present. Tests cover per-class, per-session, failing, and per-test disposal. The documentation describes span placement and scope tags.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature · Severity of issue fixed: Medium

Possibly related PRs

• thomhurst/TUnit#7248: It changes shared-instance reference counting, cache eviction, and disposal in ObjectTracker, which is the disposal path instrumented here.
• thomhurst/TUnit#7391: It changes when cached shared instances are retired and disposed, which determines when the new disposal spans run.

Merge Risk: Minimal · up to b1814

The disposal-span change is ready to merge after normal checks; no merge-blocking issue was established.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "feat(engine): add dispose spans for shared data-source objects" matches the main change. The PR adds a dispose <Type> span for disposal of disposable objects with PerClass, PerAssembly, P…
Linked Issues check		Issue #7287 is the only active direct link. Its coding requirement is a dispose span around shared-object disposal, parented by the initialize rules, tagged with tunit.trace.scope. The head imp…
Out of Scope Changes check		All changed files connect to objective #7287. ObjectTracker.cs adds the dispose span and passes the TestContext needed for parenting. TUnitActivitySource.cs gains GetSharedObjectParentContext, which b…

Full details: Title check

Explanation

The title "feat(engine): add dispose spans for shared data-source objects" matches the main change. The PR adds a dispose <Type> span for disposal of disposable objects with PerClass, PerAssembly, PerTestSession, and Keyed shared scopes. The changeset includes the ObjectTracker disposal path, the shared parent-context helper in TUnitActivitySource, the docs update, and the new tests. The title is short, specific, and uses a conventional commit prefix.

Full details: Linked Issues check

Explanation

Issue #7287 is the only active direct link. Its coding requirement is a dispose <Type> span around shared-object disposal, parented by the initialize rules, tagged with tunit.trace.scope. The head implements this. In ObjectTracker.UntrackObject (src/TUnit.Core/Tracking/ObjectTracker.cs), a disposable object with a registered shared scope is disposed through DisposeWithSpanAsync. That method runs on TUnitActivitySource.LifecycleSource with the name dispose {type}, the parent from TUnitActivitySource.GetSharedObjectParentContext, and the tunit.trace.scope tag. The parent helper keeps the rules from the issue: PerTestSession, PerAssembly, and Keyed use the assembly activity; PerClass uses the class activity. The helper was moved from ObjectLifecycleService, so initialize and dispose share one implementation. The TODO that named this gap was removed. The expected 20-second dispose span follows from wrapping DisposeAsync in the span. Per-test objects keep the old path, which the issue does not require to change. The issue does not require tests, and the PR-reported test results were not re-run here.

Full details: Out of Scope Changes check

Explanation

All changed files connect to objective #7287. ObjectTracker.cs adds the dispose span and passes the TestContext needed for parenting. TUnitActivitySource.cs gains GetSharedObjectParentContext, which both init and dispose use. ObjectLifecycleService.cs removes its local copy of the helper and calls the shared one, which avoids duplicated parent rules. SharedObjectDisposeSpanTests.cs covers the new span behaviour. docs/docs/examples/opentelemetry.md adds a note on the new spans. No public API change is reported, and the diff shows only internal members. The root span for UntrackObjectAsync (static properties, no test context) follows from the null context. The issue does not specify this path, so it is not a scope violation. No unrelated change was found.

Full details: Docstring Coverage

Explanation

Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 9 functions across 4 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit traced a fixture's trail,
And marked its teardown without fail.
A class span here, a session there,
Scope tags now follow through the air.
When disposal errs, the span records,
Then hops away to document the chores.}

[thomhurst]

@codex review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-02T09:30:00.058360Z	b18141d	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

Review: dispose <Type> spans for shared objects

Looks good. It resolves the TODO in ObjectTracker.UntrackObject and the change is small.

What I checked

• Moving GetSharedObjectParentContext into TUnitActivitySource gives init and dispose spans one parenting implementation. The added null testContext handling is what the root-span case in UntrackObjectAsync needs.
• The new work only runs when the lifecycle source has a listener and the object is registered in TraceScopeRegistry with a shared scope. Otherwise it takes the old disposer.DisposeAsync path, so the hot path is unaffected. The AsTask() allocation only happens when spans are on.
• Disposal still propagates the exception. The test for that covers the error status and the returned exception.
• The new code sits under #if NET, matching the init spans. Passing testContext through UntrackObjects is internal only, with no public API change.
• The four unit tests cover the main cases. StoppedActivityScope filters by AsyncLocal, so tests running in parallel don't pollute each other.

Minor suggestions (non-blocking)

• A shared object is disposed when the last holder releases it, so the dispose span's tunit.test.class tag and parent come from whichever test happened to release last. For PerClass this is the right class. For session or assembly scope the tag is arbitrary, and the docs imply a stable association. You could omit tunit.test.class for non-class scopes, or note this in the docs.
• The tags array construction in DisposeWithSpanAsync could use one array plus a conditional, but it's fine as written.

Approving from my side.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: e7317f6024

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[greptile-apps]

Confidence Score: 5/5

[Low impact] The PR appears safe to merge; the previous test gap is fixed and no new actionable issue was found.

Summary

Adds dispose <Type> spans for shared data-source objects on TUnit.Lifecycle.

• Shared data-source cleanup now emits matching disposal spans.

_{Reviews (2) · Last reviewed commit: "test: assert dispose spans have explicit..." · Reviewed by Greptile}

[thomhurst]

Dispositions for the non-blocking notes in the Claude review:

• tunit.test.class on non-class scopes: no change. The initialize span for shared objects (ObjectLifecycleService.InitializeWithSpanAsync) sets the same tag from whichever test initializes first, so the dispose span matches it. Dropping the tag from both spans for non-class scopes would change existing init-span output, which is out of scope here. tunit.trace.scope is the tag that defines the lifetime.
• tags array construction: no change, as the review says it is fine as written.

The Greptile thread (parent assertions could pass with no parent) is fixed in b18141d.

[thomhurst]

@codex review

[github-actions]

Review of PR 7396: dispose spans for shared data-source objects

I found no blocking issues. This reads as a clean, well-scoped change.

What I checked

• Shared parent logic. Moving the parent-context helper to TUnitActivitySource.GetSharedObjectParentContext means the init and dispose spans can't drift apart. The null testContext guard is what makes the static-property path (UntrackObjectAsync) work.
• Zero cost when unused. TryGetSharedDisposeScope checks HasListeners() and the TraceScopeRegistry scope before anything is allocated. Per-test objects and runs with no listener take the old disposer.DisposeAsync path unchanged. This respects the hot-path allocation guidance in AGENTS.md.
• Error handling. RunWithSpanAsync records the error on the span and the exception still propagates, so the existing cleanupExceptions aggregation in UntrackBucketAsync is unaffected. The test covers this.
• Both modes. The change is in the shared execution path (TUnit.Core and the engine's ObjectLifecycleService), so there is no source-generator/reflection divergence. The change is internal-only, so there is no public API impact. #if NET is consistent with the existing init-span code.
• Tests. The tests use a distinct explicit activity for each parent, so the assertions can't pass by both sides falling back to default. The AsyncLocal-scoped listener avoids cross-test leakage when tests run in parallel.

Minor, non-blocking suggestions

1. The parent span's lifetime is the one thing to be aware of. A per-class object is disposed by whichever test releases it last, and its parent is the class activity from that test's context. If the class activity has already been stopped by then, the span is still parented correctly by its context ID, but it will start after its parent's end time in the timeline. A short comment on DisposeWithSpanAsync stating this assumption would help future readers.
2. The two tag-array branches in DisposeWithSpanAsync could be a single array. Build it from testContext?.ClassContext.ClassType.FullName and omit the class tag when null. As written it's fine, but it duplicates the scope tag.
3. A test for the UntrackObjectAsync root-span path (no context, scope tag only) would round out the coverage. The PR describes that behaviour but no test exercises it.

Good work. Closing out the long-standing TODO with a shared helper is the right design.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: b18141d72a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[thomhurst]

Dispositions for the non-blocking notes in the second Claude review (head b18141d):

1. Parent lifetime: no change. The assumption does not apply. TestCoordinator calls _objectTracker.UntrackObjects(test.Context) (TestCoordinator.cs:228) before ExecuteAfterClassAssemblyHooks (:239), and that call is the one that finishes the class and assembly activities. So the last releasing test disposes the shared object while its class activity is still open, and the dispose span sits inside its parent on the timeline.
2. Single tag array: no change. As the review says, it's fine as written, and the first review already raised the same point.
3. Test for the UntrackObjectAsync root-span path: deferred, not blocking. That path differs from the tested paths only in the testContext is null guard in GetSharedObjectParentContext, which returns default. The tag and error handling go through the same DisposeWithSpanAsync the four tests already cover.

[github-actions]

Review: dispose <Type> spans for shared data-source objects

I read the diff only and did not run the tests. I found no blocking issues.

What looks good

• The new span code sits behind HasListeners() and the TraceScopeRegistry shared-scope check. Per-test objects and runs with no listener take the old disposer.DisposeAsync path unchanged, which matches the hot-path guidance.
• GetSharedObjectParentContext now lives in TUnitActivitySource, so init and dispose spans share one parenting rule instead of two copies. The testContext is null guard also covers UntrackObjectAsync.
• A failing dispose is recorded on the span and still reaches the caller. The test checks both.
• The tests give the class and assembly contexts distinct activities, so a parent assertion can't pass by both sides falling back to default. They also filter spans per async flow, so parallel tests don't leak into each other.
• Every new member is internal, so there is no public API change. Execution is on the shared path, so both discovery modes behave the same.

Minor, non-blocking

1. For PerAssembly, session and keyed objects, tunit.test.class is the class of whichever test released the last reference, not the class that created the object. That may mislead when reading a trace. Either omit the tag for non-PerClass scopes or say so in the docs. The tunit.trace.scope tag already carries the lifetime.
2. Both DisposeWithSpanAsync and the init span build tag arrays by hand. A small shared helper in TUnitActivitySource would stop the tag sets drifting apart over time.
3. .AsTask() allocates on every traced dispose. That only happens with a listener, so it's acceptable. If RunWithSpanAsync gains a ValueTask overload later, this call site could use it.

Nice, focused change.