Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

feat: Add new captureFeedback API to RN SDK#4320

Merged
krystofwoldrich merged 13 commits intomaingetsentry/sentry-react-native:mainfrom
antonis/3859-newCaptureFeedbackAPIgetsentry/sentry-react-native:antonis/3859-newCaptureFeedbackAPICopy head branch name to clipboard
Dec 2, 2024
Merged

feat: Add new captureFeedback API to RN SDK#4320
krystofwoldrich merged 13 commits intomaingetsentry/sentry-react-native:mainfrom
antonis/3859-newCaptureFeedbackAPIgetsentry/sentry-react-native:antonis/3859-newCaptureFeedbackAPICopy head branch name to clipboard

Conversation

@antonis
Copy link
Contributor

@antonis antonis commented Nov 27, 2024 •
edited
Loading

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Notes:

💡 Motivation and Context

Fixes #3859 (part of #4302)

💚 How did you test it?

CI, Manual testing with the sample app:

Screenshot 2024-11-27 at 1 58 32 PM

📝 Checklist

  • 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.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Nov 27, 2024 •
edited
Loading

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

Generated by 🚫 dangerJS against 2bb104b

@github-actions
Copy link
Contributor

github-actions bot commented Nov 27, 2024 •
edited
Loading

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 473.12 ms 490.78 ms 17.66 ms
Size 17.74 MiB 20.09 MiB 2.35 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
728164b 414.34 ms 449.22 ms 34.88 ms
484813b 434.55 ms 452.31 ms 17.75 ms
86d6d2c+dirty 332.90 ms 352.45 ms 19.55 ms
7fd512a 442.18 ms 437.57 ms -4.61 ms
9cd0e9f 449.65 ms 433.39 ms -16.26 ms
ad6c299 375.94 ms 382.02 ms 6.08 ms
0d3e677 422.82 ms 411.90 ms -10.92 ms
d8668ce 452.13 ms 435.06 ms -17.07 ms
0677344 327.74 ms 337.14 ms 9.40 ms
e540498 436.26 ms 433.00 ms -3.26 ms

App size

Revision Plain With Sentry Diff
728164b 17.73 MiB 19.85 MiB 2.12 MiB
484813b 17.73 MiB 20.07 MiB 2.33 MiB
86d6d2c+dirty 17.73 MiB 20.04 MiB 2.31 MiB
7fd512a 17.74 MiB 20.08 MiB 2.35 MiB
9cd0e9f 17.74 MiB 20.08 MiB 2.34 MiB
ad6c299 17.73 MiB 19.75 MiB 2.02 MiB
0d3e677 17.74 MiB 20.07 MiB 2.34 MiB
d8668ce 17.74 MiB 20.08 MiB 2.34 MiB
0677344 17.73 MiB 19.81 MiB 2.07 MiB
e540498 17.73 MiB 20.11 MiB 2.37 MiB

Previous results on branch: antonis/3859-newCaptureFeedbackAPI

Startup times

Revision Plain With Sentry Diff
a673305 436.21 ms 443.04 ms 6.84 ms
b2f7f82 436.78 ms 435.20 ms -1.58 ms
68362f2 461.20 ms 457.04 ms -4.16 ms
beb77ad 510.00 ms 534.31 ms 24.31 ms

App size

Revision Plain With Sentry Diff
a673305 17.74 MiB 20.09 MiB 2.35 MiB
b2f7f82 17.74 MiB 20.09 MiB 2.35 MiB
68362f2 17.74 MiB 20.09 MiB 2.35 MiB
beb77ad 17.74 MiB 20.09 MiB 2.35 MiB

@antonis antonis marked this pull request as ready for review November 27, 2024 12:30
@github-actions
Copy link
Contributor

github-actions bot commented Nov 27, 2024 •
edited
Loading

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 401.81 ms 426.80 ms 24.99 ms
Size 7.15 MiB 8.37 MiB 1.22 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
a989877+dirty 383.04 ms 400.92 ms 17.88 ms
e1ea4a8+dirty 451.98 ms 497.58 ms 45.60 ms
5bb8d5f+dirty 356.71 ms 389.65 ms 32.94 ms
86d6d2c+dirty 267.21 ms 325.24 ms 58.04 ms
e2b64fe+dirty 258.82 ms 304.26 ms 45.44 ms
f06c879+dirty 361.27 ms 407.88 ms 46.61 ms
4a6664f+dirty 357.02 ms 394.91 ms 37.89 ms
ac41368+dirty 395.91 ms 451.17 ms 55.26 ms
9cab16b+dirty 370.82 ms 416.37 ms 45.55 ms
63ed251+dirty 485.02 ms 531.16 ms 46.14 ms

App size

