Skip to content
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

Change SDK name of Sentry Native for Flutter #1468

Merged
merged 3 commits into from
May 23, 2023
Merged

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented May 22, 2023

#skip-changelog

📜 Description

Update SDK names set in native plugins.

💡 Motivation and Context

Closes #1333

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

@denrase
Copy link
Collaborator Author

denrase commented May 22, 2023

@marandaneto I tried to check the SDK names on sentry.io through the example app, but i'm not seeing those values set, neither for iOS nor Android. Am I looking in the wrong place?

Bildschirmfoto 2023-05-22 um 11 10 07

@marandaneto
Copy link
Contributor

@marandaneto I tried to check the SDK names on sentry.io through the example app, but i'm not seeing those values set, neither for iOS nor Android. Am I looking in the wrong place?

Bildschirmfoto 2023-05-22 um 11 10 07

The place is right, but events captured on the Dart layer will be this name.
See the AndroidExample class in main.dart.

When this method is executed


SDK name should be sentry.java.android.flutter (it is already).

When those 2 methods are called

"crash" -> {
crash()
}
"cpp_capture_message" -> {
message()
}

SDK name it should be called sentry.native.android.flutter -> this is the one that needs fixing.

@codecov
Copy link

codecov bot commented May 22, 2023

Codecov Report

Patch coverage has no change and project coverage change: +1.13 🎉

Comparison is base (754c630) 90.22% compared to head (54b2a59) 91.35%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1468      +/-   ##
==========================================
+ Coverage   90.22%   91.35%   +1.13%     
==========================================
  Files         181      160      -21     
  Lines        5798     5148     -650     
==========================================
- Hits         5231     4703     -528     
+ Misses        567      445     -122     

see 21 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@marandaneto
Copy link
Contributor

@marandaneto marandaneto marked this pull request as ready for review May 23, 2023 12:07
@marandaneto marandaneto enabled auto-merge (squash) May 23, 2023 12:08
@github-actions
Copy link
Contributor

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 349.50 ms 392.00 ms 42.50 ms
Size 6.15 MiB 7.13 MiB 1000.07 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
bf4aed7 311.24 ms 365.66 ms 54.42 ms
0be962b 325.54 ms 382.83 ms 57.29 ms
8fa3934 340.64 ms 407.92 ms 67.28 ms
e66e71e 296.84 ms 345.43 ms 48.59 ms
ef2f368 350.06 ms 429.44 ms 79.38 ms
62dde43 339.21 ms 423.06 ms 83.85 ms
a094100 388.02 ms 459.50 ms 71.48 ms
b2cbbc8 347.80 ms 395.31 ms 47.51 ms
a61674e 331.35 ms 391.06 ms 59.71 ms
0a82a1e 321.02 ms 393.82 ms 72.80 ms

App size

Revision Plain With Sentry Diff
bf4aed7 6.06 MiB 7.03 MiB 997.04 KiB
0be962b 6.06 MiB 7.03 MiB 990.29 KiB
8fa3934 6.06 MiB 7.09 MiB 1.03 MiB
e66e71e 6.06 MiB 7.09 MiB 1.03 MiB
ef2f368 5.94 MiB 6.89 MiB 975.81 KiB
62dde43 5.94 MiB 6.96 MiB 1.02 MiB
a094100 5.94 MiB 6.96 MiB 1.02 MiB
b2cbbc8 6.06 MiB 7.03 MiB 995.45 KiB
a61674e 6.06 MiB 7.03 MiB 990.29 KiB
0a82a1e 6.15 MiB 7.11 MiB 981.82 KiB

Previous results on branch: enha/set-sdk-name

Startup times

Revision Plain With Sentry Diff
02e696e 336.69 ms 408.72 ms 72.03 ms

App size

Revision Plain With Sentry Diff
02e696e 6.15 MiB 7.13 MiB 1000.02 KiB

@marandaneto marandaneto merged commit f79eecf into main May 23, 2023
@marandaneto marandaneto deleted the enha/set-sdk-name branch May 23, 2023 12:22
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.

Change SDK name of Sentry Native for Flutter
2 participants