Include unfinished spans in transactions

[bruno-garcia]

When a transaction is finished, the SDK tries to send the data to Sentry.

If an open Span (one that was not finished) exist in that transaction, the SDK removes that span before sending the data to Sentry.

That was done because Sentry will discard the transaction if unfinished spans are included. More context on this thread: getsentry/develop#274

An alternative solution used by the JavaScript SDK is to complete those spans with the timestamp of the finishing transaction and setting the status to deadline_exceeded . We should align with this approach as it shows something was going on even though it wasn't completed during the transaction.

Java: getsentry/sentry-java#1690
Cocoa: getsentry/sentry-cocoa#1303

[SimonCropp]

blocked by https://getsentry.atlassian.net/browse/INGEST-1109 (internal)

[bruno-garcia]

Created related issue on Relay: getsentry/relay#1244
Blocked by this ^

[bruno-garcia]

Worth noting once we start adding unfinished Spans we'll need to notify the minimum Self Hosted version required since the old version of Relay dropped transactions completely if they had an unfinished span (reason why all SDKs compensated that with dropping or lying about a span being finished)

[bruno-garcia]

@jamescrosswell @bitsandfoxes this is unblocked now and since we're on a major we should definitely include this one.

Android/Java SDKs added on 7.0.0 and it's the reason they mention new Self hosted min version 22.12:
From their README:

Since version 7.0.0 of this SDK, Sentry version >= 22.12.0 is required to properly ingest transactions with unfinished spans.

Related PRs:

• feat(general): Don't fail events containing a span with missing timestamp relay#1690
• Refrain from overwriting the span status for unfinished spans sentry-dart#1577
• feat: Include unfinished spans in transactions sentry-cocoa#1592
• Fix don't overwrite the span status of unfinished spans sentry-java#2859

[jamescrosswell]

From what I can tell, this was implemented about 3 years ago in #1296 so there's nothing to do here:

sentry-dotnet/src/Sentry/TransactionTracer.cs

Lines 373 to 380 in 175623c

	foreach (var span in _spans)
	{
	if (!span.IsFinished)
	{
	_options?.LogDebug("Deadline exceeded for Transaction {0} -> Span {1}.", SpanId, span.SpanId);
	span.Finish(SpanStatus.DeadlineExceeded);
	}
	}

[bruno-garcia]

From what I can tell, this was implemented about 3 years ago in #1296 so there's nothing to do here:

sentry-dotnet/src/Sentry/TransactionTracer.cs

Lines 373 to 380 in 175623c

	foreach (var span in _spans)
	{
	if (!span.IsFinished)
	{
	_options?.LogDebug("Deadline exceeded for Transaction {0} -> Span {1}.", SpanId, span.SpanId);
	span.Finish(SpanStatus.DeadlineExceeded);
	}
	}

The goal is to delete that code. We don't need to "fake" finish stuff since Relay will deal with that.
4.0 is a good time to change this (since it'll break transactions on self hosted Sentry older then 22.12.0