Feat View hierarchy #1189

marandaneto · 2022-12-14T08:04:54Z

📜 Description

View hierarchy as an attachment

💡 Motivation and Context

getsentry/team-mobile#64

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions · 2022-12-14T08:06:24Z

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

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

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

- Feat View hierarchy ([#1189](https://github.com/getsentry/sentry-dart/pull/1189))

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 82eccb8

flutter/lib/src/view_hierarchy/sentry_tree_walker.dart

codecov-commenter · 2022-12-14T08:07:32Z

Codecov Report

❗ No coverage uploaded for pull request base (v7.0.0@20def04). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff            @@
##             v7.0.0    #1189   +/-   ##
=========================================
  Coverage          ?   88.02%           
=========================================
  Files             ?      122           
  Lines             ?     3816           
  Branches          ?        0           
=========================================
  Hits              ?     3359           
  Misses            ?      457           
  Partials          ?        0

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.

flutter/lib/src/view_hierarchy/sentry_tree_walker.dart

marandaneto · 2022-12-14T08:37:30Z

flutter/lib/src/view_hierarchy/sentry_tree_walker.dart

+
+  SentryViewHierarchy? toSentryViewHierarchy() {
+    final rootNode = _toSentryViewHierarchyElement(rootElement);
+    rootElement.visitChildElements(_visitor(rootNode));


@ueman so the result of this for simple screens is quite deep in-depth, and it looks way more deeper in depth than the view hierarchy of the dev tooling, have you noticed that, or did I mess up something?

Unfortunately yes. See getsentry/team-mobile#64 (comment) for some ideas on how to tackle that.

Edit: Copied it over:

The view hierarchy in Flutter is way deeper than those of Android and iOS. Classes of views (views are called widgets in Flutter) prefixed with a _ are private (that's how it works in Dart). It might be worth to ignore private classes since you most probably can't look them up in docs and so on. Public children of private classes should be shown again, though. Private classes are often used to improve the structuring of a widget, so they are kinda an implementation detail. Classes not prefixed with an underscore are public and one should be able to look them up in docs. Ignoring private classes is currently not done in my code. Also, you don't know whether a class is from user code, 3-party code or framework code, since there's no reflection/introspection in Dart. When enabling obfuscation in Flutter, the class names are of course not really human readable, but I guess that's similar for Android/iOS.

In addition to the names of the widgets, you could also add the content of well known widgets like Text and *Buttons, as well as the on screen size and coordinates.

Oh indeed, missed that, thank you for sharing your thoughts, let me try this out.

I wonder if the _ check for private classes also works in obfuscated code

I will check, also found this https://docs.flutter.dev/development/tools/devtools/inspector#track-widget-creation

There are also a few methods that print the tree, but only on debug mode https://docs.flutter.dev/testing/code-debugging#debug-flags-performance

@ueman That does not work when obfuscated is enabled because everything is renamed at compile time.
Still worth keeping this quick workaround; it improves at least when it's disabled.

dart/lib/src/protocol/sentry_view_hierarchy_element.dart

marandaneto · 2022-12-15T09:28:44Z

@markushi @brustolin @krystofwoldrich early feedback is welcomed since it's a working solution already.
Views are really nested, unfortunately.
Unclear what the extra params could be, we have to figure out what's useful besides the default ones.
It's not working on Android due to an issue on the bridge most likely, trying to figure out what's happening during de/serialization on Android.
Edit, issue.

flutter/lib/src/view_hierarchy/sentry_tree_walker.dart

markushi · 2022-12-20T12:36:04Z

flutter/lib/src/view_hierarchy/view_hierarchy_event_processor.dart

+    if (instance == null) {
+      return event;
+    }
+    final sentryViewHierarchy = walkWidgetTree(instance);


Being not too familiar with Flutter, it would be interesting to know:

how fast walking the tree is

if there are any multi-threading concerns (e.g. walking the tree whilst it get's modified)

Dart is single-threaded, so no multi-threading concern, its also a sync call.
Running on debug mode (Profile mode isn't supported on iOS simulator, Debug is much slower) takes ~30ms, for our sample app/main screen, this is dynamic because it depends on the app's view, so hard to measure.
Final JSON around ~50-60kb

if there are any multi-threading concerns (e.g. walking the tree whilst it get's modified)

I actually think that's a valid concern because that's also a common problem when creating screenshots. Sometime you'll get an error which says it's not possible to create a screenshot because the current on screen content was already invalidated. But I also think it's something not a lot of people will run into, since it should be fast enough.

The feature is best effort, if for some reason you can't read the tree or take the screenshot, that's fine, there will be always a state that the framework won't allow you to do it, and we don't have control over that.

markushi · 2022-12-20T13:30:34Z

Looks good to me, nice one!

Regarding the deep nesting: I'm wondering if it could make sense to aggregate certain "known" widgets.
E.g.

{
  "opacity": 0.5,
  "children": [{
    "visible": true,
    "children": [{
      "identifier": "button_login"
    }]
  }]
}

turns into

{
  "identifier": "button_login",
  "opacity": 0.5,
  "visible": true,
}

marandaneto · 2022-12-20T13:35:51Z

Looks good to me, nice one!

Regarding the deep nesting: I'm wondering if it could make sense to aggregate certain "known" widgets. E.g.
{
  "opacity": 0.5,
  "children": [{
    "visible": true,
    "children": [{
      "identifier": "button_login"
    }]
  }]
}
turns into
{
  "identifier": "button_login",
  "opacity": 0.5,
  "visible": true,
}

I believe this should be an optimization for the FE, where we can fix/improve over time rather than changing on the SDK level and waiting for people to upgrade their SDKs before getting a meaningful view, but I agree whenever possible, the super nested view should be improved when debugging issues, but the SDK should report as it is.

marandaneto · 2022-12-20T14:20:31Z

@krystofwoldrich its a Draft because there's a bug on Relay, but its ready to review, test are failing locally, working on them.

markushi · 2022-12-21T07:59:00Z

I believe this should be an optimization for the FE

I agree, that's something which could be done on FE! Looping in @narsaynorath

dart/lib/src/sentry_attachment/sentry_attachment.dart

flutter/lib/src/view_hierarchy/view_hierarchy_event_processor.dart

krystofwoldrich · 2022-12-21T12:27:56Z

The CI is failing but otherwise looks good to me.

marandaneto · 2022-12-21T12:54:47Z

The CI is failing but otherwise looks good to me.

CI should fail, that's expected (just the analyze) step.

marandaneto · 2022-12-21T13:49:00Z

Docs: getsentry/sentry-docs#5986

narsaynorath · 2022-12-21T15:12:30Z

Is the event from your sample screenshot available in prod? I'd be interested in seeing how deep it actually ends up being. Seeing it would also help me understand the aggregated optimization mentioned above.

When this example was provided:

{
  "opacity": 0.5,
  "children": [{
    "visible": true,
    "children": [{
      "identifier": "button_login"
    }]
  }]
}

is this form the result of ignoring the private views? And the idea to aggregate the children is because the private views will show up in the data as nodes, but without anything meaningful since we're ignoring them? Is there a chance that a private view contains two (or more) children or is it always a 1:1 relationship in those cases?

marandaneto · 2023-01-10T10:09:14Z

Is the event from your sample screenshot available in prod? I'd be interested in seeing how deep it actually ends up being. Seeing it would also help me understand the aggregated optimization mentioned above.

When this example was provided:
{
  "opacity": 0.5,
  "children": [{
    "visible": true,
    "children": [{
      "identifier": "button_login"
    }]
  }]
}
is this form the result of ignoring the private views? And the idea to aggregate the children is because the private views will show up in the data as nodes, but without anything meaningful since we're ignoring them? Is there a chance that a private view contains two (or more) children or is it always a 1:1 relationship in those cases?

Shared the JSON in the channel a few weeks ago.
Yes, private views are ignored, but it only works if minification isn't enabled.
I can't tell if it's always a 1:1 relationship, views are super nested, so I'd assume it's not.

marandaneto · 2023-01-10T10:09:53Z

@krystofwoldrich @brustolin this is ready for final review.

dart/lib/src/sentry_attachment/sentry_attachment.dart

flutter/test/view_hierarchy/sentry_tree_walker_test.dart

krystofwoldrich

LGTM 🎉🚀🚀

I know I asked about the failing analyzer before, but why does it fail/why is it expected?

marandaneto · 2023-01-10T13:40:05Z

LGTM 🎉🚀🚀

I know I asked about the failing analyzer before, but why does it fail/why is it expected?

You can ignore this, it will be automatically solved once it's published.

markushi · 2023-01-19T08:59:40Z

flutter/lib/src/view_hierarchy/sentry_tree_walker.dart

+    double? alpha;
+
+    // Widget has to be RenderBox to have a size
+    if (widget is RenderBox) {


Hey @marandaneto, is it possible that this should be
if (element.renderObject is RenderBox) {
instead?
@narsaynorath is seeing elements in the View Hierarchy which have a x/y coordinate but no width/height.

You're right, #1258

Feat View hierrchy

51e10fd

marandaneto changed the title ~~Feat/remove parameter from manual setup classes (#816)~~ Feat View hierarchy Dec 14, 2022

marandaneto commented Dec 14, 2022

View reviewed changes

flutter/lib/src/view_hierarchy/sentry_tree_walker.dart Outdated Show resolved Hide resolved

marandaneto commented Dec 14, 2022

View reviewed changes

flutter/lib/src/view_hierarchy/sentry_tree_walker.dart Outdated Show resolved Hide resolved

marandaneto commented Dec 14, 2022

View reviewed changes

Merge branch 'v7.0.0' into feat/view_hierarchy

fedbb54

marandaneto mentioned this pull request Dec 14, 2022

Support View hierarchy getsentry/team-mobile#64

Closed

Ref

c0221de

ueman reviewed Dec 15, 2022

View reviewed changes

dart/lib/src/protocol/sentry_view_hierarchy_element.dart Outdated Show resolved Hide resolved

marandaneto added 4 commits December 15, 2022 08:46

Ref

0cf61fc

fix

07be5a8

integration test use fake dsn

4aabfdb

fix comment

97cfec3

marandaneto requested a review from markushi December 15, 2022 09:27

marandaneto added 2 commits December 15, 2022 12:37

fix

41d2fe9

fix

0d4f838

markushi reviewed Dec 20, 2022

View reviewed changes

flutter/lib/src/view_hierarchy/sentry_tree_walker.dart Show resolved Hide resolved

markushi reviewed Dec 20, 2022

View reviewed changes

krystofwoldrich reviewed Dec 21, 2022

View reviewed changes

dart/lib/src/sentry_attachment/sentry_attachment.dart Outdated Show resolved Hide resolved

krystofwoldrich reviewed Dec 21, 2022

View reviewed changes

flutter/lib/src/view_hierarchy/view_hierarchy_event_processor.dart Outdated Show resolved Hide resolved

fix typo

a25fdd2

marandaneto mentioned this pull request Dec 21, 2022

View hierarchy docs for Flutter getsentry/sentry-docs#5986

Merged

marandaneto added 5 commits December 22, 2022 09:40

Merge branch 'v7.0.0' into feat/view_hierarchy

a6d248f

add pending tests

1a919cd

add missing tests

28ecbfe

Merge branch 'v7.0.0' into feat/view_hierarchy

d7a3063

fix tests

d0e79d8

marandaneto marked this pull request as ready for review January 10, 2023 10:09

marandaneto requested a review from brustolin as a code owner January 10, 2023 10:09

krystofwoldrich reviewed Jan 10, 2023

View reviewed changes

dart/lib/src/sentry_attachment/sentry_attachment.dart Outdated Show resolved Hide resolved

fix

7c4c195

krystofwoldrich reviewed Jan 10, 2023

View reviewed changes

flutter/test/view_hierarchy/sentry_tree_walker_test.dart Outdated Show resolved Hide resolved

krystofwoldrich approved these changes Jan 10, 2023

View reviewed changes

marandaneto added 2 commits January 10, 2023 14:41

fix typo

1d6dbb5

fix

82eccb8

marandaneto merged commit 1275c1e into v7.0.0 Jan 10, 2023

marandaneto deleted the feat/view_hierarchy branch January 10, 2023 14:40

markushi reviewed Jan 19, 2023

View reviewed changes

marandaneto mentioned this pull request Mar 16, 2023

Remove View Hierarchy from Flutter Web #1336

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat View hierarchy #1189

Feat View hierarchy #1189

marandaneto commented Dec 14, 2022 •

edited

Loading

github-actions bot commented Dec 14, 2022 •

edited

Loading

codecov-commenter commented Dec 14, 2022 •

edited

Loading

marandaneto Dec 14, 2022

ueman Dec 14, 2022 •

edited

Loading

marandaneto Dec 14, 2022

ueman Dec 14, 2022

marandaneto Dec 14, 2022

marandaneto Dec 14, 2022

marandaneto Dec 15, 2022

marandaneto commented Dec 15, 2022 •

edited

Loading

markushi Dec 20, 2022

marandaneto Dec 20, 2022

ueman Dec 21, 2022 •

edited

Loading

marandaneto Dec 21, 2022

markushi commented Dec 20, 2022

marandaneto commented Dec 20, 2022

marandaneto commented Dec 20, 2022

markushi commented Dec 21, 2022 •

edited

Loading

krystofwoldrich commented Dec 21, 2022

marandaneto commented Dec 21, 2022

marandaneto commented Dec 21, 2022

narsaynorath commented Dec 21, 2022

marandaneto commented Jan 10, 2023

marandaneto commented Jan 10, 2023

krystofwoldrich left a comment •

edited

Loading

marandaneto commented Jan 10, 2023

markushi Jan 19, 2023

marandaneto Feb 1, 2023

Feat View hierarchy #1189

Feat View hierarchy #1189

Conversation

marandaneto commented Dec 14, 2022 • edited Loading

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Dec 14, 2022 • edited Loading

Instructions and example for changelog

codecov-commenter commented Dec 14, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

ueman Dec 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marandaneto commented Dec 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ueman Dec 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markushi commented Dec 20, 2022

marandaneto commented Dec 20, 2022

marandaneto commented Dec 20, 2022

markushi commented Dec 21, 2022 • edited Loading

krystofwoldrich commented Dec 21, 2022

marandaneto commented Dec 21, 2022

marandaneto commented Dec 21, 2022

narsaynorath commented Dec 21, 2022

marandaneto commented Jan 10, 2023

marandaneto commented Jan 10, 2023

krystofwoldrich left a comment • edited Loading

Choose a reason for hiding this comment

marandaneto commented Jan 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marandaneto commented Dec 14, 2022 •

edited

Loading

github-actions bot commented Dec 14, 2022 •

edited

Loading

codecov-commenter commented Dec 14, 2022 •

edited

Loading

ueman Dec 14, 2022 •

edited

Loading

marandaneto commented Dec 15, 2022 •

edited

Loading

ueman Dec 21, 2022 •

edited

Loading

markushi commented Dec 21, 2022 •

edited

Loading

krystofwoldrich left a comment •

edited

Loading