From e13fcaf8655b6f65b8630f2405c50371454a4a2e Mon Sep 17 00:00:00 2001 From: Ethan <39577870+ethanndickson@users.noreply.github.com> Date: Mon, 22 Sep 2025 17:45:45 +1000 Subject: [PATCH] refactor(scaletest): support exposing arbitrary metrics on scaletest runs (#19886) Relates to https://github.com/coder/internal/issues/889 The existing implementation for exposing read and written bytes was a little awkward - we're going to be adding a bunch of scaletest runners / load generators that *don't* transfer any bytes. This PR has the scaletest reports expose a map of arbitrary string-keyed metrics instead. FWIW, the latest iteration of the scaletesting infrastructure doesn't parse these reports right now - they're just logged to stdout, so we're good to break the json schema here. --- scaletest/harness/results.go | 38 +++++++------- scaletest/harness/results_test.go | 87 ++++++++++++++++++------------- scaletest/harness/run.go | 21 ++++---- scaletest/harness/run_test.go | 24 +++++---- scaletest/workspacetraffic/run.go | 19 ++++--- 5 files changed, 106 insertions(+), 83 deletions(-) diff --git a/scaletest/harness/results.go b/scaletest/harness/results.go index 67bdef55b2..8e2c181927 100644 --- a/scaletest/harness/results.go +++ b/scaletest/harness/results.go @@ -27,16 +27,15 @@ type Results struct { // RunResult is the result of a single test run. type RunResult struct { - FullID string `json:"full_id"` - TestName string `json:"test_name"` - ID string `json:"id"` - Logs string `json:"logs"` - Error error `json:"error"` - StartedAt time.Time `json:"started_at"` - Duration httpapi.Duration `json:"duration"` - DurationMS int64 `json:"duration_ms"` - TotalBytesRead int64 `json:"total_bytes_read"` - TotalBytesWritten int64 `json:"total_bytes_written"` + FullID string `json:"full_id"` + TestName string `json:"test_name"` + ID string `json:"id"` + Logs string `json:"logs"` + Error error `json:"error"` + StartedAt time.Time `json:"started_at"` + Duration httpapi.Duration `json:"duration"` + DurationMS int64 `json:"duration_ms"` + Metrics map[string]any `json:"metrics,omitempty"` } // MarshalJSON implements json.Marhshaler for RunResult. @@ -61,16 +60,15 @@ func (r *TestRun) Result() RunResult { } return RunResult{ - FullID: r.FullID(), - TestName: r.testName, - ID: r.id, - Logs: r.logs.String(), - Error: r.err, - StartedAt: r.started, - Duration: httpapi.Duration(r.duration), - DurationMS: r.duration.Milliseconds(), - TotalBytesRead: r.bytesRead, - TotalBytesWritten: r.bytesWritten, + FullID: r.FullID(), + TestName: r.testName, + ID: r.id, + Logs: r.logs.String(), + Error: r.err, + StartedAt: r.started, + Duration: httpapi.Duration(r.duration), + DurationMS: r.duration.Milliseconds(), + Metrics: r.metrics, } } diff --git a/scaletest/harness/results_test.go b/scaletest/harness/results_test.go index 48e6e55606..ac16075169 100644 --- a/scaletest/harness/results_test.go +++ b/scaletest/harness/results_test.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/scaletest/harness" + "github.com/coder/coder/v2/scaletest/workspacetraffic" ) type testError struct { @@ -36,40 +37,46 @@ func Test_Results(t *testing.T) { TotalFail: 2, Runs: map[string]harness.RunResult{ "test-0/0": { - FullID: "test-0/0", - TestName: "test-0", - ID: "0", - Logs: "test-0/0 log line 1\ntest-0/0 log line 2", - Error: xerrors.New("test-0/0 error"), - StartedAt: now, - Duration: httpapi.Duration(time.Second), - DurationMS: 1000, - TotalBytesRead: 1024, - TotalBytesWritten: 2048, + FullID: "test-0/0", + TestName: "test-0", + ID: "0", + Logs: "test-0/0 log line 1\ntest-0/0 log line 2", + Error: xerrors.New("test-0/0 error"), + StartedAt: now, + Duration: httpapi.Duration(time.Second), + DurationMS: 1000, + Metrics: map[string]any{ + workspacetraffic.BytesReadMetric: 1024, + workspacetraffic.BytesWrittenMetric: 2048, + }, }, "test-0/1": { - FullID: "test-0/1", - TestName: "test-0", - ID: "1", - Logs: "test-0/1 log line 1\ntest-0/1 log line 2", - Error: nil, - StartedAt: now.Add(333 * time.Millisecond), - Duration: httpapi.Duration(time.Second), - DurationMS: 1000, - TotalBytesRead: 512, - TotalBytesWritten: 1024, + FullID: "test-0/1", + TestName: "test-0", + ID: "1", + Logs: "test-0/1 log line 1\ntest-0/1 log line 2", + Error: nil, + StartedAt: now.Add(333 * time.Millisecond), + Duration: httpapi.Duration(time.Second), + DurationMS: 1000, + Metrics: map[string]any{ + workspacetraffic.BytesReadMetric: 512, + workspacetraffic.BytesWrittenMetric: 1024, + }, }, "test-0/2": { - FullID: "test-0/2", - TestName: "test-0", - ID: "2", - Logs: "test-0/2 log line 1\ntest-0/2 log line 2", - Error: testError{hidden: xerrors.New("test-0/2 error")}, - StartedAt: now.Add(666 * time.Millisecond), - Duration: httpapi.Duration(time.Second), - DurationMS: 1000, - TotalBytesRead: 2048, - TotalBytesWritten: 4096, + FullID: "test-0/2", + TestName: "test-0", + ID: "2", + Logs: "test-0/2 log line 1\ntest-0/2 log line 2", + Error: testError{hidden: xerrors.New("test-0/2 error")}, + StartedAt: now.Add(666 * time.Millisecond), + Duration: httpapi.Duration(time.Second), + DurationMS: 1000, + Metrics: map[string]any{ + workspacetraffic.BytesReadMetric: 2048, + workspacetraffic.BytesWrittenMetric: 4096, + }, }, }, Elapsed: httpapi.Duration(time.Second), @@ -115,9 +122,11 @@ Test results: "started_at": "2023-10-05T12:03:56.395813665Z", "duration": "1s", "duration_ms": 1000, - "total_bytes_read": 1024, - "total_bytes_written": 2048, - "error": "test-0/0 error:\n github.com/coder/coder/v2/scaletest/harness_test.Test_Results\n [working_directory]/results_test.go:43" + "metrics": { + "bytes_read": 1024, + "bytes_written": 2048 + }, + "error": "test-0/0 error:\n github.com/coder/coder/v2/scaletest/harness_test.Test_Results\n [working_directory]/results_test.go:44" }, "test-0/1": { "full_id": "test-0/1", @@ -127,8 +136,10 @@ Test results: "started_at": "2023-10-05T12:03:56.728813665Z", "duration": "1s", "duration_ms": 1000, - "total_bytes_read": 512, - "total_bytes_written": 1024, + "metrics": { + "bytes_read": 512, + "bytes_written": 1024 + }, "error": "\u003cnil\u003e" }, "test-0/2": { @@ -139,8 +150,10 @@ Test results: "started_at": "2023-10-05T12:03:57.061813665Z", "duration": "1s", "duration_ms": 1000, - "total_bytes_read": 2048, - "total_bytes_written": 4096, + "metrics": { + "bytes_read": 2048, + "bytes_written": 4096 + }, "error": "test-0/2 error" } } diff --git a/scaletest/harness/run.go b/scaletest/harness/run.go index 06d34017fa..ec8c717a14 100644 --- a/scaletest/harness/run.go +++ b/scaletest/harness/run.go @@ -31,11 +31,11 @@ type Cleanable interface { Cleanup(ctx context.Context, id string, logs io.Writer) error } -// Collectable is an optional extension to Runnable that allows to get metrics from the runner. +// Collectable is an optional extension to Runnable that exposes additional +// metrics from the runner. type Collectable interface { Runnable - // Gets the bytes transferred - GetBytesTransferred() (int64, int64) + GetMetrics() map[string]any } // AddRun creates a new *TestRun with the given name, ID and Runnable, adds it @@ -73,13 +73,12 @@ type TestRun struct { id string runner Runnable - logs *syncBuffer - done chan struct{} - started time.Time - duration time.Duration - err error - bytesRead int64 - bytesWritten int64 + logs *syncBuffer + done chan struct{} + started time.Time + duration time.Duration + err error + metrics map[string]any } func NewTestRun(testName string, id string, runner Runnable) *TestRun { @@ -111,7 +110,7 @@ func (r *TestRun) Run(ctx context.Context) (err error) { if !ok { return } - r.bytesRead, r.bytesWritten = c.GetBytesTransferred() + r.metrics = c.GetMetrics() }() defer func() { e := recover() diff --git a/scaletest/harness/run_test.go b/scaletest/harness/run_test.go index 898a5bf5a0..245d80542e 100644 --- a/scaletest/harness/run_test.go +++ b/scaletest/harness/run_test.go @@ -17,22 +17,28 @@ type testFns struct { RunFn func(ctx context.Context, id string, logs io.Writer) error // CleanupFn is optional if no cleanup is required. CleanupFn func(ctx context.Context, id string, logs io.Writer) error - // getBytesTransferred is optional if byte transfer tracking is required. - getBytesTransferred func() (int64, int64) + // GetMetricsFn is optional if no metric collection is required. + GetMetricsFn func() map[string]any } +var ( + _ harness.Runnable = &testFns{} + _ harness.Cleanable = &testFns{} + _ harness.Collectable = &testFns{} +) + // 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) { - if fns.getBytesTransferred == nil { - return 0, 0 +func (fns testFns) GetMetrics() map[string]any { + if fns.GetMetricsFn == nil { + return nil } - return fns.getBytesTransferred() + return fns.GetMetricsFn() } // Cleanup implements Cleanable. @@ -65,9 +71,9 @@ func Test_TestRun(t *testing.T) { atomic.AddInt64(&cleanupCalled, 1) return nil }, - getBytesTransferred: func() (int64, int64) { + GetMetricsFn: func() map[string]any { atomic.AddInt64(&collectableCalled, 1) - return 0, 0 + return nil }, } ) @@ -132,7 +138,7 @@ func Test_TestRun(t *testing.T) { RunFn: func(ctx context.Context, id string, logs io.Writer) error { return nil }, - getBytesTransferred: nil, + GetMetricsFn: nil, }) err := run.Run(context.Background()) diff --git a/scaletest/workspacetraffic/run.go b/scaletest/workspacetraffic/run.go index 7dd7cb6803..cbdc4f96e1 100644 --- a/scaletest/workspacetraffic/run.go +++ b/scaletest/workspacetraffic/run.go @@ -28,8 +28,9 @@ type Runner struct { } var ( - _ harness.Runnable = &Runner{} - _ harness.Cleanable = &Runner{} + _ harness.Runnable = &Runner{} + _ harness.Cleanable = &Runner{} + _ harness.Collectable = &Runner{} ) // func NewRunner(client *codersdk.Client, cfg Config, metrics *Metrics) *Runner { @@ -210,10 +211,16 @@ func (r *Runner) Run(ctx context.Context, _ string, logs io.Writer) (err error) } } -func (r *Runner) GetBytesTransferred() (bytesRead, bytesWritten int64) { - bytesRead = r.cfg.ReadMetrics.GetTotalBytes() - bytesWritten = r.cfg.WriteMetrics.GetTotalBytes() - return bytesRead, bytesWritten +const ( + BytesReadMetric = "bytes_read" + BytesWrittenMetric = "bytes_written" +) + +func (r *Runner) GetMetrics() map[string]any { + return map[string]any{ + BytesReadMetric: r.cfg.ReadMetrics.GetTotalBytes(), + BytesWrittenMetric: r.cfg.WriteMetrics.GetTotalBytes(), + } } // Cleanup does nothing, successfully.