-
-
Notifications
You must be signed in to change notification settings - Fork 444
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
Defaults for Cron - MonitorConfig #3195
Conversation
|
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
8fd337b | 349.16 ms | 459.22 ms | 110.06 ms |
86f0096 | 381.33 ms | 476.58 ms | 95.25 ms |
2a3dd50 | 363.61 ms | 440.18 ms | 76.57 ms |
2465853 | 411.39 ms | 461.10 ms | 49.72 ms |
5e04ee8 | 380.63 ms | 501.84 ms | 121.21 ms |
c554ca2 | 364.98 ms | 433.64 ms | 68.66 ms |
4e29063 | 376.38 ms | 390.98 ms | 14.60 ms |
a3c77bc | 375.80 ms | 445.85 ms | 70.06 ms |
4e260b3 | 378.73 ms | 454.18 ms | 75.45 ms |
eecfab6 | 399.27 ms | 461.82 ms | 62.55 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
8fd337b | 1.72 MiB | 2.27 MiB | 555.00 KiB |
86f0096 | 1.72 MiB | 2.29 MiB | 576.50 KiB |
2a3dd50 | 1.72 MiB | 2.27 MiB | 555.05 KiB |
2465853 | 1.70 MiB | 2.27 MiB | 583.82 KiB |
5e04ee8 | 1.70 MiB | 2.27 MiB | 584.64 KiB |
c554ca2 | 1.70 MiB | 2.27 MiB | 582.25 KiB |
4e29063 | 1.72 MiB | 2.29 MiB | 578.38 KiB |
a3c77bc | 1.72 MiB | 2.29 MiB | 577.53 KiB |
4e260b3 | 1.72 MiB | 2.27 MiB | 554.95 KiB |
eecfab6 | 1.72 MiB | 2.27 MiB | 558.56 KiB |
Previous results on branch: feat/cron-defaults
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
4f3fae8 | 422.57 ms | 497.73 ms | 75.15 ms |
d4fa6bc | 413.08 ms | 499.18 ms | 86.10 ms |
3565505 | 304.96 ms | 351.73 ms | 46.78 ms |
82e7c8f | 424.41 ms | 504.44 ms | 80.03 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
4f3fae8 | 1.70 MiB | 2.27 MiB | 584.99 KiB |
d4fa6bc | 1.70 MiB | 2.27 MiB | 586.75 KiB |
3565505 | 1.70 MiB | 2.28 MiB | 588.45 KiB |
82e7c8f | 1.70 MiB | 2.27 MiB | 584.80 KiB |
…lly set in external options
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md # sentry/src/main/java/io/sentry/SentryOptions.java # sentry/src/test/java/io/sentry/SentryOptionsTest.kt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forgot to send some comments
private @Nullable Long defaultFailureIssueThreshold; | ||
private @Nullable Long defaultRecoveryThreshold; | ||
|
||
public Cron( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m
do we need this ctor? Adding new options will just start off a pyramid, so may be better to just skip this completely and rely on setters instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
# Conflicts: # sentry/src/main/java/io/sentry/SentryOptions.java
📜 Description
Allow configuring global default values for
checkinMargin
,maxRuntime
,timezone
,recoveryThreshold
andfailureIssueThreshold
ofMonitorConfig
throughSentryOptions
.💡 Motivation and Context
Fixes #3122
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps