-
Notifications
You must be signed in to change notification settings - Fork 375
Add jank events for slow app rendering #2157
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
Changes from 14 commits
e6842c8
363586f
ec5e9ce
b595d48
d4a0d6f
0e2ac95
1cdcb8a
6d596b5
a13d954
459ba52
d3787ee
aecd2ad
9226be0
cf8a17d
7cfd1ed
723789b
37fc32d
bb3d1cc
4e7c077
9dce756
7db392d
c182d6c
ed15336
060b832
06071f2
6fc0e4d
5731f60
35b3e06
5074db9
fe90e52
e364dcc
6421152
a497728
978aafe
f0ed490
7e21a36
05dba6d
c338232
b32b0d9
83b5544
a413c6f
2621f69
0ab3f56
8b52ebd
8b6bdbe
4b2c0e6
3b64cb3
2c7acd7
2ba21a8
16c9c52
8638e7b
cc79a58
728e5d1
45dea8c
7a2ed6c
463b7f5
8dd48ff
1b0847a
65f3ecd
3f75abd
1c08e9a
84bf9b1
0728578
b52d42e
d7f6727
94d43d3
87da051
0887e29
08c3cf1
1a3a9f1
990b0a5
8c83b04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| # Use this changelog template to create an entry for release notes. | ||
| # | ||
| # If your change doesn't affect end users you should instead start | ||
| # your pull request title with [chore] or use the "Skip Changelog" label. | ||
|
|
||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
| change_type: enhancement | ||
|
|
||
| # The name of the area of concern in the attributes-registry, (e.g. http, cloud, db) | ||
| component: app | ||
|
|
||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
| note: Defines a new jank event in the app domain | ||
|
|
||
| # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. | ||
| # The values here must be integers. | ||
| issues: [2157] | ||
|
|
||
| # (Optional) One or more lines of additional information to render under the primary note. | ||
| # These lines will be padded with 2 spaces and then inserted directly into the document. | ||
| # Use pipe (|) for multiline entries. | ||
| subtext: |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,8 @@ This document defines events related to client-side applications | |
| - [Click Events](#click-events) | ||
| - [Event: `app.screen.click`](#event-appscreenclick) | ||
| - [Event: `app.widget.click`](#event-appwidgetclick) | ||
| - [Jank Events](#jank-events) | ||
| - [Event: `app.jank`](#event-appjank) | ||
| - [Attributes](#attributes) | ||
|
|
||
| <!-- tocstop --> | ||
|
|
@@ -80,9 +82,62 @@ Use this event to indicate that visual application component has been clicked, t | |
| <!-- END AUTOGENERATED TEXT --> | ||
| <!-- endsemconv --> | ||
|
|
||
| ## Jank Events | ||
|
|
||
| ### Event: `app.jank` | ||
|
|
||
| Jank is a disruption in UI rendering, resulting in a display that can feel | ||
| sluggish or even unresponsive/frozen. Applications that are able to detect | ||
| jank can report it with the following events: | ||
|
|
||
| <!-- semconv event.app.jank --> | ||
| <!-- NOTE: THIS TEXT IS AUTOGENERATED. DO NOT EDIT BY HAND. --> | ||
| <!-- see templates/registry/markdown/snippet.md.j2 --> | ||
| <!-- prettier-ignore-start --> | ||
| <!-- markdownlint-capture --> | ||
| <!-- markdownlint-disable --> | ||
|
|
||
| **Status:**  | ||
|
|
||
| The event name MUST be `app.jank`. | ||
|
|
||
| This event indicates that the application has detected substandard UI rendering performance. | ||
|
|
||
| Jank happens when the UI is rendered slowly enough for the user to experience some disruption or sluggishness. Jank may be classified with varying degrees, such as `slow` or `frozen`, when rendering times exceed a given threshold. | ||
|
|
||
| | Attribute | Type | Description | Examples | [Requirement Level](https://opentelemetry.io/docs/specs/semconv/general/attribute-requirement-level/) | Stability | | ||
| |---|---|---|---|---|---| | ||
| | [`app.jank.count`](/docs/registry/attributes/app.md) | int | A count of the number of frame renders that experienced jank. [1] | `9`; `42` | `Required` |  | | ||
| | [`app.activity.name`](/docs/registry/attributes/app.md) | string | The name of the Activity at the time of the event or span start. [2] | `myActivity`; `checkout` | `Recommended` |  | | ||
| | [`app.jank.period`](/docs/registry/attributes/app.md) | double | The time period, in seconds, for which this jank is being reported. | `0.5`; `1.0`; `10.0` | `Recommended` |  | | ||
| | [`app.jank.threshold_ms`](/docs/registry/attributes/app.md) | int | The minimum rendering threshold for this type of jank, in milliseconds. | `16`; `700`; `1024` | `Recommended` |  | | ||
| | [`app.jank.type`](/docs/registry/attributes/app.md) | string | A categorization of the type of jank. | `slow`; `frozen` | `Recommended` |  | | ||
|
|
||
| **[1] `app.jank.count`:** Depending on platform limitations, the value provided MAY be approximation. | ||
|
|
||
| **[2] `app.activity.name`:** This is an Android specific attribute. | ||
|
|
||
| --- | ||
|
|
||
| `app.jank.type` has the following list of well-known values. If one of them applies, then the respective value MUST be used; otherwise, a custom value MAY be used. | ||
|
breedx-splk marked this conversation as resolved.
|
||
|
|
||
| | Value | Description | Stability | | ||
| |---|---|---| | ||
| | `frozen` | Jank that causes the user to suspect that the application is frozen. [3] |  | | ||
| | `slow` | Jank is classified as observably slow. [4] |  | | ||
|
|
||
| **[3]:** Recommended threshold for frozen is 700ms. | ||
|
|
||
| **[4]:** Recommended threshold for slow is 16ms. | ||
|
|
||
| <!-- markdownlint-restore --> | ||
| <!-- prettier-ignore-end --> | ||
| <!-- END AUTOGENERATED TEXT --> | ||
| <!-- endsemconv --> | ||
|
|
||
| ## Attributes | ||
|
|
||
| See the [app attributes](/docs/registry/attributes/app.md) registry for all | ||
| See the [app attributes](/docs/attributes-registry/app.md) registry for all | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if you revert this change it will fix the link check failure |
||
| application-related attributes that may appear on telemetry items. | ||
|
|
||
| [DocumentStatus]: https://opentelemetry.io/docs/specs/otel/document-status | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,3 +37,26 @@ groups: | |
| requirement_level: opt_in | ||
| - ref: app.screen.coordinate.y | ||
| requirement_level: opt_in | ||
| - id: event.app.jank | ||
| stability: development | ||
| type: event | ||
| name: app.jank | ||
|
breedx-splk marked this conversation as resolved.
|
||
| brief: > | ||
| This event indicates that the application has detected substandard UI | ||
| rendering performance. | ||
| note: > | ||
| Jank happens when the UI is rendered slowly enough for the user to | ||
| experience some disruption or sluggishness. Jank may be classified with | ||
| varying degrees, such as `slow` or `frozen`, when rendering times exceed | ||
| a given threshold. | ||
| attributes: | ||
|
breedx-splk marked this conversation as resolved.
|
||
| - ref: app.jank.count | ||
| requirement_level: required | ||
|
Comment on lines
+53
to
+54
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might also be better suited as a body attribute. also am unsure how this could be >1 as events should be be emitted in a similar fashion to logs ie as aggregation not possible.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So jank events are modelled as some number of dropped frames over a certain period in such a way that it is perceivable by a user. It is not that every single dropped frame will create an event - this is not the dropped frame event, after all. This is why the count and period attributes are important: to describe what kind of jank event was captured. |
||
| - ref: app.jank.type | ||
| requirement_level: recommended | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't it be required?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. by allowing it to be
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what benefit reporting just the threshold would be |
||
| - ref: app.jank.threshold_ms | ||
| requirement_level: recommended | ||
|
Comment on lines
+57
to
+58
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would do this as a body attribute as it doesn't need to be in the registry. |
||
| - ref: app.jank.period | ||
| requirement_level: recommended | ||
| - ref: app.activity.name | ||
| requirement_level: recommended | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,3 +62,43 @@ groups: | |
| A widget is an application component, typically an on-screen visual | ||
| GUI element. | ||
| examples: [ "submit", "attack", "Clear Cart" ] | ||
| - id: app.jank.count | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not needed if moved to body attribute or metric
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, but metrics aren't on the table right now for mobile...and using flat/basic attributes is currently much simpler than defining structured event bodies. There is prior art for defining attributes for events, and these events are pretty simple (eg. not a lot of deep structure or complicated types involved).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the overall idea, @breedx-splk. What's very confusing to me is that it looks like a metric, swims like a metric, and quacks like a metric... And yet it's defined as a log event? I noticed that you recently mentioned that you've added a period attribute to keep the count somewhere in memory and only sending the event after that period of time, and that's something that already happens with metrics too! 😅 - I just don't understand the idea of having to reinvent metrics as log events, it makes me wonder why do we have different signals then? In theory, I guess we can also replace spans by logs, but it doesn't seem like that's the way to go. I was thinking what would make sense to me for this to be a log event, and as such, I think it would be simpler, like "There was a UI jank at this point in time" with maybe an attribute that shows for how long the UI was stuck and where in the app that happened. It would be helpful for me as an app developer to know that there are UI janks and where, and with the time attr I would be able to correlate that data with the host device info so that I can check how much priority I should give this issue since it's not uncommon to have UI jank in older devices. The problem with that approach is that we could potentially send hundreds of events in a very short period of time (unless we limit the amount of events per second or something like that, where we would only capture the first jank within a second and then ignore the rest during that time, for example), which is why I guess we came up with the "count" attribute. But by introducing a count value and a period of time, we essentially turn this into a metric (unless I missed something important), so it seems like it's just the nature of this event. We've recently added metrics support in OTel Android, so I'm not sure why it would be a problem to go with it as a metric. It's a tricky event to define for sure, it's probably a good topic for the SIG meeting.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree it certainly appears to me to be a metric to me so I would rather see it expressed in that manner. The event should be recorded as soon as it happens to provide an indication that something happened.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mentioned this in @breedx-splk's PR over in the Android repo, but I don't think making this a metric, thereby erasing the "when did it happen?" dimension from the signal is a good idea. That is the most important thing when this data is being used, and no amount of aggregation will give you more information than knowing when it happened within a session and what preceded and followed it. Count in this case is metadata for the jank event and gives some qualification as to the magnitude of the jank event being seen by the user. Totalling up all the dropped frames but not stating when they were dropped renders this data pretty useless. We are not trying to count all the dropped frames in the entire fleet (or even on a single device) - we are trying to model the fact that users are seeing janky scrolling or unresponsive UI.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually metrics support exemplars which preserve that link hence not rendering them useless in fact for me make them more useful then events as you can see how widespread it is and then drill down. |
||
| type: int | ||
| stability: development | ||
| brief: A count of the number of frame renders that experienced jank. | ||
| note: > | ||
| Depending on platform limitations, the value provided | ||
| MAY be approximation. | ||
| examples: [ 9, 42 ] | ||
| - id: app.jank.type | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this be app.rendering.state so that it is generic?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not my preference, because this is attempting to specifically define a type of jank (slow screen rendering performance). It's not really a "rendering state", so I don't think
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wouldn't add this attribute, I think it might be more confusing than helpful as its items don't depend on constants but instead users can choose what values to set as thresholds, so if they decide to change them at some point (or just switch from an instrumentation impl to another with different defaults), then they would get misleading values across time. Plus, I think this categorization makes more sense to do on the query side, where I can define my threshold at query time, without having to rely on how it was categorized in the past. On top of that, it adds more size to the event itself, making it not too disk space friendly. If someone needs it in the future, and they explain their use case in a way that we can get a better idea of how to make this attribute useful for them, I think we should add it then. But right now, since we don't know exactly what's needed for this event, I wouldn't add it because I think it's better to add stuff as the need arises, rather than having to make breaking changes and/or removing stuff later.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we were to add it then the naming should be generic rather than jank so that it can be used on other os.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Jank is a specific type of UI artifact whereby some amount of frames were not rendered, leading to choppy or frozen UI. It's not state, per se, but more like the occurrence of something bad that is perceivable to users. I can see the desire to classify this into different types, but in a way that is what the period and count attributes do. Having a high-level box to put it into would require defining the parameters of said box, and I don't think we know enough how the usage to clearly say what they would be. I think we should probably skip this for now. And if we want to go ahead and do it, I wouldn't make it more generic, as the whole idea of semantic conventions is to make them specific and meaningful. If there are other UI screen events we want to track, we can define a new event, as the underlying attributes would likely not be the same as what we have for jank. |
||
| stability: development | ||
| brief: A categorization of the type of jank. | ||
| type: | ||
| members: | ||
| - id: slow | ||
| value: 'slow' | ||
| brief: 'Jank is classified as observably slow.' | ||
| note: 'Recommended threshold for slow is 16ms.' | ||
| stability: development | ||
| - id: frozen | ||
| value: 'frozen' | ||
| brief: 'Jank that causes the user to suspect that the application is frozen.' | ||
| note: 'Recommended threshold for frozen is 700ms.' | ||
| stability: development | ||
| examples: [slow, frozen] | ||
| - id: app.jank.threshold_ms | ||
|
breedx-splk marked this conversation as resolved.
|
||
| stability: development | ||
| type: int | ||
| brief: The minimum rendering threshold for this type of jank, in milliseconds. | ||
| examples: [16, 700, 1024] | ||
| - id: app.jank.period | ||
| stability: development | ||
| type: double | ||
| brief: The time period, in seconds, for which this jank is being reported. | ||
| examples: [ 0.5, 1.0, 10.0 ] | ||
| - id: app.activity.name | ||
| stability: development | ||
| type: string | ||
| brief: The name of the Activity at the time of the event or span start. | ||
| note: This is an Android specific attribute. | ||
| examples: ['myActivity', 'checkout'] | ||
Uh oh!
There was an error while loading. Please reload this page.