-
-
Notifications
You must be signed in to change notification settings - Fork 4
Autoinstall Profiler #222
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
base: main
Are you sure you want to change the base?
Autoinstall Profiler #222
Changes from all commits
cc3948a
04b26ee
3172dfc
41221cf
90cf614
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,7 @@ | |
| <additionalSourceDirsForSourceContext> | ||
| <value>src/main/extra_param</value> | ||
| </additionalSourceDirsForSourceContext> | ||
| <installProfiler>false</installProfiler> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| </configuration> | ||
| <executions> | ||
| <execution> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| package io.sentry.autoinstall.profiler; | ||
|
|
||
| import io.sentry.autoinstall.AbstractIntegrationInstaller; | ||
| import io.sentry.autoinstall.AutoInstallState; | ||
| import io.sentry.semver.Version; | ||
| import java.util.List; | ||
| import org.eclipse.aether.artifact.Artifact; | ||
| import org.eclipse.aether.artifact.DefaultArtifact; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| public class ProfilerInstallStrategy extends AbstractIntegrationInstaller { | ||
| public static final @NotNull String SENTRY_ASYNC_PROFILER_ID = "sentry-async-profiler"; | ||
|
|
||
| public ProfilerInstallStrategy() { | ||
| this(LoggerFactory.getLogger(ProfilerInstallStrategy.class)); | ||
| } | ||
|
|
||
| public ProfilerInstallStrategy(final @NotNull Logger logger) { | ||
| super(logger); | ||
| } | ||
|
|
||
| @Override | ||
| protected @Nullable Artifact findThirdPartyDependency( | ||
| final @NotNull List<Artifact> resolvedArtifacts) { | ||
| // Return a dummy artifact as there are no third-party dependencies required | ||
| return new DefaultArtifact("dummy:dummy:1.0.0"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks fine as a workaround for now. We should consider a cleaner implementation if there's more of these in the future. |
||
| } | ||
|
|
||
| @Override | ||
| protected boolean shouldInstallModule(final @NotNull AutoInstallState autoInstallState) { | ||
| return autoInstallState.isInstallProfiler(); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Misleading log message when profiler not opted intoMedium Severity When Additional Locations (1)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could just add "or its auto installation has been disabled" to the log message. |
||
|
|
||
| @Override | ||
| protected @NotNull Version minSupportedSentryVersion() { | ||
| return Version.create(8, 23, 0); | ||
| } | ||
|
|
||
| @Override | ||
| protected @NotNull String sentryModuleId() { | ||
| return SENTRY_ASYNC_PROFILER_ID; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,10 @@ public class PluginConfig { | |
| private boolean skipValidateSdkDependencyVersions = DEFAULT_SKIP_VALIDATE_SDK_DEPENDENCY_VERSIONS; | ||
| private @NotNull String additionalSourceDirsForSourceContext = | ||
| DEFAULT_ADDITIONAL_SOURCE_DIRS_FOR_SOURCE_CONTEXT; | ||
| private boolean installProfiler = DEFAULT_INSTALL_PROFILER; | ||
|
|
||
| public static final boolean DEFAULT_INSTALL_PROFILER = false; | ||
| public static final @NotNull String DEFAULT_INSTALL_PROFILER_STRING = "false"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused constant
|
||
|
|
||
| private @Nullable String org; | ||
| private @Nullable String project; | ||
|
|
@@ -100,6 +104,10 @@ public void setSkipValidateSdkDependencyVersions( | |
| this.skipValidateSdkDependencyVersions = skipValidateSdkDependencyVersions; | ||
| } | ||
|
|
||
| public void setInstallProfiler(final boolean installProfiler) { | ||
| this.installProfiler = installProfiler; | ||
| } | ||
|
|
||
| public boolean isSkipAutoInstall() { | ||
| return skipAutoInstall || skip; | ||
| } | ||
|
|
@@ -124,6 +132,10 @@ public boolean isSkipValidateSdkDependencyVersions() { | |
| return skipValidateSdkDependencyVersions; | ||
| } | ||
|
|
||
| public boolean isInstallProfiler() { | ||
| return installProfiler; | ||
| } | ||
|
|
||
| public @Nullable String getOrg() { | ||
| return org; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| package io.sentry.integration.autoinstall.profiler | ||
|
|
||
| import basePom | ||
| import io.sentry.autoinstall.util.SdkVersionInfo | ||
| import io.sentry.integration.installMavenWrapper | ||
| import org.apache.maven.it.VerificationException | ||
| import org.apache.maven.it.Verifier | ||
| import org.junit.jupiter.api.BeforeEach | ||
| import org.junit.jupiter.api.Test | ||
| import org.junit.jupiter.api.io.TempDir | ||
| import java.io.File | ||
| import java.io.IOException | ||
| import java.nio.file.Files | ||
| import java.nio.file.StandardOpenOption | ||
| import kotlin.io.path.Path | ||
|
|
||
| class ProfilerAutoInstallTestIT { | ||
| @TempDir() | ||
| lateinit var file: File | ||
|
|
||
| @BeforeEach | ||
| fun installWrapper() { | ||
| installMavenWrapper(file, "3.8.6") | ||
| } | ||
|
|
||
| fun getPOM( | ||
| enableInstallProfilerFlag: Boolean = true, | ||
| profilerAlreadyInstalled: Boolean = false, | ||
| sentryProfilerVersion: String = SdkVersionInfo.getSentryVersion() ?: "8.23.0", | ||
| installedSentryVersion: String? = null, | ||
| ): String { | ||
| var dependencies = "" | ||
| var pluginConfiguration = "" | ||
|
|
||
| if (profilerAlreadyInstalled) { | ||
| dependencies = | ||
| """ | ||
| <dependency> | ||
| <groupId>io.sentry</groupId> | ||
| <artifactId>sentry-async-profiler</artifactId> | ||
| <version>$sentryProfilerVersion</version> | ||
| </dependency> | ||
| """.trimIndent() | ||
| } | ||
|
|
||
| if (enableInstallProfilerFlag) { | ||
| pluginConfiguration = | ||
| """ | ||
| <configuration> | ||
| <installProfiler>true</installProfiler> | ||
| </configuration> | ||
| """.trimIndent() | ||
| } | ||
|
|
||
| val pomContent = basePom(dependencies, installedSentryVersion, pluginConfiguration) | ||
|
|
||
| Files.write(Path("${file.absolutePath}/pom.xml"), pomContent.toByteArray(), StandardOpenOption.CREATE) | ||
|
|
||
| return file.absolutePath | ||
| } | ||
|
|
||
| @Test | ||
| @Throws(VerificationException::class, IOException::class) | ||
| fun `when installProfiler flag is false does not install`() { | ||
| val path = getPOM(enableInstallProfilerFlag = false) | ||
| val verifier = Verifier(path) | ||
| verifier.isAutoclean = false | ||
| verifier.executeGoal("install") | ||
| verifier.verifyFileNotPresent("target/lib/sentry-async-profiler-${SdkVersionInfo.getSentryVersion()}.jar") | ||
| verifier.resetStreams() | ||
| } | ||
|
|
||
| @Test | ||
| @Throws(VerificationException::class, IOException::class) | ||
| fun `when sentry-async-profiler is a direct dependency does not auto-install`() { | ||
| val path = getPOM(enableInstallProfilerFlag = true, profilerAlreadyInstalled = true) | ||
| val verifier = Verifier(path) | ||
| verifier.isAutoclean = false | ||
| verifier.executeGoal("install") | ||
| verifier.verifyTextInLog("sentry-async-profiler won't be installed because it was already installed directly") | ||
| verifier.verifyFilePresent("target/lib/sentry-async-profiler-${SdkVersionInfo.getSentryVersion()}.jar") | ||
| verifier.resetStreams() | ||
| } | ||
|
|
||
| @Test | ||
| @Throws(VerificationException::class, IOException::class) | ||
| fun `installs sentry-async-profiler with info message`() { | ||
| val path = getPOM() | ||
| val verifier = Verifier(path) | ||
| verifier.deleteDirectory("target") | ||
| verifier.isAutoclean = false | ||
| verifier.executeGoal("install") | ||
| verifier.verifyTextInLog("sentry-async-profiler was successfully installed with version: ${SdkVersionInfo.getSentryVersion()}") | ||
| verifier.verifyFilePresent("target/lib/sentry-async-profiler-${SdkVersionInfo.getSentryVersion()}.jar") | ||
| verifier.resetStreams() | ||
| verifier.deleteDirectory(path) | ||
| } | ||
|
|
||
| @Test | ||
| @Throws(VerificationException::class, IOException::class) | ||
| fun `auto-installed sentry-async-profiler version matches sentry version`() { | ||
| val sentryVersion = "8.23.0" | ||
| val path = getPOM(installedSentryVersion = sentryVersion) | ||
| val verifier = Verifier(path) | ||
| verifier.deleteDirectory("target") | ||
| verifier.isAutoclean = false | ||
| verifier.executeGoal("install") | ||
| verifier.verifyTextInLog("sentry-async-profiler was successfully installed with version: $sentryVersion") | ||
| verifier.verifyFilePresent("target/lib/sentry-async-profiler-$sentryVersion.jar") | ||
| verifier.resetStreams() | ||
| verifier.deleteDirectory(path) | ||
| } | ||
|
|
||
| @Test | ||
| @Throws(VerificationException::class, IOException::class) | ||
| fun `does not install when sentry version is too low`() { | ||
| val sentryVersion = "8.22.0" | ||
| val path = getPOM(installedSentryVersion = sentryVersion) | ||
| val verifier = Verifier(path) | ||
| verifier.deleteDirectory("target") | ||
| verifier.isAutoclean = false | ||
| verifier.executeGoal("install") | ||
| verifier.verifyTextInLog("sentry-async-profiler won't be installed because the current version") | ||
| verifier.verifyFileNotPresent("target/lib/sentry-async-profiler-$sentryVersion.jar") | ||
| verifier.resetStreams() | ||
| verifier.deleteDirectory(path) | ||
| } | ||
| } |


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.
We should mention the name of the option to enable it