Publish service provider mapping on startup #12740
Conversation
| @@ -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.
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.
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
There was a problem hiding this comment.
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 providers once 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
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 to
b881564
Compare
b881564 to
f1db54e
Compare
|
Currently, only patch changes are allowed on main. Your PR labels (semver: minor) indicate that it cannot be merged into the main at this time. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 2h 22m 8s ⏱️ Results for commit f1db54e. |
Motivation
We have the need to track which providers are used for different AWS services. We can do this by a startup hook which basically sends the service config on startup.
/cc @mmaureenliu
Changes