Repository navigation
Analytics: Track feature adoption with setting values and a config snapshot - #1370
Draft
gabrielcld2 wants to merge 4 commits into
Draft
gabrielcld2 wants to merge 4 commits into
gabrielcld2 wants to merge 4 commits into
Conversation
…fig snapshot `settings_saved` only reported which keys changed, and features left at their defaults were never reported at all, so adoption of lazy loading, responsive images, formats or adaptive streaming couldn't be measured. - `settings_saved` now carries `changed_values` for an allowlist of image, lazy loading, responsive and video settings. Anything outside the list, including the connection string, is never sent. - Values are normalised: toggles to booleans, numbers to ints, free-text transformations to a presence flag, and crop sizes to a count of custom crops. - A one-time `feature_config_snapshot` sends the full feature config on the first admin load once connected, once per cloud name. It covers wizard setups, constant-based connections, account switches and existing installs upgrading.
- Send the snapshot only on admin page loads by administrators, never from admin-ajax requests or for other roles. - Autoload the one-time flag so checking it costs no query, and gate the first send with add_option() so concurrent requests can't both send it. - Leave missing setting values out instead of reporting them as off. - Send the lazy-loading threshold only when it is a CSS length; it is a free-text field. - Allowlist feature settings per settings page. - Report `changed_values` as read back after the save, so they reflect any save filters. - Remove the analytics one-time flags on uninstall.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes feature adoption measurable: which delivery features each install runs, and with which values.
The problem
settings_savedonly carrieschanged_keys, not the new values.Approach
Values on
settings_savedchanged_valuesobject sits next tochanged_keys, e.g.{ "image_quality": "80", "image_format": "avif" }.Analytics::FEATURE_SETTINGS, keyed by settings page: image, video, lazy loading and responsive images. Everything else, including the connection string, is never sent.changed_valuesis left out when no allowlisted setting changed.cloudinary_settings_save_settingfilters rather than what was submitted.100px.image_freeform,video_freeform) can contain public IDs, overlay text or URLs, so they are only sent as a presence boolean.crop_sizesis sent as the number of image sizes with a custom crop.One-time
feature_config_snapshotevent (categoryfeatures)add_option(), so concurrent first requests can't both send it.CLOUDINARY_CONNECTION_STRINGconstant, account switches, and existing installs upgrading to this version (the baseline backfill).cloudinary_analytics_enabled, so it is still sent if they are turned on later.first_api_consumptionone) are now removed by the uninstall clean-up, so a reinstall starts fresh.Open questions
These may still change the payload. Keeping this as a draft until they are answered:
changed_valuesobject, or should it be flat prefixed fields, or one event per changed setting?enable_breakpoints,adaptive_streaming_mode) as now, or the names proposed in the request (responsive, "mode")?breakpoints) has no default, so it is missing from the snapshot until a site sets it. Should it get a default?QA notes
npm run test:unit -- --filter Test_Analytics_Featurecovers the allowlist, each kind of normalisation, and the snapshot rules: once per account, re-sent after an account switch, not sent while disconnected, with analytics off, for non-administrators or from admin-ajax, and only one of several concurrent first requests sending it.tests/e2e/settings-analytics.spec.jscheckschanged_valueson a save of each feature page: image, video, lazy loading and responsive.tests/e2e/features-analytics.spec.jschecks the snapshot's admin wiring. It is tagged@serialbecause its one-time flag is shared by the whole site.wp option delete _cloudinary_feature_snapshot_emitted.feature_config_snapshotevent, with the current feature values.settings_savedevent should carry the new values underchanged_values.settings_savedevent is sent, it has nochanged_values.