feat: Add accessor for current session id#1810
Conversation
refs: RUM-1962
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1810 +/- ##
===========================================
+ Coverage 83.44% 83.47% +0.04%
===========================================
Files 466 466
Lines 16375 16381 +6
Branches 2455 2456 +1
===========================================
+ Hits 13663 13674 +11
+ Misses 2047 2030 -17
- Partials 665 677 +12
🚀 New features to boost your workflow:
|
| * The current active session id. May return null if a session has not been started yet. | ||
| * Will return a "null session id" (all zeros) if the current session is not tracked. |
There was a problem hiding this comment.
| * The current active session id. May return null if a session has not been started yet. | |
| * Will return a "null session id" (all zeros) if the current session is not tracked. | |
| * The current active session ID. May return null if a session has not been started yet. | |
| * Will return a "null session ID" (all zeros) if the current session is not tracked. |
| * The current active session id. May return null if a session has not been started yet. | ||
| * Will return a "null session id" (all zeros) if the current session is not tracked. |
There was a problem hiding this comment.
To be precise, it won't be all zeros, but 00000000-0000-0000-0000-000000000000, so user cannot write something like if (currentSessionId.all {it == '0'}), so maybe for the sake of comparison with null session mentioned in the documentation, we need to expose such value?
|
|
||
| // region RumMonitor | ||
|
|
||
| override val currentSessionId: String? |
There was a problem hiding this comment.
This method is not thread-safe, because all the values inside RUM scopes hierarchy are set from the dedicated RUM thread, but currentSessionId can be called from any user thread. IMO it is not a big deal here, but if use-case from the user side requires consistency and precision, we may need to make this one thread-safe to avoid concurrency issues.
| } | ||
|
|
||
| @Test | ||
| fun `M returns null id for current session W no session started`() { |
There was a problem hiding this comment.
| fun `M returns null id for current session W no session started`() { | |
| fun `M return null ID for current session W currentSessionId { no session started }`() { |
| } | ||
|
|
||
| @Test | ||
| fun `M currentSessionId returns correct sessionId W session started { sampled in }`( |
There was a problem hiding this comment.
| fun `M currentSessionId returns correct sessionId W session started { sampled in }`( | |
| fun `M return correct sessionId W currentSessionId { session started, sampled in }`( |
| mockSessionListener | ||
| ) | ||
|
|
||
| assertThat(testedMonitor.currentSessionId).isEqualTo(RumContext.NULL_UUID) |
There was a problem hiding this comment.
This is not aligned to what documentation of currentSessionId is saying:
May return null if a session has not been started yet.
| } | ||
|
|
||
| @Test | ||
| fun `M currentSessionId returns null session W session started { sampled out }`( |
There was a problem hiding this comment.
| fun `M currentSessionId returns null session W session started { sampled out }`( | |
| fun `M return null session W currentSessionId { session started, sampled out }`( |
| val sessionScope = applicationScope?.activeSession as? RumSessionScope | ||
| if (sessionScope != null && sessionScope.sessionState == RumSessionScope.State.NOT_TRACKED) { | ||
| // Aligns with iOS which assigns a NULL session id when the session is not tracked | ||
| return RumContext.NULL_UUID |
There was a problem hiding this comment.
For "readability" purposes, in link with Nikita's comment above, maybe we should return null instead here. This would make it easier for customers to not use a "000…" value which doesn't make sense?
0xnm
left a comment
0xnm
left a comment
There was a problem hiding this comment.
lgtm! I've added a minor suggestion
| // Then | ||
| completableFuture.thenAccept { | ||
| assertThat(it).isEqualTo(null) | ||
| }.orTimeout(100, TimeUnit.MILLISECONDS) |
There was a problem hiding this comment.
| }.orTimeout(100, TimeUnit.MILLISECONDS) | |
| }.orTimeout(PROCESSING_DELAY, TimeUnit.MILLISECONDS) |
same to change for similar lines below
28d15ef to
6eeebd2
Compare
5 participants
What does this PR do?
There are use cases (usually support use cases) where clients what to know the current session id without depending on the
sessionStartedevent. This provides that accessor.refs: RUM-1962
Review checklist (to be filled by reviewers)