Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds performance optimizations for the webhook collector to reduce cold start latency:
- Implements public key cache pre-warming during function initialization to reduce first-request latency
- Adds optional provisioned concurrency configuration for AWS Lambda to maintain warm instances
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| java/impl/gcp/src/main/java/co/worklytics/psoxy/GcpWebhookCollectionHandler.java | Adds warmPublicKeyCache() method in constructor to pre-fetch KMS keys during Cloud Function startup |
| java/impl/aws/src/main/java/co/worklytics/psoxy/AwsWebhookCollectionModeHandler.java | Adds warmPublicKeyCache() in static initialization and reorganizes imports |
| infra/modules/aws-webhook-collector/variables.tf | Defines keep_warm_instances variable with validation for provisioned concurrency |
| infra/modules/aws-webhook-collector/main.tf | Creates Lambda alias and provisioned concurrency resources when keep_warm_instances is set |
| infra/modules/aws-host/variables.tf | Exposes keep_warm_instances parameter in webhook_collectors configuration |
| infra/modules/aws-host/main.tf | Passes keep_warm_instances to webhook collector module |
| infra/examples-dev/aws/variables.tf | Documents keep_warm_instances in example configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
👍
The SDK could always make a request to Sorry, I hadn’t read your latest comments on Slack before reviewing this. So, OK—I’ll apply your suggestions in the SDK./.well-known/openid-configuration before sending the actual payloads. The trade-off would be introducing another configuration parameter for that.
| variable "keep_warm_instances" { | ||
| type = number | ||
| description = "Number of Lambda execution environments to keep warm (at minimum). If null (default), Lambda will cold-start as needed. If set to 1 or more, AWS will keep at least that many instances ready, eliminating cold starts for the JWKS endpoint used by the JWT authorizer. This significantly improves webhook collection reliability. Cost: ~$0.015/hour per instance (~$11/month for 1 instance)." | ||
| default = null |
There was a problem hiding this comment.
Should be default 0 instead of null ? So you can remove the conditional comparison in. var.keep_warm_instances != null ?
There was a problem hiding this comment.
i like to pass null for "unset", rather than 0; that way if we see 0, we know was explicitly set as such by customer.
* Bump org.apache.httpcomponents.client5:httpclient5 in /java/gateway-core Bumps [org.apache.httpcomponents.client5:httpclient5](https://github.com/apache/httpcomponents-client) from 5.4.1 to 5.4.3. - [Changelog](https://github.com/apache/httpcomponents-client/blob/rel/v5.4.3/RELEASE_NOTES.txt) - [Commits](apache/httpcomponents-client@rel/v5.4.1...rel/v5.4.3) --- updated-dependencies: - dependency-name: org.apache.httpcomponents.client5:httpclient5 dependency-version: 5.4.3 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> * prep v0.5.12 * Salesforce rules refresh (#1039) * Update rules * Support example with POST * bump many dependencies (#1038) * apache commons codec to 1.19.0 * more dep updates * webhook collector auth perf (#1036) * add logic to keep lambdas warm * fix style * init public keys in mem for webhook collectors on start-up * aws replay tool (#1034) * aws replace tool * add documentation * webhook collector output prefix support (#1035) * improve webhook collector mode configuration * support webhook output prefix * fix style * fix path stuff * fix missed rename * fix various test issues * gmail empty header fix (#1040) * deal with empty header case more correctly; that's quite clearly a bug * add test of empty CC case * GCP VPC doc improvements (#1042) * improve GCP vpc docs + conditions * fix style, unrelated * improve error feedback on network connectivity issues * doc that GCP VPC needs external connectivity via router/nat for non-google sources * move validation up to dop * try to skip compile in tests * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix some references to role constants (#1043) * config to deploy packages as mvn libs via GitHub Packages (#1041) * improve OAuth token refresh (#1037) * interface to secret store to get multiple versions of things * refactor how token refreshin works, so potentially more proactive / better re-use * pick most recent token based on expirationDate * cr feedback * fix refactor * remove stray bracket * Update java/core/src/main/java/co/worklytics/psoxy/gateway/impl/oauth/OAuthRefreshTokenSourceAuthStrategy.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CompositeConfigService.java Co-authored-by: aperez-worklytics <75276364+aperez-worklytics@users.noreply.github.com> * drop sort on lastAccessDate; only date-level granularity, so why bother --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: aperez-worklytics <75276364+aperez-worklytics@users.noreply.github.com> * more psoxy constants (#1044) * expose more outputs relevant to gcp-hosted deployments * gcp min perms to host * expose gcp network roles, for completenes * doc gcp vpc roles * doc that Project IAM Admin is required * fix docs * Update infra/modules/psoxy-constants/main.tf Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update infra/modules/psoxy-constants/outputs.tf Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update docs/gcp/vpc.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * prep v0.5.12 (#1046) * fix bad comment * try to make sbom stuff more robust * drop mvn plugins that are problematic, see to cause build errors * use reactor build * sboms * fix sbom generation * update package-lock in psoxy-test * fix gcp release artifact script * prompt user if gcp artifact has already been published * prompt user if aws artifact has already been published * warn if we're NOT running in the expected place --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: aperez-worklytics <75276364+aperez-worklytics@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Features
also explored adding authorizer caching / API gateway caching, but can't do much. It can't truly cache routes in the API gateway; and authorizer responses for JWT can't be cached (it seems); or at least its' not configurable.
Change implications