Skip to content
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.

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

Session handling #229

Open
wants to merge 3 commits into
base: web-sdk
Choose a base branch
from

Conversation

martinkuba
Copy link
Collaborator

This PR includes a draft of managing session ID.

  • adds a new package web-common, so that the components can be used outside of the Web SDK
  • adds two SpanProcessors and LogRecordProcessor for adding the session.id attribute
  • adds SessionManager concept: class that is responsible for managing the session (storage, expiration etc.), it has a single getSessionId() method
  • the two processors have a dependency on the manager
  • the manager can have different implementations

@martinkuba martinkuba changed the title session handling Session handling Jun 25, 2024
Copy link

@jpmunz jpmunz left a comment

Choose a reason for hiding this comment

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

the web app in opentelemetry-demo does a manual version of this, once merged could be a good demonstrate of the improvement to update it to use this manager instead:

if (sessionId === null) {
const generateSessionId = getIdGenerator(16);
sessionId = generateSessionId();
sessionStorage.setItem(SESSION_STORAGE_KEY, sessionId);
Copy link

Choose a reason for hiding this comment

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

it may be worth also storing the created/last accessed time and allowing the manager optionally to be configured with how much time can pass before considering the old session to be expired, here's some of that being done on Android for reference: https://github.com/open-telemetry/opentelemetry-android/blob/main/android-agent/src/main/java/io/opentelemetry/android/SessionId.java

* returns true.
* @param span the Span that just ended.
*/
onEnd(_: ReadableSpan): void {

Choose a reason for hiding this comment

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

Do we want to clear the session once it ends?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This function is called for each span when it ends. As far as clearing the session.id attribute, it is retrieved from the manager for every span (not cached).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants