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

feat: Removed children from segments. #2689

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

bizob2828
Copy link
Member

@bizob2828 bizob2828 commented Oct 31, 2024

Description

This removes storing a segments children on the segment itself. Instead I've introduced a segments property on the transaction trace and added parentId to every segment. This will build the tree of segments when we serialize the trace but also get the children of a segment when we calculate the exclusive time. I'd like to improve the code in a follow up to only generate the tree for a segment once which will introduce some sort of linked list. I also want to remove storing the root segment on every segment and instead store this in the context manager

Related Issues

Closes #2657

@bizob2828 bizob2828 changed the base branch from main to next October 31, 2024 15:14
@bizob2828 bizob2828 marked this pull request as ready for review October 31, 2024 15:14
@bizob2828 bizob2828 requested a review from jsumners-nr October 31, 2024 15:14
@bizob2828 bizob2828 force-pushed the remove-child-segments branch from d1383c6 to e45d857 Compare October 31, 2024 15:17
@bizob2828 bizob2828 changed the title Remove child segments feat: Removed children from segments. Oct 31, 2024
@bizob2828 bizob2828 force-pushed the remove-child-segments branch from e45d857 to 8a8c5e3 Compare October 31, 2024 15:29
Copy link

codecov bot commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.31%. Comparing base (27d8484) to head (3fe4345).
Report is 1 commits behind head on next.

Additional details and impacted files
@@             Coverage Diff             @@
##             next    #2689       +/-   ##
===========================================
+ Coverage   79.12%   97.31%   +18.18%     
===========================================
  Files         281      291       +10     
  Lines       45230    46309     +1079     
===========================================
+ Hits        35790    45065     +9275     
+ Misses       9440     1244     -8196     
Flag Coverage Δ
integration-tests-cjs-18.x 74.24% <96.05%> (?)
integration-tests-cjs-20.x 74.25% <96.05%> (?)
integration-tests-cjs-22.x 74.28% <96.05%> (?)
integration-tests-esm-18.x 49.83% <37.50%> (?)
integration-tests-esm-20.x 49.84% <37.50%> (?)
integration-tests-esm-22.x 49.86% <37.50%> (?)
unit-tests-18.x 88.91% <99.34%> (?)
unit-tests-20.x 88.91% <99.34%> (?)
unit-tests-22.x 88.92% <99.34%> (?)
versioned-tests-18.x 79.10% <94.07%> (+0.01%) ⬆️
versioned-tests-20.x 79.11% <94.07%> (+0.01%) ⬆️
versioned-tests-22.x 79.11% <94.07%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

jsumners-nr
jsumners-nr previously approved these changes Oct 31, 2024
lib/transaction/trace/index.js Show resolved Hide resolved
@bizob2828 bizob2828 merged commit 0f0d0a6 into newrelic:next Oct 31, 2024
23 checks passed
@bizob2828 bizob2828 deleted the remove-child-segments branch October 31, 2024 18:56
bizob2828 added a commit that referenced this pull request Oct 31, 2024
bizob2828 added a commit that referenced this pull request Oct 31, 2024
bizob2828 added a commit that referenced this pull request Nov 7, 2024
bizob2828 added a commit that referenced this pull request Nov 7, 2024
bizob2828 added a commit that referenced this pull request Nov 8, 2024
bizob2828 added a commit that referenced this pull request Nov 8, 2024
bizob2828 added a commit that referenced this pull request Nov 13, 2024
bizob2828 added a commit that referenced this pull request Nov 19, 2024
bizob2828 added a commit that referenced this pull request Dec 4, 2024
bizob2828 added a commit that referenced this pull request Dec 9, 2024
bizob2828 added a commit that referenced this pull request Dec 12, 2024
jsumners-nr pushed a commit to jsumners-nr/node-newrelic that referenced this pull request Dec 13, 2024
bizob2828 added a commit that referenced this pull request Dec 13, 2024
bizob2828 added a commit that referenced this pull request Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Flatten context to a list of segments on transaction trace
2 participants