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(js-sdk): support release health #2546

Draft
wants to merge 58 commits into
base: feat/js-sdk-integration
Choose a base branch
from

Conversation

buenaflor
Copy link
Contributor

📜 Description

Built on top of PR #2489

💡 Motivation and Context

Adds support for sessions / release health.

💚 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

🔮 Next steps

Copy link
Contributor

github-actions bot commented Jan 2, 2025

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- support release health ([#2546](https://github.com/getsentry/sentry-dart/pull/2546))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against a60012f

Copy link

codecov bot commented Jan 2, 2025

Codecov Report

Attention: Patch coverage is 84.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 88.12%. Comparing base (be08651) to head (a60012f).
Report is 69 commits behind head on feat/js-sdk-integration.

Files with missing lines Patch % Lines
flutter/lib/src/native/c/sentry_native.dart 33.33% 2 Missing ⚠️
flutter/lib/src/native/sentry_native_channel.dart 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                     Coverage Diff                      @@
##           feat/js-sdk-integration    #2546       +/-   ##
============================================================
+ Coverage                    70.98%   88.12%   +17.14%     
============================================================
  Files                           17      119      +102     
  Lines                          579     4397     +3818     
============================================================
+ Hits                           411     3875     +3464     
- Misses                         168      522      +354     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@buenaflor buenaflor changed the title feat: support release health feat(web): support release health Jan 2, 2025
@buenaflor buenaflor changed the title feat(web): support release health feat(js-sdk): support release health Jan 2, 2025
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

the definition of session in JS is about to change on v9
might be a good idea to line it up, so only ship the Dart on JS session using the v9 sdk. you could piggy back on their alpha to ship your on alphas etc

dart/lib/src/native/c/sentry_native.dart Outdated Show resolved Hide resolved
@@ -276,7 +276,7 @@ class SentryOptions {
/// breadcrumbs.
/// In a Flutter environment, this setting also toggles recording of `debugPrint` calls.
/// `debugPrint` calls are only recorded in release builds, though.
bool enablePrintBreadcrumbs = true;
bool enablePrintBreadcrumbs = false;
Copy link
Member

Choose a reason for hiding this comment

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

Is this change needed to be part of the web support? it could have side effects of itself worth tracking as a separate change probably done before the big web support.

Copy link
Contributor Author

@buenaflor buenaflor Jan 2, 2025

Choose a reason for hiding this comment

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

this is temporary for testing, on web with the js sdk integration this seems to run into a recursion so I've disabled it for now I will revert it back after I fixed the issue

@buenaflor
Copy link
Contributor Author

note: currently the PR that this is based on is not merged yet so bunch of unrelated changes will show up for now

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.

2 participants