Session handling implementation

[martinkuba]

Related to #952

This is a proposal to implement sessions for web. The session.id attribute is now defined in semantic conventions (link). The attribute should be added to any span or log generated by a web instrumentation.

Implementation proposal:

• implement SessionIdSpanProcessor and SessionIdLogRecordProcessor
• these two processors will have a dependency on a SessionManager
• when adding the attribute, the processors will get the session ID from the manager
• the manager is responsible for managing the session ID value (generating, persisting and expiring)
• we provide a default implementation of SessionManager that will use LocalStorage and configurable expiration
• users can provide their own implementation of a SessionManager
• all of this functionality will live in a new package called web-common

SessionManager interface

export interface SessionManager {
  getSessionId(): string
}

A prototype of this proposal is available here.

Prior implementations:

• the OTel demo app already has a similar implementation
  • https://github.com/open-telemetry/opentelemetry-demo/blob/main/src/frontend/utils/telemetry/SessionIdProcessor.ts
  • https://github.com/open-telemetry/opentelemetry-demo/blob/main/src/frontend/gateways/Session.gateway.ts

I am looking for any feedback on this approach. If there are no objections, I will open a PR in line with the prototype implementation.

[breedx-splk]

Nice. This seems like a very reasonable approach. It matches pretty closely with Android, although we don't have an explicit interface: https://github.com/open-telemetry/opentelemetry-android/blob/main/android-agent/src/main/java/io/opentelemetry/android/SessionId.java#L50

Instead of SessionManager we have it spelled SessionId, which encapsulates the 3 things you described: generating, persisting (not really a thing in android yet, but it might need to be for background workers etc.), and expiring. We don't have the actual id value generation abstracted, but that would be reasonably simple thing (and I like that idea).

One thing I think is also valuable is to have the event generation (session start, end, change) being decoupled from the SessionId class. I'd encourage the same for you, even if you spell it differently.

[t2t2]

This isn't necessary for first PR, but any thoughts on global registration for session manager? (eg. via otel-api's registerGlobal/getGlobal)

Two use cases that this would greatly simplify:

1. When separating SDK/Agents into smaller bundles based on components the site uses, it could be used to share session easily across different components

As a more concrete real world example, for splunk/signalfx we have 2 separate components - core with tracing instrumentation and session replay as extension that relies on first. Since we index RUM data based on session id, want it to be consistent, and some customers use bundled js files while others npm install, then the easiest way is to register our core onto otel global, and component reuses core's Resource & session manager

2. Sharing session id with other systems

For example if a site has a live chat-box support, these systems generally include an API that can be used to include custom data on the conversation, so it can be useful to include otel's current session id so it can be used to debug if user has issues with the site. So then there's the similar issue of how to access the session manager

ChatBot.onConversationStarted(() => {
    ChatBot.updateMetadata({
        'otel.session_id': /* get session manger */.getSessionId()
    });
});

(in my experience with customers who are all-in on otel they definitely prefer a via otel api approach over via vendor api whenever possible, the lack of vendor lock-in kinda is the selling point of otel for them)

---

on that note why is this issue under contrib?

[martinkuba]

@t2t2 I can see how it would be useful to have access to the session ID from a central place. I am not opposed to adding it to the API. I would prefer to not include it in the first version (as you mentioned) in order to get something basic implemented first.

on that note why is this issue under contrib?

I thought of it as similar to sql-common or propagation-utils in the packages folder here, not as a core feature. But I am curious to hear if you feel otherwise. I suppose if we have a web-common package that core packages might reference, then it would need to live in the core.

[martinkuba]

To decouple generating session start/end events, I can think of two approaches:

1. Observer pattern

interface SessionObserver {
  onSessionStarted(sessionId: string): void;
  onSessionEnded(sessionId: string): void;
}

export interface SessionManager {
  getSessionId(): string;
  addObserver(observer: SessionObserver): void;
}

2. EventEmitter pattern

export interface SessionManager {
  getSessionId(): string;
  addEventListener(event: string, listener: (...args: any[]) => void): void;
}

Maybe the SessionManager interface should be an abstract class instead, so that every implementation does not need to implement its own event emitting mechanism. If we did not need multiple observers/listeners, it would simplify things (similar to Android setSessionIdChangeListener).

[t2t2]

I thought of it as similar to sql-common or propagation-utils in the packages folder here, not as a core feature. But I am curious to hear if you feel otherwise.

I guess since it is optional and technically optional -> contrib, just to me contrib (like just the existence of the word, regardless of how false the interpretation may be) always has the feel of being on the 2nd level of stuff being supported just like using otel for web so far

---

1. Observer pattern
2. EventEmitter pattern

