Skip to content

Hide symbols from public api #1278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Hide symbols from public api #1278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions android-agent/api/android-agent.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,27 +77,3 @@ public final class io/opentelemetry/android/agent/session/SessionConfig$Companio
public final fun withDefaults ()Lio/opentelemetry/android/agent/session/SessionConfig;
}

public abstract interface class io/opentelemetry/android/agent/session/SessionIdGenerator {
public abstract fun generateSessionId ()Ljava/lang/String;
}

public final class io/opentelemetry/android/agent/session/SessionIdGenerator$DEFAULT : io/opentelemetry/android/agent/session/SessionIdGenerator {
public static final field INSTANCE Lio/opentelemetry/android/agent/session/SessionIdGenerator$DEFAULT;
public fun generateSessionId ()Ljava/lang/String;
}

public abstract interface class io/opentelemetry/android/agent/session/SessionStorage {
public abstract fun get ()Lio/opentelemetry/android/session/Session;
public abstract fun save (Lio/opentelemetry/android/session/Session;)V
}

public final class io/opentelemetry/android/agent/session/SessionStorage$InMemory : io/opentelemetry/android/agent/session/SessionStorage {
public fun <init> ()V
public fun <init> (Lio/opentelemetry/android/session/Session;)V
public synthetic fun <init> (Lio/opentelemetry/android/session/Session;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun get ()Lio/opentelemetry/android/session/Session;
public final fun getSession ()Lio/opentelemetry/android/session/Session;
public fun save (Lio/opentelemetry/android/session/Session;)V
public final fun setSession (Lio/opentelemetry/android/session/Session;)V
}

19 changes: 19 additions & 0 deletions ...agent/src/main/kotlin/io/opentelemetry/android/agent/session/DefaultSessionIdGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.android.agent.session

import io.opentelemetry.api.trace.TraceId
import kotlin.random.Random

internal class DefaultSessionIdGenerator(
private val random: Random,
) : SessionIdGenerator {
override fun generateSessionId(): String {
// The OTel TraceId has exactly the same format as a RUM SessionId, so let's re-use it here,
// rather than re-inventing the wheel.
return TraceId.fromLongs(random.nextLong(), random.nextLong())
}
}
18 changes: 18 additions & 0 deletions ...id-agent/src/main/kotlin/io/opentelemetry/android/agent/session/InMemorySessionStorage.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.android.agent.session

import io.opentelemetry.android.session.Session

internal class InMemorySessionStorage(
@Volatile var session: Session = Session.NONE,
) : SessionStorage {
override fun get(): Session = session

override fun save(newSession: Session) {
this.session = newSession
}
}
14 changes: 1 addition & 13 deletions android-agent/src/main/kotlin/io/opentelemetry/android/agent/session/SessionIdGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,6 @@

package io.opentelemetry.android.agent.session

import io.opentelemetry.api.trace.TraceId
import java.util.Random

interface SessionIdGenerator {
internal fun interface SessionIdGenerator {
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I think this was public on purpose. There have been users who previously wanted to provide their own session id values....

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see any public-facing API that allows setting a custom SessionIdGenerator or SessionStorage - what's the expected usage?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not currently something users can provide via the initializer. The options we have are either hide them until needed, as proposed with these changes, or expose them right away, maybe via the SessionConfig object or the like. I'm fine either way tbh, though if we expose them right away, it's probably good to mark them all as incubating. Wdyt, @breedx-splk?

fun generateSessionId(): String

object DEFAULT : SessionIdGenerator {
override fun generateSessionId(): String {
val random = Random()
// The OTel TraceId has exactly the same format as a RUM SessionId, so let's re-use it here,
// rather than re-inventing the wheel.
return TraceId.fromLongs(random.nextLong(), random.nextLong())
}
}
}
2 changes: 2 additions & 0 deletions ...d-agent/src/main/kotlin/io/opentelemetry/android/agent/session/SessionIdTimeoutHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package io.opentelemetry.android.agent.session

import io.opentelemetry.android.Incubating
import io.opentelemetry.android.internal.services.applifecycle.ApplicationStateListener
import io.opentelemetry.sdk.common.Clock
import kotlin.time.Duration
Expand Down Expand Up @@ -34,6 +35,7 @@ internal class SessionIdTimeoutHandler(
private var state = State.FOREGROUND

// for testing
@OptIn(Incubating::class)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed, given that the constructor is internal?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes - @Incubating propagates so that any usage of an annotated symbol will require an opt-in, regardless of visibility. This suppresses a couple of compiler warnings that got introduced in the PR that added it. The alternative is to remove the annotation from SessionConfig

internal constructor(sessionConfig: SessionConfig) : this(
Clock.getDefault(),
sessionConfig.backgroundInactivityTimeout,
Expand Down
7 changes: 5 additions & 2 deletions android-agent/src/main/kotlin/io/opentelemetry/android/agent/session/SessionManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,21 @@

package io.opentelemetry.android.agent.session

import io.opentelemetry.android.Incubating
import io.opentelemetry.android.session.Session
import io.opentelemetry.android.session.SessionObserver
import io.opentelemetry.android.session.SessionProvider
import io.opentelemetry.android.session.SessionPublisher
import io.opentelemetry.sdk.common.Clock
import java.util.Collections.synchronizedList
import kotlin.random.Random
import kotlin.time.Duration

internal class SessionManager(
private val clock: Clock = Clock.getDefault(),
private val sessionStorage: SessionStorage = SessionStorage.InMemory(),
private val sessionStorage: SessionStorage = InMemorySessionStorage(),
private val timeoutHandler: SessionIdTimeoutHandler,
private val idGenerator: SessionIdGenerator = SessionIdGenerator.DEFAULT,
private val idGenerator: SessionIdGenerator = DefaultSessionIdGenerator(Random.Default),
private val maxSessionLifetime: Duration,
) : SessionProvider,
SessionPublisher {
Expand Down Expand Up @@ -70,6 +72,7 @@ internal class SessionManager(
}

companion object {
@OptIn(Incubating::class)
@JvmStatic
fun create(
timeoutHandler: SessionIdTimeoutHandler,
Expand Down
12 changes: 1 addition & 11 deletions android-agent/src/main/kotlin/io/opentelemetry/android/agent/session/SessionStorage.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,8 @@ package io.opentelemetry.android.agent.session

import io.opentelemetry.android.session.Session

interface SessionStorage {
internal interface SessionStorage {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment -- I think this was visible because some users wanted to be able to provide their own storage mechanism?

fun get(): Session

fun save(newSession: Session)

class InMemory(
@Volatile var session: Session = Session.NONE,
) : SessionStorage {
override fun get(): Session = session

override fun save(newSession: Session) {
this.session = newSession
}
}
}