Skip to content

[RUM-9272] Add configurable time for classifying resources for TNS metric#835

Merged
sbarrio merged 1 commit into
developfrom
sbarrio/RUM-9272/add-configurable-threshold-for-resources-on-TNS-metric
Apr 4, 2025
Merged

[RUM-9272] Add configurable time for classifying resources for TNS metric#835
sbarrio merged 1 commit into
developfrom
sbarrio/RUM-9272/add-configurable-threshold-for-resources-on-TNS-metric

Conversation

@sbarrio
Copy link
Copy Markdown
Contributor

@sbarrio sbarrio commented Mar 31, 2025

What does this PR do?

This PR adds a new config property called initialResourceThreshold, which allows users to set the maximum time threshold (counting from view start, in seconds) inside of which a resource should be considered for the "Time to Network Settled" metric.

Motivation

Exposing custom TNS settings for clients.

Additional notes

RN config
Screenshot 2025-03-31 at 16 38 46

iOS custom TimeBasedNetworkSettledPredicate set
Screenshot 2025-03-31 at 16 38 38

Android custom initialResourceIdentifier set
Screenshot 2025-03-31 at 16 46 41

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)
  • If this PR is auto-generated, please make sure also to manually update the code related to the change

@sbarrio sbarrio self-assigned this Mar 31, 2025
@sbarrio sbarrio marked this pull request as ready for review April 1, 2025 07:24
@sbarrio sbarrio requested a review from a team as a code owner April 1, 2025 07:24
@sbarrio sbarrio force-pushed the sbarrio/RUM-9272/add-configurable-threshold-for-resources-on-TNS-metric branch 2 times, most recently from fbb8158 to 3ab1116 Compare April 1, 2025 14:42
Copy link
Copy Markdown
Member

@marco-saia-datadog marco-saia-datadog left a comment

Choose a reason for hiding this comment

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

Looks good! My only suggestion is to add a few tests on the React Native layer as well.

Comment thread packages/core/ios/Tests/DdSdkTests.swift Outdated
@sbarrio
Copy link
Copy Markdown
Contributor Author

sbarrio commented Apr 3, 2025

Looks good! My only suggestion is to add a few tests on the React Native layer as well.

Agree, will do!

@marco-saia-datadog
Copy link
Copy Markdown
Member

An iOS check is also failing:

❌ /Users/ec2-user/builds/JBp7x9hf6/0/DataDog/dd-sdk-reactnative/packages/core/ios/Tests/DdSdkTests.swift:1138:57: cannot find 'onThresholdLImitResource' in scope
        XCTAssertTrue(predicate.isInitialResource(from: onThresholdLImitResource))

@sbarrio
Copy link
Copy Markdown
Contributor Author

sbarrio commented Apr 3, 2025

An iOS check is also failing:

❌ /Users/ec2-user/builds/JBp7x9hf6/0/DataDog/dd-sdk-reactnative/packages/core/ios/Tests/DdSdkTests.swift:1138:57: cannot find 'onThresholdLImitResource' in scope
        XCTAssertTrue(predicate.isInitialResource(from: onThresholdLImitResource))

Ah, of course, I renamed it but didn't change the test. On it!

@sbarrio sbarrio force-pushed the sbarrio/RUM-9272/add-configurable-threshold-for-resources-on-TNS-metric branch from 7da80eb to 4637a8f Compare April 3, 2025 10:44
Copy link
Copy Markdown
Member

@marco-saia-datadog marco-saia-datadog left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@sbarrio sbarrio merged commit a30c270 into develop Apr 4, 2025
8 checks passed
@sbarrio sbarrio deleted the sbarrio/RUM-9272/add-configurable-threshold-for-resources-on-TNS-metric branch April 4, 2025 07:45
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.

2 participants