Fix duplicative Screen size changed breadcrumbs

[lavinov-mercury]

📜 Description

This PR adds private var to save last sent metrics breadcrumb & prevent sending new breadcrumbs if data didn't change.
I'm not sure if mutable var is fine here, however, it works & passes all tests (including new ones). Any proposals on better ways to implement this are welcome 😄
I've also updated link to onMetricsChanged docs, as current one seems to be outdated

💡 Motivation and Context

There are tons of repetitive & absolutely useless Screen size changed events in our project. Funniest thing here is that size didn't change actually. One of the reasons for that is that didChangeMetrics also reacts on viewInsets changes, which means that it fires on keyboard opening and closing. And it's 30+ events on my iOS device for each keyboard appearance. So, it would be nice to add new breadcrumbs only when reporting values really changes.

💚 How did you test it?

I've changed related test & added new ones, checked that tests are still passing. Also i've checked it with the example app with new Sentry project. I changed app orientation several times, then caused error and checked appeared issue in Sentry. Then i've added TextField to open & close keyboard, caused one more error & checked new Sentry event. In both cases Screen size changed only when it was expected.

📝 Checklist

• I reviewed submitted code
• I added tests to verify changes
• I updated the docs if needed
• All tests passing
• No breaking changes

[ueman]

This sounds like potential fix for a variant of this issue #400

[marandaneto]

I understand that Screen size changed all the time is annoying and noisy, but it's what the OS/framework triggers anyway.
I'd be fine adding a enableWindowMetricBreadcrumbs flag but I'd rather prefer the debouncing in another PR, because it can be reused across different sources of duplication.
I'd rather prefer a debouncing algorithm that works generically instead of a local var which requires the implementation everywhere, how does that sound?

[lavinov-mercury]

@marandaneto I'm not sure that debouncing is right solution here. It's also incorrect, that each time client sends event to Sentry means that framework reports about screen size change.
According to the Flutter docs, onMetricsChanged method can be called whenever the devicePixelRatio, physicalSize, padding, viewInsets, PlatformDispatcher.views, or systemGestureInsets values change (docs). As you can see, there are few more values that has no relation to screen size and can trigger event on update. As i mentioned in the description, i have 30+ events each time user opens a keyboard, while screen size, obviously, persists the same (it's viewInsets changes when keyboard opens).

However, i agree, that variable + if statement doesn't look like an enterprise solution, i'm going to spend some time to refactor it to something more generic & easy to reuse, but i still believe that this solution should aim to send only distinct events, not debounced.

[lavinov-mercury]

@marandaneto @brustolin I've refactored this to use stream controller with distinct to avoid duplicates

[marandaneto]

@lavinov-mercury thanks, I'll review it early next week.

[marandaneto]

@lavinov-mercury missing changelog entry.
Thanks for doing this.

[lavinov-mercury]

@marandaneto I've updated changelog, could you please check, if it's done right?

[marandaneto]

@marandaneto I've updated changelog, could you please check, if it's done right?

All good, thanks.
CI is going to fail due to #938 but its a different issue.

[codecov-commenter]

Codecov Report

Merging #888 (f3acb7b) into main (bdc6e0f) will increase coverage by 0.12%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #888      +/-   ##
==========================================
+ Coverage   89.81%   89.94%   +0.12%     
==========================================
  Files         104        9      -95     
  Lines        3211      169    -3042     
==========================================
- Hits         2884      152    -2732     
+ Misses        327       17     -310     

Impacted Files	Coverage Δ
...lib/src/environment/_io_environment_variables.dart
dart/lib/src/sentry_span_interface.dart
dart/lib/src/http_client/sentry_http_client.dart
dart/lib/src/platform_checker.dart
dart/lib/src/protocol/sentry_culture.dart
dart/lib/src/diagnostic_logger.dart
dart/lib/src/protocol/sentry_app.dart
dart/lib/src/sentry_sampling_context.dart
dart/lib/src/client_reports/discard_reason.dart
dart/lib/src/protocol/sentry_request.dart
... and 85 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdc6e0f...f3acb7b. Read the comment docs.

[lavinov-mercury]

@marandaneto Hi! It failed to merge as branch became outdated again, could you please start workflows?