add product and edition to session analytics #12198
Conversation
|
@ackdav @thrau @dataclasses.dataclass(frozen=True)
class LocalStackEdition:
ENTERPRISE: Literal["localstack-enterprise"] = "localstack-enterprise"
PRO: Literal["localstack-pro"] = "localstack-pro"
COMMUNITY: Literal["localstack"] = "localstack"
AZURE_ALPHA: Literal["localstack-azure-alpha"] = "localstack-azure-alpha"
UNKNOWN: Literal["unknown"] = "unknown"
def get_localstack_edition() -> Optional[str]:
image_name = get_docker_image_to_start()
edition = image_name.removeprefix("localstack/")
return edition if edition in {
LocalStackEdition.ENTERPRISE,
LocalStackEdition.PRO,
LocalStackEdition.COMMUNITY,
LocalStackEdition.AZURE_ALPHA
} else LocalStackEdition.UNKNOWNThen, we could use this function to set the ❓ Are there any planned changes to the naming of the Docker images? |
| @@ -60,6 +62,8 @@ def read_client_metadata() -> ClientMetadata: | |||
| is_ci=os.getenv("CI") is not None, | |||
| is_docker=config.is_in_docker, | |||
| is_testing=config.is_local_test_mode(), | |||
| product="aws", | |||
There was a problem hiding this comment.
@ackdav As briefly discussed, perhaps we can add an environment variable override here, to allow specialized images (separate products like Azure, Snowflake) to define custom product strings. (this environment variable can then be defined in the respective Dockerfiles of the specific products)
Something like (simplified):
| product="aws", | |
| product=os.getenv("LOCALSTACK_TELEMETRY_PRODUCT") or "aws", |
Same for the edition, and also use the existing get_localstack_edition() function as the default.
There was a problem hiding this comment.
I think this is a nice small and concise change! 💯 🚀
Maybe we could - here in the PR description - add some examples of what the current products and editions are? That would make it more concrete for me.
@vittoriopolverino: Concerning your question on the edition. The edition is right now defined by the "*-version" file in /usr/lib/localstack/ added when building the Docker image. For example, for community this is happening here:
Line 176 in 1fca388
Does this clarify your question? Happy to discuss this further! :)
| runtime = MockRuntime() | ||
| set_current_runtime(runtime) | ||
| yield | ||
| set_current_runtime(None) |
There was a problem hiding this comment.
nitpick (not blocking): This really just affects the analytics module, but it might make sense to play safe and really only set it if the runtime is really not set. Something like this:
| runtime = MockRuntime() | |
| set_current_runtime(runtime) | |
| yield | |
| set_current_runtime(None) | |
| try: | |
| # don't do anything if a runtime is set | |
| get_current_runtime() | |
| yield | |
| except ValueError: | |
| # set a mock runtime if no runtime is set | |
| set_current_runtime(MockRuntime()) | |
| yield | |
| set_current_runtime(None) |
| product=os.getenv("LOCALSTACK_TELEMETRY_PRODUCT") or runtime.components.name or "unknown", | ||
| edition=os.getenv("LOCALSTACK_TELEMETRY_EDITION") or get_localstack_edition(), |
There was a problem hiding this comment.
praise: Nice and simple enhancement! 💯
| @@ -60,6 +64,8 @@ def read_client_metadata() -> ClientMetadata: | |||
| is_ci=os.getenv("CI") is not None, | |||
| is_docker=config.is_in_docker, | |||
| is_testing=config.is_local_test_mode(), | |||
| product=os.getenv("LOCALSTACK_TELEMETRY_PRODUCT") or runtime.components.name or "unknown", | |||
There was a problem hiding this comment.
super nitpick (absolutely not blocking): I think it might make sense to create a new function here as well, similar to get_localstack_edition. This gives a nice little abstraction layer which allows us to easily adjust the way we determine the product later on.
There was a problem hiding this comment.
LGTM, modulo Alex' comments. 👍 Kudos for driving this topic forward @ackdav @vittoriopolverino ! 🚀
Yes, +1 on highlighting all the expected values for the different Docker images we are currently providing. |
Motivation
This PR adds
productandeditionto the session'sClientMetadata. Given LocalStack's growing number of products, the distinction will help distinguish where a session is coming from.Both
productandeditioncan be overwritten by environment variables to cover cases where a runtime is not necessarily given or correct (e.g. when packaging an extension).Changes
productandeditionto the session'sClientMetadatamock_runtimefixture. Analytics unit tests rely on client metadata - butget_current_runtime()throws an error when not having an initialized runtime.