POTEL 53 - Automatically set span factory based on presence of OpenTelemetry (#3858)

adinauer · web-flow · commit b795cb4da8af · 2024-11-11T16:38:40.000+01:00
* Auto config for Spring Boot combined with OTel but without agent

* try to cleanup otel classloader

* make agent, no agent and agent without auto init work for spring boot

* Fix ignored instrumentation for OTel without agent; separate sample for no agent

* fix test result upload on CI

* automatically detect otel and use OtelSpanFactory

* changelog
diff --git a/.github/workflows/system-tests-backend.yml b/.github/workflows/system-tests-backend.yml
@@ -92,7 +92,7 @@ jobs:
 
       - name: Start server and run integration test for sentry-cli commands
         run: |
-          test/system-test-sentry-server-start.sh > sentry-mock-server.txt 2>&1 & test/system-test-spring-server-start.sh "${{ matrix.sample }}" "${{ matrix.agent }}" "${{ matrix.agent-auto-init }}" > spring-server.txt 2>&1 & test/wait-for-spring.sh && ./gradlew :sentry-samples:${{ matrix.sample }}:systemTest
+          test/system-test-run.sh "${{ matrix.sample }}" "${{ matrix.agent }}" "${{ matrix.agent-auto-init }}"
 
       - name: Upload test results
         if: always()
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,8 @@
   - The `sentry-opentelemetry-extra` module has been removed again, most classes have been moved to `sentry-opentelemetry-bootstrap` which is loaded into the bootstrap classloader (i.e. `null`) when our Java agent is used. The rest has been moved into `sentry-opentelemetry-agentcustomization` and is loaded into the agent classloader when our Java agent is used.
   - The `sentry-opentelemetry-bootstrap` and `sentry-opentelemetry-agentcustomization` modules can be used without the agent as well, in which case all classes are loaded into the application classloader. Check out our `sentry-samples-spring-boot-jakarta-opentelemetry-noagent` sample.
   - In this mode the SDK makes use of `GlobalOpenTelemetry`
+- Automatically set span factory based on presence of OpenTelemetry ([#3858](https://github.com/getsentry/sentry-java/pull/3858))
+  - `SentrySpanFactoryHolder` has been removed as it is no longer required.
 - Add a sample for showcasing Sentry with OpenTelemetry for Spring Boot 3 with our Java agent (`sentry-samples-spring-boot-jakarta-opentelemetry`) ([#3856](https://github.com/getsentry/sentry-java/pull/3828))
 - Add a sample for showcasing Sentry with OpenTelemetry for Spring Boot 3 without our Java agent (`sentry-samples-spring-boot-jakarta-opentelemetry-noagent`) ([#3856](https://github.com/getsentry/sentry-java/pull/3856))
 - Add `globalHubMode` to options ([#3805](https://github.com/getsentry/sentry-java/pull/3805))
diff --git a/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/src/main/java/io/sentry/opentelemetry/SentryAutoConfigurationCustomizerProvider.java b/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/src/main/java/io/sentry/opentelemetry/SentryAutoConfigurationCustomizerProvider.java
@@ -9,7 +9,6 @@
 import io.sentry.Sentry;
 import io.sentry.SentryIntegrationPackageStorage;
 import io.sentry.SentryOptions;
-import io.sentry.SentrySpanFactoryHolder;
 import io.sentry.protocol.SdkVersion;
 import io.sentry.protocol.SentryPackage;
 import java.io.IOException;
@@ -34,9 +33,6 @@ public void customize(AutoConfigurationCustomizer autoConfiguration) {
     ensureSentryOtelStorageIsInitialized();
     final @Nullable VersionInfoHolder versionInfoHolder = createVersionInfo();
 
-    final @NotNull OtelSpanFactory spanFactory = new OtelSpanFactory();
-    SentrySpanFactoryHolder.setSpanFactory(spanFactory);
-
     if (isSentryAutoInitEnabled()) {
       Sentry.init(
           options -> {
diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OpenTelemetryUtil.java b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OpenTelemetryUtil.java
@@ -1,7 +1,6 @@
 package io.sentry.opentelemetry;
 
 import io.sentry.SentryOptions;
-import io.sentry.SentrySpanFactoryHolder;
 import io.sentry.util.SpanUtils;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.Nullable;
@@ -12,7 +11,6 @@ public final class OpenTelemetryUtil {
   public static void applyOpenTelemetryOptions(
       final @Nullable SentryOptions options, final boolean isAgent) {
     if (options != null) {
-      options.setSpanFactory(SentrySpanFactoryHolder.getSpanFactory());
       options.setIgnoredSpanOrigins(SpanUtils.ignoredSpanOriginsForOpenTelemetry(isAgent));
     }
   }
diff --git a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/it/SentrySpringIntegrationTest.kt b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/it/SentrySpringIntegrationTest.kt
@@ -1,8 +1,10 @@
 package io.sentry.spring.boot.jakarta.it
 
+import io.sentry.DefaultSpanFactory
 import io.sentry.IScopes
 import io.sentry.ITransportFactory
 import io.sentry.Sentry
+import io.sentry.SentryOptions
 import io.sentry.checkEvent
 import io.sentry.checkTransaction
 import io.sentry.spring.jakarta.tracing.SentrySpan
@@ -223,6 +225,12 @@ open class App {
 
     @Bean
     open fun mockTransport() = transport
+
+    @Bean
+    open fun optionsCallback() = Sentry.OptionsConfiguration<SentryOptions> { options ->
+        // due to OTel being on the classpath we need to set the default again
+        options.spanFactory = DefaultSpanFactory()
+    }
 }
 
 @RestController
diff --git a/sentry-spring-jakarta/api/sentry-spring-jakarta.api b/sentry-spring-jakarta/api/sentry-spring-jakarta.api
@@ -204,7 +204,6 @@ public final class io/sentry/spring/jakarta/graphql/SentrySpringSubscriptionHand
 
 public class io/sentry/spring/jakarta/opentelemetry/SentryOpenTelemetryAgentWithoutAutoInitConfiguration {
 	public fun <init> ()V
-	public static fun openTelemetrySpanFactory ()Lio/sentry/ISpanFactory;
 	public fun sentryOpenTelemetryOptionsConfiguration ()Lio/sentry/Sentry$OptionsConfiguration;
 }
 
diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/opentelemetry/SentryOpenTelemetryAgentWithoutAutoInitConfiguration.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/opentelemetry/SentryOpenTelemetryAgentWithoutAutoInitConfiguration.java
@@ -1,12 +1,10 @@
 package io.sentry.spring.jakarta.opentelemetry;
 
 import com.jakewharton.nopen.annotation.Open;
-import io.sentry.ISpanFactory;
 import io.sentry.Sentry;
 import io.sentry.SentryIntegrationPackageStorage;
 import io.sentry.SentryOptions;
 import io.sentry.opentelemetry.OpenTelemetryUtil;
-import io.sentry.opentelemetry.OtelSpanFactory;
 import org.jetbrains.annotations.NotNull;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.context.annotation.Bean;
@@ -16,12 +14,6 @@
 @Open
 public class SentryOpenTelemetryAgentWithoutAutoInitConfiguration {
 
-  @Bean
-  @ConditionalOnMissingBean
-  public static ISpanFactory openTelemetrySpanFactory() {
-    return new OtelSpanFactory();
-  }
-
   @Bean
   @ConditionalOnMissingBean(name = "sentryOpenTelemetryOptionsConfiguration")
   public @NotNull Sentry.OptionsConfiguration<SentryOptions>
diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api
@@ -3175,12 +3175,6 @@ public final class io/sentry/SentryReplayOptions$SentryReplayQuality : java/lang
 	public static fun values ()[Lio/sentry/SentryReplayOptions$SentryReplayQuality;
 }
 
-public final class io/sentry/SentrySpanFactoryHolder {
-	public fun <init> ()V
-	public static fun getSpanFactory ()Lio/sentry/ISpanFactory;
-	public static fun setSpanFactory (Lio/sentry/ISpanFactory;)V
-}
-
 public final class io/sentry/SentrySpanStorage {
 	public fun get (Ljava/lang/String;)Lio/sentry/ISpan;
 	public static fun getInstance ()Lio/sentry/SentrySpanStorage;
@@ -3486,6 +3480,11 @@ public abstract interface class io/sentry/SpanDataConvention {
 	public static final field THREAD_NAME Ljava/lang/String;
 }
 
+public final class io/sentry/SpanFactoryFactory {
+	public fun <init> ()V
+	public static fun create (Lio/sentry/util/LoadClass;Lio/sentry/ILogger;)Lio/sentry/ISpanFactory;
+}
+
 public abstract interface class io/sentry/SpanFinishedCallback {
 	public abstract fun execute (Lio/sentry/Span;)V
 }
diff --git a/sentry/src/main/java/io/sentry/ScopesStorageFactory.java b/sentry/src/main/java/io/sentry/ScopesStorageFactory.java
@@ -1,6 +1,7 @@
 package io.sentry;
 
 import io.sentry.util.LoadClass;
+import io.sentry.util.Platform;
 import java.lang.reflect.InvocationTargetException;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
@@ -21,23 +22,25 @@ public final class ScopesStorageFactory {
 
   private static @NotNull IScopesStorage createInternal(
       final @NotNull LoadClass loadClass, final @NotNull ILogger logger) {
-    if (loadClass.isClassAvailable(OTEL_SCOPES_STORAGE, logger)) {
-      Class<?> otelScopesStorageClazz = loadClass.loadClass(OTEL_SCOPES_STORAGE, logger);
-      if (otelScopesStorageClazz != null) {
-        try {
-          final @Nullable Object otelScopesStorage =
-              otelScopesStorageClazz.getDeclaredConstructor().newInstance();
-          if (otelScopesStorage != null && otelScopesStorage instanceof IScopesStorage) {
-            return (IScopesStorage) otelScopesStorage;
+    if (Platform.isJvm()) {
+      if (loadClass.isClassAvailable(OTEL_SCOPES_STORAGE, logger)) {
+        Class<?> otelScopesStorageClazz = loadClass.loadClass(OTEL_SCOPES_STORAGE, logger);
+        if (otelScopesStorageClazz != null) {
+          try {
+            final @Nullable Object otelScopesStorage =
+                otelScopesStorageClazz.getDeclaredConstructor().newInstance();
+            if (otelScopesStorage != null && otelScopesStorage instanceof IScopesStorage) {
+              return (IScopesStorage) otelScopesStorage;
+            }
+          } catch (InstantiationException e) {
+            // TODO log
+          } catch (IllegalAccessException e) {
+            // TODO log
+          } catch (InvocationTargetException e) {
+            // TODO log
+          } catch (NoSuchMethodException e) {
+            // TODO log
           }
-        } catch (InstantiationException e) {
-          // TODO log
-        } catch (IllegalAccessException e) {
-          // TODO log
-        } catch (InvocationTargetException e) {
-          // TODO log
-        } catch (NoSuchMethodException e) {
-          // TODO log
         }
       }
     }
diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java
@@ -21,6 +21,7 @@
 import io.sentry.transport.NoOpTransportGate;
 import io.sentry.util.AutoClosableReentrantLock;
 import io.sentry.util.LazyEvaluator;
+import io.sentry.util.LoadClass;
 import io.sentry.util.Platform;
 import io.sentry.util.SampleRateUtils;
 import io.sentry.util.StringUtils;
@@ -2543,7 +2544,7 @@ public SentryOptions() {
   private SentryOptions(final boolean empty) {
     experimental = new ExperimentalOptions(empty);
     if (!empty) {
-      setSpanFactory(new DefaultSpanFactory());
+      setSpanFactory(SpanFactoryFactory.create(new LoadClass(), NoOpLogger.getInstance()));
       // SentryExecutorService should be initialized before any
       // SendCachedEventFireAndForgetIntegration
       executorService = new SentryExecutorService();
diff --git a/sentry/src/main/java/io/sentry/SentrySpanFactoryHolder.java b/sentry/src/main/java/io/sentry/SentrySpanFactoryHolder.java
diff --git a/sentry/src/main/java/io/sentry/SpanFactoryFactory.java b/sentry/src/main/java/io/sentry/SpanFactoryFactory.java
@@ -0,0 +1,42 @@
+package io.sentry;
+
+import io.sentry.util.LoadClass;
+import io.sentry.util.Platform;
+import java.lang.reflect.InvocationTargetException;
+import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+@ApiStatus.Internal
+public final class SpanFactoryFactory {
+
+  private static final String OTEL_SPAN_FACTORY = "io.sentry.opentelemetry.OtelSpanFactory";
+
+  public static @NotNull ISpanFactory create(
+      final @NotNull LoadClass loadClass, final @NotNull ILogger logger) {
+    if (Platform.isJvm()) {
+      if (loadClass.isClassAvailable(OTEL_SPAN_FACTORY, logger)) {
+        Class<?> otelSpanFactoryClazz = loadClass.loadClass(OTEL_SPAN_FACTORY, logger);
+        if (otelSpanFactoryClazz != null) {
+          try {
+            final @Nullable Object otelSpanFactory =
+                otelSpanFactoryClazz.getDeclaredConstructor().newInstance();
+            if (otelSpanFactory != null && otelSpanFactory instanceof ISpanFactory) {
+              return (ISpanFactory) otelSpanFactory;
+            }
+          } catch (InstantiationException e) {
+            // TODO log
+          } catch (IllegalAccessException e) {
+            // TODO log
+          } catch (InvocationTargetException e) {
+            // TODO log
+          } catch (NoSuchMethodException e) {
+            // TODO log
+          }
+        }
+      }
+    }
+
+    return new DefaultSpanFactory();
+  }
+}
diff --git a/test/system-test-run-all.sh b/test/system-test-run-all.sh
@@ -0,0 +1,8 @@
+#!/usr/bin/env bash
+
+./test/system-test-local-run.sh "sentry-samples-spring-boot" "0" "true"
+./test/system-test-run.sh "sentry-samples-spring-boot-webflux-jakarta" "0" "true"
+./test/system-test-run.sh "sentry-samples-spring-boot-webflux" "0" "true"
+./test/system-test-run.sh "sentry-samples-spring-boot-jakarta-opentelemetry-noagent" "0" "true"
+./test/system-test-run.sh "sentry-samples-spring-boot-jakarta-opentelemetry" "1" "true"
+./test/system-test-run.sh "sentry-samples-spring-boot-jakarta-opentelemetry" "1" "false"
diff --git a/test/system-test-run.sh b/test/system-test-run.sh
@@ -0,0 +1,22 @@
+#!/usr/bin/env bash
+
+readonly SAMPLE_MODULE=$1
+readonly JAVA_AGENT=$2
+readonly JAVA_AGENT_AUTO_INIT=$3
+
+test/system-test-sentry-server-start.sh
+MOCK_SERVER_PID=$(cat sentry-mock-server.pid)
+echo "started mock server ${SAMPLE_MODULE}-${JAVA_AGENT}-${JAVA_AGENT_AUTO_INIT} with PID ${MOCK_SERVER_PID}"
+
+test/system-test-spring-server-start.sh "${SAMPLE_MODULE}" "${JAVA_AGENT}" "${JAVA_AGENT_AUTO_INIT}"
+SUT_PID=$(cat spring-server.pid)
+echo "started spring server ${SAMPLE_MODULE}-${JAVA_AGENT}-${JAVA_AGENT_AUTO_INIT} with PID ${SUT_PID}"
+
+test/wait-for-spring.sh
+
+./gradlew :sentry-samples:${SAMPLE_MODULE}:systemTest
+
+echo "killing mock server ${SAMPLE_MODULE}-${JAVA_AGENT}-${JAVA_AGENT_AUTO_INIT} with PID ${MOCK_SERVER_PID}"
+kill $SUT_PID
+echo "killing spring server ${SAMPLE_MODULE}-${JAVA_AGENT}-${JAVA_AGENT_AUTO_INIT} with PID ${SUT_PID}"
+kill $MOCK_SERVER_PID
diff --git a/test/system-test-sentry-server-start.sh b/test/system-test-sentry-server-start.sh
@@ -1,3 +1,4 @@
 #!/usr/bin/env bash
 
-python3 test/system-test-sentry-server.py
+python3 test/system-test-sentry-server.py > sentry-mock-server.txt 2>&1 &
+echo $! > sentry-mock-server.pid
diff --git a/test/system-test-spring-server-start.sh b/test/system-test-spring-server-start.sh
@@ -15,4 +15,5 @@ fi
 
 echo "$JAVA_AGENT_STRING"
 
-SENTRY_DSN="http://502f25099c204a2fbf4cb16edc5975d1@localhost:8000/0" SENTRY_AUTO_INIT=${JAVA_AGENT_AUTO_INIT} SENTRY_TRACES_SAMPLE_RATE=1.0 OTEL_TRACES_EXPORTER=none OTEL_METRICS_EXPORTER=none OTEL_LOGS_EXPORTER=none java ${JAVA_AGENT_STRING} -jar sentry-samples/${SAMPLE_MODULE}/build/libs/${SAMPLE_MODULE}-0.0.1-SNAPSHOT.jar
+SENTRY_DSN="http://502f25099c204a2fbf4cb16edc5975d1@localhost:8000/0" SENTRY_AUTO_INIT=${JAVA_AGENT_AUTO_INIT} SENTRY_TRACES_SAMPLE_RATE=1.0 OTEL_TRACES_EXPORTER=none OTEL_METRICS_EXPORTER=none OTEL_LOGS_EXPORTER=none java ${JAVA_AGENT_STRING} -jar sentry-samples/${SAMPLE_MODULE}/build/libs/${SAMPLE_MODULE}-0.0.1-SNAPSHOT.jar > spring-server.txt 2>&1 &
+echo $! > spring-server.pid

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,6 @@`
`1`	`1`	`package io.sentry.opentelemetry;`
`2`	`2`
`3`	`3`	`import io.sentry.SentryOptions;`
`4`		`-import io.sentry.SentrySpanFactoryHolder;`
`5`	`4`	`import io.sentry.util.SpanUtils;`
`6`	`5`	`import org.jetbrains.annotations.ApiStatus;`
`7`	`6`	`import org.jetbrains.annotations.Nullable;`
`@@ -12,7 +11,6 @@ public final class OpenTelemetryUtil {`
`12`	`11`	`public static void applyOpenTelemetryOptions(`
`13`	`12`	`final @Nullable SentryOptions options, final boolean isAgent) {`
`14`	`13`	`if (options != null) {`
`15`		`- options.setSpanFactory(SentrySpanFactoryHolder.getSpanFactory());`
`16`	`14`	`options.setIgnoredSpanOrigins(SpanUtils.ignoredSpanOriginsForOpenTelemetry(isAgent));`
`17`	`15`	`}`
`18`	`16`	`}`
Original file line number	Diff line number	Diff line change
`@@ -204,7 +204,6 @@ public final class io/sentry/spring/jakarta/graphql/SentrySpringSubscriptionHand`
`204`	`204`
`205`	`205`	`public class io/sentry/spring/jakarta/opentelemetry/SentryOpenTelemetryAgentWithoutAutoInitConfiguration {`
`206`	`206`	`public fun <init> ()V`
`207`		`- public static fun openTelemetrySpanFactory ()Lio/sentry/ISpanFactory;`
`208`	`207`	`public fun sentryOpenTelemetryOptionsConfiguration ()Lio/sentry/Sentry$OptionsConfiguration;`
`209`	`208`	`}`
`210`	`209`