Skip to content

Fix #5459 — Marten spans re-parented to their grandparent - #5465

Merged
jeremydmiller merged 1 commit into
masterfrom
fix/5459-marten-activity-parent
Sep 21, 2026
Merged

Fix #5459 — Marten spans re-parented to their grandparent#5465
jeremydmiller merged 1 commit into
masterfrom
fix/5459-marten-activity-parent

Conversation

@jeremydmiller

@jeremydmiller jeremydmiller commented Sep 21, 2026

Copy link
Copy Markdown
Member

Closes #5459. @bohdan-hukivskyi diagnosed this correctly from the source — parentActivity?.ParentId was indeed the mistake.

The defect

MartenTracing.StartActivity passed parentActivity?.ParentId to ActivitySource.StartActivity. That overload takes the parent's id, so handing it the parent id of the activity we were given parents the new span to its grandparent.

Concretely, EventTracingConnectionLifetime captures Activity.Current and passes it in. When that current activity is an incoming Server/Consumer span carrying a remote traceparent, every marten.connection span attached to the sending service's span rather than to the span that actually did the work — exactly as reported.

Why it went unnoticed

It is invisible in the two easy cases, which is worth spelling out because it shapes the tests:

ambient activity ParentId result correct?
none null span is a root yes
a root null null parent id → ActivitySource falls back to Activity.Current yes, by accident
has a parent the grandparent's id span parents to the grandparent no

So only a current activity that has a parent of its own exposes it, and any test written against a single root span would have passed.

The fix

Parent to the activity we were handed, falling back to Activity.Current when none is supplied. I passed the ActivityContext rather than the id string so the sampling flags and tracestate travel with the parent — strictly better than parentActivity?.Id, which would have fixed the parenting but dropped tracestate.

Tests

marten_activity_parenting, four tests:

  • marten_span_parents_to_the_current_activity_when_that_activity_has_a_remote_parent — the reported shape. Fails on master.
  • marten_span_parents_to_the_nearest_activity_not_an_ancestor — three deep, so a fix that reached for the root instead would still be caught. Fails on master.
  • marten_span_parents_to_the_current_activity_when_that_activity_is_a_root and marten_span_is_a_root_when_there_is_no_ambient_activity — the two accidentally-correct cases, pinned so the fix can't regress them. Both pass on master and must keep passing.

I verified the first two actually fail before the fix rather than assuming it.

Local runs, net9.0, PostgreSQL 17.10: the OTel-related CoreTests filter is 43 passed / 0 failed. The full CoreTests suite is 673 passed / 5 failed, and those same 5 (Bug_4185_codegen_conflict_projection_with_secondary_store_dependency, both Bug_4187_ancillary_store_isolation tests, rolling_range_partitioning.the_host_startup_pass_rolls_the_window_forward_and_retires_the_aged_partitions, jasper_fx_mechanics.divergent_application_assembly_reuse_warning_is_buffered_and_logged) fail identically on an unmodified tree in the same worktree — they are pre-existing and unrelated. Three of the five pass when run in isolation, so they look order- or parallelism-dependent; happy to raise that separately if it isn't already known.

🤖 Generated with Claude Code

https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3

@jeremydmiller @claude
Fix #5459 -- Marten spans re-parented to their grandparent
29e4b11 
MartenTracing.StartActivity passed `parentActivity?.ParentId` to
ActivitySource.StartActivity, which parents the new span to the parent OF the
activity it was handed rather than to that activity itself. Every Marten span
therefore hung off its grandparent whenever the ambient activity had a parent --
the reported shape, where an incoming Server/Consumer span carries a remote
traceparent and Marten's spans attach to the sending service's span instead of
to the span that actually did the work.

It survived because it is invisible in the two easy cases. With no ambient
activity ParentId is null and the span is correctly a root; with an ambient root
ParentId is also null, and a null parent id makes ActivitySource fall back to
Activity.Current, which is the right answer by accident. Only a current activity
that has a parent of its own exposes it.

Hands over the ActivityContext rather than the id string, so the sampling flags
and tracestate travel with the parent, and falls back to Activity.Current when
no parent is supplied.

