🐛 [RUM-2720] Send logs without session id when session inactive

[amortemousque]

Motivation

Do not drop logs when session inactive

⚠ it's worth reviewing commit by commit

Changes

• Keep untracked session in sessionContextHistory
• Allow to find expired session
• Send logs when a session is expired
• Only set the session id if the session is active

Testing

• Local
• Staging
• Unit
• End to end

---

I have gone over the contributing documentation.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.32%. Comparing base (5825bd2) to head (ff8b196).
Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2578   +/-   ##
=======================================
  Coverage   93.31%   93.32%           
=======================================
  Files         241      242    +1     
  Lines        7062     7069    +7     
  Branches     1561     1567    +6     
=======================================
+ Hits         6590     6597    +7     
  Misses        472      472           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[amortemousque]

/to-staging

[dd-devflow]

🚂 Branch Integration: starting soon, merge in < 8m

Commit 4a5df21851 will soon be integrated into staging-05.

This build is going to start soon! (estimated merge in less than 8m)

Use /to-staging -c to cancel this operation!

[dd-devflow]

🚨 Branch Integration: The build pipeline was cancelled for this merge request

Build pipeline was cancelled for cb25103

If you think this is a mistake, you can re-add your pull request to the queue!

Details

gitlab pipeline was cancelled link

If you need support, contact us on Slack #ci-interfaces with those details!

[cit-pr-commenter]

Bundles Sizes Evolution

📦 Bundle Name	Base Size	Local Size	𝚫	𝚫%	Status
Rum	157.94 KiB	157.97 KiB	35 B	+0.02%	✅
Logs	56.42 KiB	56.72 KiB	303 B	+0.52%	✅
Rum Slim	104.43 KiB	104.46 KiB	35 B	+0.03%	✅
Worker	25.21 KiB	25.21 KiB	0 B	0.00%	✅

🚀 CPU Performance

Action Name	Base Average Cpu Time (ms)	Local Average Cpu Time (ms)	𝚫
adderror	0.049	0.032	-0.017
addaction	0.021	0.015	-0.006
logmessage	0.006	0.005	-0.002
startview	1.565	0.806	-0.759
startstopsessionreplayrecording	1.020	0.705	-0.316
addtiming	0.001	0.001	-0.000
addglobalcontext	0.002	0.001	-0.001

[nulrich]

/to-staging

[dd-devflow]

🚂 Branch Integration: starting soon, merge in < 9m

Commit f1c07e04b4 will soon be integrated into staging-20.

This build is going to start soon! (estimated merge in less than 9m)

Use /to-staging -c to cancel this operation!

[dd-devflow]

🚂 Branch Integration: This commit was successfully integrated

Commit f1c07e04b4 has been merged into staging-20 in merge commit 4e866dcbea.

Check out the triggered pipeline on Gitlab 🦊

[amortemousque]

💬 suggestion: ‏Could be nice to add a comment: // TODO next major: remove this option and make it the default behaviour

[nulrich]

/to-staging

[dd-devflow]

🚂 Branch Integration: starting soon, merge in < 9m

Commit 3782d12e3c will soon be integrated into staging-20.

This build is going to start soon! (estimated merge in less than 9m)

Use /to-staging -c to cancel this operation!

[dd-devflow]

🚂 Branch Integration: This commit was successfully integrated

Commit 3782d12e3c has been merged into staging-20 in merge commit fda0dd1e82.

Check out the triggered pipeline on Gitlab 🦊

[BenoitZugmeyer]

I can see there is three ways of getting inactive sessions:

• in valueHistory, there is the AFTER_ENTRY_START predicate
• in sessionManager, there is a different method findInactiveOrExpiredSession
• in logsSessionManager, there is a { returnExpired } option argument.

I would expect to use the same for all three. I like the option object, but not so much the "expired" name, as we already use "active", I think we should use "inactive".

So my suggestion:

• in valueHistory, have find(startTime, { returnInactive })
• in sessionManager, introduce a findSession(startTime, { returnInactive }), that handles both cases (remove other find*Session methods)
• in logsSessionManager, have findTrackedSession(startTime, { returnInactive })

(returnInactive should default to false in valueHistory, and you can just pass the options down without worrying about the default in other places)

[BenoitZugmeyer]

🥜 nitpick: ‏this should be in a test helper, as we're doing it in many places

[BenoitZugmeyer]

💬 suggestion: ‏I think isActiveAt complicates things quite a bit. It might be simpler to just call findTrackedSession multiple times:

const session = sessionManager.findTrackedSession(startTime)

if (
  !session &&
  (!configuration.sendLogsAfterSessionExpiration || !sessionManager.findTrackedSession(startTime, { returnInactive: true }))
) {
  return
}

// and then

session_id: session?.id

[amortemousque]

Interesting! It removes the needs to add endTime to SessionContext. I like it 👍