-
Couldn't load subscription status.
- Fork 73
feat(instrumentation): Add FirstDraw spans to activity instrumentation #1281
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1281 +/- ##
==========================================
- Coverage 64.30% 64.26% -0.04%
==========================================
Files 142 145 +3
Lines 3012 3087 +75
Branches 296 307 +11
==========================================
+ Hits 1937 1984 +47
- Misses 998 1025 +27
- Partials 77 78 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution. These changes seem to be related to this semconv PR where I've posted some concerns about things such as the type of signal to use for these kinds of events, as well as the utility of certain attributes such as nodes and depth followed by a lot of questions on what to do with them around certain scenarios. I'd like to get to a consensus over there first before moving forward with code changes.
|
The problem with detecting the first frame to be drawn is that, as you mentioned, it doesn't mean the Activity is ready to be consumed, it also doesn't necessarily mean the view tree is complete. If we are dealing with a Compose-based Activity, a minimal view tree will first be drawn and the rest will be filled in after the first frame is delivered. This means that the page complexity attribute being logged may be inaccurate - or at least not represent what you think it does. (I think there are further issues with using that metric in the first place, but one thing at a time 😅 ) I think tracking this event offers good utility, but we should probably properly contextualize it, and perhaps not add in extra data that may be noisy or misleading. |
|
i think the user has to tell when the activity is ready to be used (vs fully drawn etc) |
|
https://github.com/square/papa/tree/main also has a few things related to what we do here |
Agree, but that isn't really the purpose of tracking this event. As it stands the best way to track when an Activity is ready to interact / consume (ie a "Time to interactive") would be to track the Activity This PR is concerned with the TimeToInitialDisplay vital.
Yeah InitialDraw doesn't work too well in the context of Compose Activities. Open to ideas.
The complexity / depth attributes are removed! I think they aren't really needed for this new type of telemetry tbh
Agreed. I've removed it.
Yeah, the dev has to instrument something that can broadcast when an app / activity is ready to interact with. But we can automatically instrument, at least, when the Activity is ready to draw or has begun drawing. |
|
Hi @Doohl Again, thank you for taking the time to make this contribution and for your patience. I'm following up here based on this comment from the related semconv PR. I've covered a lot of details there that seem relevant to this work, and, in a nutshell, I'd like to try and see if this PR can help make progress in the semconv one. Please take a look at that comment for more details. Before diving deep into the implementation details that you propose here, I'd like to make sure we're all on the same page in terms of what the expected outcome is that you'd like to achieve. I know that the idea behind the semconv PR is to define a platform-agnostic span, which I think would be great, but for practical purposes, I'd like to get a better understanding of what it means specifically for Android. So, I'd like to use a visualization that relies on Android-specific terms to see if that helps to get a better understanding overall:
The image shows 3 possible scenarios (used to be 4, but then I realized that 3 is enough) that can be covered with a span. Which scenario do you think better covers the outcome that you'd expect from this implementation? |
| [DecorView](https://developer.android.com/reference/android/view/Window#getDecorView()) | ||
| * Attributes: | ||
| * `activity.name`: name of activity | ||
| * `screen.name`: name of screen |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: We've recently aligned on using app.screen.name for the attribute key, could we update to use that? Related PR: https://github.com/open-telemetry/semantic-conventions/pull/2744/files
| ### First Draw | ||
|
|
||
| * Type: Span | ||
| * Name: `FirstDraw` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: I'm currently proposing app.screen.time_to_first_draw in open-telemetry/semantic-conventions#2831. PR is still open so it may change but I don't think we should be using Pascal case regardless. I know that existing spans e.g. AppStart are in Pascal case but I recall both Jason and Hanson saying that this is something that needs to be fixed in the OTel Android SDK since it doesn't align with the usual casing convention of OTel.

fixes #1143
What is this change?
This PR adds instrumentation to track the initial draw time of Android activities. A new
FirstDrawspan is created as a child of the activity creation span (e.g.,AppStartorCreated), measuring the time from activity creation until the first frame is rendered on screen.The implementation:
ViewTreeObserver.OnDrawListenerto detect when the first draw occursWhat are the points of contention?
1. Screen view complexity attributes
Two new attributes are added to the
FirstDrawspan to capture screen complexity:screen.view.nodes(long): Total count of all View nodes in the view hierarchyscreen.view.depth(long): Maximum depth of the view hierarchyWe believe these metrics will add some value if users try to debug long
FirstDrawspans. Feedback here would be appreciated, though!2. FirstDraw value
FirstDrawspans aren't a perfect signal for when Activities become 'interactable'. Rather, this can help users identify issues with the UI pipeline. For that, users would have to manually instrument spans or events that fire off when the app or activity enters a state where the end user can actually start interacting with the app.3. FirstDraw span lifecycle
FirstDrawspans are children of theCreatedspans. However,FirstDrawspan can end long after the parent span has ended. This can create a confusing tracing experience, but we probably don't want to change the definition of the existingCreatedspan.Does it make sense to keep FirstDraw as a child of Created?
How was this tested?
Tested some scenarios with the demo app (API versions 28 and 30) to validate the correctness of the instrumentation. If you have any ideas how this could be better tested, feel free to opine!
Raw trace: https://www.codebin.cc/code/cmg9yjmxb0001jz037mtuj2v0:HXhjtG6XV3Vhfh8Ewy6r1S6KfapJtmgM1S1beucxBmpG