Skip to content

feat(analytics): make retention windows configurable #719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kriszyp merged 2 commits into from
May 23, 2026
+19 −2
Merged

feat(analytics): make retention windows configurable #719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a666252
feat(analytics): make retention windows configurable
kriszyp May 22, 2026
fe8f8b0
fix(analytics): treat aggregateRetentionMs=0 as disabled, not mass-de…
kriszyp May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions config-root.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,16 @@
"type": "number",
"description": "Number of aggregation cycles between node storage measurements. 0 to disable. Default: 10"
},
"rawRetentionMs": {
"type": "number",
"minimum": 0,
"description": "Milliseconds to retain raw (pre-aggregation) analytics. Clamped to at least aggregatePeriod to prevent data loss. Default: 3600000 (1 hour)"
},
"aggregateRetentionMs": {
"type": "number",
"minimum": 0,
"description": "Milliseconds to retain aggregated analytics. 0 disables cleanup (keep forever). Default: 31536000000 (1 year)"
},
"logging": { "$ref": "#/definitions/loggerConfig" }
}
}
Expand Down
9 changes: 7 additions & 2 deletions resources/analytics/write.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,11 +715,16 @@ function startScheduledTasks() {
nodeStorageInterval = envGet(CONFIG_PARAMS.ANALYTICS_STORAGEINTERVAL) ?? DEFAULT_STORAGE_INTERVAL;
const AGGREGATE_PERIOD = envGet(CONFIG_PARAMS.ANALYTICS_AGGREGATEPERIOD) * 1000;
if (AGGREGATE_PERIOD) {
// Clamp raw retention to at least one full aggregation period so raw records
// are never deleted before they can be rolled up.
const rawRetentionMs = Math.max(envGet(CONFIG_PARAMS.ANALYTICS_RAWRETENTIONMS) ?? RAW_EXPIRATION, AGGREGATE_PERIOD);
const aggregateRetentionMs = envGet(CONFIG_PARAMS.ANALYTICS_AGGREGATERETENTIONMS) ?? AGGREGATE_EXPIRATION;
Copy link
Copy Markdown
Contributor

@claude claude Bot May 22, 2026

Choose a reason for hiding this comment

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

aggregateRetentionMs: 0 silently deletes all aggregate data.

cleanup() computes end = Date.now() - expiration. If aggregateRetentionMs is 0, every cleanup cycle removes every aggregate record (end = Date.now() → all keys qualify). The schema allows minimum: 0 without documenting this semantics, so an operator who sets 0 expecting "disabled" instead loses all historical aggregates.

rawRetentionMs is protected by the Math.max(…, AGGREGATE_PERIOD) clamp, but aggregateRetentionMs has no guard.

Suggested fix — skip cleanup when the configured value is zero:

Suggested change
const aggregateRetentionMs = envGet(CONFIG_PARAMS.ANALYTICS_AGGREGATERETENTIONMS) ?? AGGREGATE_EXPIRATION;
const aggregateRetentionMs = envGet(CONFIG_PARAMS.ANALYTICS_AGGREGATERETENTIONMS) ?? AGGREGATE_EXPIRATION;
if (!aggregateRetentionMs) return; // 0 = disabled

Or, if full deletion is the intended "purge" semantics for 0, document it explicitly in the schema description and raise the minimum to a meaningful floor (e.g., 1).

setInterval(
async () => {
await aggregation(analyticsDelay, AGGREGATE_PERIOD);
await cleanup(getRawAnalyticsTable(), RAW_EXPIRATION);
await cleanup(getAnalyticsTable(), AGGREGATE_EXPIRATION);
await cleanup(getRawAnalyticsTable(), rawRetentionMs);
// 0 means "keep forever" — skip aggregate cleanup, matching storageInterval: 0 convention
if (aggregateRetentionMs) await cleanup(getAnalyticsTable(), aggregateRetentionMs);
},
Math.min(AGGREGATE_PERIOD / 2, 0x7fffffff)
).unref();
Expand Down
2 changes: 2 additions & 0 deletions utility/hdbTerms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,8 @@ export const LEGACY_CONFIG_PARAMS = {
*/
export const CONFIG_PARAMS = {
ANALYTICS_AGGREGATEPERIOD: 'analytics_aggregatePeriod',
ANALYTICS_AGGREGATERETENTIONMS: 'analytics_aggregateRetentionMs',
ANALYTICS_RAWRETENTIONMS: 'analytics_rawRetentionMs',
ANALYTICS_REPLICATE: 'analytics_replicate',
ANALYTICS_STORAGEINTERVAL: 'analytics_storageInterval',
AUTHENTICATION_AUTHORIZELOCAL: 'authentication_authorizeLocal',
Expand Down
Loading