[performance] add API load testing script #4237

freemvmt · 2025-01-31T15:08:14Z

Main thrust of this PR is the addition of the test_api.py load testing script (see final commit).

However, to get it working I had to make some other changes, such as adding a mechanism to bypass rate limiting, and exposing a file deletion endpoint. They are probably best considered on a per commit basis. I'm happy to split this up into multiple PRs if that's preferable :)

I'll also do a review and tag this up with some additional context.

Once this is merged we should be able to run the new script against the API on staging, to get an idea of whether it can handle the same load we are confident the data layer can (i.e. Hasura + RDS).

When we do that I'll track results here.

github-actions · 2025-01-31T15:30:29Z

Pizza

Deployed 440c499 to https://4237.planx.pizza.

Useful links:

DafyddLlyr · 2025-02-03T12:06:13Z

Rebase to main - please see https://opensystemslab.slack.com/archives/C04EEBR1E0P/p1738574105777659

…#3989)

…), update README

freemvmt · 2025-02-04T22:06:34Z

.env.example

@@ -39,6 +39,8 @@ FILE_API_KEY_DONCASTER=👻
 FILE_API_KEY_GLOUCESTER=👻
 FILE_API_KEY_TEWKESBURY=👻

+# Used to circumvent API rate limiting for development purposes (e.g. load testing)
+SKIP_RATE_LIMIT_SECRET=👻


I've already generated a 40char alphanumeric secret and pushed it up to S3, so it's ready to pull down

Can confirm I can pull this down ✔️

freemvmt · 2025-02-04T22:11:28Z

api.planx.uk/modules/auth/passport.ts

@@ -14,7 +19,29 @@ export default async (): Promise<Authenticator> => {
  const customPassport = new passport.Passport();

  // instantiate Microsoft OIDC client, and use it to build the related strategy
-  const microsoftIssuer = await Issuer.discover(MICROSOFT_OPENID_CONFIG_URL);
+  // we also keep said config as a fixture to enable offline local development


this change was literally just to allow me to run the API for development purposes (i.e. do pnpm dev) while I was on a train with no internet, so it doesn't necessarily need to make it into the codebase, but could be useful for others?

afaik fetching the config for the Microsoft OIDC stuff is the only call the code makes when spinning up, so it was possible to run pnpm dev to completion while offline before we added that SSO option, but now it isn't (without this change)

I'd say no harm in keeping this around! Fixture may additionally come in handy for future E2E test mocks if we ever want Microsoft SSO coverage?

freemvmt · 2025-02-04T22:14:08Z

api.planx.uk/modules/file/routes.ts

  privateDownloadController,
 );

+router.delete(


would definitely like to hear whether anyone thinks there's a risk associated with exposing this function? I've made it accessible only to anyone with platform level admin powers.

the utility of it here is that we can upload as many files to staging S3 as we like during load testing without worrying about incurring additional cost, because we can also delete them immediately

Exposing to platformAdmins seems fine by me !

"Public" files are pretty exclusively flow content (eg vector drawings like roof pitch examples) used throughout services uploaded by editors; all have original copies on OSL's Google Drive, so low risk here! (Whereas "/file/private/..." are uploaded by applicants, so it's ideal to keep deletion of those restricted to session sanitation jobs.)

freemvmt · 2025-02-04T22:18:00Z

api.planx.uk/modules/file/service/utils.ts

