Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RUMF-1169] cleanup compute stack trace #1335

Merged
merged 5 commits into from
Feb 15, 2022

Conversation

BenoitZugmeyer
Copy link
Member

@BenoitZugmeyer BenoitZugmeyer commented Feb 11, 2022

Motivation

Reduce bundles size by removing old browser support in tracekit imported code. Those browsers are not supported by our SDK in the first place (the code won't execute at all), so it won't have any impact.

Changes

Remove various strategies to compute the call stack. I made sure that removed code cannot be used in supported browsers:

  • old Opera (presto-based) browsers don't support WeakMaps, which is a requirement for our SDK

  • the IE11 errors may not have a stack (they need to be thrown to have a stack), but the fallback strategy to produce a stack by walking the caller stack wasn't working because our SDK is evaluated in strict mode. Also, this strategy would probably return something similar to the "handling stack" instead of the actual error stack.

Before:

du -b packages/*/bundle/*.js
35349	packages/logs/bundle/datadog-logs.js
66922	packages/rum-slim/bundle/datadog-rum-slim.js
116266	packages/rum/bundle/datadog-rum.js

After

du -b packages/*/bundle/*.js
32539	packages/logs/bundle/datadog-logs.js
64113	packages/rum-slim/bundle/datadog-rum-slim.js
113457	packages/rum/bundle/datadog-rum.js

8.6% size reduction for logs bundle 🎉

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

@codecov-commenter
Copy link

Codecov Report

Merging #1335 (2fca316) into main (f4a4758) will increase coverage by 0.98%.
The diff coverage is 97.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1335      +/-   ##
==========================================
+ Coverage   89.98%   90.96%   +0.98%     
==========================================
  Files         105      105              
  Lines        4382     4238     -144     
  Branches      990      946      -44     
==========================================
- Hits         3943     3855      -88     
+ Misses        439      383      -56     
Impacted Files Coverage Δ
...ages/core/src/domain/tracekit/computeStackTrace.ts 98.36% <97.91%> (+26.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4a4758...2fca316. Read the comment docs.

@BenoitZugmeyer BenoitZugmeyer merged commit 525f22e into main Feb 15, 2022
@BenoitZugmeyer BenoitZugmeyer deleted the benoit/cleanup-compute-stack-trace branch February 15, 2022 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants