Performance Test changes for EE #8453
Conversation
|
Issues linked to changelog: |
github-actions
bot
added
the
keep pr updated
…-io/gloo into upstream-builder-healthchecks
| // Tests are run as part of the "Nightly" action in a GHA using the default Linux runner | ||
| // More info on that machine can be found here: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources | ||
| // When developing new tests, users should manually run that action in order to test performance under the same parameters | ||
| // Results can then be found in the logs for that instance of the action | ||
| var _ = Describe("Translation - Benchmarking Tests", decorators.Performance, Label(labels.Performance), func() { | ||
| var _ = Describe("Translation - Benchmarking Tests", Ordered, decorators.Performance, Label(labels.Performance), func() { |
There was a problem hiding this comment.
any reason this needs to be ordered?
There was a problem hiding this comment.
BeforeAll and AfterAll can only be present in ordered blocks, and, since the tests are all in a Table and the benchmarking measurement locks the OS thread I didn’t really see a downside to this, whereas switching the log level in BeforeEach and AfterEach seemed like overkill and didn’t really capture the intended logic
There was a problem hiding this comment.
Do all performance tests need to be Ordered, or just this one? If it's all, could we bake this into the Peformance decorator, so other devs don't have to remember it?
There was a problem hiding this comment.
The requirement to be Ordered is purely because we're using a BeforeAll and AfterAll
| // Tests are run as part of the "Nightly" action in a GHA using the default Linux runner | ||
| // More info on that machine can be found here: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources | ||
| // When developing new tests, users should manually run that action in order to test performance under the same parameters | ||
| // Results can then be found in the logs for that instance of the action | ||
| var _ = Describe("Translation - Benchmarking Tests", decorators.Performance, Label(labels.Performance), func() { | ||
| var _ = Describe("Translation - Benchmarking Tests", Ordered, decorators.Performance, Label(labels.Performance), func() { |
There was a problem hiding this comment.
Do all performance tests need to be Ordered, or just this one? If it's all, could we bake this into the Peformance decorator, so other devs don't have to remember it?
…-io/gloo into upstream-builder-healthchecks
…-io/gloo into upstream-builder-healthchecks
Description
BenchmarkConfigandGenerateBenchmarkDesc()for use in EEbenchmarkMatchersfield inBenchmarkConfigwithGhaMatchersandLocalMatchersGetMatchers()function toBenchmarkConfigwhich returns appropriate matchers depending on run contextWithHealthChecks()function toUpstreamBuilderContext
These changes are designed to allow EE to reuse performance test infrastructure developed for OSS
The use of different targets for benchmarking depending on test running environment is inspired by this implementation in EE
Note that if an entry is missing matchers for the machine tests are being run on, the experiment will still take place and results logged, however no assertions will be made and the entry will trivially pass
See solo-io/solo-projects#5157 for proposed use of these changes in EE
Log level is reduced in order to eliminate noise from each translation iteration logging
Here is a run of the performance tests without all of the noisy logs
Here a run of the performance tests as of 50a22e4
My local environment is much faster than GHA, hence the targets that are 25x faster than the existing targets for GHA
Additionally the existing GHA targets do not account for the lowered log level which also improves performance
These targets are (still) mainly intended for demonstration purposes and are not tied too customer requirements or other external expectations
Checklist:
make -B install-go-tools generated-codeto ensure there will be no code diff