Deprecate: metrics api #2312
Deprecate: metrics api #2312
Conversation
buenaflor
requested review from
krystofwoldrich,
stefanosiano and
martinhaintz
as code owners
|
@stefanosiano is this the only place that needs the deprecated tag? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2312 +/- ##
==========================================
- Coverage 88.07% 87.07% -1.01%
==========================================
Files 247 104 -143
Lines 8607 3706 -4901
==========================================
- Hits 7581 3227 -4354
+ Misses 1026 479 -547 ☔ View full report in Codecov by Sentry. |
Android Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 8f88a49 | 364.98 ms | 413.78 ms | 48.80 ms |
| 5e8d2b3 | 342.17 ms | 375.83 ms | 33.66 ms |
| 2d3b03d | 309.53 ms | 353.40 ms | 43.87 ms |
| c73ab67 | 353.82 ms | 408.71 ms | 54.90 ms |
| 5baa201 | 389.26 ms | 462.83 ms | 73.57 ms |
| 5f2f77b | 429.06 ms | 507.74 ms | 78.68 ms |
| 2e1e4ae | 413.34 ms | 509.24 ms | 95.90 ms |
| 72eeb80 | 421.38 ms | 520.81 ms | 99.44 ms |
| f770c4c | 385.88 ms | 449.86 ms | 63.98 ms |
| f2db4ec | 372.46 ms | 469.72 ms | 97.26 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 8f88a49 | 6.34 MiB | 7.30 MiB | 979.60 KiB |
| 5e8d2b3 | 6.15 MiB | 7.13 MiB | 1000.11 KiB |
| 2d3b03d | 6.06 MiB | 7.09 MiB | 1.03 MiB |
| c73ab67 | 6.15 MiB | 7.13 MiB | 999.97 KiB |
| 5baa201 | 6.35 MiB | 7.33 MiB | 1005.56 KiB |
| 5f2f77b | 6.35 MiB | 7.40 MiB | 1.05 MiB |
| 2e1e4ae | 6.35 MiB | 7.42 MiB | 1.06 MiB |
| 72eeb80 | 6.35 MiB | 7.42 MiB | 1.06 MiB |
| f770c4c | 6.33 MiB | 7.26 MiB | 950.37 KiB |
| f2db4ec | 6.06 MiB | 7.03 MiB | 990.27 KiB |
Previous results on branch: deprecate/metrics
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 935da64 | 469.32 ms | 523.04 ms | 53.72 ms |
| d59414b | 523.71 ms | 590.06 ms | 66.35 ms |
| 3a66f24 | 463.77 ms | 523.09 ms | 59.31 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 935da64 | 6.49 MiB | 7.56 MiB | 1.07 MiB |
| d59414b | 6.49 MiB | 7.56 MiB | 1.07 MiB |
| 3a66f24 | 6.49 MiB | 7.56 MiB | 1.07 MiB |
iOS Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 039058a | 1227.90 ms | 1256.23 ms | 28.34 ms |
| 691aa3b | 1265.57 ms | 1282.13 ms | 16.55 ms |
| 8e133ad | 1268.19 ms | 1277.37 ms | 9.18 ms |
| 5aa047a | 1236.57 ms | 1241.02 ms | 4.45 ms |
| d5f600b | 1220.79 ms | 1232.93 ms | 12.14 ms |
| 7f14ddd | 1247.36 ms | 1269.89 ms | 22.53 ms |
| c732386 | 1233.20 ms | 1252.08 ms | 18.88 ms |
| d5696bf | 1232.96 ms | 1254.50 ms | 21.54 ms |
| 014c3ea | 1298.73 ms | 1351.24 ms | 52.51 ms |
| af2d175 | 1280.37 ms | 1282.24 ms | 1.88 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 039058a | 8.38 MiB | 9.71 MiB | 1.34 MiB |
| 691aa3b | 8.16 MiB | 9.17 MiB | 1.01 MiB |
| 8e133ad | 8.10 MiB | 9.16 MiB | 1.07 MiB |
| 5aa047a | 8.29 MiB | 9.39 MiB | 1.10 MiB |
| d5f600b | 8.32 MiB | 9.38 MiB | 1.05 MiB |
| 7f14ddd | 8.33 MiB | 9.64 MiB | 1.31 MiB |
| c732386 | 8.28 MiB | 9.33 MiB | 1.05 MiB |
| d5696bf | 8.38 MiB | 9.73 MiB | 1.35 MiB |
| 014c3ea | 8.33 MiB | 9.39 MiB | 1.06 MiB |
| af2d175 | 8.15 MiB | 9.12 MiB | 986.22 KiB |
Previous results on branch: deprecate/metrics
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3a66f24 | 1251.10 ms | 1276.04 ms | 24.94 ms |
| d59414b | 1256.88 ms | 1268.69 ms | 11.82 ms |
| 935da64 | 1235.12 ms | 1250.37 ms | 15.24 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3a66f24 | 8.38 MiB | 9.74 MiB | 1.36 MiB |
| d59414b | 8.38 MiB | 9.74 MiB | 1.36 MiB |
| 935da64 | 8.38 MiB | 9.74 MiB | 1.36 MiB |
|
@philipphofmann some of the metrics tests are very flaky (as described here) and metrics in its current state will be gone as a product so these tests won't be useful anymore and only a liability going forward, if we need them for a future implementation we can easily reference them again in the git history |
| @@ -316,6 +316,8 @@ class Sentry { | |||
| static ISentrySpan? getSpan() => _hub.getSpan(); | |||
|
|
|||
| /// Gets access to the metrics API for the current hub. | |||
| @Deprecated( | |||
| 'Metrics will be deprecated and removed in the next major release. Sentry will reject all metrics sent after October 7, 2024. Learn more: https://sentry.zendesk.com/hc/en-us/articles/26369339769883-Upcoming-API-Changes-to-Metrics') | |||
There was a problem hiding this comment.
m: Aren't metrics experimental? If yes, we can also remove them in a nonmajor update. I don't think it makes sense to keep the code if, anyways, we are going to reject all metrics after October 7th.
There was a problem hiding this comment.
Hmm, I don't have any strong opinion on that.
@kahest what do you think?
There was a problem hiding this comment.
I'd deprecate now and leave some time to remove - not because it needs to be a major (can be a minor in a few weeks), but to give app maintainers at least a bit of time to learn about the deprecation and remove the code before we break their builds. even if it's likely a very small change they need to do, we don't need to force them to do it right now
There was a problem hiding this comment.
Sounds good, I guess we can approve this PR
I'd like to include that in the release
I'd put the Hub.metricsApi() one, too, in case a user uses the hub directly (it should warn the user about it being internal, but let's ensure they know its deprecation) |
…e-for-events' of https://github.com/getsentry/sentry-dart into feat/only-send-debug-images-referenced-in-the-stacktrace-for-events * 'feat/only-send-debug-images-referenced-in-the-stacktrace-for-events' of https://github.com/getsentry/sentry-dart: Fix TTID timing issue (#2326) fix: dart2wasm test (#2327) Deprecate: metrics api (#2312)
In this PR: remove metrics tests, deprecate public metrics api
📜 Description
Why we deprecate it: https://sentry.zendesk.com/hc/en-us/articles/26369339769883-Upcoming-API-Changes-to-Metrics
Closes #2277