Spans Table View #781
Spans Table View #781
Conversation
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
vvvprabhakar
requested review from
everett980,
rubenvp8510 and
tiffon
as code owners
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
…into ISSUE_690
|
The tests seem to be failing. Are you able to sort them out? |
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
|
@jpkrohling Need your help in fixing this unit case(1 case) |
|
Hi @vvvprabhakar there is a small bug on the Check this commit I did on my own fork: It was a coincidence that in the past With this change your tests should pass, at least it pass on my local. :) The code overall looks good, Once the tests are fixed I'll do another review. |
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
|
@rubenvp8510 Thanks a lot man |
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
…into ISSUE_690
|
@rubenvp8510 can you review once |
|
Once you get time can you please review this |
|
ping @rubenvp8510 |
Codecov Report
@@ Coverage Diff @@
## master #781 +/- ##
==========================================
+ Coverage 95.07% 95.09% +0.01%
==========================================
Files 230 231 +1
Lines 7010 7097 +87
Branches 1745 1759 +14
==========================================
+ Hits 6665 6749 +84
- Misses 339 342 +3
Partials 6 6
Continue to review full report at Codecov.
|
|
I'll review this tomorrow morning, overall looks good but I want to see it on detail |
packages/jaeger-ui/src/components/TracePage/TraceSpanView/index.tsx
Outdated
Show resolved
Hide resolved
packages/jaeger-ui/src/components/TracePage/TraceSpanView/index.tsx
Outdated
Show resolved
Hide resolved
| const operationNames = serviceNameOperationsMap.get(span.process.serviceName) || []; | ||
| operationNames.push(span.operationName); | ||
| serviceNameOperationsMap.set(span.process.serviceName, operationNames); | ||
| return { serviceNamesList, operationNamesList }; |
There was a problem hiding this comment.
Why are your returning this ? where this is used?
There was a problem hiding this comment.
We are not using it removed
|
Can we add a couple of tests to the new views? at least basic testing? |
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
|
@rubenvp8510 made the changes you proposed |
|
Thanks for doing the changes! I'll review tomorrow morning! |
|
Did you get a chance to see this? |
| it('Should change value when onChange was called', () => { | ||
| const event = ['service2']; | ||
| wrapper = shallow(<TraceSpanView {...defaultProps} />); | ||
| // wrapper.state('filtered.process.serviceName', ['service1', 'service2']); |
There was a problem hiding this comment.
Remove this comment please.
|
Small comment, overall looks good. I just need to do a quick smoke test and will merge. |
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
|
@rubenvp8510 Removed the line and pushed it |
|
@rubenvp8510 did you get time to see this ? |
|
I've already approved but it will be good if we can increase the coverage a little bit. may be we can write tests for https://github.com/jaegertracing/jaeger-ui/pull/781/files#diff-00ec9fc101b0b3db820b0352d451f88833915d001216cf78d40ac1178a65be28R148 |
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Head branch was pushed to by a user without write access
|
Thanks for reviewing it |
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
…into ISSUE_690
Head branch was pushed to by a user without write access
|
Coverage was 0.05% less, so covered it, when you have time just check this @rubenvp8510 |
Signed-off-by: vvvprabhakar vvvprabhakar@gmail.com
Why trace span table? Issue(#690)
A trace with many spans trace (~20K) spans can be broken down into pieces and analyzed. Have a link with
traceTimeline Viewas well to link both views and get the analysis of traceGives a detailed view of a service or an operation and its impact on a trace,
What can we achieve?
Filter Spans with multiple serviceName values
Filter Spans with multiple OperationName values
Filter with both ServiceName and OperationName
Sort based on duration and startTime
Each row in the table has spanID link which will take the user to that particular span in
traceTimelineViewEither search with text on serviceName and OperatioName Columns
Pagination for the spans
What can we improve from here?
Dependencies Used
**
**