feat(web): Add session handling implementation #5173
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5173 +/- ##
==========================================
+ Coverage 93.19% 93.25% +0.06%
==========================================
Files 314 315 +1
Lines 8077 8144 +67
Branches 1622 1636 +14
==========================================
+ Hits 7527 7595 +68
+ Misses 550 549 -1
|
| * SessionManager is responsible for managing the active session including starting, | ||
| * resetting, and persisting. | ||
| */ | ||
| export class SessionManager implements SessionProvider, SessionPublisher { |
There was a problem hiding this comment.
Hmm, is there any reason for users to inherit from SessionManager? 🤔
If yes, would they be able to accomplish the same goal by using composition instead?
If everything can be done can be done with composition, I'd recommend only exporting a factory function that returns a SessionProvider & SessionPublisher - this way we can cut down on the public API quite a bit which will simplify future changes.
| @@ -13,6 +13,8 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se | |||
|
|
|||
| ### :rocket: (Enhancement) | |||
|
|
|||
| * feat(web): add session handling implementation [5173](https://github.com/open-telemetry/opentelemetry-js/pull/5173) | |||
There was a problem hiding this comment.
| * feat(web): add session handling implementation [5173](https://github.com/open-telemetry/opentelemetry-js/pull/5173) | |
| * feat(web): add session handling implementation [#5173](https://github.com/open-telemetry/opentelemetry-js/pull/5173) @martinkuba |
|
|
||
| import { SessionIdGenerator } from './types/SessionIdGenerator'; | ||
|
|
||
| export class DefaultIdGenerator implements SessionIdGenerator { |
There was a problem hiding this comment.
same question here - if users can use composition instead, I'd advocate against exporting the class to make future changes easier. 🙂
| sessionManager = new SessionManager(config); | ||
|
|
||
| sessionManager.getSessionId(); // Starts session-1 | ||
| await wait(600); |
There was a problem hiding this comment.
Hmm, would it be possible to use sinon fake timers instead to avoid depending on actual timing for these tests? 🤔
Which problem is this PR solving?
Follow up to #4972
Discussed in issue open-telemetry/opentelemetry-js-contrib#2358
Short description of the changes
This adds an implementation of session management for web. It introduces a
SessionManagerclass that is responsible for starting, ending, and persisting sessions.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Checklist: