🐛 [RUM-4434] fix timing matching for the same resource requested twice at the same time

[thomas-lebeau]

Motivation

Fixes an issue where resource timing where not matched to a resource when the same resource is requested twice at the same time.

e.g.

Promise.all([
    fetch("http://localhost:8080/"),
    fetch("http://localhost:8080/")
])

The issue is that when the second request resolves, we have 2 resource timings matching the request hence we were not able to decide which one is the correct one.

Changes

When a match is found between a resource timing and a request, we store the timing in a WeakMap, that way we can filter it out of the available timings when the 2nd resource resolve.

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	157.60 KiB	157.93 KiB	340 B	+0.21%	✅
Logs	56.29 KiB	56.29 KiB	0 B	0.00%	✅
Rum Slim	104.09 KiB	104.42 KiB	340 B	+0.32%	✅
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.030	0.042	0.012
addaction	0.014	0.017	0.002
logmessage	0.005	0.005	0.000
startview	0.804	0.947	0.143
startstopsessionreplayrecording	0.762	0.830	0.068
addtiming	0.001	0.001	-0.000
addglobalcontext	0.001	0.001	-0.000

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.28%. Comparing base (b0d638c) to head (84d8e5e).
Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2747      +/-   ##
==========================================
+ Coverage   93.27%   93.28%   +0.01%     
==========================================
  Files         241      241              
  Lines        7034     7049      +15     
  Branches     1554     1555       +1     
==========================================
+ Hits         6561     6576      +15     
  Misses        473      473              

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

[bcaudan]

❓ question: any evidence that when two entries are matching the same timing, one entry has already matched another timing?

[thomas-lebeau]

In the case of fetch, having two timing matching the same resource without being already matched, means that both timings are available before the then of the first fetch is called.

This is not the case when doing Promise.all() but I don't exclude the possibility of it happening. In that case no timing will be matched.

[bcaudan]

Would it be worth doing more experiments on browser behaviours around that? or measure the effectiveness of this change somehow?

[thomas-lebeau]

I'm adding a e2e test, however I've noticed that safari doesn't do like everyone and both PerformanceResourceTiming might be available at the time the first resource is matching it's timings.

This means that this PR reduce the amount of occurrence of missing resource timing on concurrent event (mainly chrome/FF) but does not prevent it totally (safari)

anyone has a better idea?

[thomas-lebeau]

as discussed during parking lot, this PR won't solve everything but is improving in some cases. So I've skipped the e2e on non chromium browsers as it's not reliable there.

[thomas-lebeau]

/to-staging

[dd-devflow]

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

Commit 84d8e5e469 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 already integrated

Commit 84d8e5e469 had already been merged into staging-20

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