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

✨ [RUM-6567] Generate new web vitals attribution fields #3251

Merged

Conversation

sethfowler-datadog
Copy link
Contributor

Motivation

This PR adds code to generate the new web vitals attribution fields that were added to the event format in this PR.

For now, the old web vitals fields remain. To remove them safely without breaking backwards compatibility, we need backend changes, so we'll hold off until those are done.

Changes

  • The rum-events-format submodule has been updated to include the new fields.
  • The new fields have been added to the appropriate type definitions.
  • The code to populate the new fields has been added to viewCollection.ts.
  • The tests have been updated appropriately.

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

@sethfowler-datadog sethfowler-datadog requested a review from a team as a code owner January 6, 2025 15:47
@sethfowler-datadog sethfowler-datadog force-pushed the seth.fowler/new-web-vitals-attribution-fields branch from cefc56e to 3954e43 Compare January 6, 2025 15:50
@codecov-commenter
Copy link

codecov-commenter commented Jan 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.50%. Comparing base (e263716) to head (dde4a81).
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3251      +/-   ##
==========================================
- Coverage   93.51%   93.50%   -0.01%     
==========================================
  Files         288      288              
  Lines        7599     7606       +7     
  Branches     1730     1735       +5     
==========================================
+ Hits         7106     7112       +6     
- Misses        493      494       +1     

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

@sethfowler-datadog
Copy link
Contributor Author

/to-staging

@dd-devflow
Copy link
Contributor

dd-devflow bot commented Jan 8, 2025

Devflow running: /to-staging

View all feedbacks in Devflow UI.


2025-01-08 14:19:04 UTC ℹ️ Branch Integration: starting soon, median merge time is 11m40s

Commit 9102f3f453 will soon be integrated into staging-02.


2025-01-08 14:19:27 UTC 🚨 Branch Integration: This merge request has conflicts

We couldn't automatically merge the commit 9102f3f453 into staging-02!

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

dd-devflow bot added a commit that referenced this pull request Jan 8, 2025
@sethfowler-datadog
Copy link
Contributor Author

/to-staging -c

@dd-devflow
Copy link
Contributor

dd-devflow bot commented Jan 8, 2025

Devflow running: /to-staging -c

View all feedbacks in Devflow UI.


2025-01-08 15:43:11 UTCDevflow: /to-staging -c

Cannot cancel integration of 9102f3f453 into staging-02:

This merge request was already processed and can't be unqueued anymore.

To get help about command usage, write /to-staging --help

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

Base automatically changed from v6 to main January 9, 2025 09:14
@sethfowler-datadog
Copy link
Contributor Author

/to-staging

@dd-devflow
Copy link
Contributor

dd-devflow bot commented Jan 9, 2025

Devflow running: /to-staging

View all feedbacks in Devflow UI.


2025-01-09 09:31:39 UTC ℹ️ Branch Integration: starting soon, median merge time is 11m40s

Commit 9102f3f453 will soon be integrated into staging-02.


2025-01-09 09:41:21 UTC 🚨 Branch Integration: The build pipeline contains failing jobs for this merge request

We couldn't automatically merge the commit 9102f3f453 into staging-02.
Build pipeline has failing jobs for ad59a41:

⚠️ Do NOT retry failed jobs directly (why?).

What to do next?

  • Investigate the failures and when ready, re-add your pull request to the queue!
  • If your PR checks are green, try to rebase/merge. It might be because the CI run is a bit old.
  • Any question, go check the FAQ.

If you think the errors come from a logical conflict with the target branch, you can create a fix by commenting this pull request with /create-fix-branch -b staging-02

Details

Since those jobs are not marked as being allowed to fail, the pipeline will most likely fail.
Therefore, and to allow other builds to be processed, this merge request has been rejected and the pipeline got canceled.

@sethfowler-datadog sethfowler-datadog force-pushed the seth.fowler/new-web-vitals-attribution-fields branch from 9102f3f to 6999e09 Compare January 10, 2025 15:30
@sethfowler-datadog
Copy link
Contributor Author

/to-staging

@dd-devflow
Copy link
Contributor

dd-devflow bot commented Jan 10, 2025

Devflow running: /to-staging

View all feedbacks in Devflow UI.


2025-01-10 16:12:23 UTC ℹ️ Branch Integration: starting soon, median merge time is 11m40s

Commit 6999e09ce2 will soon be integrated into staging-02.


