Migrate sampler to otel package (take 2) (#503)

* create session id early very early in the lifecycle. Use same instance when creating sampler.

* move sampler to otel package

* tighten access after refactor

* address code review comments in #503

splunk-otel-android/src/main/java/com/splunk/rum/RumInitializer.java

@@ -46,6 +46,7 @@
 import io.opentelemetry.rum.internal.GlobalAttributesSpanAppender;
 import io.opentelemetry.rum.internal.OpenTelemetryRum;
 import io.opentelemetry.rum.internal.OpenTelemetryRumBuilder;
+import io.opentelemetry.rum.internal.SessionIdRatioBasedSampler;
 import io.opentelemetry.rum.internal.instrumentation.activity.VisibleScreenTracker;
 import io.opentelemetry.rum.internal.instrumentation.anr.AnrDetector;
 import io.opentelemetry.rum.internal.instrumentation.crash.CrashReporter;
@@ -159,11 +160,11 @@ SplunkRum initialize(
         if (builder.sessionBasedSamplerEnabled) {
             otelRumBuilder.addTracerProviderCustomizer(
                     (tracerProviderBuilder, app) -> {
-                        // TODO: this is hacky behavior that utilizes a mutable variable, fix this!
-                        return tracerProviderBuilder.setSampler(
+                        SessionIdRatioBasedSampler sampler =
                                 new SessionIdRatioBasedSampler(
                                         builder.sessionBasedSamplerRatio,
-                                        () -> SplunkRum.getInstance().getRumSessionId()));
+                                        otelRumBuilder.getSessionId());
+                        return tracerProviderBuilder.setSampler(sampler);
                     });
         }
 

splunk-otel-android/src/main/java/io/opentelemetry/rum/internal/OpenTelemetryRumBuilder.java

@@ -41,6 +41,7 @@
  */
 public final class OpenTelemetryRumBuilder {
 
+    private final SessionId sessionId;
     private Resource resource = Resource.getDefault();
     private final List<BiFunction<SdkTracerProviderBuilder, Application, SdkTracerProviderBuilder>>
             tracerProviderCustomizers = new ArrayList<>();
@@ -51,7 +52,10 @@ public final class OpenTelemetryRumBuilder {
     private final List<Consumer<InstrumentedApplication>> instrumentationInstallers =
             new ArrayList<>();
 
-    OpenTelemetryRumBuilder() {}
+    OpenTelemetryRumBuilder() {
+        SessionIdTimeoutHandler timeoutHandler = new SessionIdTimeoutHandler();
+        this.sessionId = new SessionId(timeoutHandler);
+    }
 
     /**
      * Assign a {@link Resource} to be attached to all telemetry emitted by the {@link
@@ -135,6 +139,10 @@ public OpenTelemetryRumBuilder addInstrumentation(
         return this;
     }
 
+    public SessionId getSessionId() {
+        return sessionId;
+    }
+
     /**
      * Creates a new instance of {@link OpenTelemetryRum} with the settings of this {@link
      * OpenTelemetryRumBuilder}.
@@ -151,9 +159,7 @@ public OpenTelemetryRum build(Application application) {
         ApplicationStateWatcher applicationStateWatcher = new ApplicationStateWatcher();
         application.registerActivityLifecycleCallbacks(applicationStateWatcher);
 
-        SessionIdTimeoutHandler timeoutHandler = new SessionIdTimeoutHandler();
-        SessionId sessionId = new SessionId(timeoutHandler);
-        applicationStateWatcher.registerListener(timeoutHandler);
+        applicationStateWatcher.registerListener(sessionId.getTimeoutHandler());
 
         OpenTelemetrySdk openTelemetrySdk =
                 OpenTelemetrySdk.builder()

splunk-otel-android/src/main/java/io/opentelemetry/rum/internal/SessionId.java

@@ -54,6 +54,10 @@ private static String createNewId() {
         return TraceId.fromLongs(random.nextLong(), random.nextLong());
     }
 
+    SessionIdTimeoutHandler getTimeoutHandler() {
+        return timeoutHandler;
+    }
+
     String getSessionId() {
         // value will never be null
         String oldValue = requireNonNull(value.get());

splunk-otel-android/src/main/java/com/splunk/rum/SessionIdRatioBasedSampler.java → splunk-otel-android/src/main/java/io/opentelemetry/rum/internal/SessionIdRatioBasedSampler.java

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package com.splunk.rum;
+package io.opentelemetry.rum.internal;
 
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.SpanKind;
@@ -23,7 +23,6 @@
 import io.opentelemetry.sdk.trace.samplers.Sampler;
 import io.opentelemetry.sdk.trace.samplers.SamplingResult;
 import java.util.List;
-import java.util.function.Supplier;
 
 /**
  * Session ID ratio based sampler. Uses {@link Sampler#traceIdRatioBased(double)} sampler
@@ -32,12 +31,12 @@
  * io.opentelemetry.api.trace.TraceId#fromLongs(long, long)} internally to generate random session
  * IDs.
  */
-class SessionIdRatioBasedSampler implements Sampler {
+public class SessionIdRatioBasedSampler implements Sampler {
     private final Sampler ratioBasedSampler;
-    private final Supplier<String> sessionIdSupplier;
+    private final SessionId sessionid;
 
-    SessionIdRatioBasedSampler(double ratio, Supplier<String> splunkRumSupplier) {
-        this.sessionIdSupplier = splunkRumSupplier;
+    public SessionIdRatioBasedSampler(double ratio, SessionId sessionId) {
+        this.sessionid = sessionId;
         // SessionId uses the same format as TraceId, so we can reuse trace ID ratio sampler.
         this.ratioBasedSampler = Sampler.traceIdRatioBased(ratio);
     }
@@ -52,7 +51,7 @@ public SamplingResult shouldSample(
             List<LinkData> parentLinks) {
         // Replace traceId with sessionId
         return ratioBasedSampler.shouldSample(
-                parentContext, sessionIdSupplier.get(), name, spanKind, attributes, parentLinks);
+                parentContext, sessionid.getSessionId(), name, spanKind, attributes, parentLinks);
     }
 
     @Override

splunk-otel-android/src/test/java/com/splunk/rum/SessionIdRatioBasedSamplerTest.java → splunk-otel-android/src/test/java/io/opentelemetry/rum/internal/SessionIdRatioBasedSamplerTest.java

@@ -14,9 +14,10 @@
  * limitations under the License.
  */
 
-package com.splunk.rum;
+package io.opentelemetry.rum.internal;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.when;
 
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.Span;
@@ -29,27 +30,28 @@
 import io.opentelemetry.sdk.trace.samplers.SamplingDecision;
 import java.util.Collections;
 import java.util.List;
-import java.util.function.Supplier;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
 class SessionIdRatioBasedSamplerTest {
     private static final String HIGH_ID = "00000000000000008fffffffffffffff";
-    private static final Supplier<String> HIGH_ID_SUPPLIER = () -> HIGH_ID;
     private static final String LOW_ID = "00000000000000000000000000000000";
-    private static final Supplier<String> LOW_ID_SAMPLER = () -> LOW_ID;
     private static final IdGenerator idsGenerator = IdGenerator.random();
 
+    @Mock SessionId sessionId;
     private final String traceId = idsGenerator.generateTraceId();
     private final Context parentContext = Context.root().with(Span.getInvalid());
     private final List<LinkData> parentLinks =
             Collections.singletonList(LinkData.create(SpanContext.getInvalid()));
 
     @Test
     void samplerDropsHigh() {
-        SessionIdRatioBasedSampler sampler = new SessionIdRatioBasedSampler(0.5, HIGH_ID_SUPPLIER);
+        when(sessionId.getSessionId()).thenReturn(HIGH_ID);
+
+        SessionIdRatioBasedSampler sampler = new SessionIdRatioBasedSampler(0.5, sessionId);
 
         // Sampler drops if TraceIdRatioBasedSampler would drop this sessionId
         assertEquals(shouldSample(sampler), SamplingDecision.DROP);
@@ -58,25 +60,35 @@ void samplerDropsHigh() {
     @Test
     void samplerKeepsLowestId() {
         // Sampler accepts if TraceIdRatioBasedSampler would accept this sessionId
-        SessionIdRatioBasedSampler sampler = new SessionIdRatioBasedSampler(0.5, LOW_ID_SAMPLER);
+        when(sessionId.getSessionId()).thenReturn(LOW_ID);
+
+        SessionIdRatioBasedSampler sampler = new SessionIdRatioBasedSampler(0.5, sessionId);
         assertEquals(shouldSample(sampler), SamplingDecision.RECORD_AND_SAMPLE);
     }
 
     @Test
     void zeroRatioDropsAll() {
-        SessionIdRatioBasedSampler samplerHigh =
-                new SessionIdRatioBasedSampler(0.0, HIGH_ID_SUPPLIER);
+        when(sessionId.getSessionId()).thenReturn(HIGH_ID);
+
+        SessionIdRatioBasedSampler samplerHigh = new SessionIdRatioBasedSampler(0.0, sessionId);
         assertEquals(shouldSample(samplerHigh), SamplingDecision.DROP);
-        SessionIdRatioBasedSampler samplerLow = new SessionIdRatioBasedSampler(0.0, LOW_ID_SAMPLER);
+
+        when(sessionId.getSessionId()).thenReturn(LOW_ID);
+
+        SessionIdRatioBasedSampler samplerLow = new SessionIdRatioBasedSampler(0.0, sessionId);
         assertEquals(shouldSample(samplerLow), SamplingDecision.DROP);
     }
 
     @Test
     void oneRatioAcceptsAll() {
-        SessionIdRatioBasedSampler samplerHigh =
-                new SessionIdRatioBasedSampler(1.0, HIGH_ID_SUPPLIER);
+        when(sessionId.getSessionId()).thenReturn(HIGH_ID);
+
+        SessionIdRatioBasedSampler samplerHigh = new SessionIdRatioBasedSampler(1.0, sessionId);
         assertEquals(shouldSample(samplerHigh), SamplingDecision.RECORD_AND_SAMPLE);
-        SessionIdRatioBasedSampler samplerLow = new SessionIdRatioBasedSampler(1.0, LOW_ID_SAMPLER);
+
+        when(sessionId.getSessionId()).thenReturn(LOW_ID);
+
+        SessionIdRatioBasedSampler samplerLow = new SessionIdRatioBasedSampler(1.0, sessionId);
         assertEquals(shouldSample(samplerLow), SamplingDecision.RECORD_AND_SAMPLE);
     }