Preserve moments across session boundaries #541
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #541 +/- ##
==========================================
+ Coverage 79.73% 79.80% +0.07%
==========================================
Files 409 409
Lines 10938 10933 -5
Branches 1614 1614
==========================================
+ Hits 8721 8725 +4
+ Misses 1569 1564 -5
+ Partials 648 644 -4
|
| @@ -54,8 +52,7 @@ internal class EmbraceEventService( | |||
| /** | |||
| * Timeseries of event IDs, keyed on the start time of the event. | |||
| */ | |||
| private val eventIds: NavigableMap<Long, String> = ConcurrentSkipListMap() | |||
| private val eventIdsCache = CacheableValue<List<String>> { eventIds.size } | |||
| private val eventIds = ConcurrentLinkedQueue<String>() | |||
There was a problem hiding this comment.
just curious, why a queue?
There was a problem hiding this comment.
I wanted a collection that can be concurrently modified, had reasonable insertion time, and didn't get copied every time (like CopyOnWriteArrayList) as many writes but few reads are expected here.
| fakeClock.setCurrentTime(fakeClock.now() + 1L) | ||
| assertEquals(4, eventService.findEventIdsForSession(sessionBeginTime, fakeClock.now()).size) | ||
| } | ||
|
|
||
| @Test | ||
| fun `verify close clears the existing events`() { |
There was a problem hiding this comment.
I feel like we could remove this test and just call close on the test below. Or maybe improve the naming on this one because "existing events" is a bit ambiguous.
There was a problem hiding this comment.
Fair enough - removed the old test case.
| eventIds.clear() | ||
| activeEvents.clear() | ||
| logDeveloper("EmbraceEventService", "collections cleaned") | ||
| val activeEventIds = activeEvents.values.map { it.event.eventId } |
There was a problem hiding this comment.
Does the active moments list get updated on timeout?
There was a problem hiding this comment.
Yep, the EventDescription contains a callback to endEvent that is invoked on the timeout. The item is removed from the collection at that point
fractalwrench
force-pushed
the
fix-moment-boundary
branch
from
3cad15b
to
04e8810
Compare
fractalwrench
force-pushed
the
fix-moment-boundary
branch
from
04e8810
to
8191745
Compare
Goal
Preserves moments across session boundaries if they are considered 'active' (i.e. they have not ended yet). The current implementation simply cleared all the events in our collections; however, it does not seem this behavior was ever hooked up properly until the recent
SessionOrchestratorrefactor. This meant that moments would be cleared when the session boundary was transitioned - which happened frequently for the app startup moment.Testing
Added integration + unit test coverage.