add config logging on agent startup #835
Conversation
|
|
||
| private static Sampler delegate = newSampler(DEFAULT_TRACEIDRATIO_SAMPLE_RATIO); | ||
| private static volatile double latestRatio; | ||
| private static volatile Sampler delegate = newSampler(DEFAULT_TRACEIDRATIO_SAMPLE_RATIO); |
There was a problem hiding this comment.
for reviewer - the ratio can be changed on the fly from any thread. The latestRatio is not functionally needed, it's purely for nice output from config logging
| throw new IllegalStateException( | ||
| "expecting exactly one agent jar file in ../agent/build/libs"); | ||
| } | ||
| return jar[0].getAbsolutePath(); |
There was a problem hiding this comment.
for reviewer. As noted by Jonas, this is a cyclic dependency. But in almost all cases of CI testing the jar file is created first, and I think it's more maintainable to have the test here in the module rather than in a different module just for the odd case that would cause failure and is unlikely to ever be actually run (eg gradle clean check)
There was a problem hiding this comment.
I think it's possible to configure the test task to depend on the jar one. This doesn't seem to be a problem right now, so maybe it's not worth adding this config, though I mention it in case it helps in the future.
There was a problem hiding this comment.
There is something like this in the contrib repository: https://github.com/open-telemetry/opentelemetry-java-contrib/blob/main/jmx-scraper/build.gradle.kts#L69, thus I would suggest to try reproducing it.
Unlike with Maven, where artifact packaging comes before after tests, with Gradle this might not be an issue as it will just ensure that the packaged jar is made available before running the tests. Also, doing it like this is cleaner as it prevents having to use relative paths which is brittle, even if we are unlikely to change folder structure soon.
| throw new IllegalStateException( | ||
| "expecting exactly one agent jar file in ../agent/build/libs"); | ||
| } | ||
| return jar[0].getAbsolutePath(); |
There was a problem hiding this comment.
I think it's possible to configure the test task to depend on the jar one. This doesn't seem to be a problem right now, so maybe it's not worth adding this config, though I mention it in case it helps in the future.
|
adjusted per feedback, gradle feeds in the path, thanks deffo better! |
custom/build.gradle.kts
Outdated
| tasks.withType<Test>().configureEach { | ||
| val agentJarTask = project(":agent").tasks.named<Jar>("jar") | ||
| dependsOn(agentJarTask) | ||
| inputs.files(layout.files(agentJarTask)) |
There was a problem hiding this comment.
nit: this line is probably not needed.
jackshirazi
dismissed stale reviews from SylvainJuge and LikeTheSalad
via
99fea7d
3 participants
closes #730