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

feat(feedback): capture envelopes #4535

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented Nov 15, 2024

for #4272; #skip-changelog

capture the user feedback in an envelope. currently, an error is required to have been captured, with its event ID supplied in the user feedback object. so, a dummy error is currently created to satisfy this requirement to be able to see them in the dashboard (see linked issue for progress/results)

TODOs

Copy link

github-actions bot commented Nov 15, 2024

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

Generated by 🚫 dangerJS against ded938e

@armcknight armcknight marked this pull request as ready for review November 15, 2024 00:51
Copy link

codecov bot commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 62.71186% with 66 lines in your changes missing coverage. Please review.

Project coverage is 90.617%. Comparing base (c810e58) to head (ded938e).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
Sources/Sentry/SentryClient.m 35.897% 25 Missing ⚠️
...ations/UserFeedback/SentryUserFeedbackWidget.swift 0.000% 9 Missing ⚠️
Sources/Sentry/SentryHub.m 0.000% 6 Missing ⚠️
...Feedback/SentryUserFeedbackIntegrationDriver.swift 0.000% 6 Missing ⚠️
Sources/Sentry/SentryUserFeedbackIntegration.m 0.000% 5 Missing ⚠️
...grations/UserFeedback/SentryUserFeedbackForm.swift 0.000% 5 Missing ⚠️
Sources/Sentry/SentryEnvelope.m 77.777% 4 Missing ⚠️
Sources/Sentry/SentrySDK.m 0.000% 3 Missing ⚠️
Sources/Sentry/SentryDataCategoryMapper.m 90.909% 1 Missing ⚠️
Sources/Sentry/SentryQueueableRequestManager.m 80.000% 1 Missing ⚠️
... and 1 more
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4535       +/-   ##
=============================================
+ Coverage   90.606%   90.617%   +0.010%     
=============================================
  Files          621       623        +2     
  Lines        71095     71289      +194     
  Branches     25309     25990      +681     
=============================================
+ Hits         64417     64600      +183     
- Misses        6587      6591        +4     
- Partials        91        98        +7     
Files with missing lines Coverage Δ
SentryTestUtils/TestTransportAdapter.swift 84.210% <ø> (+11.483%) ⬆️
Sources/Sentry/SentryHttpTransport.m 98.127% <100.000%> (+0.028%) ⬆️
...ts/Integrations/Feedback/SentryFeedbackTests.swift 100.000% <100.000%> (ø)
...sts/Networking/SentryDataCategoryMapperTests.swift 100.000% <100.000%> (ø)
Sources/Sentry/SentryDataCategoryMapper.m 98.198% <90.909%> (-0.850%) ⬇️
Sources/Sentry/SentryQueueableRequestManager.m 96.666% <80.000%> (-3.334%) ⬇️
...ift/Integrations/UserFeedback/SentryFeedback.swift 97.058% <97.058%> (ø)
Sources/Sentry/SentrySDK.m 87.423% <0.000%> (-0.812%) ⬇️
Sources/Sentry/SentryEnvelope.m 89.320% <77.777%> (-1.206%) ⬇️
Sources/Sentry/SentryUserFeedbackIntegration.m 41.666% <0.000%> (-20.834%) ⬇️
... and 5 more

... and 28 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

Copy link

github-actions bot commented Nov 15, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1231.06 ms 1248.88 ms 17.82 ms
Size 22.32 KiB 766.09 KiB 743.77 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
5e579af 1239.16 ms 1248.49 ms 9.33 ms
5e66a38 1209.10 ms 1233.90 ms 24.79 ms
3277f18 1229.29 ms 1248.92 ms 19.63 ms
3cba0e8 1250.86 ms 1258.39 ms 7.53 ms
7fe37ab 1228.92 ms 1243.86 ms 14.94 ms
0ecf042 1228.25 ms 1250.67 ms 22.42 ms
0970e44 1237.32 ms 1251.53 ms 14.21 ms
e773cad 1221.82 ms 1240.55 ms 18.73 ms
f4a6925 1237.29 ms 1249.35 ms 12.07 ms
216bdf9 1193.69 ms 1217.90 ms 24.21 ms

App size

Revision Plain With Sentry Diff
5e579af 21.58 KiB 656.60 KiB 635.01 KiB
5e66a38 22.85 KiB 408.88 KiB 386.03 KiB
3277f18 22.84 KiB 402.88 KiB 380.03 KiB
3cba0e8 22.84 KiB 403.19 KiB 380.34 KiB
7fe37ab 21.58 KiB 542.28 KiB 520.70 KiB
0ecf042 21.58 KiB 631.82 KiB 610.24 KiB
0970e44 22.32 KiB 761.50 KiB 739.18 KiB
e773cad 21.58 KiB 681.75 KiB 660.17 KiB
f4a6925 21.58 KiB 706.47 KiB 684.89 KiB
216bdf9 21.58 KiB 418.13 KiB 396.54 KiB

