Use of SessionIdTimeoutHandler class

[ddas09]

Hi Team, I am trying to understand the use of the SessionIdTimeoutHandler class. As per my understanding there are two classes -

• SessionManagerImpl - this allows a session to be active for a maximum of 4 hours (if not overridden using the OtelRumConfig here
• SessionIdTimeoutHandler - this class handles expiring session if the app remains if background for more than 15 minutes (if not overridden using the OtelRumConfig here without any activity (span, logs).

The only scenario when I think the SessionIdTimeoutHandler class hasTimedOut check to be useful is if the timeout is not overridden from the OtelRumConfig API. In this case the handler class would timeout the session If the app is in the background and no activity (spans) happens for >15 minutes

But if the timeout is set in the config API it would override the value in both the SessionIdTimeoutHandler and SessionManagerImpl class. In this case If the app is in the background and no activity (spans) happens for >15 minutes, the next time the app comes foreground and any span is created, the check in the SessionManagerImpl class only would fail. Since the sessionLifetimeNanos is 15 minutes now.

Please help me to understand if I am missing something!

[breedx-splk]

I think you got it.

Sessions are intended to timeout after some period of inactivity, default is 15 minutes as you noted. Inactivity is defined as no user behavior, which means no telemetry being created. A foregrounded app is not considered inactive.

Hopefully that helps.

[breedx-splk]

Also keep in mind that the SessionIdTimeoutHandler class is in the internal package and is not intended for direct use by users.

[ddas09]

But if the timeout is set in the config API it would override the value in both the SessionIdTimeoutHandler and SessionManagerImpl class. In this case If the app is in the background and no activity (spans) happens for >15 minutes, the next time the app comes foreground and any span is created, the check in the SessionManagerImpl class only would fail. Since the sessionLifetimeNanos is 15 minutes now.

Thank you @breedx-splk for the explanation. Can you also confirm this?

[breedx-splk]

I think that's right. Even if the app is backgrounded, if the max time for a session to live expires, a new session should id should be created. Is this a problem or not matching your expectations?

Can you try it? I added

config.sessionTimeout = Duration.ofSeconds(10)
into the startup of the demo app, you I could watch the session id change every 10 seconds or so (you have to click the icon or generate some telemetry, and the way to observe that is thru the docker logs of the collector).

I backgrounded the app and saw no changes while backgrounded, but certainly once foregrounded again the session changed soon after.

[ddas09]

@breedx-splk, I was just wondering why we have these two classes, when the check inside the session manager class itself is enough to expire a session and create a new session.

[breedx-splk]

@breedx-splk, I was just wondering why we have these two classes, when the check inside the session manager class itself is enough to expire a session and create a new session.

Ok, I think I understand what you're asking now. Maybe I can clarify:

• The SessionManagerImpl is the implementation of the SessionManager interface, which combiners SessionProvider and SessionPublisher interfaces, so you get two external methods -- getSessionId() and addObserver(). Through the interface, this implementation is how users can get the session id and subscribe observers for changes.
• The SessionTimeoutHandler is itself an ApplicationStateListener that watches for application foreground/background state changes in order to determine (though encapsulation) if the timeout should be considered expired.

These two classes are collaborators -- the SessionManagerImpl leverages the SessionIdTimeoutHandler.

I think what you were trying to suggest is that one of the timeout checks might be redundant because they are ORed together. I think what you might be overlooking (or that the code doesn't yet make clear enough) is that there are intended to be two separate timeouts --

1. The overall maximum lifetime that a session can exist. Regardless of app state, a new session is supposed to be generated after this time passes. Original default is intended to be 4 hours.
2. The session idle timeout -- time after which an inactive app should create a new session. Original default is intended to be 15 minutes.

I think we've broken something about this along the way through refactoring, though.

[ddas09]

I think what you might be overlooking (or that the code doesn't yet make clear enough) is that there are intended to be two separate timeouts

@breedx-splk, Thanks for the detailed explanation! I got it that both the classes are used for different timeouts. But when the user overrides the default timeouts the checks become redundant as you shared. Maybe this is how it’s supposed to work?

[breedx-splk]

I think that's not how it's supposed to work, so I submitted #887 to address this. Thanks for talking this out.

[ddas09]

Thanks @breedx-splk! I saw the changes. The new config makes the timeout usages very clear.

[breedx-splk]

Thanks @ddas09. Please feel welcome to submit a PR review. This kind of feedback is very helpful, and a gray checkmark of approval with/without comments is still very much needed in the otel community. 🙏🏻