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
LocalStack Community integration with Pro 2 files 2 suites 1h 43m 22s ⏱️ Results for commit 41d7a49. ♻️ This comment has been updated with latest results. |
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 34s ⏱️ Results for commit 41d7a49. ♻️ This comment has been updated with latest results. |
f1db54e to
13c54d3
Compare
There was a problem hiding this comment.
Hey @silv-io , sorry for not reviewing sooner!
We should be using the global analytics logger because it implicitly does the session handshake. The way the hook is implemented now will cause the analytics event not be associated with the session data.
| event = Event( | ||
| name="ls_service_provider_assignment", metadata=metadata, payload=provider_assignment | ||
| ) | ||
|
|
||
| AnalyticsClientPublisher().publish([event]) |
There was a problem hiding this comment.
we should use analytics.log.event event here:
from localstack.utils import analytics
@hooks.on_runtime_ready()
def publish_provider_assignment():
...
analytics.log.event("ls_service_provider_assignment", provider_assignment)There was a problem hiding this comment.
note: just to add more context, this way we should receive the event only after the session event. This is important for us due to some payload enrichment logic in the analytics-backend.
I hadn’t realized Thomas had already explained it above
There was a problem hiding this comment.
Awesome, thanks. This was very confusing to me at first. Will fix it when I'm back next week
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