POTEL 19 - Review feedback

sentry-opentelemetry/sentry-opentelemetry-agentcustomization/src/main/java/io/sentry/opentelemetry/SentryAutoConfigurationCustomizerProvider.java

@@ -6,7 +6,6 @@
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import io.opentelemetry.sdk.trace.SdkTracerProviderBuilder;
 import io.opentelemetry.sdk.trace.export.BatchSpanProcessor;
-import io.sentry.Instrumenter;
 import io.sentry.Sentry;
 import io.sentry.SentryIntegrationPackageStorage;
 import io.sentry.SentryOptions;
@@ -38,10 +37,6 @@ public void customize(AutoConfigurationCustomizer autoConfiguration) {
       Sentry.init(
           options -> {
             options.setEnableExternalConfiguration(true);
-            // TODO [POTEL] deprecate
-            options.setInstrumenter(Instrumenter.OTEL);
-            // TODO [POTEL] do we still need this?
-            options.addEventProcessor(new OpenTelemetryLinkErrorEventProcessor());
             options.setIgnoredSpanOrigins(SpanUtils.ignoredSpanOriginsForOpenTelemetry());
             options.setSpanFactory(new OtelSpanFactory());
             final @Nullable SdkVersion sdkVersion = createSdkVersion(options, versionInfoHolder);
@@ -153,8 +148,6 @@ private static class VersionInfoHolder {
 
   private SdkTracerProviderBuilder configureSdkTracerProvider(
       SdkTracerProviderBuilder tracerProvider, ConfigProperties config) {
-    // TODO [POTEL] configurable or separate packages for old vs new way
-    //    return tracerProvider.addSpanProcessor(new SentrySpanProcessor());
     return tracerProvider
         .setSampler(new SentrySampler())
         .addSpanProcessor(new PotelSentrySpanProcessor())

sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api

@@ -42,7 +42,6 @@ public final class io/sentry/opentelemetry/OtelSpanWrapper : io/sentry/ISpan {
 	public fun getData ()Ljava/util/Map;
 	public fun getData (Ljava/lang/String;)Ljava/lang/Object;
 	public fun getDescription ()Ljava/lang/String;
-	public fun getEventId ()Lio/sentry/protocol/SentryId;
 	public fun getFinishDate ()Lio/sentry/SentryDate;
 	public fun getLocalMetricsAggregator ()Lio/sentry/metrics/LocalMetricsAggregator;
 	public fun getMeasurements ()Ljava/util/Map;

sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelContextScopesStorage.java

@@ -14,7 +14,7 @@
 public final class OtelContextScopesStorage implements IScopesStorage {
 
   @Override
-  public ISentryLifecycleToken set(@Nullable IScopes scopes) {
+  public @NotNull ISentryLifecycleToken set(@Nullable IScopes scopes) {
     final @NotNull Scope otelScope =
         Context.current().with(SENTRY_SCOPES_KEY, scopes).makeCurrent();
     return new OtelStorageToken(otelScope);

sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelSpanWrapper.java

@@ -476,12 +476,6 @@ public Map<String, MeasurementValue> getMeasurements() {
     return context.getSamplingDecision();
   }
 
-  @Override
-  public @NotNull SentryId getEventId() {
-    // TODO [POTEL]
-    return new SentryId(getOtelSpanId().toString());
-  }
-
   @ApiStatus.Internal
   public @NotNull IScopes getScopes() {
     return scopes;

sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelTransactionSpanForwarder.java

@@ -257,7 +257,8 @@ public boolean isNoOp() {
 
   @Override
   public @NotNull SentryId getEventId() {
-    return rootSpan.getEventId();
+    // TODO [POTEL]
+    return new SentryId();
   }
 
   @ApiStatus.Internal

sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryWeakSpanStorage.java

@@ -7,14 +7,8 @@
 import org.jetbrains.annotations.Nullable;
 
 /**
- * This class may have to be moved to a new gradle module to include it in the bootstrap
- * classloader.
- *
- * <p>This uses multiple maps instead of a single one with a wrapper object as porting this to
- * Android would mean there's no access to methods like compute etc. before API level 24. There's
- * also no easy way to pre-initialize the map for all keys as spans are used as keys. For span IDs
- * it would also not work as they are random. For client report storage we know beforehand what keys
- * can exist.
+ * Weakly references wrappers for OpenTelemetry spans meaning they'll be cleaned up when the
+ * OpenTelemetry span is garbage collected.
  */
 @ApiStatus.Internal
 public final class SentryWeakSpanStorage {

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryLinkErrorEventProcessor.java

@@ -11,17 +11,23 @@
 import io.sentry.ScopesAdapter;
 import io.sentry.SentryEvent;
 import io.sentry.SentryLevel;
-import io.sentry.SentrySpanStorage;
 import io.sentry.SpanContext;
 import io.sentry.protocol.SentryId;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.TestOnly;
 
+/**
+ * @deprecated this is no longer needed for the latest version of our OpenTelemetry integration.
+ */
+@Deprecated
 public final class OpenTelemetryLinkErrorEventProcessor implements EventProcessor {
 
   private final @NotNull IScopes scopes;
-  private final @NotNull SentrySpanStorage spanStorage = SentrySpanStorage.getInstance();
+
+  @SuppressWarnings("deprecation")
+  private final @NotNull io.sentry.SentrySpanStorage spanStorage =
+      io.sentry.SentrySpanStorage.getInstance();
 
   public OpenTelemetryLinkErrorEventProcessor() {
     this(ScopesAdapter.getInstance());

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSamplingUtil.java

@@ -9,7 +9,7 @@
 @ApiStatus.Internal
 public final class OtelSamplingUtil {
 
-  public static @Nullable TracesSamplingDecision extractSamplingDecisionOrDefault(
+  public static @NotNull TracesSamplingDecision extractSamplingDecisionOrDefault(
       final @NotNull Attributes attributes) {
     final @Nullable TracesSamplingDecision decision = extractSamplingDecision(attributes);
     if (decision != null) {

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/PotelSentrySpanProcessor.java

@@ -80,9 +80,10 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri
         }
       }
 
-      // TODO [POTEL] what do we use as fallback here? could happen if misconfigured (i.e. sampler
-      // not in place)
-      final boolean sampled = samplingDecision != null ? samplingDecision.getSampled() : true;
+      final boolean sampled =
+          samplingDecision != null
+              ? samplingDecision.getSampled()
+              : otelSpan.getSpanContext().isSampled();
 
       final @NotNull PropagationContext propagationContext =
           sentryTraceHeader == null
@@ -128,7 +129,6 @@ public void onEnd(final @NotNull ReadableSpan spanBeingEnded) {
           new SentryLongDate(spanBeingEnded.toSpanData().getEndEpochNanos());
       sentrySpan.updateEndDate(finishDate);
     }
-    System.out.println("span ended: " + spanBeingEnded.getSpanContext().getSpanId());
   }
 
   @Override

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentryPropagator.java

@@ -14,7 +14,6 @@
 import io.sentry.ISpan;
 import io.sentry.ScopesAdapter;
 import io.sentry.SentryLevel;
-import io.sentry.SentrySpanStorage;
 import io.sentry.SentryTraceHeader;
 import io.sentry.exception.InvalidSentryTraceHeaderException;
 import java.util.Arrays;
@@ -24,11 +23,19 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+/**
+ * @deprecated please use {@link PotelSentryPropagator} instead
+ */
+@Deprecated
 public final class SentryPropagator implements TextMapPropagator {
 
   private static final @NotNull List<String> FIELDS =
       Arrays.asList(SentryTraceHeader.SENTRY_TRACE_HEADER, BaggageHeader.BAGGAGE_HEADER);
-  private final @NotNull SentrySpanStorage spanStorage = SentrySpanStorage.getInstance();
+
+  @SuppressWarnings("deprecation")
+  private final @NotNull io.sentry.SentrySpanStorage spanStorage =
+      io.sentry.SentrySpanStorage.getInstance();
+
   private final @NotNull IScopes scopes;
 
   public SentryPropagator() {

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySampler.java

@@ -6,15 +6,16 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.sdk.trace.data.LinkData;
 import io.opentelemetry.sdk.trace.samplers.Sampler;
+import io.opentelemetry.sdk.trace.samplers.SamplingDecision;
 import io.opentelemetry.sdk.trace.samplers.SamplingResult;
 import io.sentry.Baggage;
 import io.sentry.IScopes;
 import io.sentry.PropagationContext;
 import io.sentry.SamplingContext;
 import io.sentry.ScopesAdapter;
+import io.sentry.SentryOptions;
 import io.sentry.SentryTraceHeader;
 import io.sentry.SpanId;
-import io.sentry.TracesSampler;
 import io.sentry.TracesSamplingDecision;
 import io.sentry.TransactionContext;
 import io.sentry.protocol.SentryId;
@@ -25,10 +26,10 @@
 public final class SentrySampler implements Sampler {
 
   private final @NotNull SentryWeakSpanStorage spanStorage = SentryWeakSpanStorage.getInstance();
-  private final @NotNull TracesSampler tracesSampler;
+  private final @NotNull SentryOptions options;
 
   public SentrySampler(final @NotNull IScopes scopes) {
-    this.tracesSampler = scopes.getOptions().getInternalTracesSampler();
+    this.options = scopes.getOptions();
   }
 
   public SentrySampler() {
@@ -61,8 +62,11 @@ public SamplingResult shouldSample(
     }
   }
 
-  private @NotNull SentrySamplingResult handleRootOtelSpan(
+  private @NotNull SamplingResult handleRootOtelSpan(
       final @NotNull String traceId, final @NotNull Context parentContext) {
+    if (!options.isTraceSampling()) {
+      return SamplingResult.create(SamplingDecision.DROP);
+    }
     @Nullable Baggage baggage = null;
     @Nullable
     SentryTraceHeader sentryTraceHeader = parentContext.get(SentryOtelKeys.SENTRY_TRACE_KEY);
@@ -81,7 +85,7 @@ public SamplingResult shouldSample(
     final @NotNull TransactionContext transactionContext =
         TransactionContext.fromPropagationContext(propagationContext);
     final @NotNull TracesSamplingDecision sentryDecision =
-        tracesSampler.sample(new SamplingContext(transactionContext, null));
+        options.getInternalTracesSampler().sample(new SamplingContext(transactionContext, null));
     return new SentrySamplingResult(sentryDecision);
   }
 

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanExporter.java

@@ -157,21 +157,15 @@ private boolean isSentryRequest(final @NotNull SpanData spanData) {
     // TODO [POTEL] should check if enabled but multi init with different options makes testing hard
     // atm
     //    if (scopes.getOptions().isEnableSpotlight()) {
-    final @Nullable String spotlightUrl = scopes.getOptions().getSpotlightConnectionUrl();
-    if (spotlightUrl != null) {
-      if (containsSpotlightUrl(fullUrl, spotlightUrl)) {
-        return true;
-      }
-      if (containsSpotlightUrl(httpUrl, spotlightUrl)) {
-        return true;
-      }
-    } else {
-      if (containsSpotlightUrl(fullUrl, "http://localhost:8969/stream")) {
-        return true;
-      }
-      if (containsSpotlightUrl(httpUrl, "http://localhost:8969/stream")) {
-        return true;
-      }
+    final @Nullable String optionsSpotlightUrl = scopes.getOptions().getSpotlightConnectionUrl();
+    final @NotNull String spotlightUrl =
+        optionsSpotlightUrl != null ? optionsSpotlightUrl : "http://localhost:8969/stream";
+
+    if (containsSpotlightUrl(fullUrl, spotlightUrl)) {
+      return true;
+    }
+    if (containsSpotlightUrl(httpUrl, spotlightUrl)) {
+      return true;
     }
     //    }
 
@@ -344,7 +338,7 @@ private void transferSpanDetails(
             traceId);
     final SpanId sentrySpanId = new SpanId(spanId);
 
-    @NotNull String transactionName = spanInfo.getDescription();
+    @Nullable String transactionName = spanInfo.getDescription();
     @NotNull TransactionNameSource transactionNameSource = spanInfo.getTransactionNameSource();
     @Nullable SpanId parentSpanId = null;
     @Nullable Baggage baggage = null;

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanProcessor.java

@@ -24,7 +24,6 @@
 import io.sentry.SentryDate;
 import io.sentry.SentryLevel;
 import io.sentry.SentryLongDate;
-import io.sentry.SentrySpanStorage;
 import io.sentry.SentryTraceHeader;
 import io.sentry.SpanId;
 import io.sentry.SpanOptions;
@@ -40,6 +39,10 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+/**
+ * @deprecated please use {@link PotelSentrySpanProcessor} instead.
+ */
+@Deprecated
 public final class SentrySpanProcessor implements SpanProcessor {
 
   private static final String TRACE_ORIGN = "auto.otel";
@@ -48,7 +51,11 @@ public final class SentrySpanProcessor implements SpanProcessor {
       Arrays.asList(SpanKind.CLIENT, SpanKind.INTERNAL);
   private final @NotNull SpanDescriptionExtractor spanDescriptionExtractor =
       new SpanDescriptionExtractor();
-  private final @NotNull SentrySpanStorage spanStorage = SentrySpanStorage.getInstance();
+
+  @SuppressWarnings("deprecation")
+  private final @NotNull io.sentry.SentrySpanStorage spanStorage =
+      io.sentry.SentrySpanStorage.getInstance();
+
   private final @NotNull IScopes scopes;
 
   public SentrySpanProcessor() {

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/TraceData.java

@@ -6,6 +6,7 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+@Deprecated
 @ApiStatus.Internal
 public final class TraceData {
 

sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java

@@ -11,7 +11,6 @@
 import io.sentry.SentryIntegrationPackageStorage;
 import io.sentry.SentryOptions;
 import io.sentry.graphql.SentryGraphqlExceptionHandler;
-import io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor;
 import io.sentry.protocol.SdkVersion;
 import io.sentry.quartz.SentryJobListener;
 import io.sentry.spring.boot.jakarta.graphql.SentryGraphqlAutoConfiguration;
@@ -158,14 +157,16 @@ static class ContextTagsEventProcessorConfiguration {
 
     @Configuration(proxyBeanMethods = false)
     @ConditionalOnProperty(name = "sentry.auto-init", havingValue = "false")
-    @ConditionalOnClass(OpenTelemetryLinkErrorEventProcessor.class)
+    @ConditionalOnClass(io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor.class)
+    @SuppressWarnings("deprecation")
     @Open
     static class OpenTelemetryLinkErrorEventProcessorConfiguration {
 
       @Bean
       @ConditionalOnMissingBean
-      public @NotNull OpenTelemetryLinkErrorEventProcessor openTelemetryLinkErrorEventProcessor() {
-        return new OpenTelemetryLinkErrorEventProcessor();
+      public @NotNull io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor
+          openTelemetryLinkErrorEventProcessor() {
+        return new io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor();
       }
     }
 

sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java

@@ -11,7 +11,6 @@
 import io.sentry.SentryIntegrationPackageStorage;
 import io.sentry.SentryOptions;
 import io.sentry.graphql.SentryGraphqlExceptionHandler;
-import io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor;
 import io.sentry.protocol.SdkVersion;
 import io.sentry.quartz.SentryJobListener;
 import io.sentry.spring.ContextTagsEventProcessor;
@@ -156,14 +155,16 @@ static class ContextTagsEventProcessorConfiguration {
 
     @Configuration(proxyBeanMethods = false)
     @ConditionalOnProperty(name = "sentry.auto-init", havingValue = "false")
-    @ConditionalOnClass(OpenTelemetryLinkErrorEventProcessor.class)
+    @ConditionalOnClass(io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor.class)
+    @SuppressWarnings("deprecation")
     @Open
     static class OpenTelemetryLinkErrorEventProcessorConfiguration {
 
       @Bean
       @ConditionalOnMissingBean
-      public @NotNull OpenTelemetryLinkErrorEventProcessor openTelemetryLinkErrorEventProcessor() {
-        return new OpenTelemetryLinkErrorEventProcessor();
+      public @NotNull io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor
+          openTelemetryLinkErrorEventProcessor() {
+        return new io.sentry.opentelemetry.OpenTelemetryLinkErrorEventProcessor();
       }
     }
 

sentry/api/sentry.api

@@ -998,6 +998,7 @@ public abstract interface class io/sentry/ISpanFactory {
 public abstract interface class io/sentry/ITransaction : io/sentry/ISpan {
 	public abstract fun finish (Lio/sentry/SpanStatus;Lio/sentry/SentryDate;ZLio/sentry/Hint;)V
 	public abstract fun forceFinish (Lio/sentry/SpanStatus;ZLio/sentry/Hint;)V
+	public abstract fun getEventId ()Lio/sentry/protocol/SentryId;
 	public abstract fun getLatestActiveSpan ()Lio/sentry/ISpan;
 	public abstract fun getName ()Ljava/lang/String;
 	public abstract fun getSpans ()Ljava/util/List;

sentry/src/main/java/io/sentry/ISpan.java

@@ -2,7 +2,6 @@
 
 import io.sentry.metrics.LocalMetricsAggregator;
 import io.sentry.protocol.Contexts;
-import io.sentry.protocol.SentryId;
 import java.util.List;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
@@ -281,8 +280,8 @@ ISpan startChild(
   @Nullable
   TracesSamplingDecision getSamplingDecision();
 
-  @NotNull
-  SentryId getEventId();
+  //  @NotNull
+  //  SentryId getEventId();
 
   @ApiStatus.Internal
   @NotNull