-
Notifications
You must be signed in to change notification settings - Fork 52
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
SLCORE-1149 Trace execution of analysis requests and sensors #1232
Conversation
aab7fe4
to
58ef518
Compare
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. My only remark is that the tests are mostly there for coverage but do not really assert anything related to the trace and how it's reported. But it's probably fine
var sampleRateFromSystemProperty = System.getProperty(TRACES_SAMPLE_RATE_PROPERTY); | ||
return Double.parseDouble(sampleRateFromSystemProperty); | ||
} catch (RuntimeException e) { | ||
return TRACES_SAMPLE_RATE_DEFAULT; |
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.
A small log here could help. Just in case...
|
||
public SensorsExecutor(DefaultSensorContext context, SensorOptimizer sensorOptimizer, ProgressMonitor progress, Optional<List<ProjectSensor>> sensors) { | ||
public SensorsExecutor(DefaultSensorContext context, SensorOptimizer sensorOptimizer, ProgressMonitor progress, @Nullable Trace trace, Optional<List<ProjectSensor>> sensors) { |
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.
I thought Nullable parameter injection was not working since the investigation I made recently around DBD and Java SE. But maybe in some specific contexts it does work 🤔
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.
It's a bit weird yes. For the sake of consistency, we could go with Optional
(like for the list of sensors), it does not make a huge difference.
c318d46
to
9b4c482
Compare
Quality Gate passedIssues Measures |
SLCORE-1149