From f10778608f799de4165440f80f4cd5e938841c18 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 14 Aug 2020 21:55:46 +0200 Subject: [PATCH 01/27] Initial Sentry Spring Boot Starter. --- buildSrc/src/main/java/Config.kt | 11 + sentry-spring-boot-starter/build.gradle.kts | 116 +++++++ .../spring/boot/SentryAutoConfiguration.java | 108 ++++++ .../sentry/spring/boot/SentryProperties.java | 325 ++++++++++++++++++ .../main/resources/META-INF/spring.factories | 2 + .../boot/SentryAutoConfigurationTest.kt | 97 ++++++ 6 files changed, 659 insertions(+) create mode 100644 sentry-spring-boot-starter/build.gradle.kts create mode 100644 sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java create mode 100644 sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java create mode 100644 sentry-spring-boot-starter/src/main/resources/META-INF/spring.factories create mode 100644 sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt index 8b2894c9c..5664894d6 100644 --- a/buildSrc/src/main/java/Config.kt +++ b/buildSrc/src/main/java/Config.kt @@ -2,11 +2,15 @@ object Config { val kotlinVersion = "1.3.72" val kotlinStdLib = "stdlib-jdk8" + val springBootVersion = "2.3.3.RELEASE" + object BuildPlugins { val androidGradle = "com.android.tools.build:gradle:4.0.1" val kotlinGradlePlugin = "gradle-plugin" val buildConfig = "com.github.gmazzo.buildconfig" val buildConfigVersion = "2.0.2" + val springBoot = "org.springframework.boot" + val springDependencyManagement = "io.spring.dependency-management" } object Android { @@ -32,6 +36,11 @@ object Config { val lifecycleVersion = "2.2.0" val lifecycleProcess = "androidx.lifecycle:lifecycle-process:$lifecycleVersion" val lifecycleCommonJava8 = "androidx.lifecycle:lifecycle-common-java8:$lifecycleVersion" + + val springBootStarter = "org.springframework.boot:spring-boot-starter:$springBootVersion" + + val springWeb = "org.springframework:spring-webmvc" + val servletApi = "javax.servlet:javax.servlet-api" } object TestLibs { @@ -44,6 +53,7 @@ object Config { val robolectric = "org.robolectric:robolectric:4.3.1" val mockitoKotlin = "com.nhaarman.mockitokotlin2:mockito-kotlin:2.2.0" val awaitility = "org.awaitility:awaitility-kotlin:4.0.3" + val springBootStarterTest = "org.springframework.boot:spring-boot-starter-test:$springBootVersion" } object QualityPlugins { @@ -62,6 +72,7 @@ object Config { object Sentry { val SENTRY_JAVA_SDK_NAME = "sentry.java" val SENTRY_ANDROID_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.android" + val SENTRY_SPRING_BOOT_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.spring-boot" val group = "io.sentry" val description = "SDK for sentry.io" val website = "https://sentry.io" diff --git a/sentry-spring-boot-starter/build.gradle.kts b/sentry-spring-boot-starter/build.gradle.kts new file mode 100644 index 000000000..f9d0ece40 --- /dev/null +++ b/sentry-spring-boot-starter/build.gradle.kts @@ -0,0 +1,116 @@ +import com.novoda.gradle.release.PublishExtension + +plugins { + `java-library` + kotlin("jvm") + jacoco + id(Config.QualityPlugins.errorProne) + id(Config.Deploy.novodaBintray) + id(Config.QualityPlugins.gradleVersions) + id(Config.BuildPlugins.buildConfig) version Config.BuildPlugins.buildConfigVersion + id(Config.BuildPlugins.springBoot) version Config.springBootVersion apply false +} + +apply(plugin = Config.BuildPlugins.springDependencyManagement) + +the().apply { + imports { + mavenBom(org.springframework.boot.gradle.plugin.SpringBootPlugin.BOM_COORDINATES) + } +} + +configure { + sourceCompatibility = JavaVersion.VERSION_1_8 + targetCompatibility = JavaVersion.VERSION_1_8 +} + +tasks.withType().configureEach { + kotlinOptions.jvmTarget = JavaVersion.VERSION_1_8.toString() +} + +dependencies { + api(project(":sentry-core")) + implementation(Config.Libs.springBootStarter) + implementation(Config.Libs.springWeb) + implementation(Config.Libs.servletApi) + + // TODO: annotationProcessor("org.springframework.boot:spring-boot-autoconfigure-processor") + + compileOnly(Config.CompileOnly.nopen) + errorprone(Config.CompileOnly.nopenChecker) + errorprone(Config.CompileOnly.errorprone) + errorproneJavac(Config.CompileOnly.errorProneJavac8) + compileOnly(Config.CompileOnly.jetbrainsAnnotations) + + // tests + testImplementation(kotlin(Config.kotlinStdLib)) + testImplementation(Config.TestLibs.kotlinTestJunit) + testImplementation(Config.TestLibs.mockitoKotlin) + testImplementation(Config.TestLibs.springBootStarterTest) +} + +configure { + test { + java.srcDir("src/test/java") + } +} + +jacoco { + toolVersion = Config.QualityPlugins.jacocoVersion +} + +tasks.jacocoTestReport { + reports { + xml.isEnabled = true + html.isEnabled = false + } +} + +tasks { + jacocoTestCoverageVerification { + violationRules { + // TODO: Raise the minimum to a sensible value. + rule { limit { minimum = BigDecimal.valueOf(0.1) } } + } + } + check { + dependsOn(jacocoTestCoverageVerification) + dependsOn(jacocoTestReport) + } +} + +buildConfig { + useJavaOutput() + packageName("io.sentry.spring.boot") + buildConfigField("String", "SENTRY_SPRING_BOOT_SDK_NAME", "\"${Config.Sentry.SENTRY_SPRING_BOOT_SDK_NAME}\"") + buildConfigField("String", "VERSION_NAME", "\"${project.version}\"") +} + +val generateBuildConfig by tasks +tasks.withType().configureEach { + dependsOn(generateBuildConfig) +} + +//TODO: move these blocks to parent gradle file, DRY +configure { + userOrg = Config.Sentry.userOrg + groupId = project.group.toString() + publishVersion = project.version.toString() + desc = Config.Sentry.description + website = Config.Sentry.website + repoName = Config.Sentry.repoName + setLicences(Config.Sentry.licence) + setLicenceUrls(Config.Sentry.licenceUrl) + issueTracker = Config.Sentry.issueTracker + repository = Config.Sentry.repository + sign = Config.Deploy.sign + artifactId = project.name + uploadName = "${project.group}:${project.name}" + devId = Config.Sentry.userOrg + devName = Config.Sentry.devName + devEmail = Config.Sentry.devEmail + scmConnection = Config.Sentry.scmConnection + scmDevConnection = Config.Sentry.scmDevConnection + scmUrl = Config.Sentry.scmUrl +} + diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java new file mode 100644 index 000000000..3d79468c9 --- /dev/null +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -0,0 +1,108 @@ +package io.sentry.spring.boot; + +import com.jakewharton.nopen.annotation.Open; +import io.sentry.core.EventProcessor; +import io.sentry.core.HubAdapter; +import io.sentry.core.IHub; +import io.sentry.core.Integration; +import io.sentry.core.Sentry; +import io.sentry.core.SentryClient; +import io.sentry.core.SentryOptions; +import io.sentry.core.protocol.SdkVersion; +import io.sentry.core.transport.ITransport; +import io.sentry.core.transport.ITransportGate; +import org.jetbrains.annotations.NotNull; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +@ConditionalOnProperty(name = "sentry.enabled", havingValue = "true", matchIfMissing = true) +@Open +public class SentryAutoConfiguration { + + /** Registers general purpose Sentry related beans. */ + @Configuration + @ConditionalOnProperty("sentry.dsn") + @EnableConfigurationProperties(SentryProperties.class) + @Open + static class HubConfiguration { + + @Bean + @ConditionalOnMissingBean + public Sentry.OptionsConfiguration optionsOptionsConfiguration( + ObjectProvider beforeSendCallback, + ObjectProvider beforeBreadcrumbCallback, + ObjectProvider eventProcessors, + ObjectProvider integrations, + ObjectProvider transportGate, + ObjectProvider transport) { + return options -> { + beforeSendCallback.ifAvailable(options::setBeforeSend); + beforeBreadcrumbCallback.ifAvailable(options::setBeforeBreadcrumb); + eventProcessors.stream().forEach(options::addEventProcessor); + integrations.stream().forEach(options::addIntegration); + transportGate.ifAvailable(options::setTransportGate); + transport.ifAvailable(options::setTransport); + }; + } + + @Bean + IHub sentryHub( + Sentry.OptionsConfiguration optionsConfiguration, + SentryProperties properties, + ObjectProvider sentryClient) { + Sentry.init( + options -> { + properties.applyTo(options); + optionsConfiguration.configure(options); + options.setSentryClientName(BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME); + options.setSdkVersion(createSdkVersion(options)); + }); + sentryClient.ifAvailable(Sentry::bindClient); + return HubAdapter.getInstance(); + } + + /** Registers beans specific to Spring MVC. */ + @Configuration + @ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET) + @Open + static class SentryWebMvcConfiguration { + + @Bean + SentryRequestHttpServletRequestProcessor sentryEventHttpServletRequestProcessor() { + return new SentryRequestHttpServletRequestProcessor(); + } + + @Bean + SentryUserHttpServletRequestProcessor sentryUserHttpServletRequestProcessor() { + return new SentryUserHttpServletRequestProcessor(); + } + + @Bean + public FilterRegistrationBean sentryRequestFilter(IHub sentryHub) { + return new FilterRegistrationBean<>(new SentryRequestFilter(sentryHub)); + } + } + + private static @NotNull SdkVersion createSdkVersion(@NotNull SentryOptions sentryOptions) { + SdkVersion sdkVersion = sentryOptions.getSdkVersion(); + + if (sdkVersion == null) { + sdkVersion = new SdkVersion(); + } + + sdkVersion.setName(BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME); + final String version = BuildConfig.VERSION_NAME; + sdkVersion.setVersion(version); + sdkVersion.addPackage("maven:sentry-spring-boot-starter", version); + + return sdkVersion; + } + } +} diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java new file mode 100644 index 000000000..3246a347c --- /dev/null +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java @@ -0,0 +1,325 @@ +package io.sentry.spring.boot; + +import com.jakewharton.nopen.annotation.Open; +import io.sentry.core.SentryLevel; +import io.sentry.core.SentryOptions; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * Configuration for Sentry in a shape of Spring Boot configuration bean. Example: + * + * 
+ * sentry:
+ *     enabled: true
+ *     dsn: https://00059966e6224d03a77ea5eca10fbe18@sentry.mycompany.com/14
+ *     release: "1.0.1"
+ *     dist: x86
+ *     environment: staging
+ *     server-name: megaServer
+ *
+ */ +@ConfigurationProperties("sentry") +@Open +public class SentryProperties { + + /** Whether Sentry integration should be enabled. */ + private boolean enabled = true; + + /** + * The DSN tells the SDK where to send the events to. If this value is not provided, the SDK will + * just not send any events. + */ + private String dsn = ""; + + /** + * Controls how many seconds to wait before shutting down. Sentry SDKs send events from a + * background queue and this queue is given a certain amount to drain pending events. + */ + private Long shutdownTimeoutMillis; + + /** + * Controls how many seconds to wait before flushing down. Sentry SDKs cache events from a + * background queue and this queue is given a certain amount to drain pending events. + */ + private Long flushTimeoutMillis; + + /** Read timeout in milliseconds. */ + private Integer readTimeoutMillis; + + /** Whether to ignore TLS errors. */ + private Boolean bypassSecurity; + + /** + * Turns debug mode on or off. If debug is enabled SDK will attempt to print out useful debugging + * information if something goes wrong. Default is disabled. + */ + private Boolean debug; + + /** minimum LogLevel to be used if debug is enabled */ + private SentryLevel diagnosticLevel = SentryLevel.DEBUG; + + /** + * Sentry client name used for the HTTP authHeader and userAgent eg + * sentry.{language}.{platform}/{version} eg sentry.java.android/2.0.0 would be a valid case + */ + private String sentryClientName; + + /** This variable controls the total amount of breadcrumbs that should be captured. */ + private Integer maxBreadcrumbs; + + /** Sets the release. SDK will try to automatically configure a release out of the box */ + private String release; + + /** + * Sets the environment. This string is freeform and not set by default. A release can be + * associated with more than one environment to separate them in the UI Think staging vs prod or + * similar. + */ + private String environment; + + /** + * Configures the sample rate as a percentage of events to be sent in the range of 0.0 to 1.0. if + * 1.0 is set it means that 100% of events are sent. If set to 0.1 only 10% of events will be + * sent. Events are picked randomly. + */ + private Double sampleRate; + + /** + * A list of string prefixes of module names that do not belong to the app, but rather third-party + * packages. Modules considered not to be part of the app will be hidden from stack traces by + * default. + */ + private List inAppExcludes = new ArrayList<>(); + + /** + * A list of string prefixes of module names that belong to the app. This option takes precedence + * over inAppExcludes. + */ + private List inAppIncludes = new ArrayList<>(); + + /** Sets the distribution. Think about it together with release and environment */ + private String dist; + + /** When enabled, threads are automatically attached to all logged events. */ + private Boolean attachThreads; + + /** + * When enabled, stack traces are automatically attached to all threads logged. Stack traces are + * always attached to exceptions but when this is set stack traces are also sent with threads + */ + private Boolean attachStacktrace; + + /** The server name used in the Sentry messages. */ + private String serverName; + + /** + * Applies configuration from this instance to the {@link SentryOptions} instance. + * + * @param options the instance of {@link SentryOptions} to apply the configuration to + */ + public void applyTo(SentryOptions options) { + options.setDsn(this.getDsn()); + Optional.ofNullable(maxBreadcrumbs).ifPresent(options::setMaxBreadcrumbs); + Optional.ofNullable(environment).ifPresent(options::setEnvironment); + Optional.ofNullable(shutdownTimeoutMillis).ifPresent(options::setShutdownTimeout); + Optional.ofNullable(flushTimeoutMillis).ifPresent(options::setFlushTimeoutMillis); + Optional.ofNullable(readTimeoutMillis).ifPresent(options::setReadTimeoutMillis); + Optional.ofNullable(sampleRate).ifPresent(options::setSampleRate); + Optional.ofNullable(bypassSecurity).ifPresent(options::setBypassSecurity); + Optional.ofNullable(debug).ifPresent(options::setDebug); + Optional.ofNullable(attachThreads).ifPresent(options::setAttachThreads); + Optional.ofNullable(attachStacktrace).ifPresent(options::setAttachStacktrace); + Optional.ofNullable(diagnosticLevel).ifPresent(options::setDiagnosticLevel); + Optional.ofNullable(dist).ifPresent(options::setDist); + Optional.ofNullable(release).ifPresent(options::setRelease); + Optional.ofNullable(sampleRate).ifPresent(options::setSampleRate); + Optional.ofNullable(serverName).ifPresent(options::setServerName); + } + + public boolean isEnabled() { + return enabled; + } + + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } + + public String getDsn() { + return dsn; + } + + public void setDsn(String dsn) { + this.dsn = dsn; + } + + public long getShutdownTimeoutMillis() { + return shutdownTimeoutMillis; + } + + public void setShutdownTimeoutMillis(long shutdownTimeoutMillis) { + this.shutdownTimeoutMillis = shutdownTimeoutMillis; + } + + public boolean isDebug() { + return debug; + } + + public void setDebug(boolean debug) { + this.debug = debug; + } + + public SentryLevel getDiagnosticLevel() { + return diagnosticLevel; + } + + public void setDiagnosticLevel(SentryLevel diagnosticLevel) { + this.diagnosticLevel = diagnosticLevel; + } + + public String getSentryClientName() { + return sentryClientName; + } + + public void setSentryClientName(String sentryClientName) { + this.sentryClientName = sentryClientName; + } + + public int getMaxBreadcrumbs() { + return maxBreadcrumbs; + } + + public void setMaxBreadcrumbs(int maxBreadcrumbs) { + this.maxBreadcrumbs = maxBreadcrumbs; + } + + public String getRelease() { + return release; + } + + public void setRelease(String release) { + this.release = release; + } + + public String getEnvironment() { + return environment; + } + + public void setEnvironment(String environment) { + this.environment = environment; + } + + public Double getSampleRate() { + return sampleRate; + } + + public void setSampleRate(Double sampleRate) { + this.sampleRate = sampleRate; + } + + public List getInAppExcludes() { + return inAppExcludes; + } + + public void setInAppExcludes(List inAppExcludes) { + this.inAppExcludes = inAppExcludes; + } + + public List getInAppIncludes() { + return inAppIncludes; + } + + public void setInAppIncludes(List inAppIncludes) { + this.inAppIncludes = inAppIncludes; + } + + public String getDist() { + return dist; + } + + public void setDist(String dist) { + this.dist = dist; + } + + public boolean isAttachThreads() { + return attachThreads; + } + + public void setAttachThreads(boolean attachThreads) { + this.attachThreads = attachThreads; + } + + public boolean isAttachStacktrace() { + return attachStacktrace; + } + + public void setAttachStacktrace(boolean attachStacktrace) { + this.attachStacktrace = attachStacktrace; + } + + public String getServerName() { + return serverName; + } + + public void setServerName(String serverName) { + this.serverName = serverName; + } + + public void setShutdownTimeoutMillis(Long shutdownTimeoutMillis) { + this.shutdownTimeoutMillis = shutdownTimeoutMillis; + } + + public Long getFlushTimeoutMillis() { + return flushTimeoutMillis; + } + + public void setFlushTimeoutMillis(Long flushTimeoutMillis) { + this.flushTimeoutMillis = flushTimeoutMillis; + } + + public Integer getReadTimeoutMillis() { + return readTimeoutMillis; + } + + public void setReadTimeoutMillis(Integer readTimeoutMillis) { + this.readTimeoutMillis = readTimeoutMillis; + } + + public Boolean getBypassSecurity() { + return bypassSecurity; + } + + public void setBypassSecurity(Boolean bypassSecurity) { + this.bypassSecurity = bypassSecurity; + } + + public Boolean getDebug() { + return debug; + } + + public void setDebug(Boolean debug) { + this.debug = debug; + } + + public void setMaxBreadcrumbs(Integer maxBreadcrumbs) { + this.maxBreadcrumbs = maxBreadcrumbs; + } + + public Boolean getAttachThreads() { + return attachThreads; + } + + public void setAttachThreads(Boolean attachThreads) { + this.attachThreads = attachThreads; + } + + public Boolean getAttachStacktrace() { + return attachStacktrace; + } + + public void setAttachStacktrace(Boolean attachStacktrace) { + this.attachStacktrace = attachStacktrace; + } +} diff --git a/sentry-spring-boot-starter/src/main/resources/META-INF/spring.factories b/sentry-spring-boot-starter/src/main/resources/META-INF/spring.factories new file mode 100644 index 000000000..2eb623a44 --- /dev/null +++ b/sentry-spring-boot-starter/src/main/resources/META-INF/spring.factories @@ -0,0 +1,2 @@ +org.springframework.boot.autoconfigure.EnableAutoConfiguration=\ +io.sentry.spring.boot.SentryAutoConfiguration diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt new file mode 100644 index 000000000..47cd06e5e --- /dev/null +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -0,0 +1,97 @@ +package io.sentry.spring.boot + +import io.sentry.core.IHub +import io.sentry.core.Sentry +import io.sentry.core.SentryOptions +import kotlin.test.Test +import org.assertj.core.api.Assertions.assertThat +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.boot.test.context.runner.WebApplicationContextRunner +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration + +class SentryAutoConfigurationTest { + private val contextRunner = ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(SentryAutoConfiguration::class.java, WebMvcAutoConfiguration::class.java)) + + private val webContextRunner = WebApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(SentryAutoConfiguration::class.java)) + + @Test + fun `hub is not created when auto-configuration is disabled`() { + contextRunner.withPropertyValues("sentry.enabled=false", "sentry.dsn=http://key@localhost/proj") + .run { + assertThat(it).doesNotHaveBean(IHub::class.java) + } + } + + @Test + fun `hub is created when auto-configuration is enabled`() { + contextRunner.withPropertyValues("sentry.enabled=true", "sentry.dsn=http://key@localhost/proj") + .run { + assertThat(it).hasSingleBean(IHub::class.java) + } + } + + @Test + fun `hub is created when dsn is provided`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .run { + assertThat(it).hasSingleBean(IHub::class.java) + } + } + + @Test + fun `hub is not created when dsn is provided but sentry is disabled`() { + contextRunner.withPropertyValues("sentry.enabled=false", "sentry.dsn=http://key@localhost/proj") + .run { + assertThat(it).doesNotHaveBean(IHub::class.java) + } + } + + @Test + fun `OptionsConfiguration is created if custom one is not provided`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .run { + assertThat(it).hasSingleBean(Sentry.OptionsConfiguration::class.java) + } + } + + @Test + fun `OptionsConfiguration is not created if custom one is provided`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(CustomOptionsConfigurationConfiguration::class.java) + .run { + assertThat(it).hasSingleBean(Sentry.OptionsConfiguration::class.java) + assertThat(it.getBean(Sentry.OptionsConfiguration::class.java, "customOptionsConfiguration")).isNotNull + } + } + + @Test + fun `does not register event processors for non web-servlet application type`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .run { + assertThat(it).doesNotHaveBean(SentryRequestHttpServletRequestProcessor::class.java) + assertThat(it).doesNotHaveBean(SentryUserHttpServletRequestProcessor::class.java) + } + } + + @Test + fun `registers event processors for web servlet application type`() { + webContextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .run { + assertThat(it).hasSingleBean(SentryRequestHttpServletRequestProcessor::class.java) + assertThat(it).hasSingleBean(SentryUserHttpServletRequestProcessor::class.java) + } + } + + @Configuration(proxyBeanMethods = false) + open class CustomOptionsConfigurationConfiguration { + + @Bean + open fun customOptionsConfiguration() = Sentry.OptionsConfiguration() { + } + } +} From 58aab5acbc67ac1b4b9accd3c2a030c88c83f089 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Tue, 18 Aug 2020 12:30:25 +0200 Subject: [PATCH 02/27] Pass request and user data to Sentry event. --- .../spring/boot/SentryAutoConfiguration.java | 12 ++-- .../spring/boot/SentryRequestFilter.java | 31 ++++++++++ ...tryRequestHttpServletRequestProcessor.java | 57 +++++++++++++++++++ ...SentryUserHttpServletRequestProcessor.java | 43 ++++++++++++++ ...yRequestHttpServletRequestProcessorTest.kt | 56 ++++++++++++++++++ ...ntryUserHttpServletRequestProcessorTest.kt | 44 ++++++++++++++ 6 files changed, 237 insertions(+), 6 deletions(-) create mode 100644 sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java create mode 100644 sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java create mode 100644 sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java create mode 100644 sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt create mode 100644 sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index 3d79468c9..2ba958064 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -36,12 +36,12 @@ static class HubConfiguration { @Bean @ConditionalOnMissingBean public Sentry.OptionsConfiguration optionsOptionsConfiguration( - ObjectProvider beforeSendCallback, - ObjectProvider beforeBreadcrumbCallback, - ObjectProvider eventProcessors, - ObjectProvider integrations, - ObjectProvider transportGate, - ObjectProvider transport) { + ObjectProvider beforeSendCallback, + ObjectProvider beforeBreadcrumbCallback, + ObjectProvider eventProcessors, + ObjectProvider integrations, + ObjectProvider transportGate, + ObjectProvider transport) { return options -> { beforeSendCallback.ifAvailable(options::setBeforeSend); beforeBreadcrumbCallback.ifAvailable(options::setBeforeBreadcrumb); diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java new file mode 100644 index 000000000..41ba5d948 --- /dev/null +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java @@ -0,0 +1,31 @@ +package io.sentry.spring.boot; + +import com.jakewharton.nopen.annotation.Open; +import io.sentry.core.IHub; +import java.io.IOException; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.jetbrains.annotations.NotNull; +import org.springframework.web.filter.OncePerRequestFilter; + +/** Pushes new {@link io.sentry.core.Scope} on each incoming HTTP request. */ +@Open +public class SentryRequestFilter extends OncePerRequestFilter { + private final @NotNull IHub hub; + + public SentryRequestFilter(final @NotNull IHub hub) { + this.hub = hub; + } + + @Override + protected void doFilterInternal( + final @NotNull HttpServletRequest request, + final @NotNull HttpServletResponse response, + final @NotNull FilterChain filterChain) + throws ServletException, IOException { + hub.pushScope(); + filterChain.doFilter(request, response); + } +} diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java new file mode 100644 index 000000000..5a9fa6aea --- /dev/null +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java @@ -0,0 +1,57 @@ +package io.sentry.spring.boot; + +import com.jakewharton.nopen.annotation.Open; +import io.sentry.core.EventProcessor; +import io.sentry.core.SentryEvent; +import io.sentry.core.protocol.Request; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.Map; +import javax.servlet.http.HttpServletRequest; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.springframework.web.context.request.RequestAttributes; +import org.springframework.web.context.request.RequestContextHolder; +import org.springframework.web.context.request.ServletRequestAttributes; + +/** Attaches information about HTTP request to {@link SentryEvent}. */ +@Open +public class SentryRequestHttpServletRequestProcessor implements EventProcessor { + + @Override + public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Object hint) { + final RequestAttributes requestAttributes = RequestContextHolder.currentRequestAttributes(); + if (requestAttributes instanceof ServletRequestAttributes) { + final HttpServletRequest request = + ((ServletRequestAttributes) requestAttributes).getRequest(); + event.setRequest(resolveSentryRequest(request)); + } + return event; + } + + @SuppressWarnings("JdkObsolete") + private static @NotNull Request resolveSentryRequest( + final @NotNull HttpServletRequest httpRequest) { + final Request sentryRequest = new Request(); + // TODO: set cookies + sentryRequest.setMethod(httpRequest.getMethod()); + sentryRequest.setQueryString(httpRequest.getQueryString()); + sentryRequest.setUrl(httpRequest.getRequestURL().toString()); + sentryRequest.setHeaders(resolveHeadersMap(httpRequest)); + return sentryRequest; + } + + private static @NotNull Map resolveHeadersMap( + final @NotNull HttpServletRequest request) { + final Map headersMap = new HashMap<>(); + for (String headerName : Collections.list(request.getHeaderNames())) { + headersMap.put(headerName, headersToString(request.getHeaders(headerName))); + } + return headersMap; + } + + private static String headersToString(final @NotNull Enumeration headers) { + return String.join(",", Collections.list(headers)); + } +} diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java new file mode 100644 index 000000000..68a0ce5b5 --- /dev/null +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java @@ -0,0 +1,43 @@ +package io.sentry.spring.boot; + +import com.jakewharton.nopen.annotation.Open; +import io.sentry.core.EventProcessor; +import io.sentry.core.SentryEvent; +import io.sentry.core.protocol.User; +import javax.servlet.http.HttpServletRequest; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.springframework.web.context.request.RequestAttributes; +import org.springframework.web.context.request.RequestContextHolder; +import org.springframework.web.context.request.ServletRequestAttributes; + +/** Attaches user information from the HTTP request to {@link SentryEvent}. */ +@Open +public class SentryUserHttpServletRequestProcessor implements EventProcessor { + + @Override + public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Object hint) { + final RequestAttributes requestAttributes = RequestContextHolder.currentRequestAttributes(); + if (requestAttributes instanceof ServletRequestAttributes) { + final HttpServletRequest request = + ((ServletRequestAttributes) requestAttributes).getRequest(); + event.setUser(toUser(event.getUser(), request)); + } + return event; + } + + private static @NotNull User toUser( + final @Nullable User existingUser, final @NotNull HttpServletRequest request) { + final User user = existingUser != null ? existingUser : new User(); + user.setIpAddress(toIpAddress(request)); + if (request.getUserPrincipal() != null) { + user.setUsername(request.getUserPrincipal().getName()); + } + return user; + } + + private static @NotNull String toIpAddress(final @NotNull HttpServletRequest request) { + final String ipAddress = request.getHeader("X-FORWARDED-FOR"); + return ipAddress != null ? ipAddress : request.getRemoteAddr(); + } +} diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt new file mode 100644 index 000000000..12a891836 --- /dev/null +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt @@ -0,0 +1,56 @@ +package io.sentry.spring.boot + +import io.sentry.core.SentryEvent +import java.net.URI +import kotlin.test.Test +import kotlin.test.assertEquals +import org.springframework.http.MediaType +import org.springframework.mock.web.MockServletContext +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders +import org.springframework.web.context.request.RequestContextHolder +import org.springframework.web.context.request.ServletRequestAttributes + +class SentryRequestHttpServletRequestProcessorTest { + + private val eventProcessor = SentryRequestHttpServletRequestProcessor() + + @Test + fun `attaches basic information from HTTP request to SentryEvent`() { + val request = MockMvcRequestBuilders + .get(URI.create("http://example.com?param1=xyz")) + .header("some-header", "some-header value") + .accept(MediaType.APPLICATION_JSON) + .buildRequest(MockServletContext()) + + RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + val event = SentryEvent() + + eventProcessor.process(event, null) + + assertEquals("GET", event.request.method) + assertEquals(mapOf( + "some-header" to "some-header value", + "Accept" to "application/json" + ), event.request.headers) + assertEquals("http://example.com", event.request.url) + assertEquals("param1=xyz", event.request.queryString) + } + + @Test + fun `attaches header with multiple values`() { + val request = MockMvcRequestBuilders + .get(URI.create("http://example.com?param1=xyz")) + .header("another-header", "another value") + .header("another-header", "another value2") + .buildRequest(MockServletContext()) + + RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + val event = SentryEvent() + + eventProcessor.process(event, null) + + assertEquals(mapOf( + "another-header" to "another value,another value2" + ), event.request.headers) + } +} diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt new file mode 100644 index 000000000..3f3732778 --- /dev/null +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt @@ -0,0 +1,44 @@ +package io.sentry.spring.boot + +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.whenever +import io.sentry.core.SentryEvent +import kotlin.test.Test +import kotlin.test.assertEquals +import org.springframework.mock.web.MockHttpServletRequest +import org.springframework.web.context.request.RequestContextHolder +import org.springframework.web.context.request.ServletRequestAttributes +import java.security.Principal + +class SentryUserHttpServletRequestProcessorTest { + + private val eventProcessor = SentryUserHttpServletRequestProcessor() + + @Test + fun `attaches user's IP address to Sentry Event`() { + val request = MockHttpServletRequest() + request.remoteAddr = "192.168.0.1" + + RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + val event = SentryEvent() + + eventProcessor.process(event, null) + + assertEquals("192.168.0.1", event.user.ipAddress) + } + + @Test + fun `attaches username to Sentry Event`() { + val principal = mock() + whenever(principal.name).thenReturn("janesmith") + val request = MockHttpServletRequest() + request.userPrincipal = principal + + RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + val event = SentryEvent() + + eventProcessor.process(event, null) + + assertEquals("janesmith", event.user.username) + } +} From b4ef72e5d8f2d625befd2e69d218bc6389c77db1 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Tue, 18 Aug 2020 17:25:14 +0200 Subject: [PATCH 03/27] Set cookies on SentryEvent. --- ...SentryRequestHttpServletRequestProcessor.java | 10 +++++----- ...ntryRequestHttpServletRequestProcessorTest.kt | 16 ++++++++++++++++ settings.gradle.kts | 1 + 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java index 5a9fa6aea..d3ad436d6 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java @@ -20,7 +20,7 @@ public class SentryRequestHttpServletRequestProcessor implements EventProcessor { @Override - public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Object hint) { + public @NotNull SentryEvent process(final @NotNull SentryEvent event, final @Nullable Object hint) { final RequestAttributes requestAttributes = RequestContextHolder.currentRequestAttributes(); if (requestAttributes instanceof ServletRequestAttributes) { final HttpServletRequest request = @@ -34,11 +34,11 @@ public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Obj private static @NotNull Request resolveSentryRequest( final @NotNull HttpServletRequest httpRequest) { final Request sentryRequest = new Request(); - // TODO: set cookies sentryRequest.setMethod(httpRequest.getMethod()); sentryRequest.setQueryString(httpRequest.getQueryString()); sentryRequest.setUrl(httpRequest.getRequestURL().toString()); sentryRequest.setHeaders(resolveHeadersMap(httpRequest)); + sentryRequest.setCookies(toString(httpRequest.getHeaders("Cookie"))); return sentryRequest; } @@ -46,12 +46,12 @@ public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Obj final @NotNull HttpServletRequest request) { final Map headersMap = new HashMap<>(); for (String headerName : Collections.list(request.getHeaderNames())) { - headersMap.put(headerName, headersToString(request.getHeaders(headerName))); + headersMap.put(headerName, toString(request.getHeaders(headerName))); } return headersMap; } - private static String headersToString(final @NotNull Enumeration headers) { - return String.join(",", Collections.list(headers)); + private static @Nullable String toString(final @Nullable Enumeration enumeration) { + return enumeration != null ? String.join(",", Collections.list(enumeration)) : null; } } diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt index 12a891836..c58f5cedd 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt @@ -53,4 +53,20 @@ class SentryRequestHttpServletRequestProcessorTest { "another-header" to "another value,another value2" ), event.request.headers) } + + @Test + fun `attaches cookies information`() { + val request = MockMvcRequestBuilders + .get(URI.create("http://example.com?param1=xyz")) + .header("Cookie", "name=value") + .header("Cookie", "name2=value2") + .buildRequest(MockServletContext()) + + RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + val event = SentryEvent() + + eventProcessor.process(event, null) + + assertEquals("name=value,name2=value2", event.request.cookies) + } } diff --git a/settings.gradle.kts b/settings.gradle.kts index 2cbd97ad2..4bff073d4 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -5,6 +5,7 @@ include("sentry-android", "sentry-android-ndk", "sentry-android-core", "sentry-core", + "sentry-spring-boot-starter", "sentry-samples:sentry-samples-android", "sentry-samples:sentry-samples-console", "sentry-android-timber") From f2b950e4610732ec0b2a26be8e0d2fb9bc6ae906 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Thu, 20 Aug 2020 13:12:38 +0200 Subject: [PATCH 04/27] Generate properties files from Spring Boot auto-configuration and configuration properties. --- build.gradle.kts | 2 +- sentry-spring-boot-starter/build.gradle.kts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 42d9b10d6..1812809fa 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -51,7 +51,7 @@ allprojects { dependsOn("cleanTest") } withType { - options.compilerArgs.addAll(arrayOf("-Xlint:all", "-Werror", "-Xlint:-classfile")) + options.compilerArgs.addAll(arrayOf("-Xlint:all", "-Werror", "-Xlint:-classfile", "-Xlint:-processing")) } } } diff --git a/sentry-spring-boot-starter/build.gradle.kts b/sentry-spring-boot-starter/build.gradle.kts index f9d0ece40..0d96ce658 100644 --- a/sentry-spring-boot-starter/build.gradle.kts +++ b/sentry-spring-boot-starter/build.gradle.kts @@ -34,7 +34,8 @@ dependencies { implementation(Config.Libs.springWeb) implementation(Config.Libs.servletApi) - // TODO: annotationProcessor("org.springframework.boot:spring-boot-autoconfigure-processor") + annotationProcessor("org.springframework.boot:spring-boot-autoconfigure-processor") + annotationProcessor("org.springframework.boot:spring-boot-configuration-processor") compileOnly(Config.CompileOnly.nopen) errorprone(Config.CompileOnly.nopenChecker) From fada413490a93374c4ec6ccd27959fbe2ca60199 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 21 Aug 2020 12:38:40 +0200 Subject: [PATCH 05/27] Raise required test coverage to 60% --- sentry-spring-boot-starter/build.gradle.kts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sentry-spring-boot-starter/build.gradle.kts b/sentry-spring-boot-starter/build.gradle.kts index 0d96ce658..86d53b6c9 100644 --- a/sentry-spring-boot-starter/build.gradle.kts +++ b/sentry-spring-boot-starter/build.gradle.kts @@ -70,8 +70,7 @@ tasks.jacocoTestReport { tasks { jacocoTestCoverageVerification { violationRules { - // TODO: Raise the minimum to a sensible value. - rule { limit { minimum = BigDecimal.valueOf(0.1) } } + rule { limit { minimum = BigDecimal.valueOf(0.6) } } } } check { From 3588a518def72179120816e41e4ec78e4895cf82 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 21 Aug 2020 14:18:10 +0200 Subject: [PATCH 06/27] Refactor & add more auto-configuration tests. --- .../src/main/java/io/sentry/core/Sentry.java | 9 ++ .../spring/boot/SentryAutoConfiguration.java | 26 ++-- .../sentry/spring/boot/SentryProperties.java | 16 +-- .../boot/SentryAutoConfigurationTest.kt | 120 ++++++++++++++++++ 4 files changed, 144 insertions(+), 27 deletions(-) diff --git a/sentry-core/src/main/java/io/sentry/core/Sentry.java b/sentry-core/src/main/java/io/sentry/core/Sentry.java index edb39f592..1171417a0 100644 --- a/sentry-core/src/main/java/io/sentry/core/Sentry.java +++ b/sentry-core/src/main/java/io/sentry/core/Sentry.java @@ -123,6 +123,15 @@ public static void init( init(options, globalHubMode); } + /** + * Initializes the SDK with a SentryOptions. + * + * @param options options the SentryOptions + */ + public static void init(final @NotNull SentryOptions options) { + init(options, GLOBAL_HUB_DEFAULT_MODE); + } + /** * Initializes the SDK with a SentryOptions and globalHubMode * diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index 2ba958064..f2a3ea6c0 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -6,7 +6,6 @@ import io.sentry.core.IHub; import io.sentry.core.Integration; import io.sentry.core.Sentry; -import io.sentry.core.SentryClient; import io.sentry.core.SentryOptions; import io.sentry.core.protocol.SdkVersion; import io.sentry.core.transport.ITransport; @@ -53,18 +52,19 @@ public Sentry.OptionsConfiguration optionsOptionsConfiguration( } @Bean - IHub sentryHub( - Sentry.OptionsConfiguration optionsConfiguration, - SentryProperties properties, - ObjectProvider sentryClient) { - Sentry.init( - options -> { - properties.applyTo(options); - optionsConfiguration.configure(options); - options.setSentryClientName(BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME); - options.setSdkVersion(createSdkVersion(options)); - }); - sentryClient.ifAvailable(Sentry::bindClient); + public SentryOptions sentryOptions(Sentry.OptionsConfiguration optionsConfiguration, + SentryProperties properties) { + final SentryOptions options = new SentryOptions(); + optionsConfiguration.configure(options); + properties.applyTo(options); + options.setSentryClientName(BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME); + options.setSdkVersion(createSdkVersion(options)); + return options; + } + + @Bean + IHub sentryHub(SentryOptions sentryOptions) { + Sentry.init(sentryOptions); return HubAdapter.getInstance(); } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java index 3246a347c..1005a360c 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java @@ -61,12 +61,6 @@ public class SentryProperties { /** minimum LogLevel to be used if debug is enabled */ private SentryLevel diagnosticLevel = SentryLevel.DEBUG; - /** - * Sentry client name used for the HTTP authHeader and userAgent eg - * sentry.{language}.{platform}/{version} eg sentry.java.android/2.0.0 would be a valid case - */ - private String sentryClientName; - /** This variable controls the total amount of breadcrumbs that should be captured. */ private Integer maxBreadcrumbs; @@ -137,6 +131,8 @@ public void applyTo(SentryOptions options) { Optional.ofNullable(release).ifPresent(options::setRelease); Optional.ofNullable(sampleRate).ifPresent(options::setSampleRate); Optional.ofNullable(serverName).ifPresent(options::setServerName); + Optional.ofNullable(inAppExcludes).ifPresent(excludes -> excludes.forEach(options::addInAppExclude)); + Optional.ofNullable(inAppIncludes).ifPresent(includes -> includes.forEach(options::addInAppInclude)); } public boolean isEnabled() { @@ -179,14 +175,6 @@ public void setDiagnosticLevel(SentryLevel diagnosticLevel) { this.diagnosticLevel = diagnosticLevel; } - public String getSentryClientName() { - return sentryClientName; - } - - public void setSentryClientName(String sentryClientName) { - this.sentryClientName = sentryClientName; - } - public int getMaxBreadcrumbs() { return maxBreadcrumbs; } diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index 47cd06e5e..349b358a4 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -1,8 +1,15 @@ package io.sentry.spring.boot +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.check +import io.sentry.core.Breadcrumb import io.sentry.core.IHub import io.sentry.core.Sentry +import io.sentry.core.SentryEvent +import io.sentry.core.SentryLevel import io.sentry.core.SentryOptions +import io.sentry.core.transport.ITransport import kotlin.test.Test import org.assertj.core.api.Assertions.assertThat import org.springframework.boot.autoconfigure.AutoConfigurations @@ -69,6 +76,56 @@ class SentryAutoConfigurationTest { } } + @Test + fun `properties are applied to SentryOptions`() { + contextRunner.withPropertyValues( + "sentry.dsn=http://key@localhost/proj", + "sentry.read-timeout-millis=10", + "sentry.shutdown-timeout-millis=20", + "sentry.flush-timeout-millis=30", + "sentry.bypass-security=true", + "sentry.debug=true", + "sentry.diagnostic-level=INFO", + "sentry.sentry-client-name=my-client", + "sentry.max-breadcrumbs=100", + "sentry.release=1.0.3", + "sentry.environment=production", + "sentry.sample-rate=0.2", + "sentry.in-app-excludes[0]=org.springframework", + "sentry.in-app-includes[0]=com.myapp", + "sentry.dist=my-dist", + "sentry.attach-threads=true", + "sentry.attach-stacktrace=true", + "sentry.server-name=host-001" + ).run { + val options = it.getBean(SentryOptions::class.java) + assertThat(options.readTimeoutMillis).isEqualTo(10) + assertThat(options.shutdownTimeout).isEqualTo(20) + assertThat(options.flushTimeoutMillis).isEqualTo(30) + assertThat(options.isBypassSecurity).isTrue() + assertThat(options.isDebug).isTrue() + assertThat(options.diagnosticLevel).isEqualTo(SentryLevel.INFO) + assertThat(options.maxBreadcrumbs).isEqualTo(100) + assertThat(options.release).isEqualTo("1.0.3") + assertThat(options.environment).isEqualTo("production") + assertThat(options.sampleRate).isEqualTo(0.2) + assertThat(options.inAppExcludes).containsOnly("org.springframework") + assertThat(options.inAppIncludes).containsOnly("com.myapp") + assertThat(options.dist).isEqualTo("my-dist") + assertThat(options.isAttachThreads).isEqualTo(true) + assertThat(options.isAttachStacktrace).isEqualTo(true) + assertThat(options.serverName).isEqualTo("host-001") + } + } + + @Test + fun `sets sentryClientName property on SentryOptions`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .run { + assertThat(it.getBean(SentryOptions::class.java).sentryClientName).isEqualTo("sentry.java.spring-boot") + } + } + @Test fun `does not register event processors for non web-servlet application type`() { contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") @@ -87,6 +144,40 @@ class SentryAutoConfigurationTest { } } + @Test + fun `sets SDK version on sent events`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(MockTransportConfiguration::class.java) + .run { + Sentry.captureMessage("Some message") + val transport = it.getBean(ITransport::class.java) + verify(transport).send(check { event: SentryEvent -> + assertThat(event.sdk.version).isEqualTo(BuildConfig.VERSION_NAME) + assertThat(event.sdk.name).isEqualTo(BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME) + assertThat(event.sdk.packages).anyMatch { pkg -> + pkg.name == "maven:sentry-spring-boot-starter" && pkg.version == BuildConfig.VERSION_NAME } + }) + } + } + + @Test + fun `registers beforeSendCallback on SentryOptions`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(CustomBeforeSendCallbackConfiguration::class.java) + .run { + assertThat(it.getBean(SentryOptions::class.java).beforeSend).isInstanceOf(CustomBeforeSendCallback::class.java) + } + } + + @Test + fun `registers beforeBreadcrumbCallback on SentryOptions`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(CustomBeforeBreadcrumbCallbackConfiguration::class.java) + .run { + assertThat(it.getBean(SentryOptions::class.java).beforeBreadcrumb).isInstanceOf(CustomBeforeBreadcrumbCallback::class.java) + } + } + @Configuration(proxyBeanMethods = false) open class CustomOptionsConfigurationConfiguration { @@ -94,4 +185,33 @@ class SentryAutoConfigurationTest { open fun customOptionsConfiguration() = Sentry.OptionsConfiguration() { } } + + @Configuration(proxyBeanMethods = false) + open class MockTransportConfiguration { + + @Bean + open fun sentryTransport() = mock() + } + + @Configuration(proxyBeanMethods = false) + open class CustomBeforeSendCallbackConfiguration { + + @Bean + open fun beforeSendCallback() = CustomBeforeSendCallback() + } + + class CustomBeforeSendCallback : SentryOptions.BeforeSendCallback { + override fun execute(event: SentryEvent, hint: Any?): SentryEvent? = null + } + + @Configuration(proxyBeanMethods = false) + open class CustomBeforeBreadcrumbCallbackConfiguration { + + @Bean + open fun beforeBreadcrumbCallback() = CustomBeforeBreadcrumbCallback() + } + + class CustomBeforeBreadcrumbCallback : SentryOptions.BeforeBreadcrumbCallback { + override fun execute(breadcrumb: Breadcrumb, hint: Any?): Breadcrumb? = null + } } From 7714510ce32f700865bade43f50a8b10e5975dc7 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 24 Aug 2020 13:06:06 +0200 Subject: [PATCH 07/27] Polish. --- .../spring/boot/SentryAutoConfiguration.java | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index f2a3ea6c0..7f99d991a 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -34,13 +34,13 @@ static class HubConfiguration { @Bean @ConditionalOnMissingBean - public Sentry.OptionsConfiguration optionsOptionsConfiguration( - ObjectProvider beforeSendCallback, - ObjectProvider beforeBreadcrumbCallback, - ObjectProvider eventProcessors, - ObjectProvider integrations, - ObjectProvider transportGate, - ObjectProvider transport) { + public @NotNull Sentry.OptionsConfiguration optionsOptionsConfiguration( + final @NotNull ObjectProvider beforeSendCallback, + final @NotNull ObjectProvider beforeBreadcrumbCallback, + final @NotNull ObjectProvider eventProcessors, + final @NotNull ObjectProvider integrations, + final @NotNull ObjectProvider transportGate, + final @NotNull ObjectProvider transport) { return options -> { beforeSendCallback.ifAvailable(options::setBeforeSend); beforeBreadcrumbCallback.ifAvailable(options::setBeforeBreadcrumb); @@ -52,8 +52,8 @@ public Sentry.OptionsConfiguration optionsOptionsConfiguration( } @Bean - public SentryOptions sentryOptions(Sentry.OptionsConfiguration optionsConfiguration, - SentryProperties properties) { + public @NotNull SentryOptions sentryOptions(final @NotNull Sentry.OptionsConfiguration optionsConfiguration, + final @NotNull SentryProperties properties) { final SentryOptions options = new SentryOptions(); optionsConfiguration.configure(options); properties.applyTo(options); @@ -63,7 +63,7 @@ public SentryOptions sentryOptions(Sentry.OptionsConfiguration op } @Bean - IHub sentryHub(SentryOptions sentryOptions) { + @NotNull IHub sentryHub(final @NotNull SentryOptions sentryOptions) { Sentry.init(sentryOptions); return HubAdapter.getInstance(); } @@ -75,22 +75,22 @@ IHub sentryHub(SentryOptions sentryOptions) { static class SentryWebMvcConfiguration { @Bean - SentryRequestHttpServletRequestProcessor sentryEventHttpServletRequestProcessor() { + @NotNull SentryRequestHttpServletRequestProcessor sentryEventHttpServletRequestProcessor() { return new SentryRequestHttpServletRequestProcessor(); } @Bean - SentryUserHttpServletRequestProcessor sentryUserHttpServletRequestProcessor() { + @NotNull SentryUserHttpServletRequestProcessor sentryUserHttpServletRequestProcessor() { return new SentryUserHttpServletRequestProcessor(); } @Bean - public FilterRegistrationBean sentryRequestFilter(IHub sentryHub) { + public @NotNull FilterRegistrationBean sentryRequestFilter(final @NotNull IHub sentryHub) { return new FilterRegistrationBean<>(new SentryRequestFilter(sentryHub)); } } - private static @NotNull SdkVersion createSdkVersion(@NotNull SentryOptions sentryOptions) { + private static @NotNull SdkVersion createSdkVersion(final @NotNull SentryOptions sentryOptions) { SdkVersion sdkVersion = sentryOptions.getSdkVersion(); if (sdkVersion == null) { From 73a4d567ecfeca6fda9be461a68032f3acbb40df Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 24 Aug 2020 13:15:04 +0200 Subject: [PATCH 08/27] Add more auto-configuration tests. --- .../boot/SentryAutoConfigurationTest.kt | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index 349b358a4..ff0cdf2b9 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -4,12 +4,15 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.check import io.sentry.core.Breadcrumb +import io.sentry.core.EventProcessor import io.sentry.core.IHub +import io.sentry.core.Integration import io.sentry.core.Sentry import io.sentry.core.SentryEvent import io.sentry.core.SentryLevel import io.sentry.core.SentryOptions import io.sentry.core.transport.ITransport +import io.sentry.core.transport.ITransportGate import kotlin.test.Test import org.assertj.core.api.Assertions.assertThat import org.springframework.boot.autoconfigure.AutoConfigurations @@ -178,6 +181,33 @@ class SentryAutoConfigurationTest { } } + @Test + fun `registers event processor on SentryOptions`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(CustomEventProcessorConfiguration::class.java) + .run { + assertThat(it.getBean(SentryOptions::class.java).eventProcessors).anyMatch { processor -> processor.javaClass == CustomEventProcessor::class.java } + } + } + + @Test + fun `registers transport gate on SentryOptions`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(CustomTransportGateConfiguration::class.java) + .run { + assertThat(it.getBean(SentryOptions::class.java).transportGate).isInstanceOf(CustomTransportGate::class.java) + } + } + + @Test + fun `registers custom integration on SentryOptions`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(CustomIntegration::class.java) + .run { + assertThat(it.getBean(SentryOptions::class.java).integrations).anyMatch { integration -> integration.javaClass == CustomIntegration::class.java } + } + } + @Configuration(proxyBeanMethods = false) open class CustomOptionsConfigurationConfiguration { @@ -214,4 +244,37 @@ class SentryAutoConfigurationTest { class CustomBeforeBreadcrumbCallback : SentryOptions.BeforeBreadcrumbCallback { override fun execute(breadcrumb: Breadcrumb, hint: Any?): Breadcrumb? = null } + + @Configuration(proxyBeanMethods = false) + open class CustomEventProcessorConfiguration { + + @Bean + open fun customEventProcessor() = CustomEventProcessor() + } + + class CustomEventProcessor : EventProcessor { + override fun process(event: SentryEvent?, hint: Any?) = null + } + + @Configuration(proxyBeanMethods = false) + open class CustomIntegrationConfiguration { + + @Bean + open fun customIntegration() = CustomIntegration() + } + + class CustomIntegration : Integration { + override fun register(hub: IHub?, options: SentryOptions?) {} + } + + @Configuration(proxyBeanMethods = false) + open class CustomTransportGateConfiguration { + + @Bean + open fun customTransportGate() = CustomTransportGate() + } + + class CustomTransportGate : ITransportGate { + override fun isConnected() = true + } } From b9adb070e1cc9c2f607e9a22baf9179ab8a14cde Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 24 Aug 2020 14:17:08 +0200 Subject: [PATCH 09/27] Set Git commit id as release version. --- .../spring/boot/SentryAutoConfiguration.java | 38 ++++++++++------ .../sentry/spring/boot/SentryProperties.java | 17 +++++++- ...tryRequestHttpServletRequestProcessor.java | 3 +- .../boot/SentryAutoConfigurationTest.kt | 43 ++++++++++++++++++- ...ntryUserHttpServletRequestProcessorTest.kt | 2 +- 5 files changed, 84 insertions(+), 19 deletions(-) diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index 7f99d991a..46d04d136 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -16,6 +16,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.info.GitProperties; import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -35,12 +36,13 @@ static class HubConfiguration { @Bean @ConditionalOnMissingBean public @NotNull Sentry.OptionsConfiguration optionsOptionsConfiguration( - final @NotNull ObjectProvider beforeSendCallback, - final @NotNull ObjectProvider beforeBreadcrumbCallback, - final @NotNull ObjectProvider eventProcessors, - final @NotNull ObjectProvider integrations, - final @NotNull ObjectProvider transportGate, - final @NotNull ObjectProvider transport) { + final @NotNull ObjectProvider beforeSendCallback, + final @NotNull ObjectProvider + beforeBreadcrumbCallback, + final @NotNull ObjectProvider eventProcessors, + final @NotNull ObjectProvider integrations, + final @NotNull ObjectProvider transportGate, + final @NotNull ObjectProvider transport) { return options -> { beforeSendCallback.ifAvailable(options::setBeforeSend); beforeBreadcrumbCallback.ifAvailable(options::setBeforeBreadcrumb); @@ -52,10 +54,18 @@ static class HubConfiguration { } @Bean - public @NotNull SentryOptions sentryOptions(final @NotNull Sentry.OptionsConfiguration optionsConfiguration, - final @NotNull SentryProperties properties) { + public @NotNull SentryOptions sentryOptions( + final @NotNull Sentry.OptionsConfiguration optionsConfiguration, + final @NotNull SentryProperties properties, + final @NotNull ObjectProvider gitProperties) { final SentryOptions options = new SentryOptions(); optionsConfiguration.configure(options); + gitProperties.ifAvailable( + git -> { + if (properties.isUseGitCommitIdAsRelease()) { + options.setRelease(git.getCommitId()); + } + }); properties.applyTo(options); options.setSentryClientName(BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME); options.setSdkVersion(createSdkVersion(options)); @@ -63,7 +73,7 @@ static class HubConfiguration { } @Bean - @NotNull IHub sentryHub(final @NotNull SentryOptions sentryOptions) { + public @NotNull IHub sentryHub(final @NotNull SentryOptions sentryOptions) { Sentry.init(sentryOptions); return HubAdapter.getInstance(); } @@ -75,22 +85,24 @@ static class HubConfiguration { static class SentryWebMvcConfiguration { @Bean - @NotNull SentryRequestHttpServletRequestProcessor sentryEventHttpServletRequestProcessor() { + public @NotNull SentryRequestHttpServletRequestProcessor sentryEventHttpServletRequestProcessor() { return new SentryRequestHttpServletRequestProcessor(); } @Bean - @NotNull SentryUserHttpServletRequestProcessor sentryUserHttpServletRequestProcessor() { + public @NotNull SentryUserHttpServletRequestProcessor sentryUserHttpServletRequestProcessor() { return new SentryUserHttpServletRequestProcessor(); } @Bean - public @NotNull FilterRegistrationBean sentryRequestFilter(final @NotNull IHub sentryHub) { + public @NotNull FilterRegistrationBean sentryRequestFilter( + final @NotNull IHub sentryHub) { return new FilterRegistrationBean<>(new SentryRequestFilter(sentryHub)); } } - private static @NotNull SdkVersion createSdkVersion(final @NotNull SentryOptions sentryOptions) { + private static @NotNull SdkVersion createSdkVersion( + final @NotNull SentryOptions sentryOptions) { SdkVersion sdkVersion = sentryOptions.getSdkVersion(); if (sdkVersion == null) { diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java index 1005a360c..6c36100e0 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java @@ -109,6 +109,9 @@ public class SentryProperties { /** The server name used in the Sentry messages. */ private String serverName; + /** Weather to use Git commit id as a release. */ + private boolean useGitCommitIdAsRelease = true; + /** * Applies configuration from this instance to the {@link SentryOptions} instance. * @@ -131,8 +134,10 @@ public void applyTo(SentryOptions options) { Optional.ofNullable(release).ifPresent(options::setRelease); Optional.ofNullable(sampleRate).ifPresent(options::setSampleRate); Optional.ofNullable(serverName).ifPresent(options::setServerName); - Optional.ofNullable(inAppExcludes).ifPresent(excludes -> excludes.forEach(options::addInAppExclude)); - Optional.ofNullable(inAppIncludes).ifPresent(includes -> includes.forEach(options::addInAppInclude)); + Optional.ofNullable(inAppExcludes) + .ifPresent(excludes -> excludes.forEach(options::addInAppExclude)); + Optional.ofNullable(inAppIncludes) + .ifPresent(includes -> includes.forEach(options::addInAppInclude)); } public boolean isEnabled() { @@ -310,4 +315,12 @@ public Boolean getAttachStacktrace() { public void setAttachStacktrace(Boolean attachStacktrace) { this.attachStacktrace = attachStacktrace; } + + public boolean isUseGitCommitIdAsRelease() { + return useGitCommitIdAsRelease; + } + + public void setUseGitCommitIdAsRelease(boolean useGitCommitIdAsRelease) { + this.useGitCommitIdAsRelease = useGitCommitIdAsRelease; + } } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java index d3ad436d6..b4ae8eb47 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java @@ -20,7 +20,8 @@ public class SentryRequestHttpServletRequestProcessor implements EventProcessor { @Override - public @NotNull SentryEvent process(final @NotNull SentryEvent event, final @Nullable Object hint) { + public @NotNull SentryEvent process( + final @NotNull SentryEvent event, final @Nullable Object hint) { final RequestAttributes requestAttributes = RequestContextHolder.currentRequestAttributes(); if (requestAttributes instanceof ServletRequestAttributes) { final HttpServletRequest request = diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index ff0cdf2b9..83f185866 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -1,8 +1,9 @@ package io.sentry.spring.boot +import com.nhaarman.mockitokotlin2.check import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify -import com.nhaarman.mockitokotlin2.check +import com.nhaarman.mockitokotlin2.whenever import io.sentry.core.Breadcrumb import io.sentry.core.EventProcessor import io.sentry.core.IHub @@ -17,6 +18,7 @@ import kotlin.test.Test import org.assertj.core.api.Assertions.assertThat import org.springframework.boot.autoconfigure.AutoConfigurations import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration +import org.springframework.boot.info.GitProperties import org.springframework.boot.test.context.runner.ApplicationContextRunner import org.springframework.boot.test.context.runner.WebApplicationContextRunner import org.springframework.context.annotation.Bean @@ -208,11 +210,37 @@ class SentryAutoConfigurationTest { } } + @Test + fun `sets release on SentryEvents if Git integration is configured`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(MockTransportConfiguration::class.java, MockGitPropertiesConfiguration::class.java) + .run { + Sentry.captureMessage("Some message") + val transport = it.getBean(ITransport::class.java) + verify(transport).send(check { event: SentryEvent -> + assertThat(event.release).isEqualTo("git-commit-id") + }) + } + } + + @Test + fun `sets custom release on SentryEvents if release property is set and Git integration is configured`() { + contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.release=my-release") + .withUserConfiguration(MockTransportConfiguration::class.java, MockGitPropertiesConfiguration::class.java) + .run { + Sentry.captureMessage("Some message") + val transport = it.getBean(ITransport::class.java) + verify(transport).send(check { event: SentryEvent -> + assertThat(event.release).isEqualTo("my-release") + }) + } + } + @Configuration(proxyBeanMethods = false) open class CustomOptionsConfigurationConfiguration { @Bean - open fun customOptionsConfiguration() = Sentry.OptionsConfiguration() { + open fun customOptionsConfiguration() = Sentry.OptionsConfiguration { } } @@ -277,4 +305,15 @@ class SentryAutoConfigurationTest { class CustomTransportGate : ITransportGate { override fun isConnected() = true } + + @Configuration(proxyBeanMethods = false) + open class MockGitPropertiesConfiguration { + + @Bean + open fun gitProperties(): GitProperties { + val git = mock() + whenever(git.commitId).thenReturn("git-commit-id") + return git + } + } } diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt index 3f3732778..802705ae2 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt @@ -3,12 +3,12 @@ package io.sentry.spring.boot import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever import io.sentry.core.SentryEvent +import java.security.Principal import kotlin.test.Test import kotlin.test.assertEquals import org.springframework.mock.web.MockHttpServletRequest import org.springframework.web.context.request.RequestContextHolder import org.springframework.web.context.request.ServletRequestAttributes -import java.security.Principal class SentryUserHttpServletRequestProcessorTest { From fa510f35c74af19a062da8d6a114cf735c3e6707 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 24 Aug 2020 15:19:14 +0200 Subject: [PATCH 10/27] Improve printing failed tests reason. --- .../sentry/spring/boot/SentryAutoConfiguration.java | 6 ++++-- .../spring/boot/SentryAutoConfigurationTest.kt | 13 +++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index 46d04d136..5ba1a633a 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -85,12 +85,14 @@ static class HubConfiguration { static class SentryWebMvcConfiguration { @Bean - public @NotNull SentryRequestHttpServletRequestProcessor sentryEventHttpServletRequestProcessor() { + public @NotNull SentryRequestHttpServletRequestProcessor + sentryEventHttpServletRequestProcessor() { return new SentryRequestHttpServletRequestProcessor(); } @Bean - public @NotNull SentryUserHttpServletRequestProcessor sentryUserHttpServletRequestProcessor() { + public @NotNull SentryUserHttpServletRequestProcessor + sentryUserHttpServletRequestProcessor() { return new SentryUserHttpServletRequestProcessor(); } diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index 83f185866..8d9e2e06d 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -1,5 +1,6 @@ package io.sentry.spring.boot +import com.nhaarman.mockitokotlin2.argumentCaptor import com.nhaarman.mockitokotlin2.check import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify @@ -217,9 +218,9 @@ class SentryAutoConfigurationTest { .run { Sentry.captureMessage("Some message") val transport = it.getBean(ITransport::class.java) - verify(transport).send(check { event: SentryEvent -> - assertThat(event.release).isEqualTo("git-commit-id") - }) + val captor = argumentCaptor() + verify(transport).send(captor.capture()) + assertThat(captor.firstValue.release).isEqualTo("git-commit-id") } } @@ -230,9 +231,9 @@ class SentryAutoConfigurationTest { .run { Sentry.captureMessage("Some message") val transport = it.getBean(ITransport::class.java) - verify(transport).send(check { event: SentryEvent -> - assertThat(event.release).isEqualTo("my-release") - }) + val captor = argumentCaptor() + verify(transport).send(captor.capture()) + assertThat(captor.firstValue.release).isEqualTo("my-release") } } From fdb3b7e3eacfe71b4711e248dea6a422da0f917d Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 24 Aug 2020 16:02:21 +0200 Subject: [PATCH 11/27] Await assertions as events are sent asynchronously. --- sentry-spring-boot-starter/build.gradle.kts | 1 + .../boot/SentryAutoConfigurationTest.kt | 31 ++++++++++++------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/sentry-spring-boot-starter/build.gradle.kts b/sentry-spring-boot-starter/build.gradle.kts index 86d53b6c9..c6b4b8f92 100644 --- a/sentry-spring-boot-starter/build.gradle.kts +++ b/sentry-spring-boot-starter/build.gradle.kts @@ -48,6 +48,7 @@ dependencies { testImplementation(Config.TestLibs.kotlinTestJunit) testImplementation(Config.TestLibs.mockitoKotlin) testImplementation(Config.TestLibs.springBootStarterTest) + testImplementation(Config.TestLibs.awaitility) } configure { diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index 8d9e2e06d..ccc81d28d 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -17,6 +17,7 @@ import io.sentry.core.transport.ITransport import io.sentry.core.transport.ITransportGate import kotlin.test.Test import org.assertj.core.api.Assertions.assertThat +import org.awaitility.kotlin.await import org.springframework.boot.autoconfigure.AutoConfigurations import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration import org.springframework.boot.info.GitProperties @@ -157,12 +158,14 @@ class SentryAutoConfigurationTest { .run { Sentry.captureMessage("Some message") val transport = it.getBean(ITransport::class.java) - verify(transport).send(check { event: SentryEvent -> - assertThat(event.sdk.version).isEqualTo(BuildConfig.VERSION_NAME) - assertThat(event.sdk.name).isEqualTo(BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME) - assertThat(event.sdk.packages).anyMatch { pkg -> - pkg.name == "maven:sentry-spring-boot-starter" && pkg.version == BuildConfig.VERSION_NAME } - }) + await.untilAsserted { + verify(transport).send(check { event: SentryEvent -> + assertThat(event.sdk.version).isEqualTo(BuildConfig.VERSION_NAME) + assertThat(event.sdk.name).isEqualTo(BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME) + assertThat(event.sdk.packages).anyMatch { pkg -> + pkg.name == "maven:sentry-spring-boot-starter" && pkg.version == BuildConfig.VERSION_NAME } + }) + } } } @@ -218,9 +221,11 @@ class SentryAutoConfigurationTest { .run { Sentry.captureMessage("Some message") val transport = it.getBean(ITransport::class.java) - val captor = argumentCaptor() - verify(transport).send(captor.capture()) - assertThat(captor.firstValue.release).isEqualTo("git-commit-id") + await.untilAsserted { + val captor = argumentCaptor() + verify(transport).send(captor.capture()) + assertThat(captor.firstValue.release).isEqualTo("git-commit-id") + } } } @@ -231,9 +236,11 @@ class SentryAutoConfigurationTest { .run { Sentry.captureMessage("Some message") val transport = it.getBean(ITransport::class.java) - val captor = argumentCaptor() - verify(transport).send(captor.capture()) - assertThat(captor.firstValue.release).isEqualTo("my-release") + await.untilAsserted { + val captor = argumentCaptor() + verify(transport).send(captor.capture()) + assertThat(captor.firstValue.release).isEqualTo("my-release") + } } } From 1acf62305a5797c22509ad0ae582a8b625d4554e Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 24 Aug 2020 16:18:18 +0200 Subject: [PATCH 12/27] Polish. --- .../spring/boot/SentryAutoConfigurationTest.kt | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index ccc81d28d..268dc7cbc 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -1,6 +1,5 @@ package io.sentry.spring.boot -import com.nhaarman.mockitokotlin2.argumentCaptor import com.nhaarman.mockitokotlin2.check import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify @@ -222,9 +221,9 @@ class SentryAutoConfigurationTest { Sentry.captureMessage("Some message") val transport = it.getBean(ITransport::class.java) await.untilAsserted { - val captor = argumentCaptor() - verify(transport).send(captor.capture()) - assertThat(captor.firstValue.release).isEqualTo("git-commit-id") + verify(transport).send(check { event: SentryEvent -> + assertThat(event.release).isEqualTo("git-commit-id") + }) } } } @@ -237,9 +236,9 @@ class SentryAutoConfigurationTest { Sentry.captureMessage("Some message") val transport = it.getBean(ITransport::class.java) await.untilAsserted { - val captor = argumentCaptor() - verify(transport).send(captor.capture()) - assertThat(captor.firstValue.release).isEqualTo("my-release") + verify(transport).send(check { event: SentryEvent -> + assertThat(event.release).isEqualTo("my-release") + }) } } } From 1219ad1f25555525b5071adb1560aeeb612e6178 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 24 Aug 2020 16:42:58 +0200 Subject: [PATCH 13/27] Catch exceptions thrown by event processors. --- .../java/io/sentry/core/SentryClient.java | 24 +++++++++++++++++-- .../java/io/sentry/core/SentryClientTest.kt | 8 +++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/sentry-core/src/main/java/io/sentry/core/SentryClient.java b/sentry-core/src/main/java/io/sentry/core/SentryClient.java index e65aac25b..29d1cf5a4 100644 --- a/sentry-core/src/main/java/io/sentry/core/SentryClient.java +++ b/sentry-core/src/main/java/io/sentry/core/SentryClient.java @@ -79,7 +79,17 @@ public SentryClient(final @NotNull SentryOptions options, @Nullable Connection c } for (EventProcessor processor : options.getEventProcessors()) { - event = processor.process(event, hint); + try { + event = processor.process(event, hint); + } catch (Exception e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + e, + "An exception occurred while processing event by processor: %s", + processor.getClass().getName()); + } if (event == null) { options @@ -275,7 +285,17 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec } for (EventProcessor processor : scope.getEventProcessors()) { - event = processor.process(event, hint); + try { + event = processor.process(event, hint); + } catch (Exception e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + e, + "An exception occurred while processing event by processor: %s", + processor.getClass().getName()); + } if (event == null) { options diff --git a/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt b/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt index 08cb4531d..2670f9a5b 100644 --- a/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt +++ b/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt @@ -26,6 +26,7 @@ import io.sentry.core.transport.AsyncConnection import io.sentry.core.transport.HttpTransport import io.sentry.core.transport.ITransportGate import java.io.IOException +import java.lang.RuntimeException import java.net.URL import java.util.UUID import kotlin.test.Ignore @@ -655,6 +656,13 @@ class SentryClientTest { }, anyOrNull()) } + @Test + fun `exception thrown by an event processor is handled gracefully`() { + fixture.sentryOptions.addEventProcessor { _, _ -> throw RuntimeException() } + val sut = fixture.getSut() + sut.captureEvent(SentryEvent()) + } + private fun createScope(): Scope { return Scope(SentryOptions()).apply { addBreadcrumb(Breadcrumb().apply { From fd609ce7991ed052df010c73fa74278ced100420 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 24 Aug 2020 16:43:09 +0200 Subject: [PATCH 14/27] Polish. --- .../io/sentry/spring/boot/SentryProperties.java | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java index 6c36100e0..947d750ec 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java @@ -8,19 +8,7 @@ import java.util.Optional; import org.springframework.boot.context.properties.ConfigurationProperties; -/** - * Configuration for Sentry in a shape of Spring Boot configuration bean. Example: - * - * 
- * sentry:
- *     enabled: true
- *     dsn: https://00059966e6224d03a77ea5eca10fbe18@sentry.mycompany.com/14
- *     release: "1.0.1"
- *     dist: x86
- *     environment: staging
- *     server-name: megaServer
- *
- */ +/** Configuration for Sentry integration. */ @ConfigurationProperties("sentry") @Open public class SentryProperties { From f5490cce8ab4e5703ba4729d0ab30f70a77f2135 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Tue, 25 Aug 2020 13:00:11 +0200 Subject: [PATCH 15/27] Fix attaching request and user information for uncaught exceptions. --- sentry-spring-boot-starter/build.gradle.kts | 2 + .../spring/boot/SentryAutoConfiguration.java | 19 +--- .../spring/boot/SentryRequestFilter.java | 5 + ...tryRequestHttpServletRequestProcessor.java | 15 +-- .../spring/boot/SentrySecurityFilter.java | 43 +++++++ ...SentryUserHttpServletRequestProcessor.java | 43 ++++--- .../boot/SentryAutoConfigurationTest.kt | 18 --- ...yRequestHttpServletRequestProcessorTest.kt | 13 +-- .../boot/SentrySpringIntegrationTest.kt | 107 ++++++++++++++++++ ...ntryUserHttpServletRequestProcessorTest.kt | 14 +-- 10 files changed, 193 insertions(+), 86 deletions(-) create mode 100644 sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java create mode 100644 sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt diff --git a/sentry-spring-boot-starter/build.gradle.kts b/sentry-spring-boot-starter/build.gradle.kts index c6b4b8f92..f69416d1d 100644 --- a/sentry-spring-boot-starter/build.gradle.kts +++ b/sentry-spring-boot-starter/build.gradle.kts @@ -48,6 +48,8 @@ dependencies { testImplementation(Config.TestLibs.kotlinTestJunit) testImplementation(Config.TestLibs.mockitoKotlin) testImplementation(Config.TestLibs.springBootStarterTest) + testImplementation(Config.TestLibs.springBootStarterWeb) + testImplementation(Config.TestLibs.springBootStarterSecurity) testImplementation(Config.TestLibs.awaitility) } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index 5ba1a633a..57820d14b 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -20,6 +20,7 @@ import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.Ordered; @Configuration @ConditionalOnProperty(name = "sentry.enabled", havingValue = "true", matchIfMissing = true) @@ -81,25 +82,17 @@ static class HubConfiguration { /** Registers beans specific to Spring MVC. */ @Configuration @ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET) + @ConditionalOnProperty("sentry.dsn") @Open static class SentryWebMvcConfiguration { - @Bean - public @NotNull SentryRequestHttpServletRequestProcessor - sentryEventHttpServletRequestProcessor() { - return new SentryRequestHttpServletRequestProcessor(); - } - - @Bean - public @NotNull SentryUserHttpServletRequestProcessor - sentryUserHttpServletRequestProcessor() { - return new SentryUserHttpServletRequestProcessor(); - } - @Bean public @NotNull FilterRegistrationBean sentryRequestFilter( final @NotNull IHub sentryHub) { - return new FilterRegistrationBean<>(new SentryRequestFilter(sentryHub)); + FilterRegistrationBean filterRegistrationBean = + new FilterRegistrationBean<>(new SentryRequestFilter(sentryHub)); + filterRegistrationBean.setOrder(Ordered.HIGHEST_PRECEDENCE); + return filterRegistrationBean; } } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java index 41ba5d948..31e72d9a3 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java @@ -26,6 +26,11 @@ protected void doFilterInternal( final @NotNull FilterChain filterChain) throws ServletException, IOException { hub.pushScope(); + + hub.configureScope( + scope -> { + scope.addEventProcessor(new SentryRequestHttpServletRequestProcessor(request)); + }); filterChain.doFilter(request, response); } } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java index b4ae8eb47..c38861994 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java @@ -11,23 +11,20 @@ import javax.servlet.http.HttpServletRequest; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.springframework.web.context.request.RequestAttributes; -import org.springframework.web.context.request.RequestContextHolder; -import org.springframework.web.context.request.ServletRequestAttributes; /** Attaches information about HTTP request to {@link SentryEvent}. */ @Open public class SentryRequestHttpServletRequestProcessor implements EventProcessor { + private final @NotNull HttpServletRequest request; + + public SentryRequestHttpServletRequestProcessor(final @NotNull HttpServletRequest request) { + this.request = request; + } @Override public @NotNull SentryEvent process( final @NotNull SentryEvent event, final @Nullable Object hint) { - final RequestAttributes requestAttributes = RequestContextHolder.currentRequestAttributes(); - if (requestAttributes instanceof ServletRequestAttributes) { - final HttpServletRequest request = - ((ServletRequestAttributes) requestAttributes).getRequest(); - event.setRequest(resolveSentryRequest(request)); - } + event.setRequest(resolveSentryRequest(request)); return event; } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java new file mode 100644 index 000000000..2ee62fbfa --- /dev/null +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java @@ -0,0 +1,43 @@ +package io.sentry.spring.boot; + +import com.jakewharton.nopen.annotation.Open; +import io.sentry.core.IHub; +import java.io.IOException; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.jetbrains.annotations.NotNull; +import org.springframework.web.filter.OncePerRequestFilter; + +/** + * Adds {@link SentryUserHttpServletRequestProcessor} to the scope in order to decorate {@link + * io.sentry.core.SentryEvent} with principal name and user ip address. + */ +@Open +public class SentrySecurityFilter extends OncePerRequestFilter { + private final @NotNull IHub hub; + + public SentrySecurityFilter(final @NotNull IHub hub) { + this.hub = hub; + } + + @Override + protected void doFilterInternal( + final @NotNull HttpServletRequest request, + final @NotNull HttpServletResponse response, + final @NotNull FilterChain filterChain) + throws ServletException, IOException { + hub.configureScope( + scope -> + scope.addEventProcessor( + new SentryUserHttpServletRequestProcessor( + request.getUserPrincipal(), toIpAddress(request)))); + filterChain.doFilter(request, response); + } + + private static @NotNull String toIpAddress(final @NotNull HttpServletRequest request) { + final String ipAddress = request.getHeader("X-FORWARDED-FOR"); + return ipAddress != null ? ipAddress : request.getRemoteAddr(); + } +} diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java index 68a0ce5b5..c3bf8fe0a 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java @@ -4,40 +4,35 @@ import io.sentry.core.EventProcessor; import io.sentry.core.SentryEvent; import io.sentry.core.protocol.User; -import javax.servlet.http.HttpServletRequest; +import java.security.Principal; +import java.util.Optional; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.springframework.web.context.request.RequestAttributes; -import org.springframework.web.context.request.RequestContextHolder; -import org.springframework.web.context.request.ServletRequestAttributes; -/** Attaches user information from the HTTP request to {@link SentryEvent}. */ +/** Attaches user information to the {@link SentryEvent}. */ @Open public class SentryUserHttpServletRequestProcessor implements EventProcessor { + private final @Nullable Principal principal; + private final @Nullable String ipAddress; + + public SentryUserHttpServletRequestProcessor( + final @Nullable Principal principal, final @Nullable String ipAddress) { + this.principal = principal; + this.ipAddress = ipAddress; + } @Override public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Object hint) { - final RequestAttributes requestAttributes = RequestContextHolder.currentRequestAttributes(); - if (requestAttributes instanceof ServletRequestAttributes) { - final HttpServletRequest request = - ((ServletRequestAttributes) requestAttributes).getRequest(); - event.setUser(toUser(event.getUser(), request)); - } - return event; - } + final User user = Optional.ofNullable(event.getUser()).orElseGet(User::new); - private static @NotNull User toUser( - final @Nullable User existingUser, final @NotNull HttpServletRequest request) { - final User user = existingUser != null ? existingUser : new User(); - user.setIpAddress(toIpAddress(request)); - if (request.getUserPrincipal() != null) { - user.setUsername(request.getUserPrincipal().getName()); + if (ipAddress != null) { + user.setIpAddress(ipAddress); + } + if (principal != null) { + user.setUsername(principal.getName()); } - return user; - } - private static @NotNull String toIpAddress(final @NotNull HttpServletRequest request) { - final String ipAddress = request.getHeader("X-FORWARDED-FOR"); - return ipAddress != null ? ipAddress : request.getRemoteAddr(); + event.setUser(user); + return event; } } diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index 268dc7cbc..cef98d47a 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -132,24 +132,6 @@ class SentryAutoConfigurationTest { } } - @Test - fun `does not register event processors for non web-servlet application type`() { - contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") - .run { - assertThat(it).doesNotHaveBean(SentryRequestHttpServletRequestProcessor::class.java) - assertThat(it).doesNotHaveBean(SentryUserHttpServletRequestProcessor::class.java) - } - } - - @Test - fun `registers event processors for web servlet application type`() { - webContextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") - .run { - assertThat(it).hasSingleBean(SentryRequestHttpServletRequestProcessor::class.java) - assertThat(it).hasSingleBean(SentryUserHttpServletRequestProcessor::class.java) - } - } - @Test fun `sets SDK version on sent events`() { contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt index c58f5cedd..7579e5384 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt @@ -7,13 +7,9 @@ import kotlin.test.assertEquals import org.springframework.http.MediaType import org.springframework.mock.web.MockServletContext import org.springframework.test.web.servlet.request.MockMvcRequestBuilders -import org.springframework.web.context.request.RequestContextHolder -import org.springframework.web.context.request.ServletRequestAttributes class SentryRequestHttpServletRequestProcessorTest { - private val eventProcessor = SentryRequestHttpServletRequestProcessor() - @Test fun `attaches basic information from HTTP request to SentryEvent`() { val request = MockMvcRequestBuilders @@ -21,8 +17,7 @@ class SentryRequestHttpServletRequestProcessorTest { .header("some-header", "some-header value") .accept(MediaType.APPLICATION_JSON) .buildRequest(MockServletContext()) - - RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + val eventProcessor = SentryRequestHttpServletRequestProcessor(request) val event = SentryEvent() eventProcessor.process(event, null) @@ -43,8 +38,7 @@ class SentryRequestHttpServletRequestProcessorTest { .header("another-header", "another value") .header("another-header", "another value2") .buildRequest(MockServletContext()) - - RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + val eventProcessor = SentryRequestHttpServletRequestProcessor(request) val event = SentryEvent() eventProcessor.process(event, null) @@ -61,8 +55,7 @@ class SentryRequestHttpServletRequestProcessorTest { .header("Cookie", "name=value") .header("Cookie", "name2=value2") .buildRequest(MockServletContext()) - - RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + val eventProcessor = SentryRequestHttpServletRequestProcessor(request) val event = SentryEvent() eventProcessor.process(event, null) diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt new file mode 100644 index 000000000..7ca2ca85a --- /dev/null +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt @@ -0,0 +1,107 @@ +package io.sentry.spring.boot + +import com.nhaarman.mockitokotlin2.check +import com.nhaarman.mockitokotlin2.verify +import io.sentry.core.IHub +import io.sentry.core.Sentry +import io.sentry.core.SentryEvent +import io.sentry.core.transport.ITransport +import org.assertj.core.api.Assertions.assertThat +import org.awaitility.kotlin.await +import org.junit.Test +import org.junit.runner.RunWith +import org.springframework.boot.autoconfigure.SpringBootApplication +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.boot.test.mock.mockito.MockBean +import org.springframework.boot.test.web.client.TestRestTemplate +import org.springframework.boot.web.server.LocalServerPort +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration +import org.springframework.http.HttpEntity +import org.springframework.http.HttpHeaders +import org.springframework.http.HttpMethod +import org.springframework.security.config.annotation.web.builders.HttpSecurity +import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter +import org.springframework.security.core.userdetails.User +import org.springframework.security.core.userdetails.UserDetails +import org.springframework.security.core.userdetails.UserDetailsService +import org.springframework.security.crypto.factory.PasswordEncoderFactories +import org.springframework.security.crypto.password.PasswordEncoder +import org.springframework.security.provisioning.InMemoryUserDetailsManager +import org.springframework.security.web.authentication.AnonymousAuthenticationFilter +import org.springframework.test.context.junit4.SpringRunner +import org.springframework.web.bind.annotation.GetMapping +import org.springframework.web.bind.annotation.RestController + +@RunWith(SpringRunner::class) +@SpringBootTest( + classes = [App::class], + webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT, + properties = ["sentry.dsn=http://key@localhost/proj"] +) +class SentrySpringIntegrationTest { + + @MockBean + lateinit var transport: ITransport + + @LocalServerPort + lateinit var port: Integer + + @Test + fun `attaches request and user information to SentryEvents`() { + val restTemplate = TestRestTemplate().withBasicAuth("user", "password") + val headers = HttpHeaders() + headers["X-FORWARDED-FOR"] = listOf("169.128.0.1") + val entity = HttpEntity(headers) + + restTemplate.exchange("http://localhost:$port/hello", HttpMethod.GET, entity, Void::class.java) + + await.untilAsserted { + verify(transport).send(check { event: SentryEvent -> + assertThat(event.request).isNotNull() + assertThat(event.request.url).isEqualTo("http://localhost:$port/hello") + assertThat(event.user).isNotNull() + assertThat(event.user.username).isEqualTo("user") + assertThat(event.user.ipAddress).isEqualTo("169.128.0.1") + }) + } + } +} + +@SpringBootApplication +open class App + +@RestController +class HelloController { + + @GetMapping("/hello") + fun hello() { + Sentry.captureMessage("hello") + } +} + +@Configuration +open class SecurityConfiguration(private val hub: IHub) : WebSecurityConfigurerAdapter() { + + override fun configure(http: HttpSecurity) { + http + .addFilterAfter(SentrySecurityFilter(hub), AnonymousAuthenticationFilter::class.java) + .csrf().disable() + .authorizeRequests().anyRequest().authenticated() + .and() + .httpBasic() + } + + @Bean + override fun userDetailsService(): UserDetailsService { + val encoder: PasswordEncoder = PasswordEncoderFactories.createDelegatingPasswordEncoder() + val user: UserDetails = User + .builder() + .passwordEncoder { rawPassword -> encoder.encode(rawPassword) } + .username("user") + .password("password") + .roles("USER") + .build() + return InMemoryUserDetailsManager(user) + } +} diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt index 802705ae2..d26907783 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt @@ -6,20 +6,12 @@ import io.sentry.core.SentryEvent import java.security.Principal import kotlin.test.Test import kotlin.test.assertEquals -import org.springframework.mock.web.MockHttpServletRequest -import org.springframework.web.context.request.RequestContextHolder -import org.springframework.web.context.request.ServletRequestAttributes class SentryUserHttpServletRequestProcessorTest { - private val eventProcessor = SentryUserHttpServletRequestProcessor() - @Test fun `attaches user's IP address to Sentry Event`() { - val request = MockHttpServletRequest() - request.remoteAddr = "192.168.0.1" - - RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + val eventProcessor = SentryUserHttpServletRequestProcessor(null, "192.168.0.1") val event = SentryEvent() eventProcessor.process(event, null) @@ -31,10 +23,8 @@ class SentryUserHttpServletRequestProcessorTest { fun `attaches username to Sentry Event`() { val principal = mock() whenever(principal.name).thenReturn("janesmith") - val request = MockHttpServletRequest() - request.userPrincipal = principal - RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + val eventProcessor = SentryUserHttpServletRequestProcessor(principal, null) val event = SentryEvent() eventProcessor.process(event, null) From 21bc5be966224a3969af6d37b73a24a8135abb9b Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Tue, 25 Aug 2020 13:02:59 +0200 Subject: [PATCH 16/27] Add Sentry Spring Boot Sample. --- .../build.gradle.kts | 40 ++++++++++++++ .../java/io/sentry/samples/spring/Person.java | 24 ++++++++ .../samples/spring/PersonController.java | 27 +++++++++ .../samples/spring/SecurityConfiguration.java | 55 +++++++++++++++++++ .../samples/spring/SentryDemoApplication.java | 12 ++++ .../src/main/resources/application.properties | 2 + .../src/main/resources/logback.xml | 16 ++++++ settings.gradle.kts | 19 ++++--- 8 files changed, 186 insertions(+), 9 deletions(-) create mode 100644 sentry-samples/sentry-samples-spring-boot/build.gradle.kts create mode 100644 sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/Person.java create mode 100644 sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/PersonController.java create mode 100644 sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SecurityConfiguration.java create mode 100644 sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SentryDemoApplication.java create mode 100644 sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties create mode 100644 sentry-samples/sentry-samples-spring-boot/src/main/resources/logback.xml diff --git a/sentry-samples/sentry-samples-spring-boot/build.gradle.kts b/sentry-samples/sentry-samples-spring-boot/build.gradle.kts new file mode 100644 index 000000000..b0db0af01 --- /dev/null +++ b/sentry-samples/sentry-samples-spring-boot/build.gradle.kts @@ -0,0 +1,40 @@ +import org.jetbrains.kotlin.gradle.tasks.KotlinCompile + +plugins { + id("org.springframework.boot") version "2.3.3.RELEASE" + id("io.spring.dependency-management") version "1.0.10.RELEASE" + kotlin("jvm") + kotlin("plugin.spring") version "1.3.72" +} + +group = "com.example" +version = "0.0.1-SNAPSHOT" +java.sourceCompatibility = JavaVersion.VERSION_1_8 + +repositories { + mavenCentral() +} + +dependencies { + implementation("org.springframework.boot:spring-boot-starter-security") + implementation("org.springframework.boot:spring-boot-starter-web") + implementation("org.springframework.boot:spring-boot-starter") + implementation("org.jetbrains.kotlin:kotlin-reflect") + implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk8") + implementation(project(":sentry-spring-boot-starter")) + implementation(project(":sentry-logback")) + testImplementation("org.springframework.boot:spring-boot-starter-test") { + exclude(group = "org.junit.vintage", module = "junit-vintage-engine") + } +} + +tasks.withType { + useJUnitPlatform() +} + +tasks.withType { + kotlinOptions { + freeCompilerArgs = listOf("-Xjsr305=strict") + jvmTarget = "1.8" + } +} diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/Person.java b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/Person.java new file mode 100644 index 000000000..f4588a7f6 --- /dev/null +++ b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/Person.java @@ -0,0 +1,24 @@ +package io.sentry.samples.spring; + +public class Person { + private final String firstName; + private final String lastName; + + public Person(String firstName, String lastName) { + this.firstName = firstName; + this.lastName = lastName; + } + + public String getFirstName() { + return firstName; + } + + public String getLastName() { + return lastName; + } + + @Override + public String toString() { + return "Person{" + "firstName='" + firstName + '\'' + ", lastName='" + lastName + '\'' + '}'; + } +} diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/PersonController.java b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/PersonController.java new file mode 100644 index 000000000..9a2d97316 --- /dev/null +++ b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/PersonController.java @@ -0,0 +1,27 @@ +package io.sentry.samples.spring; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +@RequestMapping("/person/") +public class PersonController { + private static final Logger LOGGER = LoggerFactory.getLogger(PersonController.class); + + @GetMapping("{id}") + Person person(@PathVariable Long id) { + throw new IllegalArgumentException("Something went wrong [id=" + id + "]"); + } + + @PostMapping + Person create(@RequestBody Person person) { + LOGGER.warn("Creating person: {}", person); + return person; + } +} diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SecurityConfiguration.java b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SecurityConfiguration.java new file mode 100644 index 000000000..d5527cdbb --- /dev/null +++ b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SecurityConfiguration.java @@ -0,0 +1,55 @@ +package io.sentry.samples.spring; + +import io.sentry.core.IHub; +import io.sentry.spring.boot.SentrySecurityFilter; +import org.jetbrains.annotations.NotNull; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; +import org.springframework.security.core.userdetails.User; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.core.userdetails.UserDetailsService; +import org.springframework.security.crypto.factory.PasswordEncoderFactories; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.security.provisioning.InMemoryUserDetailsManager; +import org.springframework.security.web.authentication.AnonymousAuthenticationFilter; + +@Configuration +public class SecurityConfiguration extends WebSecurityConfigurerAdapter { + + private final @NotNull IHub hub; + + public SecurityConfiguration(final @NotNull IHub hub) { + this.hub = hub; + } + + @Override + protected void configure(final @NotNull HttpSecurity http) throws Exception { + // register SentrySecurityFilter to attach user information to SentryEvents + http.addFilterAfter(new SentrySecurityFilter(hub), AnonymousAuthenticationFilter.class) + .csrf() + .disable() + .authorizeRequests() + .anyRequest() + .authenticated() + .and() + .httpBasic(); + } + + @Bean + @Override + public @NotNull UserDetailsService userDetailsService() { + final PasswordEncoder encoder = PasswordEncoderFactories.createDelegatingPasswordEncoder(); + + final UserDetails user = + User.builder() + .passwordEncoder(encoder::encode) + .username("user") + .password("password") + .roles("USER") + .build(); + + return new InMemoryUserDetailsManager(user); + } +} diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SentryDemoApplication.java b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SentryDemoApplication.java new file mode 100644 index 000000000..43882fe6c --- /dev/null +++ b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SentryDemoApplication.java @@ -0,0 +1,12 @@ +package io.sentry.samples.spring; + +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; + +@SpringBootApplication +public class SentryDemoApplication { + + public static void main(String[] args) { + SpringApplication.run(SentryDemoApplication.class, args); + } +} diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties new file mode 100644 index 000000000..3d8acbbc1 --- /dev/null +++ b/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties @@ -0,0 +1,2 @@ +# NOTE: Replace the test DSN below with YOUR OWN DSN to see the events from this app in your Sentry project/dashboard +sentry.dsn=https://f7f320d5c3a54709be7b28e0f2ca7081@sentry.io/1808954 diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/resources/logback.xml b/sentry-samples/sentry-samples-spring-boot/src/main/resources/logback.xml new file mode 100644 index 000000000..fe050c21c --- /dev/null +++ b/sentry-samples/sentry-samples-spring-boot/src/main/resources/logback.xml @@ -0,0 +1,16 @@ + + + + + + + + WARN + + + + + + + + diff --git a/settings.gradle.kts b/settings.gradle.kts index fcca1ef45..9bd98dbdf 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -2,12 +2,13 @@ rootProject.name = "sentry" rootProject.buildFileName = "build.gradle.kts" include("sentry-android", - "sentry-android-ndk", - "sentry-android-core", - "sentry-core", - "sentry-logback", - "sentry-spring-boot-starter", - "sentry-samples:sentry-samples-android", - "sentry-samples:sentry-samples-console", - "sentry-samples:sentry-samples-logback", - "sentry-android-timber") + "sentry-android-ndk", + "sentry-android-core", + "sentry-core", + "sentry-logback", + "sentry-spring-boot-starter", + "sentry-samples:sentry-samples-android", + "sentry-samples:sentry-samples-console", + "sentry-samples:sentry-samples-logback", + "sentry-samples:sentry-samples-spring-boot", + "sentry-android-timber") From c2f7f57a850f7b53ba460b9b6d1aa5bab77144e7 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Tue, 25 Aug 2020 13:14:52 +0200 Subject: [PATCH 17/27] Add Sentry Spring Boot Sample. --- buildSrc/src/main/java/Config.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt index 4565119e2..9fc7d1c2c 100644 --- a/buildSrc/src/main/java/Config.kt +++ b/buildSrc/src/main/java/Config.kt @@ -57,6 +57,8 @@ object Config { val mockitoKotlin = "com.nhaarman.mockitokotlin2:mockito-kotlin:2.2.0" val awaitility = "org.awaitility:awaitility-kotlin:4.0.3" val springBootStarterTest = "org.springframework.boot:spring-boot-starter-test:$springBootVersion" + val springBootStarterWeb = "org.springframework.boot:spring-boot-starter-web:$springBootVersion" + val springBootStarterSecurity = "org.springframework.boot:spring-boot-starter-security:$springBootVersion" } object QualityPlugins { From f8c5352298fcf92ac3e4a421d6e0a098f5b650ff Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Tue, 25 Aug 2020 13:20:00 +0200 Subject: [PATCH 18/27] Import classes. --- sentry-spring-boot-starter/build.gradle.kts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/sentry-spring-boot-starter/build.gradle.kts b/sentry-spring-boot-starter/build.gradle.kts index f69416d1d..ad7cba44e 100644 --- a/sentry-spring-boot-starter/build.gradle.kts +++ b/sentry-spring-boot-starter/build.gradle.kts @@ -1,4 +1,7 @@ import com.novoda.gradle.release.PublishExtension +import io.spring.gradle.dependencymanagement.dsl.DependencyManagementExtension +import org.jetbrains.kotlin.gradle.tasks.KotlinCompile +import org.springframework.boot.gradle.plugin.SpringBootPlugin plugins { `java-library` @@ -13,9 +16,9 @@ plugins { apply(plugin = Config.BuildPlugins.springDependencyManagement) -the().apply { +the().apply { imports { - mavenBom(org.springframework.boot.gradle.plugin.SpringBootPlugin.BOM_COORDINATES) + mavenBom(SpringBootPlugin.BOM_COORDINATES) } } @@ -24,7 +27,7 @@ configure { targetCompatibility = JavaVersion.VERSION_1_8 } -tasks.withType().configureEach { +tasks.withType().configureEach { kotlinOptions.jvmTarget = JavaVersion.VERSION_1_8.toString() } From 75f8d55c0e7f5348cefcdc0bd3decfba71a73656 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Tue, 25 Aug 2020 13:22:15 +0200 Subject: [PATCH 19/27] Polish. --- buildSrc/src/main/java/Config.kt | 5 +++++ sentry-spring-boot-starter/build.gradle.kts | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt index 9fc7d1c2c..67debfd76 100644 --- a/buildSrc/src/main/java/Config.kt +++ b/buildSrc/src/main/java/Config.kt @@ -46,6 +46,11 @@ object Config { val servletApi = "javax.servlet:javax.servlet-api" } + object AnnotationProcessors { + val springBootAutoConfigure = "org.springframework.boot:spring-boot-autoconfigure-processor" + val springBootConfiguration = "org.springframework.boot:spring-boot-configuration-processor" + } + object TestLibs { private val androidxTestVersion = "1.2.0" diff --git a/sentry-spring-boot-starter/build.gradle.kts b/sentry-spring-boot-starter/build.gradle.kts index ad7cba44e..100ea7aee 100644 --- a/sentry-spring-boot-starter/build.gradle.kts +++ b/sentry-spring-boot-starter/build.gradle.kts @@ -37,8 +37,8 @@ dependencies { implementation(Config.Libs.springWeb) implementation(Config.Libs.servletApi) - annotationProcessor("org.springframework.boot:spring-boot-autoconfigure-processor") - annotationProcessor("org.springframework.boot:spring-boot-configuration-processor") + annotationProcessor(Config.AnnotationProcessors.springBootAutoConfigure) + annotationProcessor(Config.AnnotationProcessors.springBootConfiguration) compileOnly(Config.CompileOnly.nopen) errorprone(Config.CompileOnly.nopenChecker) From 5a7f15eb75f0829f9c863439f10b51ab2b805a81 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Tue, 25 Aug 2020 13:22:25 +0200 Subject: [PATCH 20/27] Polish. --- .../spring/boot/SentryRequestHttpServletRequestProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java index c38861994..860b48c4a 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java @@ -28,7 +28,7 @@ public SentryRequestHttpServletRequestProcessor(final @NotNull HttpServletReques return event; } - @SuppressWarnings("JdkObsolete") + @SuppressWarnings("JdkObsolete") // httpRequest.getRequestURL() returns StringBuffer which is considered an obsolete class. private static @NotNull Request resolveSentryRequest( final @NotNull HttpServletRequest httpRequest) { final Request sentryRequest = new Request(); From 85ad96d9c8562e49254d8af9a6631195bcd1d4d7 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Wed, 26 Aug 2020 12:49:08 +0200 Subject: [PATCH 21/27] Polish. --- sentry-core/src/main/java/io/sentry/core/Sentry.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sentry-core/src/main/java/io/sentry/core/Sentry.java b/sentry-core/src/main/java/io/sentry/core/Sentry.java index 1171417a0..1e0a05fb1 100644 --- a/sentry-core/src/main/java/io/sentry/core/Sentry.java +++ b/sentry-core/src/main/java/io/sentry/core/Sentry.java @@ -7,6 +7,8 @@ import java.io.File; import java.lang.reflect.InvocationTargetException; import java.util.List; + +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -128,6 +130,7 @@ public static void init( * * @param options options the SentryOptions */ + @ApiStatus.Internal public static void init(final @NotNull SentryOptions options) { init(options, GLOBAL_HUB_DEFAULT_MODE); } From 6cba5b50ca78e9135c53874bffcc9ced3f8d55c3 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Wed, 26 Aug 2020 12:56:32 +0200 Subject: [PATCH 22/27] Polish. --- .../src/main/java/io/sentry/core/Sentry.java | 1 - .../java/io/sentry/core/SentryClient.java | 80 ++++++++----------- 2 files changed, 34 insertions(+), 47 deletions(-) diff --git a/sentry-core/src/main/java/io/sentry/core/Sentry.java b/sentry-core/src/main/java/io/sentry/core/Sentry.java index 1e0a05fb1..9459e2793 100644 --- a/sentry-core/src/main/java/io/sentry/core/Sentry.java +++ b/sentry-core/src/main/java/io/sentry/core/Sentry.java @@ -7,7 +7,6 @@ import java.io.File; import java.lang.reflect.InvocationTargetException; import java.util.List; - import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; diff --git a/sentry-core/src/main/java/io/sentry/core/SentryClient.java b/sentry-core/src/main/java/io/sentry/core/SentryClient.java index 29d1cf5a4..ab6394dc0 100644 --- a/sentry-core/src/main/java/io/sentry/core/SentryClient.java +++ b/sentry-core/src/main/java/io/sentry/core/SentryClient.java @@ -12,6 +12,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Random; import org.jetbrains.annotations.ApiStatus; @@ -78,29 +79,7 @@ public SentryClient(final @NotNull SentryOptions options, @Nullable Connection c .log(SentryLevel.DEBUG, "Event was cached so not applying scope: %s", event.getEventId()); } - for (EventProcessor processor : options.getEventProcessors()) { - try { - event = processor.process(event, hint); - } catch (Exception e) { - options - .getLogger() - .log( - SentryLevel.ERROR, - e, - "An exception occurred while processing event by processor: %s", - processor.getClass().getName()); - } - - if (event == null) { - options - .getLogger() - .log( - SentryLevel.DEBUG, - "Event was dropped by processor: %s", - processor.getClass().getName()); - break; - } - } + event = processEvent(event, hint, options.getEventProcessors()); if (event == null) { return SentryId.EMPTY_ID; @@ -137,6 +116,37 @@ public SentryClient(final @NotNull SentryOptions options, @Nullable Connection c return event.getEventId(); } + @Nullable + private SentryEvent processEvent( + @NotNull SentryEvent event, + final @Nullable Object hint, + final @NotNull List eventProcessors) { + for (EventProcessor processor : eventProcessors) { + try { + event = processor.process(event, hint); + } catch (Exception e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + e, + "An exception occurred while processing event by processor: %s", + processor.getClass().getName()); + } + + if (event == null) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Event was dropped by a processor: %s", + processor.getClass().getName()); + break; + } + } + return event; + } + /** * Updates the session data based on the event, hint and scope data * @@ -284,29 +294,7 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec event.setLevel(scope.getLevel()); } - for (EventProcessor processor : scope.getEventProcessors()) { - try { - event = processor.process(event, hint); - } catch (Exception e) { - options - .getLogger() - .log( - SentryLevel.ERROR, - e, - "An exception occurred while processing event by processor: %s", - processor.getClass().getName()); - } - - if (event == null) { - options - .getLogger() - .log( - SentryLevel.DEBUG, - "Event was dropped by scope processor: %s", - processor.getClass().getName()); - break; - } - } + event = processEvent(event, hint, scope.getEventProcessors()); } return event; } From 8e6ad892c14c5e46856e97f2ebdf3be85f5d1e25 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Wed, 26 Aug 2020 12:58:02 +0200 Subject: [PATCH 23/27] Change group id in Spring Boot Sample. --- sentry-samples/sentry-samples-spring-boot/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-spring-boot/build.gradle.kts b/sentry-samples/sentry-samples-spring-boot/build.gradle.kts index b0db0af01..d57571ab7 100644 --- a/sentry-samples/sentry-samples-spring-boot/build.gradle.kts +++ b/sentry-samples/sentry-samples-spring-boot/build.gradle.kts @@ -7,7 +7,7 @@ plugins { kotlin("plugin.spring") version "1.3.72" } -group = "com.example" +group = "io.sentry.sample.spring-boot" version = "0.0.1-SNAPSHOT" java.sourceCompatibility = JavaVersion.VERSION_1_8 From bb863f2e75309218b1ba518ce12d0986ef8189fe Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Wed, 26 Aug 2020 13:22:22 +0200 Subject: [PATCH 24/27] Handle multiple ip addresses in X-FORWARDED-FOR header. --- ...SentryRequestHttpServletRequestProcessor.java | 3 ++- .../sentry/spring/boot/SentrySecurityFilter.java | 9 ++++++++- .../spring/boot/SentrySpringIntegrationTest.kt | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java index 860b48c4a..af8c78e9f 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java @@ -28,7 +28,8 @@ public SentryRequestHttpServletRequestProcessor(final @NotNull HttpServletReques return event; } - @SuppressWarnings("JdkObsolete") // httpRequest.getRequestURL() returns StringBuffer which is considered an obsolete class. + // httpRequest.getRequestURL() returns StringBuffer which is considered an obsolete class. + @SuppressWarnings("JdkObsolete") private static @NotNull Request resolveSentryRequest( final @NotNull HttpServletRequest httpRequest) { final Request sentryRequest = new Request(); diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java index 2ee62fbfa..2525b4c7f 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java @@ -36,8 +36,15 @@ protected void doFilterInternal( filterChain.doFilter(request, response); } + // it is advised to not use `String#split` method but since we do not have 3rd party libraries + // this is our only option. + @SuppressWarnings("StringSplitter") private static @NotNull String toIpAddress(final @NotNull HttpServletRequest request) { final String ipAddress = request.getHeader("X-FORWARDED-FOR"); - return ipAddress != null ? ipAddress : request.getRemoteAddr(); + if (ipAddress != null) { + return ipAddress.contains(",") ? ipAddress.split(",")[0].trim() : ipAddress; + } else { + return request.getRemoteAddr(); + } } } diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt index 7ca2ca85a..0f6dcbe87 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt @@ -66,6 +66,22 @@ class SentrySpringIntegrationTest { }) } } + + @Test + fun `attaches first ip address if multiple addresses exist in a header`() { + val restTemplate = TestRestTemplate().withBasicAuth("user", "password") + val headers = HttpHeaders() + headers["X-FORWARDED-FOR"] = listOf("169.128.0.1, 192.168.0.1") + val entity = HttpEntity(headers) + + restTemplate.exchange("http://localhost:$port/hello", HttpMethod.GET, entity, Void::class.java) + + await.untilAsserted { + verify(transport).send(check { event: SentryEvent -> + assertThat(event.user.ipAddress).isEqualTo("169.128.0.1") + }) + } + } } @SpringBootApplication From 41b845e5c56cae3da2e5d9d6b0288e55f43233b9 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Wed, 26 Aug 2020 13:30:06 +0200 Subject: [PATCH 25/27] Polish Gradle configuration. --- sentry-samples/sentry-samples-spring-boot/build.gradle.kts | 2 +- settings.gradle.kts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry-samples/sentry-samples-spring-boot/build.gradle.kts b/sentry-samples/sentry-samples-spring-boot/build.gradle.kts index d57571ab7..5cedbd722 100644 --- a/sentry-samples/sentry-samples-spring-boot/build.gradle.kts +++ b/sentry-samples/sentry-samples-spring-boot/build.gradle.kts @@ -35,6 +35,6 @@ tasks.withType { tasks.withType { kotlinOptions { freeCompilerArgs = listOf("-Xjsr305=strict") - jvmTarget = "1.8" + jvmTarget = JavaVersion.VERSION_1_8.toString() } } diff --git a/settings.gradle.kts b/settings.gradle.kts index 9bd98dbdf..84aa5eec2 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -7,8 +7,8 @@ include("sentry-android", "sentry-core", "sentry-logback", "sentry-spring-boot-starter", + "sentry-android-timber", "sentry-samples:sentry-samples-android", "sentry-samples:sentry-samples-console", "sentry-samples:sentry-samples-logback", - "sentry-samples:sentry-samples-spring-boot", - "sentry-android-timber") + "sentry-samples:sentry-samples-spring-boot") From cb0a1a93259c30ac2be601d442ae2b84405189b9 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Wed, 26 Aug 2020 13:50:14 +0200 Subject: [PATCH 26/27] Polish Gradle configuration. --- sentry-samples/sentry-samples-spring-boot/build.gradle.kts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry-samples/sentry-samples-spring-boot/build.gradle.kts b/sentry-samples/sentry-samples-spring-boot/build.gradle.kts index 5cedbd722..9676cdb87 100644 --- a/sentry-samples/sentry-samples-spring-boot/build.gradle.kts +++ b/sentry-samples/sentry-samples-spring-boot/build.gradle.kts @@ -1,10 +1,10 @@ import org.jetbrains.kotlin.gradle.tasks.KotlinCompile plugins { - id("org.springframework.boot") version "2.3.3.RELEASE" - id("io.spring.dependency-management") version "1.0.10.RELEASE" + id(Config.BuildPlugins.springBoot) version Config.springBootVersion + id(Config.BuildPlugins.springDependencyManagement) version Config.BuildPlugins.springDependencyManagementVersion kotlin("jvm") - kotlin("plugin.spring") version "1.3.72" + kotlin("plugin.spring") version Config.kotlinVersion } group = "io.sentry.sample.spring-boot" From c31142808094aaa6bb3f7b8b3e7f90fa593df3f7 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Wed, 26 Aug 2020 14:03:34 +0200 Subject: [PATCH 27/27] Polish Gradle configuration. --- buildSrc/src/main/java/Config.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt index 67debfd76..5eeb45c99 100644 --- a/buildSrc/src/main/java/Config.kt +++ b/buildSrc/src/main/java/Config.kt @@ -11,6 +11,7 @@ object Config { val buildConfigVersion = "2.0.2" val springBoot = "org.springframework.boot" val springDependencyManagement = "io.spring.dependency-management" + val springDependencyManagementVersion = "1.0.10.RELEASE" } object Android {