Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Know when slow and frozen frames occurred during a transaction #1912

Closed
philipphofmann opened this issue Jun 22, 2022 · 9 comments
Closed

Know when slow and frozen frames occurred during a transaction #1912

philipphofmann opened this issue Jun 22, 2022 · 9 comments

Comments

@philipphofmann
Copy link
Member

Description

Currently, the SDK attaches slow and frozen frames to transactions. The problem is that the users don't know when these slow and frozen occurred during the transaction. We could attach the slow and frozen frames to individual spans or we could add timing information as proposed in #1910, but then Sentry would need to do some work to add this somehow to the transactions. We could even add spans for slow and frozen frames.

Maybe only add to UI spans, because there could be two spans running at the same time. Furthermore, slow and frozen frames might not really make sense for file IO, DB, or network spans.

@brustolin
Copy link
Contributor

We decided to create spans when a slow or frozen frame occurs.

@philipphofmann
Copy link
Member Author

philipphofmann commented Jul 4, 2022

Wait for #1910 to get merged before tackling this PR because it tracks the timestamps of slow and frozen frames.

@philipphofmann
Copy link
Member Author

FrameTimestamps are stored here

@property (nonatomic, readwrite) SentryMutableFrameInfoTimeSeries *frameTimestamps;

Tracer adds them to the profile here

frameInfo = SentryFramesTracker.sharedInstance.currentFrames;
[SentryFramesTracker.sharedInstance resetProfilingTimestamps];
SentryFramesTracker.sharedInstance.currentTracer = nil;

@kevinrenskers
Copy link
Contributor

@philipphofmann You explained the thinking behind this last week but now that I am looking into this, I am not sure how you wanted this to work.

  • We want to add a span for every slow or frozen frame to the transaction, right?
  • frameTimestamps contains the start_timestamp and end_timestamp of any slow or frozen frame. But how do we know if the frame was slow or frozen? If I loop over frameTimestamps, I have no clue what kind of frame it was? Adding a span with the text "slow or frozen frame" seems a bit weird?
  • frameTimestamps doesn't seem to hold actual timestamps (as in, dates), but something different. So how can we create spans with timestamps, so that they are shown in the UI at the correct place?

Can we go over this together? Thanks!

@armcknight
Copy link
Member

But how do we know if the frame was slow or frozen?

We talked about this in profiling but decided at the time we could compute this server-side, since we also send the expected framerates (as a time series since it can change on modern devices). But it would be easy to track them in separate arrays from each of these callsites: here and here.

And just so I understand what's ultimately being requested, I imagine we'd want to display something like this?

Screen Shot 2022-08-16 at 11 15 54 AM

@kevinrenskers
Copy link
Contributor

Check my PR where I had a working implementation using those callsites: #2028. But ultimately it doesn't result in a very useful thing in the Sentry UI, see comments in that PR as well.

@brustolin
Copy link
Contributor

We're closing this based on the result of #2028.

@philipphofmann
Copy link
Member Author

Reopened because of #2028 (comment)

@philipphofmann
Copy link
Member Author

After investigating this feature, it turned out that it is not that useful. If a lifecycle method needs 200 ms to complete, and we put a slow frame span below it, it doesn't add extra value because you can assume that already if you see that you have one slow frame. By just looking at the raw numbers of slow and frozen frames, you know that you have to fix something, and you should keep an eye on the spans that took the longest time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Status: In Progress 📈
Development

Successfully merging a pull request may close this issue.

4 participants