Fix don't overwrite the span status of unfinished spans

[markushi]

📜 Description

Span status should remain untouched when being force-finished, instead of setting DEADLINE_EXCEEDED.

💡 Motivation and Context

Fixes #2730

💚 How did you test it?

📝 Checklist

• I reviewed the submitted code.
• I added tests to verify the changes.
• No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
• I updated the docs if needed.
• Review from the native team if needed.
• No breaking change or entry added to the changelog.
• No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	329.35 ms	349.02 ms	19.67 ms
Size	1.72 MiB	2.26 MiB	547.99 KiB

Previous results on branch: fix/dont-set-deadline-exceeded-for-unfinished-spans

Startup times

Revision	Plain	With Sentry	Diff
b0815e7	286.90 ms	322.02 ms	35.12 ms
03438e5	282.12 ms	366.40 ms	84.28 ms

App size

Revision	Plain	With Sentry	Diff
b0815e7	1.72 MiB	2.29 MiB	575.25 KiB
03438e5	1.72 MiB	2.29 MiB	575.21 KiB

[codecov]

Codecov Report

Patch coverage: 87.56% and project coverage change: +0.18% 🎉

Comparison is base (9020d1b) 81.26% compared to head (964e980) 81.45%.
Report is 46 commits behind head on feat/7.0.0.

Additional details and impacted files

@@               Coverage Diff                @@
##             feat/7.0.0    #2859      +/-   ##
================================================
+ Coverage         81.26%   81.45%   +0.18%     
- Complexity         4560     4660     +100     
================================================
  Files               350      354       +4     
  Lines             16866    17152     +286     
  Branches           2272     2320      +48     
================================================
+ Hits              13706    13971     +265     
- Misses             2219     2231      +12     
- Partials            941      950       +9     

Files Changed	Coverage Δ
...racing/SentrySpanClientHttpRequestInterceptor.java	0.00% <0.00%> (ø)
...ebflux/SentryWebFilterWithThreadLocalAccessor.java	0.00% <0.00%> (ø)
...racing/SentrySpanClientHttpRequestInterceptor.java	0.00% <0.00%> (ø)
sentry/src/main/java/io/sentry/Breadcrumb.java	85.04% <ø> (ø)
sentry/src/main/java/io/sentry/HostnameCache.java	90.69% <ø> (ø)
sentry/src/main/java/io/sentry/Hub.java	75.17% <ø> (ø)
.../src/main/java/io/sentry/JsonObjectSerializer.java	84.21% <ø> (ø)
...ry/src/main/java/io/sentry/ProfilingTraceData.java	78.51% <ø> (ø)
.../main/java/io/sentry/ProfilingTransactionData.java	52.52% <ø> (ø)
sentry/src/main/java/io/sentry/Scope.java	96.72% <0.00%> (-0.36%)	⬇️
... and 83 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[romtsn]

LGTM, but only one concern - we're not executing this block for JPC stuff, do we still need to handle it (at least the start date), in case of parent transaction is aborted?

sentry-java/sentry/src/main/java/io/sentry/Span.java

Lines 193 to 222 in 695d3a3

	if (options.isTrimStart() || options.isTrimEnd()) {
	@Nullable SentryDate minChildStart = null;
	@Nullable SentryDate maxChildEnd = null;
	// The root span should be trimmed based on all children, but the other spans, like the
	// jetpack composition should be trimmed based on its direct children only
	final @NotNull List<Span> children =
	transaction.getRoot().getSpanId().equals(getSpanId())
	? transaction.getChildren()
	: getDirectChildren();
	for (final Span child : children) {
	if (minChildStart == null || child.getStartDate().isBefore(minChildStart)) {
	minChildStart = child.getStartDate();
	}
	if (maxChildEnd == null
	|| (child.getFinishDate() != null && child.getFinishDate().isAfter(maxChildEnd))) {
	maxChildEnd = child.getFinishDate();
	}
	}
	if (options.isTrimStart()
	&& minChildStart != null
	&& startTimestamp.isBefore(minChildStart)) {
	updateStartDate(minChildStart);
	}
	if (options.isTrimEnd()
	&& maxChildEnd != null
	&& (this.timestamp == null || this.timestamp.isAfter(maxChildEnd))) {
	updateEndDate(maxChildEnd);
	}
	}

[markushi]

LGTM, but only one concern - we're not executing this block for JPC stuff, do we still need to handle it (at least the start date), in case of parent transaction is aborted?

I looked into this in more detail now and I think it's fine this way as relay would anyway set the status to deadline_exceeded, rendering the meaning of start/end useless

[stefanosiano]

good!