Skip to content

feat(mobile-ui): Add slow, frozen, and total frames metrics #3473

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

Merged

Conversation

narsaynorath
Copy link
Member

@narsaynorath narsaynorath commented Apr 23, 2024

Adds gauge metrics for slow, frozen, and total frames metrics for the Mobile UI Starfish module. The tags I've chosen are common and already applied to other mobile conditions.

The data source for these metrics come from the data property on spans, which I've extracted into measurements on spans.

@narsaynorath narsaynorath requested a review from a team as a code owner April 23, 2024 03:25
Copy link
Contributor

@iker-barriocanal iker-barriocanal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transactions have these values are measurements (see test). Spans also have measurements, are these values available in there? If not, can we reuse them?

@narsaynorath
Copy link
Member Author

narsaynorath commented Apr 23, 2024

Transactions have these values are measurements (see test). Spans also have measurements, are these values available in there? If not, can we reuse them?

@iker-barriocanal We do have them there, but the problem is that these values are associated with specific spans and we can't really propagate them from the transaction down. And in the current implementation they're sent in the span data property and not measurements.

I see there's a method for extracting values from span data to span measurements here, would it be preferable if I pulled these values into that data structure?

@jjbayer
Copy link
Member

jjbayer commented Apr 23, 2024

Spans also have measurements, are these values available in there? If not, can we reuse them?

I see there's a method for extracting values from span data to span measurements here, would it be preferable if I pulled these values into that data structure?

The difference between transaction measurements and span.measurements is that span measurements are not converted to metrics automatically, so even as a measurement we would need to define a metric in defaults.rs. However, I do believe populating the measurements data structure makes sense, because it will put the numbers into the indexed spans dataset as well.

TL;DR yes, I would populate span.measurements as we do for http.decoded_response_content_length, etc.

Copy link
Member

@jjbayer jjbayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@narsaynorath could you please update the PR description before merging? Distributions -> gauges, plus maybe mention measurements. Thanks!

@narsaynorath narsaynorath merged commit f89aafb into master Apr 24, 2024
@narsaynorath narsaynorath deleted the nar/feat/mobile-ui-add-slow-and-frozen-frames-metrics branch April 24, 2024 15:38
narsaynorath added a commit that referenced this pull request Apr 25, 2024
Adds a new metric for collecting the `frames.delay` property from a
span's `data` object in the Starfish Mobile UI module.

I've added the tags that I think are necessary to query and filter for
this metric, and they are common filters for other metrics in this
config. Also noted is that the data is in the `data` object, similar to
#3473 and thus I modified measurement extraction to copy this value over
to span measurements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants