Only start a new session span if it's appropriate

[bidetofevil]

Goal

Pass down whether or not ending a session in the CurrentSessionSpan should start a new session, and make the check appropriately based on end type as well as whether background sessions are enabled.

Testing

Added new tests for the condition

[bidetofevil]

• Only start a new session span if it's appropriate #986 👈
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @bidetofevil and the rest of your teammates on Graphite

[codecov]

Codecov Report

Attention: Patch coverage is 84.44444% with 7 lines in your changes missing coverage. Please review.

Project coverage is 79.02%. Comparing base (8e95028) to head (09865d4).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #986      +/-   ##
==========================================
+ Coverage   78.97%   79.02%   +0.04%     
==========================================
  Files         416      416              
  Lines       11045    11065      +20     
  Branches     1764     1766       +2     
==========================================
+ Hits         8723     8744      +21     
  Misses       1539     1539              
+ Partials      783      782       -1     

Files	Coverage Δ
.../capture/envelope/session/SessionEnvelopeSource.kt	100.00% <100.00%> (ø)
...ture/envelope/session/SessionEnvelopeSourceImpl.kt	100.00% <100.00%> (ø)
...k/capture/envelope/session/SessionPayloadSource.kt	100.00% <100.00%> (ø)
...pture/envelope/session/SessionPayloadSourceImpl.kt	100.00% <100.00%> (ø)
...id/embracesdk/internal/spans/CurrentSessionSpan.kt	100.00% <100.00%> (ø)
.../embracesdk/session/message/FinalEnvelopeParams.kt	77.77% <100.00%> (+2.77%)	⬆️
...esdk/session/message/PayloadMessageCollatorImpl.kt	100.00% <100.00%> (ø)
...acesdk/session/orchestrator/SessionSnapshotType.kt	71.42% <80.00%> (+4.76%)	⬆️
...mbracesdk/internal/spans/CurrentSessionSpanImpl.kt	90.32% <66.66%> (+6.11%)	⬆️
...d/embracesdk/session/message/PayloadFactoryImpl.kt	93.10% <82.35%> (+0.69%)	⬆️

... and 2 files with indirect coverage changes

[fractalwrench]

This introduces undesirable behavior where calling sessionSpan.get() afterwards means folks could attempt to write to the span after the session has ended. Setting the reference to null if no new session is created should be enough to fix it?

[bidetofevil]

Good call. Will fix tomorrow!

[bidetofevil]

This brings up an interesting point about what happens if we do stuff when there is no session span. Looking at the code, we can't start a new span without a session span, and likely everything else that gets written to the session span will fail.

Before OTel, that data is just lost, right?

I'll put up the fix for this but lets figure this out before I merge this - we should have tests to verify this case and ensure we don't crash

[bidetofevil]

Added tests to verify no-session-span state. This should be good to go.

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details

Scanned Manifest Files

[bidetofevil]

Differentiated between initialization and whether a session span exists, as there doesn't necessarily have to be one now, if we are in the background and background activity isn't on.

[fractalwrench]

LGTM