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.

Sign up for GitHub

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

Jump to bottom

Refactor Critical Path code to prevent it from ever throwing exceptions #1785

Merged
merged 4 commits into from
Sep 14, 2023
Merged

Refactor Critical Path code to prevent it from ever throwing exceptions #1785

merged 4 commits into from
Sep 14, 2023

Conversation

GLVSKiriti
Copy link
Contributor

Resolves #1779

In this PR switch statement is refactored to if-else block and covered all of our decision space.

@GLVSKiriti
sanitizeOverFlowingChildren function code is refactored
a015901 
Signed-off-by: GLVS Kiriti <glvskiriti2003369@gmail.com>
@codecov
Copy link

codecov bot commented Sep 13, 2023 •
edited
Loading

Codecov Report

Patch coverage is 100.00% of modified lines.

Files Changed Coverage
...mponents/TracePage/CriticalPath/testCases/test9.js 100.00%
...CriticalPath/utils/sanitizeOverFlowingChildren.tsx 100.00%

📢 Thoughts on this report? Let us know!.

@GLVSKiriti
Reduced further nesting and modified to suggested changes
4467206 
Signed-off-by: GLVS Kiriti <glvskiriti2003369@gmail.com>
@GLVSKiriti
Correcred a typo in sanitizeOverFlowingChildren in filtering childSpa…
c127eea 
…nIds when a childSpan is dropped

Signed-off-by: GLVS Kiriti <glvskiriti2003369@gmail.com>
yurishkuro
yurishkuro previously approved these changes Sep 14, 2023
View reviewed changes
@GLVSKiriti
Added a new test case that is child span ends before parent span
9da89cc 
Signed-off-by: GLVS Kiriti <glvskiriti2003369@gmail.com>
@yurishkuro yurishkuro changed the title To Prevent Critical Path code from throwing exceptions by coverage of decision space Refactor Critical Path code to prevent it from ever throwing exceptions Sep 14, 2023
@yurishkuro yurishkuro merged commit 7ef6909 into jaegertracing:main Sep 14, 2023
9 checks passed
@yurishkuro
Copy link
Member

Thanks!

This was referenced Sep 14, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yurishkuro yurishkuro yurishkuro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: Prevent Critical Path code from throwing exceptions
2 participants
@GLVSKiriti @yurishkuro