Refactor session handling (#564)

* first pass at new session api * session work in progress * Rename .java to .kt * fix up some tests * make sampler use interface * move SessionIdTest.java to SessionManagerTest.kt and fix up * remove unused SessionId.java * remove unused SessionChangeObserver * spotless * make constants * don't send events for the NONE session * default to NONE session and cause session start after adding observer * Rename .java to .kt * migrate observer to kotlin * spotless * pass config session timeout and don't force SessionManager to be a builder field. * fix typo * fix test * fix tests * remove SessionIdEventSender (to make the PR more purely refactoring)
open-telemetry · Aug 30, 2024 · 5a230fc · 5a230fc
1 parent 798386e
commit 5a230fc
Show file tree

Hide file tree

Showing 25 changed files with 447 additions and 364 deletions.
diff --git a/core/build.gradle.kts b/core/build.gradle.kts
@@ -69,6 +69,7 @@ dependencies {
     api(platform(libs.opentelemetry.platform))
     api(libs.opentelemetry.api)
     implementation(libs.opentelemetry.sdk)
+    implementation(libs.opentelemetry.api.incubator)
     implementation(libs.opentelemetry.exporter.logging)
     implementation(libs.opentelemetry.instrumentation.api)
     implementation(libs.opentelemetry.semconv.incubating)

diff --git a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java
@@ -23,6 +23,8 @@
 import io.opentelemetry.android.internal.services.CacheStorage;
 import io.opentelemetry.android.internal.services.Preferences;
 import io.opentelemetry.android.internal.services.ServiceManager;
+import io.opentelemetry.android.session.SessionManager;
+import io.opentelemetry.android.session.SessionProvider;
 import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator;
 import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator;
 import io.opentelemetry.context.propagation.ContextPropagators;
@@ -65,7 +67,6 @@
  */
 public final class OpenTelemetryRumBuilder {
 
-    private final SessionId sessionId;
     private final Application application;
     private final List<BiFunction<SdkTracerProviderBuilder, Application, SdkTracerProviderBuilder>>
             tracerProviderCustomizers = new ArrayList<>();
@@ -76,6 +77,7 @@ public final class OpenTelemetryRumBuilder {
     private final OtelRumConfig config;
     private final List<AndroidInstrumentation> instrumentations = new ArrayList<>();
     private final List<Consumer<OpenTelemetrySdk>> otelSdkReadyListeners = new ArrayList<>();
+    private final SessionIdTimeoutHandler timeoutHandler;
     private Function<? super SpanExporter, ? extends SpanExporter> spanExporterCustomizer = a -> a;
     private Function<? super LogRecordExporter, ? extends LogRecordExporter>
             logRecordExporterCustomizer = a -> a;
@@ -97,7 +99,7 @@ public static OpenTelemetryRumBuilder create(Application application, OtelRumCon
     OpenTelemetryRumBuilder(
             Application application, OtelRumConfig config, SessionIdTimeoutHandler timeoutHandler) {
         this.application = application;
-        this.sessionId = new SessionId(timeoutHandler);
+        this.timeoutHandler = timeoutHandler;
         this.resource = AndroidResource.createDefault(application);
         this.config = config;
     }
@@ -253,10 +255,6 @@ public OpenTelemetryRumBuilder addLogRecordExporterCustomizer(
         return this;
     }
 
-    public SessionId getSessionId() {
-        return sessionId;
-    }
-
     /**
      * Creates a new instance of {@link OpenTelemetryRum} with the settings of this {@link
      * OpenTelemetryRumBuilder}.
@@ -303,10 +301,13 @@ OpenTelemetryRum build(ServiceManager serviceManager) {
         }
         initializationEvents.spanExporterInitialized(spanExporter);
 
+        SessionManager sessionManager =
+                SessionManager.create(timeoutHandler, config.getSessionTimeout().toNanos());
+
         OpenTelemetrySdk sdk =
                 OpenTelemetrySdk.builder()
                         .setTracerProvider(
-                                buildTracerProvider(sessionId, application, spanExporter))
+                                buildTracerProvider(sessionManager, application, spanExporter))
                         .setMeterProvider(buildMeterProvider(application))
                         .setLoggerProvider(buildLoggerProvider(application, logsExporter))
                         .setPropagators(buildFinalPropagators())
@@ -320,7 +321,8 @@ OpenTelemetryRum build(ServiceManager serviceManager) {
                 new SdkPreconfiguredRumBuilder(
                         application,
                         sdk,
-                        sessionId,
+                        timeoutHandler,
+                        sessionManager,
                         config.shouldDiscoverInstrumentations(),
                         serviceManager);
         instrumentations.forEach(delegate::addInstrumentation);
@@ -417,11 +419,11 @@ private void applyConfiguration(
     }
 
     private SdkTracerProvider buildTracerProvider(
-            SessionId sessionId, Application application, SpanExporter spanExporter) {
+            SessionProvider sessionProvider, Application application, SpanExporter spanExporter) {
         SdkTracerProviderBuilder tracerProviderBuilder =
                 SdkTracerProvider.builder()
                         .setResource(resource)
-                        .addSpanProcessor(new SessionIdSpanAppender(sessionId));
+                        .addSpanProcessor(new SessionIdSpanAppender(sessionProvider));
 
         BatchSpanProcessor batchSpanProcessor = BatchSpanProcessor.builder(spanExporter).build();
         tracerProviderBuilder.addSpanProcessor(batchSpanProcessor);

diff --git a/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumImpl.java b/core/src/main/java/io/opentelemetry/android/OpenTelemetryRumImpl.java
@@ -5,17 +5,18 @@
 
 package io.opentelemetry.android;
 
+import io.opentelemetry.android.session.SessionManager;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
 
 final class OpenTelemetryRumImpl implements OpenTelemetryRum {
 
     private final OpenTelemetrySdk openTelemetrySdk;
-    private final SessionId sessionId;
+    private final SessionManager sessionManager;
 
-    OpenTelemetryRumImpl(OpenTelemetrySdk openTelemetrySdk, SessionId sessionId) {
+    OpenTelemetryRumImpl(OpenTelemetrySdk openTelemetrySdk, SessionManager sessionManager) {
         this.openTelemetrySdk = openTelemetrySdk;
-        this.sessionId = sessionId;
+        this.sessionManager = sessionManager;
     }
 
     @Override
@@ -25,6 +26,6 @@ public OpenTelemetry getOpenTelemetry() {
 
     @Override
     public String getRumSessionId() {
-        return sessionId.getSessionId();
+        return sessionManager.getSessionId();
     }
 }
diff --git a/core/src/main/java/io/opentelemetry/android/SdkPreconfiguredRumBuilder.kt b/core/src/main/java/io/opentelemetry/android/SdkPreconfiguredRumBuilder.kt
@@ -9,17 +9,16 @@ import android.app.Application
 import io.opentelemetry.android.instrumentation.AndroidInstrumentation
 import io.opentelemetry.android.instrumentation.AndroidInstrumentationLoader
 import io.opentelemetry.android.internal.services.ServiceManager
+import io.opentelemetry.android.session.SessionManager
 import io.opentelemetry.sdk.OpenTelemetrySdk
 
 class SdkPreconfiguredRumBuilder
     @JvmOverloads
     internal constructor(
         private val application: Application,
         private val sdk: OpenTelemetrySdk,
-        private val sessionId: SessionId =
-            SessionId(
-                SessionIdTimeoutHandler(),
-            ),
+        private val timeoutHandler: SessionIdTimeoutHandler = SessionIdTimeoutHandler(),
+        private val sessionManager: SessionManager = SessionManager(timeoutHandler = timeoutHandler),
         private val discoverInstrumentations: Boolean,
         private val serviceManager: ServiceManager,
     ) {
@@ -49,12 +48,9 @@ class SdkPreconfiguredRumBuilder
             serviceManager.start()
             // the app state listeners need to be run in the first ActivityLifecycleCallbacks since they
             // might turn off/on additional telemetry depending on whether the app is active or not
-            appLifecycleService.registerListener(sessionId.timeoutHandler)
+            appLifecycleService.registerListener(timeoutHandler)
 
-            val tracer = sdk.getTracer(OpenTelemetryRum::class.java.simpleName)
-            sessionId.setSessionIdChangeListener(SessionIdChangeTracer(tracer))
-
-            val openTelemetryRum = OpenTelemetryRumImpl(sdk, sessionId)
+            val openTelemetryRum = OpenTelemetryRumImpl(sdk, sessionManager)
 
             // Install instrumentations
             for (instrumentation in getInstrumentations()) {

diff --git a/core/src/main/java/io/opentelemetry/android/SessionId.java b/core/src/main/java/io/opentelemetry/android/SessionId.java
diff --git a/core/src/main/java/io/opentelemetry/android/SessionIdChangeListener.java b/core/src/main/java/io/opentelemetry/android/SessionIdChangeListener.java
diff --git a/core/src/main/java/io/opentelemetry/android/SessionIdChangeTracer.java b/core/src/main/java/io/opentelemetry/android/SessionIdChangeTracer.java
diff --git a/core/src/main/java/io/opentelemetry/android/SessionIdRatioBasedSampler.java b/core/src/main/java/io/opentelemetry/android/SessionIdRatioBasedSampler.java
@@ -5,6 +5,7 @@
 
 package io.opentelemetry.android;
 
+import io.opentelemetry.android.session.SessionProvider;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.context.Context;
@@ -22,10 +23,10 @@
  */
 public class SessionIdRatioBasedSampler implements Sampler {
     private final Sampler ratioBasedSampler;
-    private final SessionId sessionId;
+    private final SessionProvider sessionProvider;
 
-    public SessionIdRatioBasedSampler(double ratio, SessionId sessionId) {
-        this.sessionId = sessionId;
+    public SessionIdRatioBasedSampler(double ratio, SessionProvider sessionProvider) {
+        this.sessionProvider = sessionProvider;
         // SessionId uses the same format as TraceId, so we can reuse trace ID ratio sampler.
         this.ratioBasedSampler = Sampler.traceIdRatioBased(ratio);
     }
@@ -40,7 +41,12 @@ public SamplingResult shouldSample(
             List<LinkData> parentLinks) {
         // Replace traceId with sessionId
         return ratioBasedSampler.shouldSample(
-                parentContext, sessionId.getSessionId(), name, spanKind, attributes, parentLinks);
+                parentContext,
+                sessionProvider.getSessionId(),
+                name,
+                spanKind,
+                attributes,
+                parentLinks);
     }
 
     @Override

diff --git a/core/src/main/java/io/opentelemetry/android/SessionIdSpanAppender.java b/core/src/main/java/io/opentelemetry/android/SessionIdSpanAppender.java
@@ -5,23 +5,25 @@
 
 package io.opentelemetry.android;
 
-import io.opentelemetry.android.common.RumConstants;
+import static io.opentelemetry.semconv.incubating.SessionIncubatingAttributes.SESSION_ID;
+
+import io.opentelemetry.android.session.SessionProvider;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.sdk.trace.ReadWriteSpan;
 import io.opentelemetry.sdk.trace.ReadableSpan;
 import io.opentelemetry.sdk.trace.SpanProcessor;
 
 final class SessionIdSpanAppender implements SpanProcessor {
 
-    private final SessionId sessionId;
+    private final SessionProvider sessionProvider;
 
-    public SessionIdSpanAppender(SessionId sessionId) {
-        this.sessionId = sessionId;
+    public SessionIdSpanAppender(SessionProvider sessionProvider) {
+        this.sessionProvider = sessionProvider;
     }
 
     @Override
     public void onStart(Context parentContext, ReadWriteSpan span) {
-        span.setAttribute(RumConstants.SESSION_ID_KEY, sessionId.getSessionId());
+        span.setAttribute(SESSION_ID, sessionProvider.getSessionId());
     }
 
     @Override

diff --git a/core/src/main/java/io/opentelemetry/android/SessionIdTimeoutHandler.java b/core/src/main/java/io/opentelemetry/android/SessionIdTimeoutHandler.java
@@ -23,7 +23,8 @@
  * <p>Consequently, when the app spent >15 minutes without any activity (spans) in the background,
  * after moving to the foreground the first span should trigger the sessionId timeout.
  */
-final class SessionIdTimeoutHandler implements ApplicationStateListener {
+// TODO: Migrate to kotlin and make internal?
+public final class SessionIdTimeoutHandler implements ApplicationStateListener {
 
     static final Duration DEFAULT_SESSION_TIMEOUT = Duration.ofMinutes(15);
     private final Duration sessionTimeout;
@@ -56,7 +57,7 @@ public void onApplicationBackgrounded() {
         state = State.BACKGROUND;
     }
 
-    boolean hasTimedOut() {
+    public boolean hasTimedOut() {
         // don't apply sessionId timeout to apps in the foreground
         if (state == State.FOREGROUND) {
             return false;
@@ -65,7 +66,7 @@ boolean hasTimedOut() {
         return elapsedTime >= sessionTimeout.toNanos();
     }
 
-    void bump() {
+    public void bump() {
         timeoutStartNanos = clock.nanoTime();
 
         // move from the temporary transition state to foreground after the first span