Disable Sentry with SentryOptions

[lbloder]

📜 Description

Adds enabled flag to SentryOptions and ExternalOptions to allow users to disable Sentry.

💡 Motivation and Context

Fixes #2833

💚 How did you test it?

📝 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

[github-actions]

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

Generated by 🚫 dangerJS against 904bf17

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	361.67 ms	422.00 ms	60.33 ms
Size	1.72 MiB	2.29 MiB	575.74 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
496bdfd	301.22 ms	343.96 ms	42.73 ms
fe10f05	314.71 ms	360.62 ms	45.90 ms
fe10f05	304.13 ms	365.65 ms	61.52 ms
f60279b	324.60 ms	345.33 ms	20.73 ms
496bdfd	272.86 ms	407.33 ms	134.48 ms
9246ed4	275.63 ms	321.31 ms	45.69 ms
87b3774	310.48 ms	362.04 ms	51.56 ms
4bf202b	331.20 ms	345.24 ms	14.04 ms
fe10f05	294.30 ms	346.84 ms	52.54 ms
4c237f8	319.84 ms	354.47 ms	34.63 ms

App size

Revision	Plain	With Sentry	Diff
496bdfd	1.72 MiB	2.28 MiB	571.82 KiB
fe10f05	1.72 MiB	2.29 MiB	575.54 KiB
fe10f05	1.72 MiB	2.29 MiB	575.54 KiB
f60279b	1.72 MiB	2.29 MiB	575.23 KiB
496bdfd	1.72 MiB	2.28 MiB	571.82 KiB
9246ed4	1.72 MiB	2.28 MiB	572.22 KiB
87b3774	1.72 MiB	2.29 MiB	575.54 KiB
4bf202b	1.72 MiB	2.29 MiB	575.54 KiB
fe10f05	1.72 MiB	2.29 MiB	575.54 KiB
4c237f8	1.72 MiB	2.29 MiB	575.58 KiB

Previous results on branch: feat/option_to_disable_sentry

Startup times

Revision	Plain	With Sentry	Diff
edb005a	306.40 ms	341.75 ms	35.35 ms
98f72a3	285.59 ms	331.38 ms	45.79 ms
175e652	321.66 ms	336.11 ms	14.45 ms
eee1bad	313.43 ms	371.42 ms	57.99 ms
7682cd0	313.29 ms	341.04 ms	27.75 ms

App size

Revision	Plain	With Sentry	Diff
edb005a	1.72 MiB	2.29 MiB	575.33 KiB
98f72a3	1.72 MiB	2.29 MiB	575.35 KiB
175e652	1.72 MiB	2.29 MiB	575.34 KiB
eee1bad	1.72 MiB	2.29 MiB	575.35 KiB
7682cd0	1.72 MiB	2.29 MiB	575.35 KiB

[codecov]

Codecov Report

Patch coverage: 92.30% and project coverage change: +0.06% 🎉

Comparison is base (7cdf121) 80.45% compared to head (260ddf4) 80.52%.

❗ Current head 260ddf4 differs from pull request most recent head 904bf17. Consider uploading reports for the commit 904bf17 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2840      +/-   ##
============================================
+ Coverage     80.45%   80.52%   +0.06%     
- Complexity     4701     4714      +13     
============================================
  Files           371      371              
  Lines         17556    17566      +10     
  Branches       2364     2365       +1     
============================================
+ Hits          14125    14145      +20     
+ Misses         2454     2447       -7     
+ Partials        977      974       -3     

Files Changed	Coverage Δ
sentry/src/main/java/io/sentry/Sentry.java	85.77% <66.66%> (+3.01%)	⬆️
...entry/src/main/java/io/sentry/ExternalOptions.java	98.05% <100.00%> (+0.05%)	⬆️
sentry/src/main/java/io/sentry/SentryOptions.java	80.20% <100.00%> (+0.24%)	⬆️

... and 2 files with indirect coverage changes

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

[adinauer]

We should check the enabled flag first and not throw if disabled regardless of DSN.

[lbloder]

Yeah, makes sense, any preference between the following two variants?

    if(!options.isEnabled() || (dsn != null && dsn.isEmpty())) {
      close();
      return false;
    } else if (dsn == null) {
      throw new IllegalArgumentException(
        "DSN is required. Use empty string or set enabled to false in SentryOptions to disable SDK.");
    }

    if (!options.isEnabled()) {
      close();
      return false;
    }

    if (dsn == null) {
      throw new IllegalArgumentException(
          "DSN is required. Use empty string or set enabled to false in SentryOptions to disable SDK.");
    } else if (dsn.isEmpty()) {
      close();
      return false;
    }

[adinauer]

Top one seems a bit better as it doesn't duplicate the code.

[adinauer]

LGTM

[romtsn]

LGTM, missing Manifest option, but we're gonna drop it in the next major most likely (v8) and this is probably anyway not gonna be used there, so it's alright

[benoit-lateltin]

Nice PR! Would it be possible to have this sentry.enabled property hot-reloadable via a Spring Cloud Config Server ? Could we use the Environment PropertyResolver to retrieve the prop when processing the event?

[adinauer]

Hello @benoit-lateltin I've created #2939 to track your feature request. Thanks for the suggestion - makes sense. Can't say when we'll get to implementing it though.