Fix ecto transaction in parallel preload

unflxw commented 3 months ago

This should fix a customer-reported issue (Intercom link) where an Ecto transaction performed inside an Ecto parallel preload causes the transaction to be closed early.

It makes sense to me, going from the event timelines reported by the customer, that this is the issue that is taking place: the sample showing the error, where Appsignal.Ecto.Repo is used, doesn't show the overarching transaction event or the query within it (because the transaction event is never closed and the query event is orphaned) which are present in the sample where Ecto.Repo is used, and it shows a lone commit event, which instead of closing the transaction (its tracer parent, within its process, which was spawned by the parallel preload) would have closed the Oban root span (its telemetry parent, from the root process that initiated the parallel preload)

That said, I have not been able to reproduce it (it is unclear to me how to trigger a transaction within a parallel preload) -- I have asked the customer to try this change, let's wait to hear back before we merge this.

Actually assert the parent is used

Instead of checking that the parent span was used or closed, a new variable named "parent" was assigned and ignored.

Prefer the tracer span to the telemetry span

When processing an Ecto event, if both a tracer span and a telemetry span are present, use the tracer span as the parent.

This should fix an issue where, when an Ecto transaction takes place inside an Ecto parallel preload, when the transaction's commit or rollback event attempts to close both its own span and the overarching transaction span (which would be the tracer parent, within the parallel preload's sub-process) it instead closes the transaction span's parent (which would be the telemetry parent, from the process that spawned the parallel preload) leaving the transaction span unclosed and potentially cutting the transaction short.