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-160] Collect PerformanceResourceTiming.responseStatus #2587

Merged
merged 3 commits into from
Feb 7, 2024

Conversation

RomanGaignault
Copy link
Contributor

@RomanGaignault RomanGaignault commented Feb 2, 2024

Motivation

To add a response status to resource other than xhr and fetch when available

Changes

use PerformanceResourceTiming.responseStatus in the resource events.
https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStatus

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

@RomanGaignault RomanGaignault requested a review from a team as a code owner February 2, 2024 10:55
@bits-bot
Copy link

bits-bot commented Feb 2, 2024

CLA assistant check
All committers have signed the CLA.

@RomanGaignault
Copy link
Contributor Author

/to-staging

@dd-devflow
Copy link
Contributor

dd-devflow bot commented Feb 2, 2024

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

Commit 1b4b2bf4f4 will soon be integrated into staging-05.

This build is going to start soon! (estimated merge in less than 8m)

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

dd-mergequeue bot added a commit that referenced this pull request Feb 2, 2024
Co-authored-by: roman.gaignault <roman.gaignault@datadoghq.com>
@dd-devflow
Copy link
Contributor

dd-devflow bot commented Feb 2, 2024

🚂 Branch Integration: This commit was successfully integrated

Commit 1b4b2bf4f4 has been merged into staging-05 in merge commit 1e41f28078.

Check out the triggered pipeline on Gitlab 🦊

@dd-devflow dd-devflow bot added the staging-05 label Feb 2, 2024
@@ -142,6 +141,7 @@ function processResourceEntry(
id: generateUUID(),
type,
url: entry.name,
status_code: entry.responseStatus,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to MDN, the responseStatus will be 0 for cross-origin requests, which might happen frequently.
To avoid confusing customers, it could be nice no to send the status_code if the value is 0.

@RomanGaignault
Copy link
Contributor Author

/to-staging

@dd-devflow
Copy link
Contributor

dd-devflow bot commented Feb 5, 2024

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

Commit d5a483da9e will soon be integrated into staging-06.

This build is going to start soon! (estimated merge in less than 9m)

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

dd-mergequeue bot added a commit that referenced this pull request Feb 5, 2024
Co-authored-by: roman.gaignault <roman.gaignault@datadoghq.com>
@dd-devflow
Copy link
Contributor

dd-devflow bot commented Feb 5, 2024

🚂 Branch Integration: This commit was successfully integrated

Commit d5a483da9e has been merged into staging-06 in merge commit 51d8f1e10f.

Check out the triggered pipeline on Gitlab 🦊

Co-authored-by: Aymeric <aymeric.mortemousque@datadoghq.com>
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (df1fccd) 92.78% compared to head (7712960) 92.78%.
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2587   +/-   ##
=======================================
  Coverage   92.78%   92.78%           
=======================================
  Files         235      235           
  Lines        6789     6791    +2     
  Branches     1487     1488    +1     
=======================================
+ Hits         6299     6301    +2     
  Misses        490      490           

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

@RomanGaignault RomanGaignault merged commit 2f0fbbc into main Feb 7, 2024
17 checks passed
@RomanGaignault RomanGaignault deleted the roman/responseStatus branch February 7, 2024 10:43
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.

6 participants