Change antctl traceflow command to output results at timeout/failure #1879
Conversation
| // Still output the Traceflow results if any. | ||
| if res == nil || len(res.Status.Results) == 0 { | ||
| return errors.New("timeout waiting for Traceflow results") | ||
| } |
There was a problem hiding this comment.
shouldn't we still return a timeout error eventually after outputting the (partial?) result?
There was a problem hiding this comment.
We can do that too. In your mind should antctl exit with an error in that case?
There was a problem hiding this comment.
Ok. Updated the code.
There was a problem hiding this comment.
if err := output(res); err != nil { should not override err?
It is a new err variable defined in the if block.
There was a problem hiding this comment.
Oh, sorry, you're right... Become careless after holiday :(
| }); err != nil { | ||
| }) | ||
| if err == wait.ErrWaitTimeout { | ||
| err = errors.New("timeout waiting for Traceflow done") |
There was a problem hiding this comment.
Please correct me if my understanding is wrong.
It seems error "timeout waiting for Traceflow done" is only for L156 to return. On L162, its value is overwritten by output(). Does it really output "timeout waiting for Traceflow done" after outputting partial traceflow result?
I think we can update code like below:
var timeoutErr error
if err == wait.ErrWaitTimeout {
timeoutErr = errors.New("timeout waiting for Traceflow done")
// Still output the Traceflow results if any.
if res == nil {
return timeoutErr
}
} else if err != nil {
return fmt.Errorf("error when retrieving Traceflow: %w", err)
}
if err := output(res); err != nil {
return fmt.Errorf("error when outputing result: %w", err)
}
return timeoutErr
There was a problem hiding this comment.
This is not a mistake. Plesae ignore.
pkg/antctl/raw/traceflow/command.go
Outdated
| return fmt.Errorf("error when retrieving Traceflow: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
| if err := output(res); err != nil { | ||
| return fmt.Errorf("error when outputing result: %w", err) |
There was a problem hiding this comment.
| return fmt.Errorf("error when outputing result: %w", err) | |
| return fmt.Errorf("error when outputting result: %w", err) |
Output the Traceflow results, as long as there are any results returned, so users can know the progress of the Traceflow session even the antctl command is timeout, or the Traceflow operation fails.
|
/test-all |
Codecov Report
@@ Coverage Diff @@
## main #1879 +/- ##
=======================================
Coverage ? 45.27%
=======================================
Files ? 199
Lines ? 17227
Branches ? 0
=======================================
Hits ? 7799
Misses ? 8390
Partials ? 1038
Flags with carried forward coverage won't be shown. Click here to find out more. |
Output the Traceflow results, as long as there are any results returned,
so users can know the progress of the Traceflow session even the antctl
command is timeout, or the Traceflow operation fails.
Example output: