Session Management: Added core infrastructure, etc. #1470
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.android.agent.session.factory | ||
|
|
||
| import android.app.Application | ||
| import io.opentelemetry.android.Incubating | ||
| import io.opentelemetry.android.agent.session.SessionConfig | ||
| import io.opentelemetry.android.agent.session.SessionIdTimeoutHandler | ||
| import io.opentelemetry.android.agent.session.SessionManager | ||
| import io.opentelemetry.android.internal.services.Services | ||
| import io.opentelemetry.android.session.SessionProvider | ||
| import io.opentelemetry.sdk.common.Clock | ||
|
|
||
| /** | ||
| * Default implementation of [SessionProviderFactory] that creates [SessionManager] instances. | ||
| * | ||
| * This implementation creates a [SessionManager] and wires it with: | ||
| * - A timeout handler configured from the session config | ||
| * - Application lifecycle integration for timeout management | ||
| * | ||
| * This class is open to allow custom implementations for specialized use cases. | ||
| * | ||
| * @param application the Android application instance used to access platform services. | ||
| * @param clock the clock instance to use for time-based operations. Defaults to [Clock.getDefault]. | ||
| * @see SessionProviderFactory | ||
| * @see SessionManager | ||
| * @see SessionConfig | ||
| */ | ||
| open class SessionManagerFactory( | ||
|
Member
There was a problem hiding this comment. I'm not a fan of exposing concrete types as part of the public API, particularly if they're
Contributor
Author
There was a problem hiding this comment. I think it relates to how we're thinking about the SDK's architecture. These are good discussions to have. 😄 Libraries vs SDKs vs FrameworksFrom my perspective, this SDK's design fits that of a framework and not a library. That distinction matters because: Libraries tend to give you code to use as-is without customization This SDK already leans into the framework side:
Making On Your DSL SuggestionAdding OpenTelemetryRumInitializer.initialize(ctx) {
sessionProvider = myCustomProvider
} // But there's still a creation question - how do DI users create that `SessionProvider` in the first place if `SessionManagerFactory` is internal?
@Provides
fun provideSessionProvider(app: Application, config: AppConfig): SessionProvider {
return ??? // Need a way to create this with custom config
} // That's why the factory needs to be public.The Codebase Already Works This Way
The What Remains Hidden The complicated stuff still stays internal:
Only the extension points are public (factory and config). The implementation stays locked down. This is how frameworks like Dagger, OkHttp, and others work. They expose factories because customization is part of the public contract. This is the architectural pattern I'm seeing throughout the SDK. If you have a different approach for how teams should create SessionProviders in DI scenarios, I'd like to understand that.
Contributor
There was a problem hiding this comment. Btw, OpenTelemetryRumBuilder already allows to set an instance of
Contributor
There was a problem hiding this comment. What's the intended use case for this factory? |
||
| private val application: Application, | ||
| private val clock: Clock = Clock.getDefault(), | ||
| ) : SessionProviderFactory { | ||
| /** | ||
| * Creates a session manager instance. | ||
| * | ||
| * This implementation creates a [SessionManager] with a timeout handler that is | ||
| * registered with the application lifecycle service. | ||
| * | ||
| * @param sessionConfig the configuration for session management. | ||
| * @return a session manager instance. | ||
| */ | ||
| @OptIn(Incubating::class) | ||
| override fun createSessionProvider(sessionConfig: SessionConfig): SessionProvider { | ||
| val timeoutHandler = SessionIdTimeoutHandler(sessionConfig, clock) | ||
| Services.get(application).appLifecycle.registerListener(timeoutHandler) | ||
| return SessionManager.create(timeoutHandler, sessionConfig, clock) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.android.agent.session.factory | ||
|
|
||
| import io.opentelemetry.android.agent.session.SessionConfig | ||
| import io.opentelemetry.android.session.SessionProvider | ||
|
|
||
| /** | ||
| * Factory for creating [SessionProvider] instances. | ||
| * | ||
| * This interface follows the Factory design pattern to enable flexible dependency injection | ||
| * and testing of session management components. Implementations can provide custom session | ||
| * providers with different behaviors while maintaining a consistent creation interface. | ||
| * | ||
| * @see SessionManagerFactory | ||
| * @see SessionProvider | ||
| */ | ||
| interface SessionProviderFactory { | ||
|
Contributor
There was a problem hiding this comment. I'm also not sure what could be the use case for this factory. Users should be able to create their own implementation of |
||
| /** | ||
| * Creates a session provider instance. | ||
| * | ||
| * @param sessionConfig the configuration for session management. | ||
| * @return a session provider instance. | ||
| */ | ||
| fun createSessionProvider(sessionConfig: SessionConfig): SessionProvider | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it'd be preferable to use the existing
SessionConfigurationin the DSL API rather than using concrete classes such asSessionConfigandSessionManagerFactory. E.g. something like this should be possible:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. So you're suggesting adding
sessionProvideras a property toOpenTelemetryConfigurationso users can inject custom providers via DSL. Is that correct? If yes, then something like this:That works for injection, but there's still a creation question of how do DI users create the
SessionProviderin the first place?For instance, when DI is used:
Here are the options I see for creating the SessionProvider:
Users can do
SessionManagerFactory(app).createSessionProvider(config)SessionProvider.create(app, timeout, maxLifetime)They rebuild SessionManager's logic (~200 lines of concurrent code, session storage, timeout handling, lifecycle integration)
Which approach makes the most sense to you? I'm leaning heavily toward option 1 (public factory) since it matches how DiskBufferingConfiguration works and keeps the actual implementation internal.