I'd vote towards 2nd as it's the pattern js devs would already be used to. Bringing it back to integrating with other systems mentioned above, there can be external APIs that want initial value + call it again with new value when it changes. (and as you can tell from this, supporting multiple listeners definitely something that's gonna be wanted)

Maybe the SessionManager interface should be an abstract class instead, so that every implementation does not need to implement its own event emitting mechanism.

There may be value in both - Interface and base class that handles most of the api, expiration checking, session refreshing, ... Maybe in the end implementations really only need to provide getValue (from storage) and setValue. Or even when written functionally

interface SessionManager {
  getSessionId(): string;
  addEventListener(event: string, listener: (...args: any[]) => void): void;
}

function createSessionManager(
    options,
    getValue: (key: string) => string | undefined | null,
    setValue: (key: string, value: string) => void
): SessionManager {
    /* all of the logic */
}

function createLocalStorageSessionManager(options: Parameters<typeof createSessionManager>[0]) {
    return createSessionManager(
        options,
        (key) => localStorage.getItem(key),
        (key, value) => localStorage.setItem(key, value)
    );
}

function createSessionStorageSessionManager(options: Parameters<typeof createSessionManager>[0]) {
    return createSessionManager(
        options,
        (key) => sessionStorage.getItem(key),
        (key, value) => sessionStorage.setItem(key, value)
    );
}

function createCookieSessionManager(options: Parameters<typeof createSessionManager>[0]) {
    return createSessionManager(
        options,
        (key) => { /* not gonna a full implmentation in example since it's quite a few lines but imagine reading document.cookie */},
        (key, value) => document.cookie = /* yeah same */
    );
}

[bidetofevil]

The general approach looks good to me and fits well with the existing thoughts on session management. I don't have specific comments on the implementation, but echoing some of the existing comments, having a component to transition the session from one to state to another, then wrapping an API to broadcast session changes in an idiomatic way for web locally, seems like the core of what this prototype should provide.

Additionally, things that can be added later (as mentioned by others too) can be additive to this initial proposal (e.g. firing session change events that can be observed external systems, providing an API that gets the current session ID, configuring how session start and ends are triggered, etc.), so keeping this as tight as possible without needing to change the existing public API methods would be nice.

[martinkuba]

Here is an alternative design -
The main difference is using a simpler interface SessionIdProvider as a dependency of the span/log processors instead of SessionManager. SessionManager is no longer an interface, but it implements SessionIdProvider, and can be customized by providing different implementations of its own dependencies (IdGenerator, storage).

[atreat]

Like the design 👍

Commented in the Client-Side SIG but will include it here as well for posterity:

1. We may want to formalize "Session Observation" into an interface and pull out the addObserver(observer: SessionObserver) method into that interface. This will help people who implement a SessionManager declare that this custom type supports session observation and it means the session observation interface is declared for people who would share SessionObserver implementations.

2. I also commented about how including a concept of a "SessionLifecycle" for vendors who want to change what triggers a session start/end. Instead of activity timeout, maybe it's when an app foregrounds or backgrounds. I think if the session observation interface is formalized this ask becomes much less urgent because the SessionManager becomes very lightweight. Implementing a new SessionManager would be(as is currently intended) the expected way to configure this logic.

[martinkuba]

Here is an updated diagram based on feedback. Updates:

• added SessionPublisher interface
• changed SessionIdProvider to SessionProvider
• added previousSession to SessionObserver.onSessionStarted()

[martinkuba]

@atreat

I also commented about how including a concept of a "SessionLifecycle" for vendors who want to change what triggers a session start/end. Instead of activity timeout, maybe it's when an app foregrounds or backgrounds.

I took a stab at this, and it might be tricky. Here is a prototype open-telemetry/opentelemetry-sandbox-web-js#238.

Some observations

• if the lifecycle object should track inactivity, then it needs to be notified somehow that there is activity (I had to include it in the interface, see below)
• if the lifecycle object should initiate start of a session, then it would also need to somehow interact with the storage (on initialization, is there an existing session or do I need to start one?)
• it almost seems like the observers should just interact with the lifecycle

It seems to me like this might get complicated. I wonder if tracking max duration and inactivity works for majority of users, and for those who want something different, they could implement a different SessionProvider?

The lifecycle interface currently looks like this

export interface SessionLifecycle {
  /** Starts tracking a new session */
  start(session: Session): void;

  /** Stop tracking the current session, reset to a clear state */
  clear(): void;

  /** Notifies this class that the session is currently active. */
  bumpSessionActive(): void;

  /** Register a listener function to call when session is ended. */
  onSessionEnded(listener: () => void): void;
}

Implementing a new SessionManager would be(as is currently intended) the expected way to configure this logic.

In the current design, a completely new SessionProvider would have to be implemented. My hope with the SessionManager was to have something that works for most users.

[atreat]

Thanks for looking into it, I agree that formalizing this would cause more complexity than necessary. I think the prescribed approach would be to implement a new SessionProvider (replace the default SessionManager).

That should be enough and providing the SessionManager as a default implementation example should be helpful enough for people would want to customize the behavior.