Throttle analytics to 300 daily events per dev machine

[amcaplan]

WHY are these changes introduced?

Some users are experiencing errors thousands of times daily, with minimal value added by analytics from those reports. We are throttling analytics (including bugsnag, monorail, and open analytics) to 300 events daily, to balance the need for continuous visibility into error conditions with the need to make those problems more visible by clearing out much of the noise from individual users with e.g. a background process retrying continuously thousands of times daily.

This isn't a perfect solution, we still are getting up to hundreds of reports daily from noisy users. But it's a quick and easy solution that gets us started down the path and eliminates the vast majority of noise immediately.

WHAT is this pull request doing?

Adds a rate limiter around the reporting functions. At the moment, there is no granularity for specific errors, though it would be great to add that in the future.

How to test your changes?

1. Run an invalid command in verbose mode, e.g. pnpm shopify app dev --path /path/to/your/app --config nonexistent --verbose
2. Note that you see output like Skipping command analytics with a payload
3. Edit your local copy in packages/cli-kit/src/private/node to have an absurdly low rate limit (e.g. once daily)
4. Now run it again, and you should see Skipping command analytics due to rate limiting with a payload

Measuring impact

How do we know this change was effective? Please choose one:

• n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
• Existing analytics will cater for this addition
• PR includes analytics changes to measure impact

Checklist

• I've considered possible cross-platform impacts (Mac, Linux, Windows)
• I've considered possible documentation changes

[github-actions]

We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

[github-actions]

Coverage report

St.❔	Category	Percentage	Covered / Total
🟡	Statements	72.92% (+0.05% 🔼)	8344/11442
🟡	Branches	69.51% (+0.06% 🔼)	4071/5857
🟡	Functions	71.7% (+0.04% 🔼)	2171/3028
🟡	Lines	73.27% (+0.05% 🔼)	7894/10774

Show files with reduced coverage 🔻

St.❔	File	Statements	Branches	Functions	Lines
🟢	... / analytics.ts	88.33% (-2.41% 🔻)	69.44% (-1.14% 🔻)	100%	88.33% (-2.41% 🔻)
🟡	... / error-handler.ts	65% (+0.11% 🔼)	68.52% (-0.71% 🔻)	57.14% (+2.14% 🔼)	65.31% (+0.09% 🔼)

Test suite run success

1874 tests passing in 850 suites.

Report generated by 🧪jest coverage report action from 5614c49

[shauns]

One suggestion but rest looks great 👌

[shauns]

Given that this is only used in a pattern where the provided task is a stub, maybe we should narrow this down, drop the task piece and just return true/false. Can always reintroduce something later should the need come.

[amcaplan]

Yeah, that makes sense. I'll probably edit runAtMinimumInterval to do the same.

[github-actions]

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

• Some seemingly private modules might be re-exported through public modules.
• If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/private/node/conf-store.d.ts

@@ -6,11 +6,13 @@ interface CacheValue<T> {
 export type IntrospectionUrlKey = ;
 export type PackageVersionKey = ;
 type MostRecentOccurrenceKey = ;
+type RateLimitKey = ;
 type ExportedKey = IntrospectionUrlKey | PackageVersionKey;
 interface Cache {
     [introspectionUrlKey: IntrospectionUrlKey]: CacheValue<string>;
     [packageVersionKey: PackageVersionKey]: CacheValue<string>;
-    [MostRecentOccurrenceKey: MostRecentOccurrenceKey]: CacheValue<boolean>;
+    [mostRecentOccurrenceKey: MostRecentOccurrenceKey]: CacheValue<boolean>;
+    [rateLimitKey: RateLimitKey]: CacheValue<number[]>;
 }
 export interface ConfSchema {
     sessionStore: string;
@@ -63,7 +65,39 @@ interface TimeInterval {
  * days, hours, minutes, and seconds properties.
  * If the most recent occurrence is older than this, the task will be executed.
  * @param task - The task to run if the most recent occurrence is older than the timeout.
- * @returns The result of the task, or undefined if the task was not run.
+ * @returns true if the task was run, or false if the task was not run.
  */
-export declare function runAtMinimumInterval(key: string, timeout: TimeInterval, task: () => Promise<void>, config?: LocalStorage<ConfSchema>): Promise<boolean | undefined>;
+export declare function runAtMinimumInterval(key: string, timeout: TimeInterval, task: () => Promise<void>, config?: LocalStorage<ConfSchema>): Promise<boolean>;
+interface RunWithRateLimitOptions {
+    /**
+     * The key to use for the cache.
+     */
+    key: string;
+    /**
+     * The number of times the task can be run within the limit
+     */
+    limit: number;
+    /**
+     * The window of time after which the rate limit is refreshed,
+     * expressed as an object with days, hours, minutes, and seconds properties.
+     * If the most recent occurrence is older than this, the task will be executed.
+     */
+    timeout: TimeInterval;
+    /**
+     * The task to run if the most recent occurrence is older than the timeout.
+     */
+    task: () => Promise<void>;
+}
+/**
+ * Execute a task with a time-based rate limit. The rate limit is enforced by
+ * checking how many times that task has been executed in a window of time ending
+ * at the current time. If the task has been executed more than the allowed number
+ * of times in that window, the task will not be executed.
+ *
+ * Note that this function has side effects, as it will also remove events prior
+ * to the window of time that is being checked.
+ * @param options - The options for the rate limiting.
+ * @returns true, or undefined if the task was not run.
+ */
+export declare function runWithRateLimit(options: RunWithRateLimitOptions, config?: LocalStorage<ConfSchema>): Promise<boolean>;
 export {};
\ No newline at end of file

packages/cli-kit/dist/private/node/constants.d.ts

@@ -53,4 +53,10 @@ export declare const pathConstants: {
 export declare const sessionConstants: {
     expirationTimeMarginInMinutes: number;
 };
-export declare const bugsnagApiKey = "9e1e6889176fd0c795d5c659225e0fae";
\ No newline at end of file
+export declare const bugsnagApiKey = "9e1e6889176fd0c795d5c659225e0fae";
+export declare const reportingRateLimit: {
+    limit: number;
+    timeout: {
+        days: number;
+    };
+};
\ No newline at end of file