Skip to content

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

Merged
merged 4 commits into from
Jun 13, 2025

Conversation

cstyan
Copy link
Contributor

@cstyan cstyan commented May 12, 2025

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.

@cstyan cstyan requested a review from mafredri May 13, 2025 16:51
@cstyan cstyan changed the title WIP: include bytes read/written in scaletests json report feat: include bytes read/written in scaletests json report May 13, 2025
@cstyan cstyan marked this pull request as ready for review May 13, 2025 16:52
@cstyan cstyan changed the title feat: include bytes read/written in scaletests json report feat: include bytes read/written stats in scaletests json report May 13, 2025
@cstyan
Copy link
Contributor Author

cstyan commented May 13, 2025

@mafredri requested a review as you opened the original issue, please lmk if I've misunderstood scaletest and any parallelism in play there, which is the reason for using atomic updates to the new bytes stat. For the existing Prometheus metric, the client library handles atomic updates to the value for us.

@github-actions github-actions bot added the stale This issue is like stale bread. label May 21, 2025
@github-actions github-actions bot closed this May 25, 2025
@cstyan cstyan reopened this May 26, 2025
@github-actions github-actions bot removed the stale This issue is like stale bread. label May 27, 2025
@cstyan cstyan force-pushed the callum-report-bytes branch from 236ce51 to 8b3fc67 Compare June 2, 2025 20:05
@github-actions github-actions bot added the stale This issue is like stale bread. label Jun 10, 2025
@github-actions github-actions bot closed this Jun 13, 2025
@cstyan cstyan reopened this Jun 13, 2025
}

// 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) {
Copy link
Contributor

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 :)

Copy link
Contributor Author

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>
@cstyan cstyan changed the title feat: include bytes read/written stats in scaletests json report feat: include read/write byte stats in scaletests JSON report Jun 13, 2025
@cstyan cstyan merged commit 068f9a0 into main Jun 13, 2025
36 checks passed
@cstyan cstyan deleted the callum-report-bytes branch June 13, 2025 21:33
@github-actions github-actions bot locked and limited conversation to collaborators Jun 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
stale This issue is like stale bread.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report bytes read/written in JSON report for coder exp scaletest workspace-traffic commands
3 participants