Skip to content

[FatalIssueReporter] Add BuiltIn as part of Configuration object #436

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 19, 2025
Merged

[FatalIssueReporter] Add BuiltIn as part of Configuration object #436

merged 3 commits into from
Jun 19, 2025

Conversation

@FranAguilera
Copy link
Contributor

@FranAguilera FranAguilera commented Jun 12, 2025 •
edited
Loading

Part of BIT-5634

What

  1. Simplifies FatalIssue reporter initialization and makes it part of Configuration object.
Logger.start(
    apiKey = bitdriftAPIKey,
    apiUrl = BITDRIFT_URL,
    sessionStrategy = SessionStrategy.ActivityBased(),
    configuration = Configuration(enableFatalIssueReporting = true), --> Part of config object now
    ...
)

  1. initIntegrationFatalIssueReporting is now hidden. Will reopen if there is interest in using this mode

  2. Took the opportunity to pass optional context to Logger.start for cases the library needs to be initialized within ContentProviders, etc

NOTE: For now initIntegrationFatalIssueReporting will be a hidden API. If we need to open this again in the future will revisit

Verification

Verified BuiltIn reporting works as expected see session

@FranAguilera FranAguilera marked this pull request as draft June 12, 2025 14:48
@FranAguilera FranAguilera changed the title Sampling built in WIP Jun 12, 2025
@FranAguilera FranAguilera force-pushed the franjam/consolidate-inits branch 5 times, most recently from e688064 to 015024a Compare June 18, 2025 11:28
@FranAguilera FranAguilera changed the title WIP [Android] [FatalIssueReporter] Add BuiltIn as part of Configuration object Jun 18, 2025
@FranAguilera FranAguilera marked this pull request as ready for review June 18, 2025 11:34
@github-actions
Copy link

github-actions bot commented Jun 18, 2025

📦 APK Size Report

Metric Size (KB)
Baseline 3602
Current 3602
Difference 0

✅ APK size unchanged.

@murki
Copy link
Contributor

murki commented Jun 18, 2025

We should change iOS as well if we're ok with this change

murki
murki reviewed Jun 18, 2025
View reviewed changes
platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/reports/FatalIssueReporter.kt
appContext,
FatalIssueReporterStorage(destinationDirectory.destinationDirectory),
)
captureUncaughtExceptionHandler.install(this)
Copy link
Contributor

@murki murki Jun 18, 2025

Choose a reason for hiding this comment

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

no need to include uninstall anymore?

Copy link
Contributor Author

@FranAguilera FranAguilera Jun 18, 2025

Choose a reason for hiding this comment

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

still needed, sorry it's a bit noisy this was part of the setup action before but now done here directly as part of the initial setup

murki
murki approved these changes Jun 18, 2025
View reviewed changes
Copy link
Contributor

@murki murki left a comment

Choose a reason for hiding this comment

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

change lgtm for android but we should have API parity on iOS

@FranAguilera
Copy link
Contributor Author

FranAguilera commented Jun 18, 2025

change lgtm for android but we should have API parity on iOS

thanks! let's discuss in sync in a bit

@FranAguilera FranAguilera force-pushed the franjam/consolidate-inits branch from af18b95 to 39a8ebe Compare June 19, 2025 10:46
@github-actions
Copy link

github-actions bot commented Jun 19, 2025

📦 APK Size Report

Metric Size (KB)
Baseline 3606
Current 3602
Difference -4

✅ APK size decreased by 4 KB.

@FranAguilera FranAguilera enabled auto-merge (squash) June 19, 2025 11:12
@FranAguilera FranAguilera force-pushed the franjam/consolidate-inits branch from 39a8ebe to 83b64b6 Compare June 19, 2025 11:18
@github-actions
Copy link

github-actions bot commented Jun 19, 2025

📦 APK Size Report

Metric Size (KB)
Baseline 3606
Current 3602
Difference -4

✅ APK size decreased by 4 KB.

@FranAguilera FranAguilera force-pushed the franjam/consolidate-inits branch from 83b64b6 to b1483ae Compare June 19, 2025 11:48
@FranAguilera FranAguilera force-pushed the franjam/consolidate-inits branch from b1483ae to 7df9a66 Compare June 19, 2025 11:55
@FranAguilera FranAguilera disabled auto-merge June 19, 2025 12:08
@github-actions
Copy link

github-actions bot commented Jun 19, 2025

📦 APK Size Report

Metric Size (KB)
Baseline 3606
Current 3602
Difference -4

✅ APK size decreased by 4 KB.

@FranAguilera FranAguilera changed the title [Android] [FatalIssueReporter] Add BuiltIn as part of Configuration object [FatalIssueReporter] Add BuiltIn as part of Configuration object Jun 19, 2025
@FranAguilera FranAguilera requested a review from kattrali June 19, 2025 12:29
@FranAguilera FranAguilera force-pushed the franjam/consolidate-inits branch 2 times, most recently from f0d4583 to b3392f7 Compare June 19, 2025 12:38
@github-actions
Copy link

github-actions bot commented Jun 19, 2025

📦 APK Size Report

Metric Size (KB)
Baseline 3606
Current 3602
Difference -4

✅ APK size decreased by 4 KB.

@FranAguilera FranAguilera force-pushed the franjam/consolidate-inits branch from b3392f7 to 0c6069d Compare June 19, 2025 13:06
@github-actions
Copy link

github-actions bot commented Jun 19, 2025

📦 APK Size Report

Metric Size (KB)
Baseline 3606
Current 3602
Difference -4

✅ APK size decreased by 4 KB.

@FranAguilera FranAguilera merged commit c99348e into main Jun 19, 2025
14 checks passed
@FranAguilera FranAguilera deleted the franjam/consolidate-inits branch June 19, 2025 14:14
@github-actions github-actions bot locked and limited conversation to collaborators Jun 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@murki murki murki approved these changes

@kattrali kattrali Awaiting requested review from kattrali

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@FranAguilera @murki