feat: add scaletest Runner for dynamicparameters load gen#19890
Conversation
a368b05 to
0b6d03c
Compare
| type Config struct { | ||
| TemplateVersion uuid.UUID `json:"template_version"` | ||
| SessionToken string `json:"session_token"` | ||
| Metrics *Metrics `json:"-"` | ||
| MetricLabelValues []string `json:"metric_label_values"` | ||
| } |
There was a problem hiding this comment.
Optional: could use a Validate function here, I've found the one on my load generator somewhat useful, such as for checking that my time.Duration timeouts aren't zero.
Emyrk
left a comment
There was a problem hiding this comment.
Overall this is a good start. It is missing tests/assertions for some more dynamic elements, modules, some terraform functions, user context.
In my experience, the verbosity required to run an exchange and test is just too cumbersome. So we should come up with some patterns like the unit tests have:
coder/enterprise/coderd/dynamicparameters_test.go
Lines 566 to 584 in 0b6d03c
Then it could be easier to add more query/response round trips and assert more properties of the response.
| case resp, ok := <-respCh: | ||
| if !ok { | ||
| return xerrors.Errorf("dynamic parameters stream closed before change response") | ||
| } | ||
| _, _ = fmt.Fprintf(logs, "change response: %+v\n", resp) | ||
| r.cfg.Metrics.LatencyChangeResponseSeconds. | ||
| WithLabelValues(r.cfg.MetricLabelValues...). | ||
| Observe(time.Since(initTime).Seconds()) | ||
| if resp.ID != 1 { | ||
| return xerrors.Errorf("unexpected response ID: %d", resp.ID) | ||
| } | ||
| if err := checkNoDiagnostics(resp); err != nil { | ||
| return xerrors.Errorf("unexpected change response diagnostics: %w", err) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
We do not assert any param condition on the response. I wonder if we can refactor the ParameterAsserter structure to be useful here:
coder/coderd/coderdtest/dynamicparameters.go
Line 122 in 0b6d03c
It really streamlines the assertion code to something like:
coderdtest.AssertParameter(t, "groups", resp.Parameters).
Exists().
Options(database.EveryoneGroup, "admin", "auditor").
Value("admin")It just currently only works in unit tests.
There was a problem hiding this comment.
I feel like this is missing the point --- we are scale testing, not functional testing. I'm not looking to check that the response we get is "correct" in any detailed sense of the term, just that the response has not thrown errors and is doing the computation we want it to.
Emyrk
left a comment
There was a problem hiding this comment.
Is this using the echo provisioner? And is that intentional?
That's the test of the scaletest runner/load generator. The actual scale test won't use the echo provisioner. |
I'm not sure what you mean by "more dynamic elements." Modules is a good call out. Are there particular terraform functions that are relevant for a scale test? The point here is not to verify the functionality of dynamic parameters, but how it behaves under load. The test template does make use of the user roles and username. Are there other elements of the user context that need special attention in terms of scale? |
0b6d03c to
cefa48c
Compare
relates to coder/internal#912
Adds a new scaletest Runner to generate dynamic parameters load.
A later PR will add the CLI command, including creating the template & version.