♻️ [RUM-958] Use a performance observable instead of the lifecycle

[amortemousque]

Motivation

The performanceCollection has too many concerns and limitations:

• it forces us to set a central configuration, while sometimes we might want to have slightly different options (ex here for durationThreshold)
• we always listen for every performance entries while in some cases it might not be needed (ex for longtask)
• we cannot use takeRecords() because the observer is centralized and it would take records for all types

The goal of this PR is to be able to use separate performance observer for each entry type:

• The first iteration uses a performanceObservable for performanceResourceTimings
• The next iteration will use it for other types of performance

Changes

• Introduce a lightweight performanceObservable which removes the performanceObservable fallback
• Move retrieveInitialDocumentResourceTiming from performanceCollection.ts in /domain/resource/retrieveInitialDocumentResourceTiming.ts
• Use the performanceObservable to collect performance resource
• Use the performanceObservable in waitPageActivityEnd

Testing

• Local
• Staging
• Unit
• End to end

---

I have gone over the contributing documentation.

[cit-pr-commenter]

Bundles Sizes Evolution

📦 Bundle Name	Base Size	Local Size	𝚫	𝚫%	Status
Rum	160.25 KiB	160.86 KiB	626 B	+0.38%	✅
Logs	57.91 KiB	57.91 KiB	4 B	+0.01%	✅
Rum Slim	108.77 KiB	109.39 KiB	626 B	+0.56%	✅
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)	𝚫
addglobalcontext	0.001	0.002	0.001
addaction	0.030	0.043	0.013
adderror	0.032	0.044	0.013
addtiming	0.001	0.001	0.000
startview	0.893	1.020	0.127
startstopsessionreplayrecording	0.771	0.974	0.203
logmessage	0.020	0.026	0.006

🧠 Memory Performance

Action Name	Base Consumption Memory (bytes)	Local Consumption Memory (bytes)	𝚫 (bytes)
addglobalcontext	22.20 KiB	20.92 KiB	-1308 B
addaction	71.94 KiB	71.11 KiB	-849 B
adderror	88.93 KiB	86.31 KiB	-2681 B
addtiming	21.90 KiB	19.44 KiB	-2520 B
startview	314.64 KiB	334.02 KiB	19.38 KiB
startstopsessionreplayrecording	13.51 KiB	15.01 KiB	1.50 KiB
logmessage	72.84 KiB	70.91 KiB	-1979 B

🔗 RealWorld

[BenoitZugmeyer]

💭 thought: ‏I think we can "clean" this up and don't fake a performance entry like this. Are you planning to do it when we'll migrate the NAVIGATION performance entry to the new createPerformanceObserver?

[amortemousque]

I thought of cleaning it but the PR is already quite substantial. Like you mention, let's discuss it in the following NAVIGATION PR ;)

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 94.35484% with 7 lines in your changes missing coverage. Please review.

Project coverage is 93.64%. Comparing base (c48a9bd) to head (96c47cc).

Files	Patch %	Lines
...s/rum-core/test/emulate/mockPerformanceObserver.ts	87.87%	4 Missing ⚠️
...ages/rum-core/src/browser/performanceObservable.ts	95.55%	2 Missing ⚠️
...ages/rum-core/src/browser/performanceCollection.ts	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2818      +/-   ##
==========================================
+ Coverage   93.31%   93.64%   +0.32%     
==========================================
  Files         263      266       +3     
  Lines        7482     7548      +66     
  Branches     1669     1679      +10     
==========================================
+ Hits         6982     7068      +86     
+ Misses        500      480      -20     

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

[cy-moi]

👏 So nice to have this

[BenoitZugmeyer]

💬 suggestion: ‏this is Safari behavior, but I'm not sure this is the standard behavior. We should at least state this as a comment. Maybe this could be done more explicitly by passing a { synchronousBufferedEntriesCallback: true } flag to mockPerformanceObserver.

[amortemousque]

Hum, the mock always triggers entries synchronously even for non buffered ones. To me, it's more by convenience to avoid playing with the clock each time we use mockPerformanceObserver.
So I'm not sure it's a good idea to add a special behaviour for buffered entries and if we generalise it for any entries adding synchronousBufferedEntriesCallback: true everywhere will be a bit cumbersome. Wdyt?

[BenoitZugmeyer]

Let's not talk about async/sync, but whether the PerformanceObserver callback is called during or after observe(). My point is not to make everything asynchronous, just to make the observe default behavior closer to the standard.

Again, I'd be fine with just documenting this with a comment so we know why it's implemented like this.

[amortemousque]

/to-staging

[dd-devflow]

🚂 Branch Integration: starting soon, median merge time is 0s

Commit 96c47cc4f5 will soon be integrated into staging-27.

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

[dd-devflow]

🚂 Branch Integration: This commit was successfully integrated

Commit 96c47cc4f5 has been merged into staging-27 in merge commit ae24d645c0.

[amortemousque]

/to-staging

[dd-devflow]

🚂 Branch Integration: starting soon, median merge time is 11m

Commit 96c47cc4f5 will soon be integrated into staging-28.

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

[dd-devflow]

🚨 Branch Integration: This merge request has conflicts

We couldn't automatically merge the commit 96c47cc4f5 into staging-28!

You can use this resolution PR: #2849 to fix the conflicts.

If you need support, contact us on Slack #devflow with those details!

[bbrzoska]

After upgrade with this change, we're seeing a new PerformanceObserver instance is created after every user click / action.
Is that the intention of this change? Can you confirm there's no performance impact due to creating and garbage collecting these objects unnecessarily?

[BenoitZugmeyer]

This change should not have a performance impact. Creating a new PerformanceObserver is pretty cheap, and the memory associated to it should be negligeable compared to memory used by the rest of the SDK and/or your webapp.

[bbrzoska]

We do see a noticeable increase in memory usage for RUM-enabled customers after upgrading the SDK from version 4 to version 5:

Dotted line is a week before comparison. You can see clearly when we rolled out the new SDK. The top yellow value is pc95.