2025-01-10 16:25:26 UTC ℹ️ Branch Integration: This commit was successfully integrated

Commit 6999e09ce2 has been merged into staging-02 in merge commit ebdac61980.

Check out the triggered pipeline on Gitlab 🦊

dd-mergequeue bot added a commit that referenced this pull request Jan 10, 2025
…staging-02

Integrated commit sha: 6999e09

Co-authored-by: Seth Fowler <seth.fowler@datadoghq.com>
@sethfowler-datadog
Copy link
Contributor Author

Things LGTM in staging. The old fields are still there:
image
The new fields reflect the same values:
image

@sethfowler-datadog
Copy link
Contributor Author

After clicking around a bit to get INP to show up, here are the old fields:
image
And the new fields:
image

The units aren't always perfectly consistent here because the developer extension hardcodes which fields to display with those units; I'll update it in a followup.

Based on these results, I think this is good to go!

@sethfowler-datadog
Copy link
Contributor Author

/to-staging

Copy link

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 144.54 KiB 145.09 KiB 568 B +0.38%
Logs 51.13 KiB 51.13 KiB 0 B 0.00%
Rum Slim 103.39 KiB 103.90 KiB 526 B +0.50%
Worker 24.50 KiB 24.50 KiB 0 B 0.00%
🚀 CPU Performance
Action Name Base Average Cpu Time (ms) Local Average Cpu Time (ms) 𝚫
addglobalcontext 0.002 0.002 -0.001
addaction 0.068 0.047 -0.021
addtiming 0.001 0.001 0.000
adderror 0.080 0.059 -0.021
startstopsessionreplayrecording 0.008 0.009 0.000
startview 0.501 0.480 -0.021
logmessage 0.030 0.028 -0.002
🧠 Memory Performance
Action Name Base Consumption Memory (bytes) Local Consumption Memory (bytes) 𝚫 (bytes)
addglobalcontext 26.92 KiB 29.22 KiB 2.30 KiB
addaction 53.87 KiB 54.82 KiB 978 B
addtiming 25.94 KiB 29.95 KiB 4.01 KiB
adderror 58.64 KiB 58.29 KiB -361 B
startstopsessionreplayrecording 25.74 KiB 27.51 KiB 1.76 KiB
startview 411.62 KiB 419.04 KiB 7.42 KiB
logmessage 55.83 KiB 58.75 KiB 2.93 KiB

🔗 RealWorld

@dd-devflow
Copy link
Contributor

dd-devflow bot commented Jan 14, 2025

Devflow running: /to-staging

View all feedbacks in Devflow UI.


2025-01-14 16:15:33 UTC ℹ️ Branch Integration: starting soon, median merge time is 10m57s

Commit dde4a81679 will soon be integrated into staging-03.


2025-01-14 16:26:46 UTC ℹ️ Branch Integration: This commit was successfully integrated

Commit dde4a81679 has been merged into staging-03 in merge commit 9cedaef91e.

Check out the triggered pipeline on Gitlab 🦊

dd-mergequeue bot added a commit that referenced this pull request Jan 14, 2025
…staging-03

Integrated commit sha: dde4a81

Co-authored-by: Seth Fowler <seth.fowler@datadoghq.com>
@sethfowler-datadog
Copy link
Contributor Author

OK, I rebased to incorporate the latest changes to the web vitals attribution format. Everything looks good on staging, so I think this is now ready to ship! I'm going to go ahead and merge.

@sethfowler-datadog sethfowler-datadog merged commit 85baa2b into main Jan 14, 2025
20 checks passed
@sethfowler-datadog sethfowler-datadog deleted the seth.fowler/new-web-vitals-attribution-fields branch January 14, 2025 16:55
RomanGaignault pushed a commit that referenced this pull request Jan 22, 2025
* ✨ [RUM-6567] Generate new web vitals attribution fields
RomanGaignault pushed a commit that referenced this pull request Jan 22, 2025
* ✨ [RUM-6567] Generate new web vitals attribution fields
RomanGaignault pushed a commit that referenced this pull request Jan 22, 2025
* ✨ [RUM-6567] Generate new web vitals attribution fields
RomanGaignault pushed a commit that referenced this pull request Jan 22, 2025
* ✨ [RUM-6567] Generate new web vitals attribution fields
RomanGaignault pushed a commit that referenced this pull request Jan 22, 2025
* ✨ [RUM-6567] Generate new web vitals attribution fields
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants