diff --git a/packages/shared/src/telemetry/client.test.ts b/packages/shared/src/telemetry/client.test.ts index 87da22ba41..af9ec68d0c 100644 --- a/packages/shared/src/telemetry/client.test.ts +++ b/packages/shared/src/telemetry/client.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { TelemetryClient } from "./client.js"; +import { resolveTelemetryConfig } from "./config.js"; import type { TelemetryConfig, TelemetryState } from "./types.js"; const TEST_STATE: TelemetryState = { @@ -101,3 +102,126 @@ describe("TelemetryClient runtime event gate", () => { ]); }); }); + +// Stubs `fetch` to reject the batch with a given non-OK HTTP status for every +// endpoint the client may try. Returns the mock so call counts can be asserted. +function stubFetchStatus(status: number) { + const fetchMock = vi.fn().mockResolvedValue({ ok: false, status }); + vi.stubGlobal("fetch", fetchMock); + return fetchMock; +} + +// Phase 1 (PAP-2862): characterization pins for today's best-effort, silent-drop +// flush. On ANY non-OK response or network error the drained batch is dropped +// with no re-queue and no second attempt, and no `batchId` is emitted. These pins +// lock the current baseline; Impl-2 (PAP-2853) replaces them when retry lands. +describe("TelemetryClient silent-drop baseline (characterization)", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("drops the batch on a 429 with no re-queue", async () => { + const fetchMock = stubFetchStatus(429); + const { client } = makeClient(); + + client.track("install.started", {}); + await client.flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + + // Queue was drained despite the failure: a second flush sends nothing. + await client.flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("drops the batch on a 413 with no re-queue", async () => { + const fetchMock = stubFetchStatus(413); + const { client } = makeClient(); + + client.track("install.started", {}); + await client.flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + + await client.flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("drops the batch on a 400 with no re-queue", async () => { + const fetchMock = stubFetchStatus(400); + const { client } = makeClient(); + + client.track("install.started", {}); + await client.flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + + await client.flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("drops the batch on network error with no re-queue", async () => { + const fetchMock = vi.fn().mockRejectedValue(new Error("network down")); + vi.stubGlobal("fetch", fetchMock); + const { client } = makeClient(); + + client.track("install.started", {}); + await client.flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + + await client.flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("emits no batchId today", async () => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: true })); + const { client } = makeClient(); + + client.track("install.started", {}); + await client.flush(); + + expect(fetch).toHaveBeenCalledTimes(1); + expect(sentBody()).not.toHaveProperty("batchId"); + }); +}); + +// Phase 2 (PAP-2862): config surface for soft caps + backoff. Fields are optional +// and additive; `resolveTelemetryConfig` fills documented defaults centrally so no +// existing caller changes behavior. Nothing reads these yet — Impl-2 is the first +// consumer. +describe("resolveTelemetryConfig caps + backoff surface", () => { + it("resolveTelemetryConfig returns default caps and backoff", () => { + const config = resolveTelemetryConfig(); + + expect(config.maxEventsPerBatch).toBe(50); + expect(config.maxBodyBytes).toBe(524288); + expect(config.maxPendingRetryBatches).toBe(20); + expect(config.backoff).toEqual({ + baseDelayMs: 1_000, + maxDelayMs: 30_000, + maxAttempts: 5, + jitterRatio: 0.25, + }); + }); + + it("honors caps/backoff overrides", () => { + const config = resolveTelemetryConfig({ + maxEventsPerBatch: 10, + maxBodyBytes: 1024, + maxPendingRetryBatches: 3, + backoff: { + baseDelayMs: 500, + maxDelayMs: 5_000, + maxAttempts: 2, + jitterRatio: 0.1, + }, + }); + + expect(config.maxEventsPerBatch).toBe(10); + expect(config.maxBodyBytes).toBe(1024); + expect(config.maxPendingRetryBatches).toBe(3); + expect(config.backoff).toEqual({ + baseDelayMs: 500, + maxDelayMs: 5_000, + maxAttempts: 2, + jitterRatio: 0.1, + }); + }); +}); diff --git a/packages/shared/src/telemetry/config.ts b/packages/shared/src/telemetry/config.ts index 0e5252fc55..7596e3adea 100644 --- a/packages/shared/src/telemetry/config.ts +++ b/packages/shared/src/telemetry/config.ts @@ -1,25 +1,79 @@ -import type { TelemetryConfig } from "./types.js"; +import type { TelemetryBackoffConfig, TelemetryConfig } from "./types.js"; const CI_ENV_VARS = ["CI", "CONTINUOUS_INTEGRATION", "BUILD_NUMBER", "GITHUB_ACTIONS", "GITLAB_CI"]; +/** + * Single source of truth for telemetry soft caps + backoff. Kept as config + * *defaults* (not hardcoded flush logic) so later work reads config, not + * literals. Exported so Impl-2's `client.ts` consumer resolves the same values. + */ +export const TELEMETRY_DEFAULTS: { + readonly maxEventsPerBatch: number; + readonly maxBodyBytes: number; + readonly maxPendingRetryBatches: number; + readonly backoff: Readonly; +} = Object.freeze({ + maxEventsPerBatch: 50, + maxBodyBytes: 512 * 1024, // 524288 + maxPendingRetryBatches: 20, + backoff: Object.freeze({ + baseDelayMs: 1_000, + maxDelayMs: 30_000, + maxAttempts: 5, + jitterRatio: 0.25, + }), +}); + +/** Caller-supplied overrides for the additive caps + backoff surface. */ +export type TelemetryConfigOverrides = Partial< + Pick< + TelemetryConfig, + "enabled" | "maxEventsPerBatch" | "maxBodyBytes" | "maxPendingRetryBatches" | "backoff" + > +>; + +type ResolvedCaps = Pick< + TelemetryConfig, + "maxEventsPerBatch" | "maxBodyBytes" | "maxPendingRetryBatches" | "backoff" +>; + +/** + * Resolves soft caps + backoff, applying `TELEMETRY_DEFAULTS` for any field the + * caller did not override. One source of truth for both the config surface and + * Impl-2's future `client.ts` consumer. + */ +export function resolveCaps(overrides?: TelemetryConfigOverrides): ResolvedCaps { + return { + maxEventsPerBatch: overrides?.maxEventsPerBatch ?? TELEMETRY_DEFAULTS.maxEventsPerBatch, + maxBodyBytes: overrides?.maxBodyBytes ?? TELEMETRY_DEFAULTS.maxBodyBytes, + maxPendingRetryBatches: + overrides?.maxPendingRetryBatches ?? TELEMETRY_DEFAULTS.maxPendingRetryBatches, + backoff: { ...TELEMETRY_DEFAULTS.backoff, ...overrides?.backoff }, + }; +} + function isCI(): boolean { return CI_ENV_VARS.some((key) => process.env[key] === "true" || process.env[key] === "1"); } -export function resolveTelemetryConfig(fileConfig?: { enabled?: boolean }): TelemetryConfig { +export function resolveTelemetryConfig( + fileConfig?: { enabled?: boolean } & TelemetryConfigOverrides, +): TelemetryConfig { + const caps = resolveCaps(fileConfig); + if (process.env.PAPERCLIP_TELEMETRY_DISABLED === "1") { - return { enabled: false }; + return { enabled: false, ...caps }; } if (process.env.DO_NOT_TRACK === "1") { - return { enabled: false }; + return { enabled: false, ...caps }; } if (isCI()) { - return { enabled: false }; + return { enabled: false, ...caps }; } if (fileConfig?.enabled === false) { - return { enabled: false }; + return { enabled: false, ...caps }; } const endpoint = process.env.PAPERCLIP_TELEMETRY_ENDPOINT || undefined; - return { enabled: true, endpoint }; + return { enabled: true, endpoint, ...caps }; } diff --git a/packages/shared/src/telemetry/index.ts b/packages/shared/src/telemetry/index.ts index 494fbbd829..61ae4e460a 100644 --- a/packages/shared/src/telemetry/index.ts +++ b/packages/shared/src/telemetry/index.ts @@ -1,5 +1,6 @@ export { TelemetryClient } from "./client.js"; -export { resolveTelemetryConfig } from "./config.js"; +export { resolveTelemetryConfig, resolveCaps, TELEMETRY_DEFAULTS } from "./config.js"; +export type { TelemetryConfigOverrides } from "./config.js"; export { loadOrCreateState } from "./state.js"; export { trackInstallStarted, @@ -18,6 +19,7 @@ export { } from "./events.js"; export type { TelemetryConfig, + TelemetryBackoffConfig, TelemetryState, TelemetryEvent, TelemetryEventEnvelope, diff --git a/packages/shared/src/telemetry/types.ts b/packages/shared/src/telemetry/types.ts index b5c2aef21b..039d99118c 100644 --- a/packages/shared/src/telemetry/types.ts +++ b/packages/shared/src/telemetry/types.ts @@ -10,11 +10,33 @@ export interface TelemetryState { firstSeenVersion: string; } +/** + * Exponential-backoff-with-jitter parameters for the (future) batched-retry + * sender. Shape mirrors the plugin worker crash-recovery backoff + * (`server/src/services/plugin-worker-manager.ts`). Consumed by Impl-2; nothing + * reads it yet. + */ +export interface TelemetryBackoffConfig { + baseDelayMs: number; + maxDelayMs: number; + maxAttempts: number; + jitterRatio: number; +} + export interface TelemetryConfig { enabled: boolean; endpoint?: string; app?: string; schemaVersion?: string; + /** + * Optional, additive soft caps + backoff. Defaulted centrally in + * `resolveTelemetryConfig`; no wire/envelope change and no consumer today — + * Impl-2 (PAP-2853) is the first reader. + */ + maxEventsPerBatch?: number; + maxBodyBytes?: number; + maxPendingRetryBatches?: number; + backoff?: TelemetryBackoffConfig; } export type TelemetryDimensionValue = string | number | boolean;