Revision Plain With Sentry Diff
a989877+dirty 7.15 MiB 8.35 MiB 1.20 MiB
e1ea4a8+dirty 7.15 MiB 8.35 MiB 1.20 MiB
5bb8d5f+dirty 7.15 MiB 8.21 MiB 1.06 MiB
86d6d2c+dirty 7.15 MiB 8.09 MiB 962.69 KiB
e2b64fe+dirty 7.15 MiB 8.07 MiB 947.16 KiB
f06c879+dirty 7.15 MiB 8.12 MiB 997.78 KiB
4a6664f+dirty 7.15 MiB 8.22 MiB 1.07 MiB
ac41368+dirty 7.15 MiB 8.39 MiB 1.24 MiB
9cab16b+dirty 7.15 MiB 8.35 MiB 1.20 MiB
63ed251+dirty 7.15 MiB 8.35 MiB 1.20 MiB

Previous results on branch: antonis/3859-newCaptureFeedbackAPI

Startup times

Revision Plain With Sentry Diff
b2f7f82+dirty 377.85 ms 440.47 ms 62.61 ms
beb77ad+dirty 393.65 ms 451.71 ms 58.06 ms
a673305+dirty 369.77 ms 414.12 ms 44.36 ms
68362f2+dirty 344.64 ms 400.24 ms 55.59 ms

App size

Revision Plain With Sentry Diff
b2f7f82+dirty 7.15 MiB 8.37 MiB 1.22 MiB
beb77ad+dirty 7.15 MiB 8.37 MiB 1.22 MiB
a673305+dirty 7.15 MiB 8.37 MiB 1.22 MiB
68362f2+dirty 7.15 MiB 8.37 MiB 1.22 MiB

@github-actions
Copy link
Contributor

github-actions bot commented Nov 27, 2024 •
edited
Loading

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1214.98 ms 1228.34 ms 13.36 ms
Size 2.36 MiB 3.11 MiB 759.80 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d7401ac+dirty 1252.38 ms 1275.04 ms 22.66 ms
07e58c9+dirty 1226.02 ms 1228.35 ms 2.33 ms
e22745e+dirty 1222.73 ms 1224.98 ms 2.25 ms
690220d+dirty 1228.27 ms 1233.55 ms 5.29 ms
575f9da+dirty 1266.22 ms 1274.84 ms 8.62 ms
728164b+dirty 1256.10 ms 1259.08 ms 2.98 ms
946a600+dirty 1238.20 ms 1236.85 ms -1.35 ms
57448c5+dirty 1228.73 ms 1235.90 ms 7.16 ms
52a8031+dirty 1280.88 ms 1289.78 ms 8.90 ms
15c80ab+dirty 1223.74 ms 1228.96 ms 5.22 ms

App size

Revision Plain With Sentry Diff
d7401ac+dirty 2.36 MiB 2.83 MiB 481.14 KiB
07e58c9+dirty 2.36 MiB 3.10 MiB 752.28 KiB
e22745e+dirty 2.36 MiB 3.10 MiB 752.32 KiB
690220d+dirty 2.36 MiB 3.10 MiB 753.57 KiB
575f9da+dirty 2.36 MiB 2.87 MiB 520.20 KiB
728164b+dirty 2.36 MiB 2.88 MiB 530.38 KiB
946a600+dirty 2.36 MiB 3.10 MiB 759.74 KiB
57448c5+dirty 2.36 MiB 3.10 MiB 752.34 KiB
52a8031+dirty 2.36 MiB 2.82 MiB 469.44 KiB
15c80ab+dirty 2.36 MiB 2.83 MiB 474.49 KiB

Previous results on branch: antonis/3859-newCaptureFeedbackAPI

Startup times

Revision Plain With Sentry Diff
b2f7f82+dirty 1224.27 ms 1232.24 ms 7.97 ms
68362f2+dirty 1236.22 ms 1228.98 ms -7.25 ms
beb77ad+dirty 1229.25 ms 1228.02 ms -1.23 ms
a673305+dirty 1221.76 ms 1224.72 ms 2.97 ms

App size

Revision Plain With Sentry Diff
b2f7f82+dirty 2.36 MiB 3.10 MiB 759.76 KiB
68362f2+dirty 2.36 MiB 3.11 MiB 759.82 KiB
beb77ad+dirty 2.36 MiB 3.10 MiB 759.77 KiB
a673305+dirty 2.36 MiB 3.11 MiB 759.83 KiB

@kahest
Copy link
Contributor

kahest commented Nov 27, 2024

@bruno-garcia FYI

@github-actions
Copy link
Contributor

github-actions bot commented Nov 27, 2024 •
edited
Loading

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1245.86 ms 1244.17 ms -1.69 ms
Size 2.92 MiB 3.67 MiB 772.42 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d7401ac+dirty 1288.10 ms 1289.54 ms 1.44 ms
07e58c9+dirty 1216.42 ms 1210.60 ms -5.82 ms
e22745e+dirty 1246.02 ms 1233.60 ms -12.42 ms
690220d+dirty 1227.45 ms 1221.67 ms -5.78 ms
575f9da+dirty 1272.00 ms 1284.38 ms 12.38 ms
728164b+dirty 1280.06 ms 1285.26 ms 5.20 ms
946a600+dirty 1236.12 ms 1234.94 ms -1.18 ms
57448c5+dirty 1244.65 ms 1249.20 ms 4.55 ms
52a8031+dirty 1255.96 ms 1273.00 ms 17.04 ms
15c80ab+dirty 1248.41 ms 1251.24 ms 2.83 ms

