DataDog / browser-sdk

Datadog Browser SDK
Apache License 2.0
294 stars 131 forks source link

🎨 Instrument fetch and XHR before trying to init consent #2834

Closed N-Boutaib closed 3 months ago

N-Boutaib commented 3 months ago

Motivation

When RUM's initialisation is delayed because of consent not being granted initially, some libraries (such as Apollo Client) do store some methods aside to be re-used (mainly window.fetch), which leads to the un-instrumented method being used for http requests, even after consent being granted.

Changes

Instrument fetch method on sdk initialisation. The instrumentation process is idempotent.

Testing


I have gone over the contributing documentation.

codecov-commenter commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.09%. Comparing base (fab7ead) to head (14807bd). Report is 7 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2834 +/- ## ========================================== - Coverage 93.10% 93.09% -0.02% ========================================== Files 248 248 Lines 7242 7246 +4 Branches 1624 1624 ========================================== + Hits 6743 6746 +3 - Misses 499 500 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cit-pr-commenter[bot] commented 3 months ago

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 160.40 KiB 160.42 KiB 18 B +0.01%
Logs 58.02 KiB 58.04 KiB 18 B +0.03%
Rum Slim 108.92 KiB 108.94 KiB 18 B +0.02%
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.000 | | addaction | 0.031 | 0.037 | 0.006 | | adderror | 0.032 | 0.035 | 0.003 | | addtiming | 0.001 | 0.001 | 0.000 | | startview | 0.898 | 0.998 | 0.100 | | startstopsessionreplayrecording | 0.860 | 0.952 | 0.092 | | logmessage | 0.019 | 0.021 | 0.002 |
🧠 Memory Performance | Action Name | Base Consumption Memory (bytes) | Local Consumption Memory (bytes) | 𝚫 (bytes) | | --- | --- | --- | --- | | addglobalcontext | 19.53 KiB | 17.88 KiB | -1698 B | | addaction | 72.93 KiB | 68.45 KiB | -4583 B | | adderror | 86.17 KiB | 85.68 KiB | -500 B | | addtiming | 19.72 KiB | 16.54 KiB | -3251 B | | startview | 314.43 KiB | 320.54 KiB | 6.11 KiB | | startstopsessionreplayrecording | 15.25 KiB | 14.13 KiB | -1155 B | | logmessage | 71.76 KiB | 69.40 KiB | -2424 B |

🔗 RealWorld

BenoitZugmeyer commented 3 months ago

🔨 warning: ‏ I think we have the same issue with logs, don't we?

❓ question: ‏ What about other instrumented methods? It might be less common for them to be used that way, but we could have the same issue if that happens, no?

For both subjects: I would wait for requests to do so before acting on it, as we don't want to over-instrument if we don't need to (in other words, it is better to not instrument if we never have user consent).

N-Boutaib commented 3 months ago

/to-staging

dd-devflow[bot] commented 3 months ago

:steam_locomotive: Branch Integration: starting soon, median merge time is 10m

Commit d3f3bd6f78 will soon be integrated into staging-26.

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

dd-devflow[bot] commented 3 months ago

:warning: Branch Integration: This commit was already integrated

Commit d3f3bd6f78 had already been merged into staging-26

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