Previous results on branch: armcknight/feat(user-feedback)/envelope

Startup times

Revision Plain With Sentry Diff
46f819e 1216.73 ms 1238.35 ms 21.61 ms
3c44525 1230.98 ms 1250.49 ms 19.51 ms
3f71d28 1231.12 ms 1253.88 ms 22.76 ms
c5dac22 1229.73 ms 1252.24 ms 22.51 ms
6fbfc91 1237.08 ms 1256.96 ms 19.88 ms
86abdac 1209.79 ms 1233.82 ms 24.02 ms
68416bc 1227.29 ms 1247.86 ms 20.57 ms

App size

Revision Plain With Sentry Diff
46f819e 22.30 KiB 759.88 KiB 737.58 KiB
3c44525 22.32 KiB 765.14 KiB 742.82 KiB
3f71d28 22.30 KiB 759.88 KiB 737.58 KiB
c5dac22 22.31 KiB 760.57 KiB 738.26 KiB
6fbfc91 21.90 KiB 746.34 KiB 724.43 KiB
86abdac 21.90 KiB 744.30 KiB 722.40 KiB
68416bc 22.31 KiB 760.79 KiB 738.48 KiB

@armcknight armcknight linked an issue Nov 15, 2024 that may be closed by this pull request
@armcknight armcknight marked this pull request as draft November 15, 2024 02:19
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

I know this is a draft, but I already gave you early feedback.

Sources/Sentry/Public/SentryUserFeedback.h Outdated Show resolved Hide resolved
Sources/Sentry/include/SentryClient+Private.h Outdated Show resolved Hide resolved
Sources/Sentry/SentryEnvelope.m Outdated Show resolved Hide resolved
Sources/Sentry/SentryUserFeedback.m Outdated Show resolved Hide resolved
Sources/Sentry/include/SentrySDK+Private.h Outdated Show resolved Hide resolved
Base automatically changed from armcknight/feat(user-feedback)/form to main November 19, 2024 19:58
@armcknight armcknight changed the base branch from main to armcknight/feat(user-feedback)/iteration December 5, 2024 22:50
@armcknight armcknight force-pushed the armcknight/feat(user-feedback)/envelope branch from 6f2ddf0 to 5038ff5 Compare December 5, 2024 22:52
@armcknight armcknight changed the title feat(user-feedback): capture envelopes feat(feedback): capture envelopes Dec 5, 2024
Base automatically changed from armcknight/feat(user-feedback)/iteration to main December 7, 2024 02:53
@armcknight armcknight force-pushed the armcknight/feat(user-feedback)/envelope branch from f84c1c7 to 2398c73 Compare December 12, 2024 21:19
@armcknight armcknight changed the base branch from main to armcknight/feat(feedback)/ui-form-use-user December 12, 2024 21:19
@armcknight armcknight force-pushed the armcknight/feat(user-feedback)/envelope branch from 2398c73 to e548832 Compare December 12, 2024 21:30
@armcknight armcknight force-pushed the armcknight/feat(user-feedback)/envelope branch from c4c1986 to 48c3f2b Compare December 13, 2024 02:42
@armcknight armcknight marked this pull request as ready for review December 13, 2024 02:44
@armcknight armcknight changed the base branch from armcknight/feat(feedback)/ui-form-use-user to armcknight/feat(feedback)/api-visibility December 13, 2024 02:48
@armcknight armcknight force-pushed the armcknight/feat(user-feedback)/envelope branch from 48c3f2b to 4dcee7a Compare December 13, 2024 02:48
@armcknight

This comment was marked as resolved.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

A few comments.

Sources/Sentry/SentryTransportAdapter.m Outdated Show resolved Hide resolved
Sources/Sentry/SentryClient.m Show resolved Hide resolved
Sources/Sentry/SentryClient.m Outdated Show resolved Hide resolved
Sources/Sentry/SentryClient.m Show resolved Hide resolved
Sources/Sentry/SentryDataCategoryMapper.m Show resolved Hide resolved
Sources/Sentry/SentryEnvelope.m Show resolved Hide resolved
Base automatically changed from armcknight/feat(feedback)/api-visibility to main December 14, 2024 00:23
@armcknight

This comment was marked as resolved.

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.

User Feedback envelope (Due: Dec 13)
4 participants