Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

analytics for non-prefixed env var forwarding in CLI#11946

Merged
alexrashed merged 1 commit intomasterfrom
env-var-prefix-data
Nov 28, 2024
Merged

analytics for non-prefixed env var forwarding in CLI#11946
alexrashed merged 1 commit intomasterfrom
env-var-prefix-data

Conversation

@alexrashed
Copy link
Copy Markdown
Member

Motivation

With #11810 we deprecated the forwarding of not-explicitly-prefixed environment variables from the system environment to the runtime environment.
However, this is definitely a change which will take quite a while to reach all our users.
In order to get an idea where we are, this PR adds a simple analytics log which tracks the environment variables (only those marked as "known" by the CLI and only the name and not the content) which are affected by this deprecation.

Changes

  • Add a simple analytics log message to the deprecation check to help us gain knowledge on the adoption of env var prefix.

@alexrashed alexrashed added the semver: patch Non-breaking changes which can be included in patch releases label Nov 27, 2024
@alexrashed alexrashed added this to the 4.0.3 milestone Nov 27, 2024
@alexrashed alexrashed self-assigned this Nov 27, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 27, 2024

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 48m 51s ⏱️ +50s
3 775 tests ±0  3 429 ✅ ±0  346 💤 ±0  0 ❌ ±0 
3 777 runs  ±0  3 429 ✅ ±0  348 💤 ±0  0 ❌ ±0 

Results for commit 8f926a8. ± Comparison against base commit 1a46f58.

♻️ This comment has been updated with latest results.

@alexrashed alexrashed marked this pull request as ready for review November 27, 2024 19:19
Copy link
Copy Markdown
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good idea to track the usage of those variables, so we can estimate the effect of the removal in the future

Comment on lines +544 to +546
log.event(
event="non_prefixed_cli_env_vars", payload={"env_vars": non_prefixed_env_vars}
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: somewhat unrelated but IMO we should start also writing tests for the analytics events to make sure they work as expected 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, I thought about adding this, but then I postponed this to a follow-up to introduce unit tests for multiple log events with some utilities around it (mocking / asserting).

@alexrashed alexrashed merged commit 4e78bdf into master Nov 28, 2024
@alexrashed alexrashed deleted the env-var-prefix-data branch November 28, 2024 11:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants