Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto add OpenTelemetryLinkErrorEventProcessor for Spring Boot #2429

Merged
merged 11 commits into from
Dec 22, 2022
Merged

Auto add OpenTelemetryLinkErrorEventProcessor for Spring Boot #2429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
baf8bd5
Add tests for linking errors to otel transactions
adinauer Dec 13, 2022
49efc4c
Add logging to OTEL code
adinauer Dec 13, 2022
183423a
Add more IDs to log statements
adinauer Dec 13, 2022
8b47a99
Add changelog
adinauer Dec 13, 2022
e3c8470
Auto add OpenTelemetryLinkErrorEventProcessor for Spring Boot
adinauer Dec 15, 2022
707ea59
Add changelog
adinauer Dec 15, 2022
ba730c2
No longer require direct dependency on OTEL SDK in spring boot app wh…
adinauer Dec 19, 2022
319b527
Merge branch 'main' into feat/test-error-otel-transaction-link
adinauer Dec 20, 2022
d7813ca
Merge branch 'feat/test-error-otel-transaction-link' into feat/auto-a…
adinauer Dec 20, 2022
da36dbf
Merge branch 'main' into feat/auto-add-otel-event-processor-for-sprin…
adinauer Dec 22, 2022
44f9f2e
Merge branch 'main' into feat/auto-add-otel-event-processor-for-sprin…
adinauer Dec 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
### Features

