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

Enable foreground tracking for everyone #961

Closed
wants to merge 3 commits into from

Conversation

ThibautGeriz
Copy link
Contributor

@ThibautGeriz ThibautGeriz commented Jul 26, 2021

Motivation

Enable foreground tracking for everyone

Changes

Remove experimental flag


I have gone over the contributing documentation.

@ThibautGeriz ThibautGeriz requested a review from a team as a code owner July 26, 2021 15:04
@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2021

Codecov Report

Merging #961 (048d68d) into main (cb113a5) will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #961      +/-   ##
==========================================
+ Coverage   89.08%   89.14%   +0.06%     
==========================================
  Files          83       83              
  Lines        3866     3861       -5     
  Branches      857      856       -1     
==========================================
- Hits         3444     3442       -2     
+ Misses        422      419       -3     
Impacted Files Coverage Δ
packages/rum-core/src/boot/startRum.ts 37.03% <100.00%> (ø)
packages/rum-core/src/domain/foregroundContexts.ts 100.00% <100.00%> (+4.76%) ⬆️

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 cb113a5...048d68d. Read the comment docs.

@ThibautGeriz ThibautGeriz force-pushed the thibaut.gery/track-foreground-by-default branch from bba4a58 to 0e57a68 Compare July 27, 2021 09:07
@@ -60,10 +51,6 @@ function addNewForegroundPeriod() {
}
const currentForegroundPeriod = foregroundPeriods[foregroundPeriods.length - 1]
if (currentForegroundPeriod !== undefined && currentForegroundPeriod.end === undefined) {
addMonitoringMessage('Previous foreground periods not closed. Continuing current one', {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about other messages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK, They are not triggered at the moment, I would like to know if we change something and those limit case start to trigger.

Copy link
Contributor

Choose a reason for hiding this comment

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

By looking at the logs I shared with you earlier, we can see some occurrences for all of them.

@ThibautGeriz
Copy link
Contributor Author

Closed for #1053

@bcaudan bcaudan deleted the thibaut.gery/track-foreground-by-default branch September 13, 2022 14:55
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