@@ -23,7 +23,7 @@ function useMinio() {
    // Points to Minio
    return {
      endpoint: `http://minio:${process.env.MINIO_PORT}`,
-      s3ForcePathStyle: true,
+      forcePathStyle: true,


this took me a good while to hunt down! a pesky little breaking change resulting from our move to v3 @aws-sdk/client-s3 in #3989 that was causing 500 errors with the following msg:

ServerError: Failed to upload public file: getaddrinfo ENOTFOUND pizza-user-uploads.minio

Thanks for shaking some of these out - went unnoticed for a good while!

freemvmt · 2025-02-04T22:20:17Z

api.planx.uk/rateLimit.ts

@@ -7,6 +7,19 @@ const apiLimiter = rateLimit({
  max: 250,
  standardHeaders: true,
  legacyHeaders: false,
+  skip: (req: Request, _res: Response) => {
+    // add a mechanism for skipping rate limit when load testing (on local or staging only)
+    if (process.env.APP_ENVIRONMENT == "production") return false;


without this (or a more convoluted solution like hosting Locust workers on remote servers with different IPs) I can't exceed the rate limit of 250 reqs per IP per 10 minute period, which of course is not sufficient for load testing purposes!

Only thing I'm not totally following here is why we need a request header and env var? Setting both and checking their equality is just an extra layer of assurance that we're bypassing intentionally?

freemvmt · 2025-02-04T22:23:19Z

infrastructure/performance/test_api.py

+          return resp.failure("Failed to delete file")
+        resp.success()
+
+  # TODO: add another task which hits the /upload-submission endpoint


I haven't managed to implement this part, but I think the above task should suffice for our purposes for this round of load testing

jessicamcinchak

Code looks great here - one minor comment to address around Pulumi secret-getting, but otherwise think this is good to merge & test on staging 🙂

Happy to have another go with the Python script instructions once merged 👍

jessicamcinchak · 2025-02-11T07:06:46Z

.env.example

@@ -39,6 +39,8 @@ FILE_API_KEY_DONCASTER=👻
 FILE_API_KEY_GLOUCESTER=👻
 FILE_API_KEY_TEWKESBURY=👻

+# Used to circumvent API rate limiting for development purposes (e.g. load testing)
+SKIP_RATE_LIMIT_SECRET=👻


Can confirm I can pull this down ✔️

jessicamcinchak · 2025-02-11T07:09:40Z

api.planx.uk/modules/auth/passport.ts

@@ -14,7 +19,29 @@ export default async (): Promise<Authenticator> => {
  const customPassport = new passport.Passport();

  // instantiate Microsoft OIDC client, and use it to build the related strategy
-  const microsoftIssuer = await Issuer.discover(MICROSOFT_OPENID_CONFIG_URL);
+  // we also keep said config as a fixture to enable offline local development


I'd say no harm in keeping this around! Fixture may additionally come in handy for future E2E test mocks if we ever want Microsoft SSO coverage?

jessicamcinchak · 2025-02-11T07:10:32Z

api.planx.uk/modules/file/controller.ts

@@ -71,15 +72,15 @@ export const publicUploadController: UploadController = async (
  }
 };

-export const downloadFileSchema = z.object({
+export const hostedFileSchema = z.object({


👍 indeed a bit more semantically clear !

jessicamcinchak · 2025-02-11T07:14:36Z

api.planx.uk/modules/file/routes.ts

  privateDownloadController,
 );

+router.delete(


Exposing to platformAdmins seems fine by me !

"Public" files are pretty exclusively flow content (eg vector drawings like roof pitch examples) used throughout services uploaded by editors; all have original copies on OSL's Google Drive, so low risk here! (Whereas "/file/private/..." are uploaded by applicants, so it's ideal to keep deletion of those restricted to session sanitation jobs.)

jessicamcinchak · 2025-02-11T07:16:03Z

api.planx.uk/modules/file/service/utils.ts

@@ -23,7 +23,7 @@ function useMinio() {
    // Points to Minio
    return {
      endpoint: `http://minio:${process.env.MINIO_PORT}`,
-      s3ForcePathStyle: true,
+      forcePathStyle: true,


Thanks for shaking some of these out - went unnoticed for a good while!

jessicamcinchak · 2025-02-11T07:18:26Z

api.planx.uk/rateLimit.ts

@@ -7,6 +7,19 @@ const apiLimiter = rateLimit({
  max: 250,
  standardHeaders: true,
  legacyHeaders: false,
+  skip: (req: Request, _res: Response) => {
+    // add a mechanism for skipping rate limit when load testing (on local or staging only)
+    if (process.env.APP_ENVIRONMENT == "production") return false;


Only thing I'm not totally following here is why we need a request header and env var? Setting both and checking their equality is just an extra layer of assurance that we're bypassing intentionally?

jessicamcinchak · 2025-02-11T07:46:37Z

infrastructure/application/index.ts

+          {
+            name: "SKIP_RATE_LIMIT_SECRET",
+            value: config.requireSecret("skip-rate-limit-secret"),
+          },


I think this will fail on our production deploy's Pulumi up step because there's no equivalent defined in Pulumi.production.yaml and value: config.requireSecret(...) is expecting a defined value?

Alternatively, it looks like config.getSecret() should do what we want here and handle this as a staging-only secret ! Docs here https://www.pulumi.com/docs/iac/concepts/secrets/#creating-secrets-programmatically

jessicamcinchak · 2025-02-11T07:48:52Z

infrastructure/performance/README.md

+
+The API load testing script requires some files to work with, which are in `/samples`. Care should be taken to make sure anything added there is in the public domain.
+
+For example, I used [Unsplash](https://unsplash.com/s/photos/tree?license=free) to search for an image with a ['free' license](https://unsplash.com/license), and printed a page from the [WikiHouse](https://www.wikihouse.cc/) site as a PDF.


freemvmt force-pushed the load-testing-api branch from 765c888 to 0b989d4 Compare January 31, 2025 15:17

DafyddLlyr force-pushed the load-testing-api branch from 0b989d4 to a8e08ff Compare February 3, 2025 12:05

freemvmt force-pushed the load-testing-api branch 2 times, most recently from 9821cc5 to b3589ff Compare February 4, 2025 22:00

freemvmt added 5 commits February 4, 2025 22:01

[api] enable offline development of API with Microsoft OIDC fixture

19734eb

[api] add mechanism to bypass rate limiting for development purposes

c1f5d6d

[api] expose file deletion endpoint (guarded by platform admin role)

52ce264

[api] fix minio params to S3 factory (introduced by aws-sdk v3 bump in …

c6097ee

…#3989)

[performance] add API load testing script (w/ sample files for upload…

4f8b013

…), update README

freemvmt force-pushed the load-testing-api branch from b3589ff to 4f8b013 Compare February 4, 2025 22:01

freemvmt requested a review from a team February 4, 2025 22:01

freemvmt marked this pull request as ready for review February 4, 2025 22:01

freemvmt commented Feb 4, 2025

View reviewed changes

freemvmt changed the title ~~WIP: add API load testing script~~ add API load testing script Feb 4, 2025

freemvmt changed the title ~~add API load testing script~~ [performance] add API load testing script Feb 4, 2025

jessicamcinchak approved these changes Feb 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[performance] add API load testing script #4237

[performance] add API load testing script #4237

freemvmt commented Jan 31, 2025 •

edited

Loading

github-actions bot commented Jan 31, 2025 •

edited

Loading

DafyddLlyr commented Feb 3, 2025

freemvmt Feb 4, 2025

jessicamcinchak Feb 11, 2025

freemvmt Feb 4, 2025

jessicamcinchak Feb 11, 2025

freemvmt Feb 4, 2025 •

edited

Loading

jessicamcinchak Feb 11, 2025

freemvmt Feb 4, 2025

jessicamcinchak Feb 11, 2025

freemvmt Feb 4, 2025

jessicamcinchak Feb 11, 2025

freemvmt Feb 4, 2025

jessicamcinchak left a comment

jessicamcinchak Feb 11, 2025

jessicamcinchak Feb 11, 2025

jessicamcinchak Feb 11, 2025

jessicamcinchak Feb 11, 2025

jessicamcinchak Feb 11, 2025

jessicamcinchak Feb 11, 2025

jessicamcinchak Feb 11, 2025

jessicamcinchak Feb 11, 2025


		The API load testing script requires some files to work with, which are in `/samples`. Care should be taken to make sure anything added there is in the public domain.

		For example, I used [Unsplash](https://unsplash.com/s/photos/tree?license=free) to search for an image with a ['free' license](https://unsplash.com/license), and printed a page from the [WikiHouse](https://www.wikihouse.cc/) site as a PDF.

[performance] add API load testing script #4237

Are you sure you want to change the base?

[performance] add API load testing script #4237

Conversation

freemvmt commented Jan 31, 2025 • edited Loading

github-actions bot commented Jan 31, 2025 • edited Loading

Pizza

DafyddLlyr commented Feb 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freemvmt Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jessicamcinchak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freemvmt commented Jan 31, 2025 •

edited

Loading

github-actions bot commented Jan 31, 2025 •

edited

Loading

freemvmt Feb 4, 2025 •

edited

Loading