Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork4.2k
Add provider info to service request counter#12740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
github-actionsbot commentedJun 11, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Test Results - Preflight, Unit21 862 tests ±0 20 203 ✅ - 2 6m 15s ⏱️ +2s For more details on these failures, seethis check. Results for commitb881564. ± Comparison against base commit8057b19. ♻️ This comment has been updated with latest results. |
| @@ -42,13 +42,15 @@ def __call__(self, chain: HandlerChain, context: RequestContext, response: Respo | |||
| err_type = self._get_err_type(context, response) if response.status_code >= 400 else None | |||
| service_name = context.operation.service_model.service_name | |||
| operation_name = context.operation.name | |||
| provider = config.SERVICE_PROVIDER_CONFIG.get_provider(service_name) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this line seems to suggest that there's a 1:1 relationship of service_name to provider, which I think is not true - did I misread the function or the 1:1 relationship actually is true in the community version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
In a running instance the relation is 1:1. Only one provider can be active at once. The active provider is defined in the config
github-actionsbot commentedJun 11, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for taking a stab at this! My high-level feedback is: if we include the service provider in each service request, we'll also need to adapt the analytics backend, specifically the materialized view that stores the AWS service request. We can do that, but this creates additional work, is quite a substantial change. Also, since the set of providers doesn't (because it currently cannot) change during the runtime of LocalStack, we'll be sending redundant data for each request.
It might be more efficient to capture the configured providersonce at startup, similar to how we log environment variables. This would avoid redundancy and keep the data leaner. Happy to provide more guidance if needed, but suggest to sync up with@vittoriopolverino on this
mmaureenliu commentedJun 17, 2025
Just want some clarification on the "capture the configured providers once at startup" suggestion - do you mean at each session start, send a map of provider to service to the data backend, and let the data backend figure out the provider for each service within the session? |
Happy to support on this. +1 to what Thomas already pointed out |
b6d93f4 tob881564Compare
Motivation
/cc@mmaureenliu
Changes