Skip to content

Leave inApp flag for stack frames undecided in SDK if unsure and let ingestion decide instead #2547

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

Merged
merged 4 commits into from
Feb 16, 2023

Conversation

adinauer
Copy link
Member

📜 Description

If a stack frame neither matches any inAppIncludes nor inAppExcludes, we now treat it as undecided by setting null and leaving the decision to the backend / ingestion.

💡 Motivation and Context

As discussed internally SDKs should only set true / false if they're certain.

💚 How did you test it?

Unit Tests

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • [-] Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Sorry, something went wrong.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 15, 2023

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

Generated by 🚫 dangerJS against 51de431

@github-actions
Copy link
Contributor

github-actions bot commented Feb 15, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 298.79 ms 362.18 ms 63.39 ms
Size 1.73 MiB 2.34 MiB 625.56 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
4b32504 315.69 ms 373.96 ms 58.27 ms
754021c 358.70 ms 361.98 ms 3.28 ms
c1399c1 345.06 ms 385.49 ms 40.43 ms
5fa24ec 326.29 ms 384.53 ms 58.24 ms
fe30606 310.82 ms 335.36 ms 24.55 ms
f6a135d 263.96 ms 383.59 ms 119.63 ms
14c083a 350.82 ms 388.86 ms 38.04 ms
4b32504 357.14 ms 404.04 ms 46.90 ms
fe30606 327.46 ms 351.74 ms 24.28 ms

App size

Revision Plain With Sentry Diff
4b32504 1.73 MiB 2.34 MiB 623.74 KiB
754021c 1.73 MiB 2.33 MiB 623.06 KiB
c1399c1 1.73 MiB 2.33 MiB 620.61 KiB
5fa24ec 1.73 MiB 2.33 MiB 620.61 KiB
fe30606 1.73 MiB 2.34 MiB 623.74 KiB
f6a135d 1.73 MiB 2.33 MiB 623.10 KiB
14c083a 1.73 MiB 2.33 MiB 620.61 KiB
4b32504 1.73 MiB 2.34 MiB 623.74 KiB
fe30606 1.73 MiB 2.34 MiB 623.74 KiB

Previous results on branch: feat/leave-in-app-decision-undecided-if-unsure

Startup times

Revision Plain With Sentry Diff
c0105c6 329.54 ms 399.04 ms 69.50 ms

App size

Revision Plain With Sentry Diff
c0105c6 1.73 MiB 2.34 MiB 624.38 KiB

@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

Base: 80.27% // Head: 80.27% // No change to project coverage 👍

Coverage data is based on head (51de431) compared to base (d5473b8).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2547   +/-   ##
=========================================
  Coverage     80.27%   80.27%           
+ Complexity     3986     3985    -1     
=========================================
  Files           327      327           
  Lines         15010    15010           
  Branches       1977     1977           
=========================================
  Hits          12050    12050           
  Misses         2183     2183           
  Partials        777      777           
Impacted Files Coverage Δ
...c/main/java/io/sentry/SentryStackTraceFactory.java 89.47% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@adinauer adinauer changed the title Leave inApp undecided if unsure Leave inApp undecided in SDK if unsure and let ingestion decide instead Feb 15, 2023
@adinauer adinauer changed the title Leave inApp undecided in SDK if unsure and let ingestion decide instead Leave inApp flag for stack frames undecided in SDK if unsure and let ingestion decide instead Feb 15, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Member

@markushi markushi left a comment

Choose a reason for hiding this comment

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

LGTM!

@adinauer adinauer merged commit efa7b3d into main Feb 16, 2023
@adinauer adinauer deleted the feat/leave-in-app-decision-undecided-if-unsure branch February 16, 2023 07:12
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.

None yet

2 participants