App size

Revision Plain With Sentry Diff
d7401ac+dirty 2.92 MiB 3.40 MiB 488.06 KiB
07e58c9+dirty 2.92 MiB 3.66 MiB 756.65 KiB
e22745e+dirty 2.92 MiB 3.66 MiB 756.73 KiB
690220d+dirty 2.92 MiB 3.66 MiB 758.77 KiB
575f9da+dirty 2.92 MiB 3.43 MiB 524.26 KiB
728164b+dirty 2.92 MiB 3.44 MiB 533.26 KiB
946a600+dirty 2.92 MiB 3.67 MiB 772.40 KiB
57448c5+dirty 2.92 MiB 3.66 MiB 756.65 KiB
52a8031+dirty 2.92 MiB 3.38 MiB 475.71 KiB
15c80ab+dirty 2.92 MiB 3.39 MiB 481.56 KiB

Previous results on branch: antonis/3859-newCaptureFeedbackAPI

Startup times

Revision Plain With Sentry Diff
b2f7f82+dirty 1242.42 ms 1234.87 ms -7.54 ms
68362f2+dirty 1232.73 ms 1226.80 ms -5.92 ms
beb77ad+dirty 1228.63 ms 1222.55 ms -6.08 ms
a673305+dirty 1239.94 ms 1236.31 ms -3.63 ms

App size

Revision Plain With Sentry Diff
b2f7f82+dirty 2.92 MiB 3.67 MiB 772.44 KiB
68362f2+dirty 2.92 MiB 3.67 MiB 772.35 KiB
beb77ad+dirty 2.92 MiB 3.67 MiB 772.36 KiB
a673305+dirty 2.92 MiB 3.67 MiB 772.46 KiB

@antonis antonis mentioned this pull request Nov 27, 2024
Merged
10 tasks
lucas-zimerman
lucas-zimerman approved these changes Nov 27, 2024
View reviewed changes
Copy link
Collaborator

@lucas-zimerman lucas-zimerman left a comment

Choose a reason for hiding this comment

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

All seems good, thank you for splitting the PR!

krystofwoldrich
krystofwoldrich reviewed Nov 28, 2024
View reviewed changes
CHANGELOG.md Show resolved Hide resolved
krystofwoldrich
krystofwoldrich reviewed Nov 28, 2024
View reviewed changes
packages/core/src/js/client.ts Outdated Show resolved Hide resolved
@antonis @krystofwoldrich
Update CHANGELOG.md
9ea5496 
Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com>
krystofwoldrich
krystofwoldrich reviewed Nov 28, 2024
View reviewed changes
packages/core/src/js/sdk.tsx Outdated Show resolved Hide resolved
krystofwoldrich
krystofwoldrich reviewed Nov 28, 2024
View reviewed changes
packages/core/src/js/client.ts Show resolved Hide resolved
@krystofwoldrich
Copy link
Contributor

krystofwoldrich commented Nov 28, 2024

https://github.com/getsentry/sentry-javascript/blob/dafd51054d8b2ab2030fa0b16ad0fd70493b6e08/packages/core/test/lib/feedback.test.ts#L14

Let's copy this tests of the API to our RN repository, to ensure it behaves as expected. I would like to start doing this going forward, as we encountered in the past bug caused by change of behavior of the API which was not obvious from the changelog and thus we missed it.

@antonis
Copy link
Contributor Author

antonis commented Nov 28, 2024

https://github.com/getsentry/sentry-javascript/blob/dafd51054d8b2ab2030fa0b16ad0fd70493b6e08/packages/core/test/lib/feedback.test.ts#L14
Let's copy this tests of the API to our RN repository, to ensure it behaves as expected. I would like to start doing this going forward, as we encountered in the past bug caused by change of behavior of the API which was not obvious from the changelog and thus we missed it.

Copied the tests with f9d2b59

krystofwoldrich
krystofwoldrich reviewed Nov 28, 2024
View reviewed changes
packages/core/src/js/client.ts Show resolved Hide resolved
@antonis antonis mentioned this pull request Dec 2, 2024
Merged
10 tasks
krystofwoldrich
krystofwoldrich approved these changes Dec 2, 2024
View reviewed changes
Copy link
Contributor

@krystofwoldrich krystofwoldrich left a comment

Choose a reason for hiding this comment

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

Looks perfect, thank you for the updates.

@krystofwoldrich krystofwoldrich merged commit 6c56eb1 into main Dec 2, 2024
@krystofwoldrich krystofwoldrich deleted the antonis/3859-newCaptureFeedbackAPI branch December 2, 2024 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucas-zimerman lucas-zimerman lucas-zimerman approved these changes

+1 more reviewer

@krystofwoldrich krystofwoldrich krystofwoldrich approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add new captureFeedback API to RN SDK

4 participants

@antonis @kahest @krystofwoldrich @lucas-zimerman
Morty Proxy This is a proxified and sanitized view of the page, visit original site.