Skip to content

feat(aspire): opt-in live forwarding of resource logs into test output (#6341)#6345

Merged
thomhurst merged 2 commits into
mainfrom
feat/6341-forward-resource-logs
Jul 1, 2026
Merged

feat(aspire): opt-in live forwarding of resource logs into test output (#6341)#6345
thomhurst merged 2 commits into
mainfrom
feat/6341-forward-resource-logs

Conversation

@thomhurst

Copy link
Copy Markdown
Owner

Closes #6341.

Problem

AspireFixture<TAppHost> captures OpenTelemetry but never surfaces a resource's raw stdout/stderr. The blind spot is worst at startup: a resource that throws during boot exits before its OTel exporter flushes, and host-builder/DI/config failures go to stderr, not OTel. So "a resource won't come up" — the single most common Aspire-test failure — shows only a generic startup timeout, never the cause.

Main already has a post-mortem error-log dump on failure (DumpResourceLogsOnFailure, #6323), but it fetches after the fact and races to "(no logs available)" for already-exited resources. This adds a live, opt-in forward.

What changed

  • New AspireFixtureOptions bag, exposed via protected virtual AspireFixtureOptions Options. Existing virtual knobs untouched.
    protected override AspireFixtureOptions Options => new() { ForwardResourceLogs = true };
  • When on, subscribes to each selected resource's ResourceLoggerService.WatchAsync and forwards every line — prefixed [name] line — into the owning test's captured output.
  • ResourceLogNames selects a subset; null/empty = every resource ShouldWaitForResource picks.

Key correctness points

  • Subscribe before StartAsync, not after. A boot crash makes StartAsync throw/hang (that is the failure), so the issue's post-start reference impl would never subscribe. WatchAsync replays the buffered backlog, so the earliest lines are still delivered.
  • The pump runs on a background thread where TestContext.Current (an AsyncLocal) does not flow. Capture the owner context once at subscribe time; fall back to stderr (LogProgress) when init runs outside a test (session-shared).
  • Cancel pumps first in teardown so nothing writes to an already-finished test's output.
  • Refactored WatchResourceLogs to share the new PumpResourceLogsAsync.

Acceptance criteria

  • ✅ Opt-in, default off — no behaviour change for existing users.
  • ✅ Forwarded logs appear in the fixture-owner test's captured output.
  • ✅ Resources that exit during startup are still captured (live sub + backlog replay).
  • ✅ Resource-name prefix + include filter.

Known limitation

Logs attach to the fixture-owner test (the test whose execution triggered init) for the subscription's life — matching the issue's "test that owns the fixture lifecycle" wording and the AsyncLocal constraint. Per-request correlation remains the OTLP receiver's job.

Tests

  • New pure (no-Docker) ResourceLogSelectionTests for SelectResourceLogNames (explicit-subset, case-sensitivity, ShouldWaitForResource fallback, empty-names). 4/4 pass.
  • TUnit.Aspire.Core builds across net8.0/net9.0/net10.0.
  • Full live forwarding needs a real AppHost + Docker (not run in this environment).

#6341)

AspireFixture captures OpenTelemetry but never surfaces a resource's raw
stdout/stderr. The blind spot is worst at startup: a resource that crashes
during boot exits before its OTel exporter flushes, and host/DI/config errors
go to stderr, not OTel — so "a resource won't come up" failures show only a
generic startup timeout, not the cause.

Add an opt-in AspireFixtureOptions.ForwardResourceLogs. When on, subscribe to
each selected resource's ResourceLoggerService.WatchAsync *before* StartAsync
(a boot crash makes StartAsync throw/hang, so a post-start subscribe would
never run; WatchAsync replays the backlog so early lines are still delivered)
and forward every line, prefixed "  [name] line", into the owning test's
captured output.

- New AspireFixtureOptions bag, exposed via virtual Options; existing knobs
  untouched.
- ResourceLogNames selects a subset; empty = every ShouldWaitForResource pick.
- Pump runs on a background thread where TestContext.Current (AsyncLocal) does
  not flow — capture the owner context once; fall back to stderr when init runs
  outside a test.
- Refactor WatchResourceLogs to share PumpResourceLogsAsync.
- Cancel pumps first in teardown so nothing writes to a finished test's output.
- Pure (no-Docker) tests for SelectResourceLogNames.
@codacy-production

codacy-production Bot commented Jul 1, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 11 complexity

Metric Results
Complexity 11

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

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Reviewed AspireFixture.cs's new opt-in resource-log forwarding (subscribe-before-StartAsync, background pump tasks per resource, teardown ordering that cancels pumps before stopping the app), the new AspireFixtureOptions bag, and ResourceLogSelectionTests.cs. Specifically verified:

  • Concurrent writes from multiple resource pumps to the shared TestContext.Output are safe (ConcurrentStringWriter is lock-guarded).
  • Pump tasks are joined via Task.WhenAll/awaited in teardown, so no orphaned tasks or unobserved faults.
  • CTS linked to the run-abort token, cancelled before app stop, disposed and nulled correctly.
  • The new test fakes (FakeResource, FakeCompute, FakeChildCompute) correctly implement IResource/IComputeResource/IResourceWithParent and compile against Aspire.Hosting.
  • No dual-mode (source-gen/reflection), snapshot, VSTest, or AOT/reflection-annotation concerns — scoped entirely to TUnit.Aspire.Core, outside those rules' stated scope.

🤖 Generated with Claude Code

- Hoist the owner-null decision out of the per-line path: bind the write sink
  once per resource pump instead of branching every line; drop the now-dead
  WriteForwardedLine helper.
- Call the async pump directly instead of wrapping in Task.Run — it yields at
  its first await, so pumps still run concurrently without a thread-pool hop.
- Make _options a local (only read in InitializeAsync); tighten teardown.
- Consolidate the duplicated fake IResource classes (this test file and
  WaitForHealthyReproductionTests) into Helpers/FakeResources.cs.
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Code review (updated after latest push)

Re-reviewed the latest commit (f72a7b8d, "tidy resource-log forwarding after review") against the prior review of 92350c68. No issues found.

The follow-up commit is a clean simplification, verified against the surrounding code:

  • Dropped the _options field in favor of a local var options = Options; — it was only ever read within the same InitializeAsync call, so caching it in a field was unnecessary.
  • Dropped the extra Task.Run wrapper around each pump — PumpResourceLogsAsync is async and yields at its first await (the WatchAsync enumeration), so invoking it directly still runs the pumps concurrently without an extra thread-pool hop.
  • Inlined WriteForwardedLine into a per-resource Action<string> bound once at subscribe time — same behavior, one fewer indirection per line.
  • Removed the _logForwardCts = null; _logForwardTask = null; resets after teardown. Confirmed safe: StopAndDisposeCoreAsync is only ever invoked once, via StopAndDisposeAsync's lock (_teardownGate) { return _teardownTask ??= StopAndDisposeCoreAsync(); } single-shot gate, so there's no risk of these fields being read again in a stale state.
  • await _logForwardTask! (non-null-forgiving) is sound: _logForwardCts and _logForwardTask are always assigned together with no branch between them in StartForwardingResourceLogs, and the guard if (_logForwardCts is not null) covers the only path where they're set.
  • Test helper consolidation (FakeResources.cs shared by ResourceLogSelectionTests and WaitForHealthyReproductionTests) removes prior duplication with no behavior change — confirmed via InternalsVisibleTo wiring in TUnit.Aspire.Core.csproj that the shared internal fakes are still accessible from both test files.

No dual-mode, snapshot, VSTest, or AOT/reflection-annotation concerns — this stays scoped to TUnit.Aspire.Core/TUnit.Aspire.Tests, outside those rules.

🤖 Generated with Claude Code

@thomhurst thomhurst enabled auto-merge (squash) July 1, 2026 15:59
@thomhurst thomhurst merged commit 9941370 into main Jul 1, 2026
13 checks passed
@thomhurst thomhurst deleted the feat/6341-forward-resource-logs branch July 1, 2026 16:20
This was referenced Jul 2, 2026
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.

[TUnit.Aspire] Forward Aspire resource logs into captured test output (opt-in)

1 participant