Calculate frame count delta when another activity starts to avoid double counting

[adinauer]

📜 Description

Builds on top of the delta calculation for frame counts but calculates the delta earlier in case another Activity is started before tracking of the previous one has ended.

💡 Motivation and Context

If users turn off auto finish of transactions and instead finish them manually in onStop of the activity we would be double reporting some of the frames that occur between onStart of the next Activity and onStop of the previous Activity.

💚 How did you test it?

Unit tests; Sample App

📝 Checklist

• I reviewed the submitted code
• I added tests to verify the changes
• I updated the docs if needed
• No breaking changes

🔮 Next steps

[adinauer]

We fall back to FrameCounts(0, 0, 0) so we don't calculate later but freeze it now. This is to avoid double counting.

[github-actions]

	Fails
🚫	Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Calculate frame count delta when another activity starts to avoid double counting ([#2273](https://github.com/getsentry/sentry-java/pull/2273))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 2306de1

[philipphofmann]

What happens if the user finishes the activity transaction manually at a random point, such as onDestroy or on a background thread?

[codecov-commenter]

Codecov Report

Base: 80.62% // Head: 80.62% // No change to project coverage 👍

Coverage data is based on head (2306de1) compared to base (5b7f10b).
Patch has no changes to coverable lines.

Additional details and impacted files

@@                         Coverage Diff                          @@
##             fix/6_4_fix_frames_tracking_delta    #2273   +/-   ##
====================================================================
  Coverage                                80.62%   80.62%           
  Complexity                                3368     3368           
====================================================================
  Files                                      240      240           
  Lines                                    12388    12388           
  Branches                                  1646     1646           
====================================================================
  Hits                                      9988     9988           
  Misses                                    1791     1791           
  Partials                                   609      609           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[adinauer]

What happens if the user finishes the activity transaction manually at a random point, such as onDestroy or on a background thread?

Then the finishedCallback of the transaction is executed, calling setMetrics. We then check if there's a pre-calculated delta of frames. If there's not we calculate frame delta as was done before. Does that answer your question?

[adinauer]

@philipphofmann @romtsn @brustolin @markushi and me just discussed the PR and decided we don't want to merge it as we think it's easier to find the root problem (causing slow / frozen frames) if we show them on any transaction that is running when the slow / frozen frames occur.

It's OK that the SDK double counts slow and frozen frames for overlapping transactions. The SDK can't track down its root cause when a slow or frozen frame happens. If the SDK adds them two both transactions, the transaction with the root cause will always have a higher slow and frozen frames rate, and users know to address this transaction first. Suppose the SDK would only add them to the transactions generated by the visible activity. In that case, it could happen that a background task kicked off by a previous activity puts slow and frozen frames on multiple activities loaded after it but never on the activity causing the slow and frozen frames.

Long term it would be nice to know about all transactions that are running at the same time as it would make it easier to spot the root cause.

The tracking code keeps counting frames until a transaction is finished and then takes all the frame counts since the transaction was started. Due to the fact that any Activity being started causes frames to be tracked in the same global place, all previous transactions also receive the counts until they finish.

For the future we could change the code to always track slow and frozen frames for every Activity. Then anyone interested can ask for snapshots of the counts at any point in time and calculate a diff. This would then allow us to calculate slow / frozen frames for UI events and any other transaction (see #2061 and #2101).

[philipphofmann]

Thank you, @adinauer, for the detailed comment.