Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
### Features

- add option to ignore source bundle upload failures ([#209](https://github.com/getsentry/sentry-maven-plugin/pull/209))
- Autoinstall Profiler ([#222](https://github.com/getsentry/sentry-maven-plugin/pull/222))
Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated changelog

- enable with `<installProfiler>true</installProfiler>`

### Dependencies

Expand Down
2 changes: 2 additions & 0 deletions examples/sentry-maven-plugin-example/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
<additionalSourceDirsForSourceContext>
<value>src/main/extra_param</value>
</additionalSourceDirsForSourceContext>
<!-- enable auto-installation of sentry-async-profiler for the profiling example below -->
<installProfiler>true</installProfiler>
</configuration>
<executions>
<execution>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import io.sentry.Hint;
import io.sentry.ISpan;
import io.sentry.ITransaction;
import io.sentry.ProfileLifecycle;
import io.sentry.Sentry;
import io.sentry.SentryEvent;
import io.sentry.SentryLevel;
Expand Down Expand Up @@ -83,6 +84,11 @@ public static void main(String[] args) throws InterruptedException {
// Set what percentage of traces should be collected
options.setTracesSampleRate(1.0); // set 0.5 to send 50% of traces

// Profiling configuration options
// Set ProfileLifecycle and ProfileSessionSampleRate
options.setProfileLifecycle(ProfileLifecycle.TRACE);
options.setProfileSessionSampleRate(1.0);

// Determine traces sample rate based on the sampling context
options.setTracesSampler(
context -> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.sentry;

import static io.sentry.config.PluginConfig.DEFAULT_INSTALL_PROFILER_STRING;
import static io.sentry.config.PluginConfig.DEFAULT_SKIP_AUTO_INSTALL_STRING;
import static io.sentry.config.PluginConfig.DEFAULT_SKIP_TELEMETRY_STRING;

Expand All @@ -21,6 +22,10 @@ public class ExtensionConfigurationHolderMojo extends AbstractMojo {
@Parameter(defaultValue = DEFAULT_SKIP_TELEMETRY_STRING)
private boolean skipTelemetry;

@SuppressWarnings("UnusedVariable")
@Parameter(defaultValue = DEFAULT_INSTALL_PROFILER_STRING)
private boolean installProfiler;

@Override
public void execute() throws MojoExecutionException, MojoFailureException {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import static io.sentry.autoinstall.jdbc.JdbcInstallStrategy.SENTRY_JDBC_ID;
import static io.sentry.autoinstall.log4j2.Log4j2InstallStrategy.SENTRY_LOG4J2_ID;
import static io.sentry.autoinstall.logback.LogbackInstallStrategy.SENTRY_LOGBACK_ID;
import static io.sentry.autoinstall.profiler.ProfilerInstallStrategy.SENTRY_ASYNC_PROFILER_ID;
import static io.sentry.autoinstall.quartz.QuartzInstallStrategy.SENTRY_QUARTZ_ID;
import static io.sentry.autoinstall.spring.Spring5InstallStrategy.SENTRY_SPRING_5_ID;
import static io.sentry.autoinstall.spring.Spring6InstallStrategy.SENTRY_SPRING_6_ID;
Expand All @@ -22,6 +23,7 @@
import io.sentry.autoinstall.jdbc.JdbcInstallStrategy;
import io.sentry.autoinstall.log4j2.Log4j2InstallStrategy;
import io.sentry.autoinstall.logback.LogbackInstallStrategy;
import io.sentry.autoinstall.profiler.ProfilerInstallStrategy;
import io.sentry.autoinstall.quartz.QuartzInstallStrategy;
import io.sentry.autoinstall.spring.*;
import io.sentry.config.ConfigParser;
Expand Down Expand Up @@ -64,7 +66,8 @@ public class SentryInstallerLifecycleParticipant extends AbstractMavenLifecycleP
GraphqlInstallStrategy.class,
Graphql22InstallStrategy.class,
JdbcInstallStrategy.class,
QuartzInstallStrategy.class)
QuartzInstallStrategy.class,
ProfilerInstallStrategy.class)
.collect(Collectors.toList());

@Inject @NotNull ArtifactResolver resolver;
Expand Down Expand Up @@ -125,6 +128,7 @@ public void afterProjectsRead(final @NotNull MavenSession session)
autoInstallState.setInstallGraphql(shouldInstallGraphQL(resolvedArtifacts));
autoInstallState.setInstallJdbc(!isModuleAvailable(resolvedArtifacts, SENTRY_JDBC_ID));
autoInstallState.setInstallQuartz(!isModuleAvailable(resolvedArtifacts, SENTRY_QUARTZ_ID));
autoInstallState.setInstallProfiler(shouldInstallProfiler(resolvedArtifacts, pluginConfig));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Profiler skip message logged for every build by default

Medium Severity

Since installProfiler defaults to false (opt-in feature) and ProfilerInstallStrategy is always in the installers list, every build will log "sentry-async-profiler won't be installed because it was already installed directly or its auto-installation has been disabled" — even for users who never requested profiler installation. Other integrations only show their skip message when already installed as a direct dependency, because their install flags default to true. This will confuse customers, as the reviewer noted.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d15cf76. Configure here.


for (final @NotNull Class<? extends AbstractIntegrationInstaller> installerClass :
installers) {
Expand Down Expand Up @@ -160,6 +164,12 @@ private boolean shouldInstallGraphQL(final @NotNull List<Artifact> resolvedArtif
|| isModuleAvailable(resolvedArtifacts, SENTRY_GRAPHQL_22_ID));
}

private boolean shouldInstallProfiler(
final @NotNull List<Artifact> resolvedArtifacts, final @NotNull PluginConfig pluginConfig) {
return pluginConfig.isInstallProfiler()
&& !(isModuleAvailable(resolvedArtifacts, SENTRY_ASYNC_PROFILER_ID));
}

public static boolean isModuleAvailable(
final @NotNull List<Artifact> resolvedArtifacts, final @NotNull String artifactId) {
return resolvedArtifacts.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ public abstract class AbstractIntegrationInstaller {

protected abstract @NotNull String sentryModuleId();

protected @NotNull String skippedInstallMessage(
final @NotNull List<Artifact> resolvedArtifacts,
final @NotNull AutoInstallState autoInstallState) {
return sentryModuleId() + " won't be installed because it was already installed directly";
}

protected AbstractIntegrationInstaller(final @NotNull org.slf4j.Logger logger) {
this.logger = logger;
}
Expand All @@ -41,8 +47,7 @@ public void install(
final @NotNull List<Artifact> resolvedArtifacts,
final @NotNull AutoInstallState autoInstallState) {
if (!shouldInstallModule(autoInstallState)) {
logger.info(
sentryModuleId() + " won't be installed because it was already installed directly");
logger.info(skippedInstallMessage(resolvedArtifacts, autoInstallState));
return;
}

Expand Down Expand Up @@ -92,7 +97,7 @@ public void install(
+ " won't be installed because the current version ("
+ sentrySemVersion
+ ") is lower than the minimum supported sentry version "
+ sentryVersion);
+ minSupportedSentryVersion());
return;
}
} catch (IllegalArgumentException ex) {
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/io/sentry/autoinstall/AutoInstallState.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public class AutoInstallState {
private boolean installGraphql = false;

private boolean installQuartz = false;
private boolean installProfiler = false;

public AutoInstallState(final @NotNull String sentryVersion) {
this.sentryVersion = sentryVersion;
Expand Down Expand Up @@ -72,4 +73,12 @@ public boolean isInstallQuartz() {
public void setInstallQuartz(boolean installQuartz) {
this.installQuartz = installQuartz;
}

public boolean isInstallProfiler() {
return installProfiler;
}

public void setInstallProfiler(boolean installProfiler) {
this.installProfiler = installProfiler;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
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");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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();
}

@Override
protected @NotNull String skippedInstallMessage(
final @NotNull List<Artifact> resolvedArtifacts,
final @NotNull AutoInstallState autoInstallState) {
return sentryModuleId()
+ " won't be installed because it was already installed directly or its auto-installation has been disabled";
}

@Override
protected @NotNull Version minSupportedSentryVersion() {
return Version.create(8, 23, 0);
}

@Override
protected @NotNull String sentryModuleId() {
return SENTRY_ASYNC_PROFILER_ID;
}
}
5 changes: 5 additions & 0 deletions src/main/java/io/sentry/config/ConfigParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public class ConfigParser {
private static final @NotNull String AUTH_TOKEN_OPTION = "authToken";
private static final @NotNull String ADDITIONAL_SOURCE_DIRS_FOR_SOURCE_CONTEXT =
"additionalSourceDirsForSourceContext";
private static final @NotNull String INSTALL_PROFILER_FLAG = "installProfiler";

public @NotNull PluginConfig parseConfig(final @NotNull MavenProject project) {
final @NotNull PluginConfig pluginConfig = new PluginConfig();
Expand Down Expand Up @@ -101,6 +102,10 @@ public class ConfigParser {
: Arrays.stream(dom.getChild(ADDITIONAL_SOURCE_DIRS_FOR_SOURCE_CONTEXT).getChildren())
.map(Xpp3Dom::getValue)
.collect(Collectors.joining(",")));

pluginConfig.setInstallProfiler(
dom.getChild(INSTALL_PROFILER_FLAG) != null
&& Boolean.parseBoolean(dom.getChild(INSTALL_PROFILER_FLAG).getValue()));
}

return pluginConfig;
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/io/sentry/config/PluginConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ public class PluginConfig {
public static final @NotNull String DEFAULT_SKIP_VALIDATE_SDK_DEPENDENCY_VERSIONS_STRING =
"false";
public static final @NotNull String DEFAULT_ADDITIONAL_SOURCE_DIRS_FOR_SOURCE_CONTEXT = "";
public static final boolean DEFAULT_INSTALL_PROFILER = false;
public static final @NotNull String DEFAULT_INSTALL_PROFILER_STRING = "false";

private boolean skip = DEFAULT_SKIP;
private boolean skipAutoInstall = DEFAULT_SKIP_AUTO_INSTALL;
Expand All @@ -36,6 +38,7 @@ 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;

private @Nullable String org;
private @Nullable String project;
Expand Down Expand Up @@ -100,6 +103,10 @@ public void setSkipValidateSdkDependencyVersions(
this.skipValidateSdkDependencyVersions = skipValidateSdkDependencyVersions;
}

public void setInstallProfiler(final boolean installProfiler) {
this.installProfiler = installProfiler;
}

public boolean isSkipAutoInstall() {
return skipAutoInstall || skip;
}
Expand All @@ -124,6 +131,10 @@ public boolean isSkipValidateSdkDependencyVersions() {
return skipValidateSdkDependencyVersions;
}

public boolean isInstallProfiler() {
return installProfiler;
}

public @Nullable String getOrg() {
return org;
}
Expand Down
Loading
Loading