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

fix: Fix view.time_spent skews #1596

Merged
merged 2 commits into from
Dec 20, 2023

Conversation

ncreated
Copy link
Member

@ncreated ncreated commented Dec 20, 2023

What and why?

🧰 Fixes the problem of view.time_spent increasing infinitely if view scope tracks dangling resources (ones that were started, but never finished).

How?

The problem originated in the condition meant to close current view scope if next view is started. This piece of logic was executed for every next view, instead of being limited to the succeeding view only.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

  • Run unit tests for Core, RUM, Trace, Logs, CR and WVT
  • Run unit tests for Session Replay
  • Run integration tests
  • Run smoke tests
  • Run tests for tools/

@ncreated ncreated self-assigned this Dec 20, 2023
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Dec 20, 2023

Datadog Report

Branch report: ncreated/fix/view-time-spent-skews
Commit report: cf106ce

❄️ dd-sdk-ios: 0 Failed, 2 New Flaky, 2469 Passed, 0 Skipped, 21m 13.97s Wall Time

New Flaky Tests (2)

  • testGivenCompositeDelegate_whenInterceptingRequests_itDetectFirstPartyHost - NetworkInstrumentationFeatureTests - Last Failure

    Expand for error
     Assertion Failure at NetworkInstrumentationFeatureTests.swift:573: XCTAssertTrue failed
    
  • testConcurrentBinding - URLSessionDataDelegateSwizzlerTests - Last Failure

    Expand for error
     Abort trap. Check error.crash_log for the full crash log.
    

@ncreated ncreated force-pushed the ncreated/fix/view-time-spent-skews branch from f14021a to 2416488 Compare December 20, 2023 13:27
@ncreated ncreated marked this pull request as ready for review December 20, 2023 13:31
@ncreated ncreated requested review from a team as code owners December 20, 2023 13:31
maxep
maxep previously approved these changes Dec 20, 2023
Copy link
Member

@maxep maxep left a comment

Choose a reason for hiding this comment

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

Nice, thanks 🙏

ganeshnj
ganeshnj previously approved these changes Dec 20, 2023
@ncreated ncreated dismissed stale reviews from ganeshnj and maxep via e47336a December 20, 2023 14:39
@ncreated ncreated force-pushed the ncreated/fix/view-time-spent-skews branch from 2416488 to e47336a Compare December 20, 2023 14:39
@ncreated ncreated changed the base branch from develop to hotfix/2.5.1 December 20, 2023 14:40
@ncreated ncreated merged commit bf9f7a0 into hotfix/2.5.1 Dec 20, 2023
9 checks passed
@ncreated ncreated deleted the ncreated/fix/view-time-spent-skews branch December 20, 2023 15:33
This was referenced Dec 20, 2023
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.

3 participants