The four new tests cover all of it: the reported remote-parent shape, a
three-deep chain that pins attachment to the immediate parent rather than any
other ancestor, and the two easy cases as regression guards. The first two fail
on master; the last two pass there and must keep passing.

Closes #5459

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3
@jeremydmiller
jeremydmiller merged commit e7d8d53 into master Sep 21, 2026
32 checks passed
@jeremydmiller
jeremydmiller deleted the fix/5459-marten-activity-parent branch September 21, 2026 13:43
This was referenced Sep 22, 2026
Merged
Merged
Closed
Closed
Closed
Closed
jeremydmiller added a commit that referenced this pull request Sep 27, 2026
@jeremydmiller @claude
Take JasperFx 2.75.1, and install the commit listeners the compliance…
7f6430f 
… suites register

#905 repairs two facts 2.75.0 shipped wrong: the tenanted-concurrency fact
contradicted itself, and the phantom-delete fact passed without discriminating. Adding
its control half turned stream_archiving_compliance red here, which is the gap working
as intended.

The cause was in Marten's own harness, not the product. The suite registers its
IDocumentCommitListener on ComplianceStoreConfig (#893) and then asserts on what
the listener saw, but MartenComplianceFixture never installed one -- so every
commit-listener fact was passing vacuously. "No phantom deletion was reported" is
trivially true of a store that reports nothing at all, and that is exactly what the new
control was written to catch.

With the listener actually running, both halves pass: Marten does not report a phantom
deletion for a document created and deleted inside one batch, and does report the real
one. No product change was needed.

The hand-built fixture store has no container, so it applies
DocumentCommitListenerAdapter directly; the hosted coordinator store registers the
listener in DI, which is the path a real application uses.

Also rides in 2.75.1: #900/#904 stops a Block worker inheriting Activity.Current
from construction time, which changes span parentage in daemon traces (the same class of
bug as #5465), and #887/#901, test-only.

EventSourcingTests 2330 tests, 0 failed. CoreTests unchanged at its 4 pre-existing
failures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3
@jeremydmiller jeremydmiller mentioned this pull request Sep 27, 2026
Merged
jeremydmiller added a commit that referenced this pull request Sep 27, 2026
@jeremydmiller @claude
Take JasperFx 2.75.1, and install the commit listeners the compliance…
1974600 
… suites register

#905 repairs two facts 2.75.0 shipped wrong: the tenanted-concurrency fact
contradicted itself, and the phantom-delete fact passed without discriminating. Adding
its control half turned stream_archiving_compliance red here, which is the gap working
as intended.

The cause was in Marten's own harness, not the product. The suite registers its
IDocumentCommitListener on ComplianceStoreConfig (#893) and then asserts on what
the listener saw, but MartenComplianceFixture never installed one -- so every
commit-listener fact was passing vacuously. "No phantom deletion was reported" is
trivially true of a store that reports nothing at all, and that is exactly what the new
control was written to catch.

With the listener actually running, both halves pass: Marten does not report a phantom
deletion for a document created and deleted inside one batch, and does report the real
one. No product change was needed.

The hand-built fixture store has no container, so it applies
DocumentCommitListenerAdapter directly; the hosted coordinator store registers the
listener in DI, which is the path a real application uses.

Also rides in 2.75.1: #900/#904 stops a Block worker inheriting Activity.Current
from construction time, which changes span parentage in daemon traces (the same class of
bug as #5465), and #887/#901, test-only.

EventSourcingTests 2330 tests, 0 failed. CoreTests unchanged at its 4 pre-existing
failures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3
jeremydmiller added a commit that referenced this pull request Sep 27, 2026
@jeremydmiller @claude
Bump JasperFx to 2.75.2 and Weasel to 9.35.2 (#5518)
d522ef9 
* Bump JasperFx to 2.75.0 and Weasel to 9.35.1

JasperFx 2.75.0 carries the three changes filed out of #5495, all of which reach a
Marten consumer through Marten's own package:

- #887/#890 — the source generator now emits
  [assembly: JasperFxSourceGeneratorApplied] from RegisterPostInitializationOutput, so
  an assembly it processed is distinguishable at runtime from one it never saw.
- #891/#895 — two copies of the generator in one compilation no longer collide
  with CS0433. Verified end-to-end against a locally packed Marten 9.39.1: a project
  that references the standalone analyzer package at the SAME version Marten bundles
  used to fail the build, and now compiles with one generator copy handed to csc.
- #892/#896 — the opt-in JasperFxEventsRequireSourceGenerator property turns a
  missing analyzer into build error JFXEVT900. Its targets live in JasperFx.Events's
  buildTransitive folder and do reach a consumer through a store package whose
  dependency carries exclude="Build,Analyzers".

#889 is the one behaviour change: the source-generated DetermineAction now
decides one action per batch rather than per event. EventSourcingTests, which is
enrolled in StreamArchivingCompliance, is green on it.

Weasel 9.35.1 is EF Core and SQL Server work (weasel#625, #627, #628, #629, #631,
#632) plus weasel#624. Nothing in it touches the Postgres paths Marten drives, and the
suites that would notice are green.

#5495 is NOT closed by this bump — see the PR description.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3

* Take JasperFx 2.75.1, and install the commit listeners the compliance suites register

#905 repairs two facts 2.75.0 shipped wrong: the tenanted-concurrency fact
contradicted itself, and the phantom-delete fact passed without discriminating. Adding
its control half turned stream_archiving_compliance red here, which is the gap working
as intended.

The cause was in Marten's own harness, not the product. The suite registers its
IDocumentCommitListener on ComplianceStoreConfig (#893) and then asserts on what
the listener saw, but MartenComplianceFixture never installed one -- so every
commit-listener fact was passing vacuously. "No phantom deletion was reported" is
trivially true of a store that reports nothing at all, and that is exactly what the new
control was written to catch.

With the listener actually running, both halves pass: Marten does not report a phantom
deletion for a document created and deleted inside one batch, and does report the real
one. No product change was needed.

The hand-built fixture store has no container, so it applies
DocumentCommitListenerAdapter directly; the hosted coordinator store registers the
listener in DI, which is the path a real application uses.

Also rides in 2.75.1: #900/#904 stops a Block worker inheriting Activity.Current
from construction time, which changes span parentage in daemon traces (the same class of
bug as #5465), and #887/#901, test-only.

EventSourcingTests 2330 tests, 0 failed. CoreTests unchanged at its 4 pre-existing
failures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3

* Take JasperFx 2.75.2 and Weasel 9.35.2, which unblocks #5495

#906/#907 fixes the inverted marker verdict this PR reported. The evidence list
now excludes a constructed generic projection type, so Snapshot<T>() /
SingleStreamProjection<T, TId> / AggregateStream<T> no longer let Marten.dll's own marker
answer a question about the user's build.

Verified against a Marten packed from this branch, in the #5495 topology (aggregate in an
assembly the analyzer never ran over, registered through SingleStreamProjection<T, Guid>).
The message now names the real cause:

  "The JasperFx.Events source generator left no trace in assembly App -- no marker
   attribute and no generated registration -- so it most likely never ran there.
   Reference Marten (or JasperFx.Events.SourceGenerator) directly from that project,
   and check the reference chain for ExcludeAssets="analyzers" or PrivateAssets="all" ..."

Before the fix that same call reported "DID run ... this is a code-shape problem, not a
build configuration one", which is the opposite of the truth for this shape.

The fix keeps the Any quantifier rather than switching to All, which matters: a
user-declared MyProjection : SingleStreamProjection<Agg, Guid> does get its evolver
emitted into its own assembly, so confirmation there is genuine even when the aggregate's
assembly has no marker.

Also in 2.75.2: #902 warns JFXEVT902 when two analyzer copies are present and the
dedupe is opted out, and puts an explanation of the CS0433 into the generated marker file
-- the only thing that reaches a compilation running none of our targets. #908
imports and exercises the shipped MSBuild targets in JasperFx's own CI.

Weasel 9.35.2 is weasel#634: an exhausted DropSchema retry is reported instead of
returning success.

EventSourcingTests 2330/0 failed. DocumentDbTests 1114/0. CoreTests unchanged at its
pre-existing set.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Oct 1, 2026
Closed
Closed
Merged
Closed
Closed
Closed
Open
Merged
Closed
This was referenced Oct 9, 2026
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Wrong parent for Marten activities when a remote parent exists

1 participant

@jeremydmiller