-
Notifications
You must be signed in to change notification settings - Fork 914
feat: include read/write byte stats in scaletests JSON report #17777
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
Conversation
@mafredri requested a review as you opened the original issue, please lmk if I've misunderstood |
Signed-off-by: Callum Styan <callumstyan@gmail.com>
236ce51
to
8b3fc67
Compare
} | ||
|
||
// Run implements Runnable. | ||
func (fns testFns) Run(ctx context.Context, id string, logs io.Writer) error { | ||
return fns.RunFn(ctx, id, logs) | ||
} | ||
|
||
// GetBytesTransferred implements Collectable. | ||
func (fns testFns) GetBytesTransferred() (bytesRead int64, bytesWritten int64) { |
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.
I'm curious what's going on with this pattern, but your addition seems totally in line with what's already 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.
yeah I don't particularly like it, feels odd/reads strangely, but it's already in use for the cleanable interface as well so 🤷
Signed-off-by: Callum Styan <callumstyan@gmail.com>
PR to fix #12157
WIP/Draft as I spend some time to confirm my understanding of the current code and the changes from @joobisb original PR here, ATM this PR is just his commits as I want to check what CI jobs were failing.