- Add logging for OpenTelemetry integration ([#2425](https://github.com/getsentry/sentry-java/pull/2425))
- Auto add `OpenTelemetryLinkErrorEventProcessor` for Spring Boot ([#2429](https://github.com/getsentry/sentry-java/pull/2429))

### Dependencies

Expand Down
5 changes: 4 additions & 1 deletion sentry-opentelemetry/sentry-opentelemetry-agentcustomization/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ tasks.withType<KotlinCompile>().configureEach {

dependencies {
compileOnly(projects.sentry)
implementation(projects.sentryOpentelemetry.sentryOpentelemetryCore)
implementation(projects.sentryOpentelemetry.sentryOpentelemetryCore) {
exclude(group = "io.opentelemetry")
exclude(group = "io.opentelemetry.javaagent")
}

compileOnly(Config.Libs.OpenTelemetry.otelSdk)
compileOnly(Config.Libs.OpenTelemetry.otelExtensionAutoconfigureSpi)
Expand Down
2 changes: 1 addition & 1 deletion sentry-opentelemetry/sentry-opentelemetry-core/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ tasks.withType<KotlinCompile>().configureEach {
dependencies {
compileOnly(projects.sentry)

compileOnly(Config.Libs.OpenTelemetry.otelSdk)
implementation(Config.Libs.OpenTelemetry.otelSdk)
Copy link
Member

Choose a reason for hiding this comment

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

hm, why did we change it to implementation? It's not possible to use this module without otel, right? Or is the assumption that people will just include our sentry-opentelemetry-core package, but not otel itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

The intention was to make it easier to use sentry-opentelemetry-agent with SENTRY_AUTO_INIT=false. Currently you need to add sentry-opentelemetry-core to the dependencies so you can enjoy linking of errors to transactions. With compileOnly you'd also have to include the correct version of the OTEL SDK, with implementation it just drags it in. For sentry-opentelemetry-agent we exclude it again, which users also have to do, if they don't want to use the agent but have a different OTEL version than we use or their build system can't figure out versions.

Another approach (for later) would be to have yet another gradle module, that we can then add to the bootstrap classloader in the agent, thus not requiring the explicit dependency on sentry-opentelemetry-core at all, when setting SENTRY_AUTO_INIT=false.

Copy link
Member

Choose a reason for hiding this comment

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

alright, just beware that implementation will override whatever version the users are using (if it's lower than ours). If that's what we want, then I'm good 👍

compileOnly(Config.Libs.OpenTelemetry.otelSemconv)

compileOnly(Config.CompileOnly.nopen)
Expand Down
2 changes: 1 addition & 1 deletion .../sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public void onEnd(final @NotNull ReadableSpan otelSpan) {
.getLogger()
.log(
SentryLevel.DEBUG,
"Unable to find Sentry span for OpenTelemetry span %s (trace %s).",
"Unable to find Sentry span for OpenTelemetry span %s (trace %s). This may simply mean it is a Sentry request.",
adinauer marked this conversation as resolved.
Show resolved Hide resolved
traceData.getSpanId(),
traceData.getTraceId());
return;
Expand Down
2 changes: 2 additions & 0 deletions sentry-spring-boot-starter-jakarta/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ dependencies {
compileOnly(Config.Libs.springBoot3StarterAop)
compileOnly(Config.Libs.springBoot3StarterSecurity)
compileOnly(Config.Libs.reactorCore)
compileOnly(projects.sentryOpentelemetry.sentryOpentelemetryCore)

annotationProcessor(Config.AnnotationProcessors.springBootAutoConfigure)
annotationProcessor(Config.AnnotationProcessors.springBootConfiguration)
Expand All @@ -77,6 +78,7 @@ dependencies {
testImplementation(Config.Libs.springBoot3StarterWebflux)
testImplementation(Config.Libs.springBoot3StarterSecurity)
testImplementation(Config.Libs.springBoot3StarterAop)
testImplementation(projects.sentryOpentelemetry.sentryOpentelemetryCore)
}

configure<SourceSetContainer> {
Expand Down
14 changes: 14 additions & 0 deletions ...-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import io.sentry.Integration;
import io.sentry.Sentry;
import io.sentry.SentryOptions;
import io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor;
import io.sentry.protocol.SdkVersion;
import io.sentry.spring.jakarta.ContextTagsEventProcessor;
import io.sentry.spring.jakarta.SentryExceptionResolver;
Expand Down Expand Up @@ -138,6 +139,19 @@ static class ContextTagsEventProcessorConfiguration {
}
}

@Configuration(proxyBeanMethods = false)
@ConditionalOnProperty(name = "sentry.auto-init", havingValue = "false")
@ConditionalOnClass(OpenTelemetryLinkErrorEventProcessor.class)
@Open
static class OpenTelemetryLinkErrorEventProcessorConfiguration {

@Bean
@ConditionalOnMissingBean
public @NotNull OpenTelemetryLinkErrorEventProcessor openTelemetryLinkErrorEventProcessor() {
return new OpenTelemetryLinkErrorEventProcessor();
}
}

/** Registers beans specific to Spring MVC. */
@Configuration(proxyBeanMethods = false)
@ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET)
Expand Down
42 changes: 42 additions & 0 deletions ...rter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import io.sentry.SentryEvent
import io.sentry.SentryLevel
import io.sentry.SentryOptions
import io.sentry.checkEvent
import io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor
import io.sentry.protocol.SentryTransaction
import io.sentry.protocol.User
import io.sentry.spring.jakarta.ContextTagsEventProcessor
Expand Down Expand Up @@ -683,6 +684,47 @@ class SentryAutoConfigurationTest {
}
}

@Test
fun `when OpenTelemetryLinkErrorEventProcessor is on the classpath and auto init off, creates OpenTelemetryLinkErrorEventProcessor`() {
lbloder marked this conversation as resolved.
Show resolved Hide resolved
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.auto-init=false")
.run {
assertThat(it).hasSingleBean(OpenTelemetryLinkErrorEventProcessor::class.java)
val options = it.getBean(SentryOptions::class.java)
assertThat(options.eventProcessors).anyMatch { processor -> processor.javaClass == OpenTelemetryLinkErrorEventProcessor::class.java }
}
}

@Test
fun `when OpenTelemetryLinkErrorEventProcessor is on the classpath but auto init on, does not create OpenTelemetryLinkErrorEventProcessor`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.auto-init=true")
.run {
assertThat(it).doesNotHaveBean(OpenTelemetryLinkErrorEventProcessor::class.java)
val options = it.getBean(SentryOptions::class.java)
assertThat(options.eventProcessors).noneMatch { processor -> processor.javaClass == OpenTelemetryLinkErrorEventProcessor::class.java }
}
}

@Test
fun `when OpenTelemetryLinkErrorEventProcessor is on the classpath but auto init default, does not create OpenTelemetryLinkErrorEventProcessor`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj")
.run {
assertThat(it).doesNotHaveBean(OpenTelemetryLinkErrorEventProcessor::class.java)
val options = it.getBean(SentryOptions::class.java)
assertThat(options.eventProcessors).noneMatch { processor -> processor.javaClass == OpenTelemetryLinkErrorEventProcessor::class.java }
}
}

@Test
fun `when OpenTelemetryLinkErrorEventProcessor is not on the classpath, does not create OpenTelemetryLinkErrorEventProcessor`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.auto-init=false")
.withClassLoader(FilteredClassLoader(OpenTelemetryLinkErrorEventProcessor::class.java))
.run {
assertThat(it).doesNotHaveBean(OpenTelemetryLinkErrorEventProcessor::class.java)
val options = it.getBean(SentryOptions::class.java)
assertThat(options.eventProcessors).noneMatch { processor -> processor.javaClass == OpenTelemetryLinkErrorEventProcessor::class.java }
}
}

@Configuration(proxyBeanMethods = false)
open class CustomOptionsConfigurationConfiguration {

Expand Down
2 changes: 2 additions & 0 deletions sentry-spring-boot-starter/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ dependencies {
compileOnly(Config.Libs.springBootStarterAop)
compileOnly(Config.Libs.springBootStarterSecurity)
compileOnly(Config.Libs.reactorCore)
compileOnly(projects.sentryOpentelemetry.sentryOpentelemetryCore)

annotationProcessor(Config.AnnotationProcessors.springBootAutoConfigure)
annotationProcessor(Config.AnnotationProcessors.springBootConfiguration)
Expand All @@ -67,6 +68,7 @@ dependencies {
testImplementation(Config.Libs.springBootStarterWebflux)
testImplementation(Config.Libs.springBootStarterSecurity)
testImplementation(Config.Libs.springBootStarterAop)
testImplementation(projects.sentryOpentelemetry.sentryOpentelemetryCore)
}

configure<SourceSetContainer> {
Expand Down
14 changes: 14 additions & 0 deletions sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import io.sentry.Integration;
import io.sentry.Sentry;
import io.sentry.SentryOptions;
import io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor;
import io.sentry.protocol.SdkVersion;
import io.sentry.spring.ContextTagsEventProcessor;
import io.sentry.spring.SentryExceptionResolver;
Expand Down Expand Up @@ -139,6 +140,19 @@ static class ContextTagsEventProcessorConfiguration {
}
}

@Configuration(proxyBeanMethods = false)
@ConditionalOnProperty(name = "sentry.auto-init", havingValue = "false")
Copy link
Member Author

Choose a reason for hiding this comment

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

When not using the agent this will not trigger. At the moment it feels like a niche use case to not use the agent JAR and there doesn't seem to be an easy way of detecting whether the agent is in use. We could add some class that serves as a marker (to the bootstrap classloader) or manipulate the options (byte code manipulation) from the agent but decided not to do that for now. If this becomes a more common use case we can revisit.

Copy link
Member Author

Choose a reason for hiding this comment

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

  • 💡 could also look at the classloader of OpenTelemetryLinkErrorEventProcessor. If it's the bootstrap classloader the agent is in use. Not sure yet how to turn that into an @Conditional for spring config.

Copy link
Member Author

@adinauer adinauer Dec 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

  • 💡 probably even easier to just have SentryOptions injected into the @Bean method and not create the bean if a OpenTelemetryLinkErrorEventProcessor has already been registered. Also not sure how to add that to the auto config yet.

@ConditionalOnClass(OpenTelemetryLinkErrorEventProcessor.class)
@Open
static class OpenTelemetryLinkErrorEventProcessorConfiguration {

@Bean
@ConditionalOnMissingBean
public @NotNull OpenTelemetryLinkErrorEventProcessor openTelemetryLinkErrorEventProcessor() {
return new OpenTelemetryLinkErrorEventProcessor();
}
}

/** Registers beans specific to Spring MVC. */
@Configuration(proxyBeanMethods = false)
@ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET)
Expand Down
42 changes: 42 additions & 0 deletions ...-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import io.sentry.SentryEvent
import io.sentry.SentryLevel
import io.sentry.SentryOptions
import io.sentry.checkEvent
import io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor
import io.sentry.protocol.SentryTransaction
import io.sentry.protocol.User
import io.sentry.spring.ContextTagsEventProcessor
Expand Down Expand Up @@ -683,6 +684,47 @@ class SentryAutoConfigurationTest {
}
}

@Test
fun `when OpenTelemetryLinkErrorEventProcessor is on the classpath and auto init off, creates OpenTelemetryLinkErrorEventProcessor`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.auto-init=false")
.run {
assertThat(it).hasSingleBean(OpenTelemetryLinkErrorEventProcessor::class.java)
val options = it.getBean(SentryOptions::class.java)
assertThat(options.eventProcessors).anyMatch { processor -> processor.javaClass == OpenTelemetryLinkErrorEventProcessor::class.java }
}
}

@Test
fun `when OpenTelemetryLinkErrorEventProcessor is on the classpath but auto init on, does not create OpenTelemetryLinkErrorEventProcessor`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.auto-init=true")
.run {
assertThat(it).doesNotHaveBean(OpenTelemetryLinkErrorEventProcessor::class.java)
val options = it.getBean(SentryOptions::class.java)
assertThat(options.eventProcessors).noneMatch { processor -> processor.javaClass == OpenTelemetryLinkErrorEventProcessor::class.java }
}
}

@Test
fun `when OpenTelemetryLinkErrorEventProcessor is on the classpath but auto init default, does not create OpenTelemetryLinkErrorEventProcessor`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj")
.run {
assertThat(it).doesNotHaveBean(OpenTelemetryLinkErrorEventProcessor::class.java)
val options = it.getBean(SentryOptions::class.java)
assertThat(options.eventProcessors).noneMatch { processor -> processor.javaClass == OpenTelemetryLinkErrorEventProcessor::class.java }
}
}

@Test
fun `when OpenTelemetryLinkErrorEventProcessor is not on the classpath, does not create OpenTelemetryLinkErrorEventProcessor`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.auto-init=false")
.withClassLoader(FilteredClassLoader(OpenTelemetryLinkErrorEventProcessor::class.java))
.run {
assertThat(it).doesNotHaveBean(OpenTelemetryLinkErrorEventProcessor::class.java)
val options = it.getBean(SentryOptions::class.java)
assertThat(options.eventProcessors).noneMatch { processor -> processor.javaClass == OpenTelemetryLinkErrorEventProcessor::class.java }
}
}

@Configuration(proxyBeanMethods = false)
open class CustomOptionsConfigurationConfiguration {

Expand Down