-
Notifications
You must be signed in to change notification settings - Fork 484
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
Usage of a helper for trace name when preparing trace data #544
Usage of a helper for trace name when preparing trace data #544
Conversation
Usage of a helper for trace name resolution when preparing traces data. It affects Search and Compare pages. Signed-off-by: Valerii Varankin <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #544 +/- ##
==========================================
- Coverage 93.00% 92.89% -0.12%
==========================================
Files 197 200 +3
Lines 4848 4912 +64
Branches 1177 1212 +35
==========================================
+ Hits 4509 4563 +54
- Misses 299 309 +10
Partials 40 40
Continue to review full report at Codecov.
|
Any updates here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can there be a unit test?
* Test for trace without an ID * Test for trace with remote (missing) root span * Test for trace with root span without refs Signed-off-by: Valerii Varankin <[email protected]>
Added some tests. |
key: 'http.status_code', | ||
type: 'float64', | ||
value: 200, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: are these tags relevant to the functionality being tested? They don't look like it. Could you please trim the fixtures to minimally required data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable, thanks :)
Removed unnecessary tags.
Tags has been removed since it are not relevant to the functionality being tested Signed-off-by: Valerii Varankin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
* Usage of getTraceName() when preparing trace data Usage of a helper for trace name resolution when preparing traces data. It affects Search and Compare pages. Signed-off-by: Valerii Varankin <[email protected]> * Tests for transform-trace-data helper * Test for trace without an ID * Test for trace with remote (missing) root span * Test for trace with root span without refs Signed-off-by: Valerii Varankin <[email protected]> * Removed unnecessary data from tests Tags has been removed since it are not relevant to the functionality being tested Signed-off-by: Valerii Varankin <[email protected]> Signed-off-by: vvvprabhakar <[email protected]>
In continue of #541.
Now there is a
getTraceName()
method that can deal with a "root spans" with missing references. But it doesn't use when traces data transforms for a "Search" and "Compare" pages.Seems like it should be using here.