-
-
Notifications
You must be signed in to change notification settings - Fork 446
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
Support SENTRY_TRACES_SAMPLE_RATE conf. via env variables. #1171
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1171 +/- ##
============================================
+ Coverage 75.25% 75.27% +0.02%
- Complexity 1662 1665 +3
============================================
Files 173 173
Lines 5812 5817 +5
Branches 568 570 +2
============================================
+ Hits 4374 4379 +5
Misses 1174 1174
Partials 264 264
Continue to review full report at Codecov.
|
@Nullable | ||
default Double getDoubleProperty(final @NotNull String property) { | ||
final String result = getProperty(property); | ||
return result != null ? Double.valueOf(result) : null; |
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.
should we account if it's not a Double? like catch NumberFormatException
and return null?
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.
Given this is done at startup and the value must be double, probably OK to blow up here
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.
that's o but kinda goes against our philosophy
https://develop.sentry.dev/sdk/philosophy/#degrade-gracefully
I'd prefer blowing up only if there's no DSN, any other reason would be a bug IMO
@Nullable | ||
default Double getDoubleProperty(final @NotNull String property) { | ||
final String result = getProperty(property); | ||
return result != null ? Double.valueOf(result) : null; |
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.
Given this is done at startup and the value must be double, probably OK to blow up here
📜 Description
Support SENTRY_TRACES_SAMPLE_RATE conf. via env variables.
💡 Motivation and Context
Fixes #1169
💚 How did you test it?
Unit tests.
📝 Checklist