Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import io.opentelemetry.android.internal.services.periodicwork.PeriodicWork;
import io.opentelemetry.android.internal.session.SessionIdTimeoutHandler;
import io.opentelemetry.android.internal.session.SessionManagerImpl;
import io.opentelemetry.android.internal.session.SessionObservingSessionProvider;
import io.opentelemetry.android.sampling.SessionIdRatioBasedSampler;
import io.opentelemetry.android.session.SessionManager;
import io.opentelemetry.android.session.SessionProvider;
import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator;
Expand Down Expand Up @@ -71,6 +73,7 @@
import io.opentelemetry.sdk.trace.SpanProcessor;
import io.opentelemetry.sdk.trace.export.BatchSpanProcessor;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -111,6 +114,7 @@ public final class OpenTelemetryRumBuilder {

@Nullable private ExportScheduleHandler exportScheduleHandler;
@Nullable private SessionManager sessionManager;
private double sessionIdSamplingRatio = Double.NEGATIVE_INFINITY;

private static TextMapPropagator buildDefaultPropagator() {
return TextMapPropagator.composite(
Expand Down Expand Up @@ -301,6 +305,24 @@ public OpenTelemetryRumBuilder addLogRecordExporterCustomizer(
return this;
}

/**
* Enables a ratio-based probabilistic sampling of sessions. When a session is sampled
* (included), then the telemetry for that session will be created and exported. If a session is
* not sampled (excluded) then the telemetry for that session will not be exported.
*
* @param ratio - A number between 0 and 1, representing a percentage of sessions to sample.
*/
public OpenTelemetryRumBuilder enableSessionBasedSampling(double ratio) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The class SessionFromContextProvider is pretty much explicit about where we are fetching the current session but not this method. If we want to continue to use a custom SessionManager (with setSessionManager) and use this method to apply a sampling, we need to know that we should save our session id in the context with a specific context key. Hard to know for someone that has limited knowledge about the internal implementation of this project.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Another proposition without the need of a context and allow us to provide a custom SessionManager if it is require by the consumer.

    public OpenTelemetryRumBuilder enableSessionBasedSampling(double ratio) {
        return enableSessionBasedSampling(ratio, SessionManagerImpl.create());
    }

    public OpenTelemetryRumBuilder enableSessionBasedSampling(double ratio, SessionManager sessionManager) {
        Sampler sampler = buildSampler(ratio, sessionManager);
        addTracerProviderCustomizer((builder, application) -> builder.setSampler(sampler));

        // TODO: When there is a mechanism for sampling logs, we need to wire
        // it up here...

        this.sessionManager = sessionManager;

        return this;
    }

    private static Sampler buildSampler(double ratio, SessionProvider sessionProvider) {
        if (ratio < 0.0 || ratio > 1.0) {
            throw new IllegalArgumentException("ratio must be in range [0.0, 1.0]");
        }
        if (ratio == 0.0) {
            return Sampler.alwaysOff();
        }
        if (ratio == 1.0) {
            return Sampler.alwaysOn();
        }
        return new SessionIdRatioBasedSampler(ratio, sessionProvider);
    }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The class SessionFromContextProvider is pretty much explicit about where we are fetching the current session but not this method. If we want to continue to use a custom SessionManager (with setSessionManager) and use this method to apply a sampling, we need to know that we should save our session id in the context with a specific context key. Hard to know for someone that has limited knowledge about the internal implementation of this project.

Yes, this is a really good point -- and I really do wish that we didn't have SessionManager as a pluggable component.

This also makes me realize that we have a loose contract around the observers as well -- the interface has addObserver() through the SessionPublisher interface (which SessionManager extends), but there's nothing to require a SessionManager implementation to notify its observers when the session changes. It's extra knowledge, and that stinks.

I don't mind keeping the session id format/value user-suppliable, but making the entire thing pluggable is turning into a headache.

if (ratio < 0.0 || ratio > 1.0) {
Log.e(
RumConstants.OTEL_RUM_LOG_TAG,
"Sampling ratio must be in range [0.0, 1.0], ignoring " + ratio);
return this;
}
sessionIdSamplingRatio = ratio;
return this;
}

/**
* Creates a new instance of {@link OpenTelemetryRum} with the settings of this {@link
* OpenTelemetryRumBuilder}.
Expand All @@ -325,6 +347,8 @@ public OpenTelemetryRum build() {
sessionManager = SessionManagerImpl.create(timeoutHandler, config.getSessionConfig());
}

configureSessionSampling();

OpenTelemetrySdk sdk =
OpenTelemetrySdk.builder()
.setTracerProvider(
Expand Down Expand Up @@ -360,6 +384,30 @@ public OpenTelemetryRum build() {
return delegate.build();
}

private void configureSessionSampling() {
if ((sessionIdSamplingRatio < 0) || (sessionManager == null)) {
Comment thread
marandaneto marked this conversation as resolved.
return;
}
SessionObservingSessionProvider observerAndProvider = new SessionObservingSessionProvider();
sessionManager.addObserver(observerAndProvider);

Sampler sampler = buildSampler(sessionIdSamplingRatio, observerAndProvider);
addTracerProviderCustomizer((builder, application) -> builder.setSampler(sampler));

// TODO: When there is a mechanism for sampling logs, we need to wire
// it up here...
}

private static Sampler buildSampler(double ratio, SessionProvider sessionProvider) {
if (ratio == 0.0) {
return Sampler.alwaysOff();
}
if (ratio == 1.0) {
return Sampler.alwaysOn();
}
return new SessionIdRatioBasedSampler(ratio, sessionProvider);
}

private void initializeExporters(
Services services,
InitializationEvents initializationEvents,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.android.internal.session

import io.opentelemetry.android.session.Session
import io.opentelemetry.android.session.SessionObserver
import io.opentelemetry.android.session.SessionProvider
import java.util.concurrent.atomic.AtomicReference

/**
* A SessionObserver that listens for session start events, stores the session,
* and is also a SessionProvider, that can return the session id.
*/
internal class SessionObservingSessionProvider :
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

From what I understand, this class was introduced due to the belief that:

Having a user-providable SessionManager interface means that none of the SessionManager behaviors can be assumed...and I hate this. There should just be one session manager, and it should opinionated...this would make all of our lives easier.

However, I'm not fully convinced this is actually a problem. The current SessionManager already extends both SessionProvider and SessionPublisher, which together define a clear contract. Any implementation (whether user-provided or internal) must satisfy this contract. While the observer pattern might make sense for external libraries that shouldn't depend directly on the OpenTelemetry Android sdk, I’m not sure it’s the best fit within the sdk itself.

The more I think about it, the more I believe that the SDK may not need to own the concept of a session at all. On Android, sessions are often used beyond telemetry (for example, for analytics or app-specific behaviors). In my own experience, I work on a kiosk application that resets itself after a configurable timeout, triggering a new session. In such scenarios, having a centralized, user-defined session manager becomes critical, as it allows multiple tools (including OpenTelemetry) to share a consistent session lifecycle.

If that's the case, then perhaps the SDK should act only as a consumer of session data rather than its owner. In this context, adopting an observer pattern makes more sense. But to fully embrace that model, maybe all session-related logic should be removed from the core sdk, with the option for users to plug in either a custom or provided basic implementation.

Just my two cents, curious to hear yours.

Copy link
Copy Markdown
Contributor

@LikeTheSalad LikeTheSalad May 9, 2025

Choose a reason for hiding this comment

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

I think I see what you mean. In this case, core is being opinionated about what a session must be. Based on the project structure that I mentioned here, I think one approach we could take to avoid it, is to move our opinionated implementation of session manager from core into the agent, and then allow the interface to be set in core. That way, endusers will still have our opinionated version of a session, while other SDKs and/or expert users can define their own within core.

Generally speaking, I think core should be as flexible and opinionated-less as possible, and if there's something that we don't want users to do with core, then we can work on what the interfaces allow them to do. It's a tricky balance to ensure that something is flexible enough without causing people to shoot themselves in the foot, and most likely we won't get it perfectly on the first attempt, though I think this project is in a state (not stable) that allows us to experiment with the API until we find a good balance.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've created this PR with the approach that I was mentioning in yesterday's SIG meeting, @breedx-splk, please have a look to see what you think. I think it should address both @GerardPaligot's issue and also this other one and it will also simplify the core API surface.

SessionProvider,
SessionObserver {
private val session = AtomicReference<Session>(Session.NONE)

override fun getSessionId(): String = session.get().getId()

override fun onSessionStarted(
newSession: Session,
previousSession: Session,
) = session.set(newSession)
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.android;
package io.opentelemetry.android.sampling;

import io.opentelemetry.android.session.SessionProvider;
import io.opentelemetry.api.common.Attributes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.when;

import io.opentelemetry.android.sampling.SessionIdRatioBasedSampler;
import io.opentelemetry.android.session.SessionProvider;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.Span;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ interface SessionObserver {
fun onSessionStarted(
newSession: Session,
previousSession: Session,
)
) {}

fun onSessionEnded(session: Session)
fun onSessionEnded(session: Session) {}
}