Skip to content

Conversation

@winor30
Copy link
Owner

@winor30 winor30 commented Apr 20, 2025

  • Refactor RUM tools for improved performance and clarity
  • Enhance tests for grouped event counts and filtering
  • Enhance RUM tool tests for robustness and coverage
  • chore: bump version to 1.5.0 in package.json

@winor30 winor30 requested a review from Copilot April 20, 2025 09:01
@codecov
Copy link

codecov bot commented Apr 20, 2025

Codecov Report

Attention: Patch coverage is 97.14286% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.66%. Comparing base (9690dd0) to head (a67efe0).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/tools/rum/tool.ts 97.14% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #30      +/-   ##
==========================================
+ Coverage   88.70%   89.66%   +0.95%     
==========================================
  Files          32       32              
  Lines        1222     1238      +16     
  Branches      135      153      +18     
==========================================
+ Hits         1084     1110      +26     
+ Misses        138      128      -10     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the RUM tool handler for improved performance and clarity by replacing an iterative attribute path extraction with a recursive helper function, and includes test enhancements for grouped event counts and filtering.

  • Replace loop-based group path extraction with the new recursive getValueByPath function
  • Skip events missing attributes earlier in the iteration
  • Remove an unused closing brace for cleaner structure
Files not reviewed (1)
  • package.json: Language not supported
Comments suppressed due to low confidence (1)

src/tools/rum/tool.ts:126

  • [nitpick] Consider renaming 'groupPath' to something like 'groupByKeys' for improved clarity on its purpose.
const groupPath = groupBy.split('.') as Array<keyof typeof event.attributes.attributes>

Comment on lines +283 to +293
if (typedObj[key] === undefined) {
return { value: null, found: false }
Copy link

Copilot AI Apr 20, 2025

Choose a reason for hiding this comment

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

If the object property exists but its value is null, the recursive call at line 287 will cast null to a Record<string, unknown>, which can lead to runtime errors. Consider adding an explicit null check to safely handle this scenario.

Suggested change
if (typedObj[key] === undefined) {
return { value: null, found: false }
if (typedObj[key] === undefined || typedObj[key] === null) {

Copilot uses AI. Check for mistakes.
winor30 added 6 commits April 20, 2025 18:59
- Simplify logic for retrieving group values and enhance performance metric handling.
- Introduce a utility function for recursive object value extraction.
- Improve error handling for missing RUM data and streamline session ID management.

Signed-off-by: katsumata <12413150+winor30@users.noreply.github.com>
- Enhance test coverage for grouped event counts in `rum.test.ts`.
- Improve clarity of test descriptions to include application-specific details.
- Add robustness to tests by handling various edge cases and invalid scenarios.

Signed-off-by: katsumata <12413150+winor30@users.noreply.github.com>
- Enhance testing suite for RUM tools to improve coverage and robustness.
- Address various edge cases including empty and invalid data inputs.
- Introduce default metric handling and nested metric path scenarios.
- Validate functionality with non-numeric metric values to ensure error resilience.

Signed-off-by: katsumata <12413150+winor30@users.noreply.github.com>
- Update version number to reflect the latest release.
- Prepare for upcoming features and improvements.

Signed-off-by: katsumata <12413150+winor30@users.noreply.github.com>
- Refactor metric initialization and attribute extraction for improved code clarity and efficiency.
- Enhance the statistics calculation method by implementing functional programming principles.
- Improve readability of conditionals when handling missing event attributes.

Signed-off-by: katsumata <12413150+winor30@users.noreply.github.com>
- Refactor the `get_rum_page_performance` function for improved clarity and efficiency.
- Enhance code readability by replacing an if statement with a conditional expression.

Signed-off-by: katsumata <12413150+winor30@users.noreply.github.com>
@winor30 winor30 merged commit c08835e into main Apr 20, 2025
9 checks passed
@winor30 winor30 deleted the release-1-5-0 branch April 20, 2025 10:01
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