Accept transactions containing unfinished spans

[bruno-garcia]

https://getsentry.atlassian.net/browse/INGEST-1109

Currently, Relay will drop transactions that contain Spans that don't have an end timestamp.

The code in question:

relay/relay-general/src/store/transactions.rs

Lines 110 to 130 in 5696786

	match (span.start_timestamp.value(), span.timestamp.value()) {
	(Some(start), Some(end)) => {
	if end < start {
	return Err(ProcessingAction::InvalidTransaction(
	"end timestamp in span is smaller than start timestamp",
	));
	}
	}
	(_, None) => {
	// XXX: Maybe do the same as event.timestamp?
	return Err(ProcessingAction::InvalidTransaction(
	"span is missing timestamp",
	));
	}
	(None, _) => {
	// XXX: Maybe copy timestamp over?
	return Err(ProcessingAction::InvalidTransaction(
	"span is missing start_timestamp",
	));
	}
	}

relay/relay-general/src/store/transactions.rs

Lines 110 to 130 in 5696786

	match (span.start_timestamp.value(), span.timestamp.value()) {
	(Some(start), Some(end)) => {
	if end < start {
	return Err(ProcessingAction::InvalidTransaction(
	"end timestamp in span is smaller than start timestamp",
	));
	}
	}
	(_, None) => {
	// XXX: Maybe do the same as event.timestamp?
	return Err(ProcessingAction::InvalidTransaction(
	"span is missing timestamp",
	));
	}
	(None, _) => {
	// XXX: Maybe copy timestamp over?
	return Err(ProcessingAction::InvalidTransaction(
	"span is missing start_timestamp",
	));
	}
	}

In practice, it's common in SDKs that a transaction is finished while an asynchronous task didn't complete yet. This in turn results in confusion, particularly when SDKs are being developed as the engineer doesn't see any transactions showing up.

Different SDKs chose different approaches on how to handle it. But it's mainly:

• Exclude Spans that are not finished before sending the transaction out
• Set the end timestamp to be the same as the transaction and status timeline_exceeded
  • Dart → https://github.com/getsentry/sentry-dart/blob/75bd585dbe9a7553f60f751614e6506521945b21/dart/lib/src/sentry_tracer.dart#L70-L76
  • Java → https://github.com/getsentry/sentry-java/blob/d1029084e146b5ff02b1359e8bdf0a7252ebf2e5/sentry/src/main/java/io/sentry/SentryTracer.java#L220-L236
  • Cocoa → https://github.com/getsentry/sentry-cocoa/blob/a92d2b74174cea7de992e184f19a38d16dbb84f1/Sources/Sentry/SentryTracer.m#L273-L281

We've identified the different implementations in the SDKs and are now leaning towards the latter with the reasoning that it's better to see what was going on during the transaction (could affect performance) even though it didn't complete. And the status code will help clearly indicate that this span didn't complete with the transaction.

Either way, handling this in the SDK adds to the complexity that now must iterate over all spans before capturing transactions to handle this.

With that, we agreed in a DACI that Relay should be changed to deal with transactions that contain unfinished spans:

Relay will no longer drop transactions that contain Spans without end timestamp

In this approach, the SDKs can remove the logic of handling unfinished spans. Relay now will be free to take the best approach to the problem:

Relay will add the transaction’s timestamp to the Span and the status deadline_exceeded, to indicate this span didn't complete during the transaction.

[jan-auer]

Thanks, added to backlog and will implement this. In the future, please feel free to directly submit a PR :)

[jjbayer]

@bruno-garcia this has now been implemented and deployed.