diff --git a/services/diagnostics-api/src/input.ts b/services/diagnostics-api/src/input.ts index e4ea0b4..49cdd08 100644 --- a/services/diagnostics-api/src/input.ts +++ b/services/diagnostics-api/src/input.ts @@ -8,14 +8,6 @@ export type InputFailure = { export type InputResult = { ok: true; value: T } | InputFailure; -export type NormalizedProperties = Record; - -export interface NormalizedBreadcrumb { - name: string; - timestampMillis: number; - properties: NormalizedProperties; -} - export interface NormalizedDevice { deviceName: string; deviceModel: string; @@ -36,16 +28,12 @@ export interface NormalizedBugPayload { contact: string; logs: string; device: NormalizedDevice; - breadcrumbs: NormalizedBreadcrumb[]; schemaVersion: 1; } export const MAX_LOG_BYTES = 192 * 1024; -export const MAX_BREADCRUMBS_JSON_BYTES = 16_000; export const MAX_DEVICE_JSON_BYTES = 4_000; -const MAX_PROPERTIES = 12; -const MAX_BREADCRUMBS = 40; const MISSING = Symbol("missing"); const UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; const UTF8_ENCODER = new TextEncoder(); @@ -177,8 +165,6 @@ export function normalizeBug(body: JsonObject): InputResult { - if (raw === MISSING) return success([]); - if (!Array.isArray(raw)) return failure(400, "invalid_breadcrumbs"); - - const breadcrumbs: NormalizedBreadcrumb[] = []; - for (const item of raw.slice(0, MAX_BREADCRUMBS)) { - if (!isPlainObject(item)) return failure(400, "invalid_breadcrumbs"); - const name = stringField(item, ["name"], 64, "invalid_breadcrumbs", true, true); - if (!name.ok) return name; - const timestamp = timestampField(item, ["timestampMillis", "timestamp_millis", "ts"]); - if (!timestamp.ok) return failure(400, "invalid_breadcrumbs"); - const properties = normalizeProperties( - pick(item, ["properties", "props"]), - "invalid_breadcrumbs", - ); - if (!properties.ok) return properties; - - breadcrumbs.push({ - name: name.value, - timestampMillis: timestamp.value, - properties: properties.value, - }); - if (jsonBytes(breadcrumbs) > MAX_BREADCRUMBS_JSON_BYTES) { - breadcrumbs.pop(); - break; - } - } - return success(breadcrumbs); -} - -function normalizeProperties( - raw: unknown | typeof MISSING, - error: string, -): InputResult { - if (raw === MISSING) return success({}); - if (!isPlainObject(raw)) return failure(400, error); - - const entries: Array<[string, string]> = []; - const normalizedKeys = new Set(); - for (const [key, value] of Object.entries(raw).slice(0, MAX_PROPERTIES)) { - if (typeof value !== "string") return failure(400, error); - const normalizedKey = truncateUtf8(key, 40); - if (normalizedKey.length === 0 || normalizedKeys.has(normalizedKey)) { - return failure(400, error); - } - normalizedKeys.add(normalizedKey); - entries.push([normalizedKey, truncateUtf8(value, 128)]); - } - return success(Object.fromEntries(entries)); -} - function normalizeDevice(raw: unknown | typeof MISSING): InputResult { if (raw === MISSING) raw = {}; if (!isPlainObject(raw)) return failure(400, "invalid_device"); diff --git a/services/diagnostics-api/src/storage.ts b/services/diagnostics-api/src/storage.ts index d5df546..19b0c3a 100644 --- a/services/diagnostics-api/src/storage.ts +++ b/services/diagnostics-api/src/storage.ts @@ -32,12 +32,12 @@ export async function storeBug( try { const result = await database .prepare( - `INSERT INTO bugs ( - id, received_at, occurred_at, install_id, app_version, platform, - what_happened, expected, steps, contact, logs_r2_key, - device_json, breadcrumbs_json, status, schema_version - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'open', ?) - ON CONFLICT(id) DO NOTHING`, + `INSERT INTO bugs ( + id, received_at, occurred_at, install_id, app_version, platform, + what_happened, expected, steps, contact, logs_r2_key, + device_json, status, schema_version + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'open', ?) + ON CONFLICT(id) DO NOTHING`, ) .bind( payload.id, @@ -52,7 +52,6 @@ export async function storeBug( payload.contact, logsKey, JSON.stringify(payload.device), - JSON.stringify(payload.breadcrumbs), payload.schemaVersion, ) .run(); diff --git a/services/diagnostics-api/test/input.test.ts b/services/diagnostics-api/test/input.test.ts index 5d343f2..fb30e08 100644 --- a/services/diagnostics-api/test/input.test.ts +++ b/services/diagnostics-api/test/input.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from "vitest"; import { - MAX_BREADCRUMBS_JSON_BYTES, MAX_DEVICE_JSON_BYTES, MAX_LOG_BYTES, normalizeBug, @@ -119,20 +118,11 @@ describe("normalizers", () => { }); }); - it("keeps logs, breadcrumbs, and device JSON within valid byte budgets", () => { - const properties = Object.fromEntries( - Array.from({ length: 12 }, (_, index) => [`key-${index}-${"\u0000".repeat(40)}`, "\u0000".repeat(128)]), - ); - const breadcrumbs = Array.from({ length: 40 }, (_, index) => ({ - name: `crumb-${index}`, - timestamp_millis: index, - properties, - })); + it("keeps logs and device JSON within valid byte budgets", () => { const result = normalizeBug( bugPayload({ include_logs: true, logs: "😀".repeat(60_000), - breadcrumbs, device: { device_name: "\u0000".repeat(200), device_model: "\u0000".repeat(200), @@ -145,14 +135,9 @@ describe("normalizers", () => { expect(result.ok).toBe(true); if (!result.ok) return; - const breadcrumbsJson = JSON.stringify(result.value.breadcrumbs); const deviceJson = JSON.stringify(result.value.device); expect(ENCODER.encode(result.value.logs).byteLength).toBe(MAX_LOG_BYTES); - expect(ENCODER.encode(breadcrumbsJson).byteLength).toBeLessThanOrEqual( - MAX_BREADCRUMBS_JSON_BYTES, - ); expect(ENCODER.encode(deviceJson).byteLength).toBeLessThanOrEqual(MAX_DEVICE_JSON_BYTES); - expect(JSON.parse(breadcrumbsJson)).toEqual(result.value.breadcrumbs); expect(JSON.parse(deviceJson)).toEqual(result.value.device); }); @@ -218,7 +203,6 @@ function bugPayload(overrides: Record = {}): Record { const row = await env.DB.prepare( `SELECT occurred_at AS occurredAt, logs_r2_key AS logsKey, - device_json AS deviceJson, breadcrumbs_json AS breadcrumbsJson + device_json AS deviceJson FROM bugs WHERE id = ?`, ) .bind(id) @@ -129,7 +129,6 @@ describe("diagnostics Worker", () => { occurredAt: number; logsKey: string; deviceJson: string; - breadcrumbsJson: string; }>(); expect(row?.occurredAt).toBe(3); expect(JSON.parse(row?.deviceJson ?? "null")).toEqual({ @@ -139,9 +138,6 @@ describe("diagnostics Worker", () => { network: "offline", batteryLevel: "90%", }); - expect(JSON.parse(row?.breadcrumbsJson ?? "null")).toEqual([ - { name: "opened", timestampMillis: 2, properties: {} }, - ]); expect(await (await env.BLOBS.get(row?.logsKey ?? "missing"))?.text()).toBe("first logs"); const objects = await env.BLOBS.list({ prefix: `bugs/${id}/` }); @@ -198,15 +194,15 @@ describe("diagnostics Worker", () => { `INSERT INTO bugs (id, received_at, occurred_at, install_id, app_version, platform, what_happened, expected, steps, contact, logs_r2_key, - device_json, breadcrumbs_json, status, schema_version) - VALUES (?, ?, ?, ?, '', '', 'failed', 'worked', '', '', ?, '{}', '[]', 'open', 1)`, + device_json, status, schema_version) + VALUES (?, ?, ?, ?, '', '', 'failed', 'worked', '', '', ?, '{}', 'open', 1)`, ).bind(oldBugId, oldReceivedAt, oldReceivedAt, INSTALL_ID, oldBugKey), env.DB.prepare( `INSERT INTO bugs (id, received_at, occurred_at, install_id, app_version, platform, what_happened, expected, steps, contact, logs_r2_key, - device_json, breadcrumbs_json, status, schema_version) - VALUES (?, ?, ?, ?, '', '', 'failed', 'worked', '', '', NULL, '{}', '[]', 'open', 1)`, + device_json, status, schema_version) + VALUES (?, ?, ?, ?, '', '', 'failed', 'worked', '', '', NULL, '{}', 'open', 1)`, ).bind(currentBugId, Date.now(), Date.now(), INSTALL_ID), ]); @@ -283,10 +279,10 @@ describe("diagnostics Worker", () => { INSERT INTO bugs ( id, received_at, occurred_at, install_id, app_version, platform, what_happened, expected, steps, contact, logs_r2_key, - device_json, breadcrumbs_json, status, schema_version + device_json, status, schema_version ) SELECT 'retention-backlog-' || printf('%04d', value), ?, ?, ?, '', '', - 'failed', 'worked', '', '', NULL, '{}', '[]', 'open', 1 + 'failed', 'worked', '', '', NULL, '{}', 'open', 1 FROM sequence`, ) .bind(oldReceivedAt, oldReceivedAt, INSTALL_ID) @@ -358,7 +354,6 @@ function bugPayload(id: string, logs: string): Record { network: "offline", batteryLevel: "90%", }, - breadcrumbs: [{ name: "opened", timestampMillis: 2, properties: {} }], schemaVersion: 1, }; } @@ -382,7 +377,6 @@ function normalizedBug(id: string, logs: string): NormalizedBugPayload { network: "offline", batteryLevel: "90%", }, - breadcrumbs: [], schemaVersion: 1, }; } diff --git a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/BreadcrumbBuffer.kt b/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/BreadcrumbBuffer.kt deleted file mode 100644 index 29c2951..0000000 --- a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/BreadcrumbBuffer.kt +++ /dev/null @@ -1,47 +0,0 @@ -package com.vnidrop.app.diagnostics - -import com.vnidrop.app.logging.platformNowMillis -import kotlin.concurrent.atomics.AtomicReference -import kotlin.concurrent.atomics.ExperimentalAtomicApi - -/** - * Fixed-size ring of high-level app breadcrumbs for crash / bug context. - * Always in-memory only; never auto-uploaded without policy + transport. - * - * Updates are best-effort under concurrency; losing a breadcrumb is preferable - * to blocking a dying process on a lock. - */ -@OptIn(ExperimentalAtomicApi::class) -class BreadcrumbBuffer( - private val capacity: Int = DefaultCapacity, -) { - init { - require(capacity > 0) { "capacity must be positive" } - } - - private val items = AtomicReference>(emptyList()) - - fun add(name: String, properties: Map = emptyMap(), timestampMillis: Long = platformNowMillis()) { - val sanitizedName = sanitizeDiagnosticName(name) - if (sanitizedName.isBlank()) return - val crumb = Breadcrumb( - name = sanitizedName, - timestampMillis = timestampMillis, - properties = sanitizeDiagnosticProperties(properties), - ) - while (true) { - val current = items.load() - if (items.compareAndSet(current, (current + crumb).takeLast(capacity))) return - } - } - - fun snapshot(): List = items.load() - - fun clear() { - items.store(emptyList()) - } - - companion object { - const val DefaultCapacity = 40 - } -} diff --git a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/BugReportService.kt b/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/BugReportService.kt index a5c2c36..128bde7 100644 --- a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/BugReportService.kt +++ b/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/BugReportService.kt @@ -21,7 +21,6 @@ data class BugReportDraft( class BugReportService( private val preferencesRepository: PreferencesRepository, private val transport: DiagnosticsTransport, - private val breadcrumbs: BreadcrumbBuffer, private val appVersion: String, private val platform: String, private val logReader: () -> String = { @@ -53,7 +52,6 @@ class BugReportService( network = deviceInfo?.network?.takeUtf8Bytes(96), batteryLevel = deviceInfo?.batteryLevel?.takeUtf8Bytes(64), ), - breadcrumbs = breadcrumbs.snapshot(), ) } diff --git a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsCoordinator.kt b/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsCoordinator.kt index e035f65..0ae44b1 100644 --- a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsCoordinator.kt +++ b/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsCoordinator.kt @@ -13,7 +13,6 @@ import kotlinx.coroutines.launch class DiagnosticsCoordinator( val preferencesRepository: PreferencesRepository, val transport: DiagnosticsTransport, - val breadcrumbs: BreadcrumbBuffer, val bugReports: BugReportService, private val scope: CoroutineScope, ) { @@ -32,18 +31,15 @@ class DiagnosticsCoordinator( scope: CoroutineScope, transport: DiagnosticsTransport = NoOpDiagnosticsTransport(), ): DiagnosticsCoordinator { - val breadcrumbs = BreadcrumbBuffer() val bugReports = BugReportService( preferencesRepository = preferencesRepository, transport = transport, - breadcrumbs = breadcrumbs, appVersion = appVersion, platform = platform, ) return DiagnosticsCoordinator( preferencesRepository = preferencesRepository, transport = transport, - breadcrumbs = breadcrumbs, bugReports = bugReports, scope = scope, ) diff --git a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsJson.kt b/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsJson.kt index 3476416..56dbecc 100644 --- a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsJson.kt +++ b/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsJson.kt @@ -8,8 +8,6 @@ internal object DiagnosticsJson { internal const val MaxInstallIdBytes = 80 internal const val MaxAppVersionBytes = 40 internal const val MaxPlatformBytes = 40 - private const val MaxBreadcrumbsJsonBytes = 16_000 - private const val MaxBreadcrumbs = 40 fun bugBody(report: BugReport): String { val logs = if (report.includeLogs) report.logs else "" @@ -72,47 +70,7 @@ internal object DiagnosticsJson { appendJsonField("network", report.device.network.orEmpty()) append(',') appendJsonField("batteryLevel", report.device.batteryLevel.orEmpty()) - append("},") - append("\"breadcrumbs\":") - appendBreadcrumbs(report.breadcrumbs) append('}') - } - - private fun StringBuilder.appendBreadcrumbs(crumbs: List) { - append('[') - var encodedBytes = 2 - var appended = 0 - for (crumb in crumbs) { - if (appended == MaxBreadcrumbs) break - val name = sanitizeDiagnosticName(crumb.name) - if (name.isBlank() || crumb.timestampMillis < 0) continue - val encoded = buildString { - append('{') - appendJsonField("name", name) - append(',') - append("\"timestampMillis\":") - append(crumb.timestampMillis) - append(',') - append("\"properties\":") - appendStringMap(crumb.properties) - append('}') - } - val additionBytes = encoded.encodeToByteArray().size + if (appended == 0) 0 else 1 - if (encodedBytes + additionBytes > MaxBreadcrumbsJsonBytes) break - if (appended > 0) append(',') - append(encoded) - encodedBytes += additionBytes - appended += 1 - } - append(']') - } - - private fun StringBuilder.appendStringMap(map: Map) { - append('{') - sanitizeDiagnosticProperties(map).entries.forEachIndexed { index, (key, value) -> - if (index > 0) append(',') - appendJsonField(key, value) - } append('}') } diff --git a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsModels.kt b/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsModels.kt index 43392df..7cab917 100644 --- a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsModels.kt +++ b/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsModels.kt @@ -5,12 +5,6 @@ package com.vnidrop.app.diagnostics * intentionally abstracted; nothing here assumes a network backend. */ -data class Breadcrumb( - val name: String, - val timestampMillis: Long, - val properties: Map = emptyMap(), -) - data class DeviceSnapshot( val deviceName: String?, val deviceModel: String?, @@ -32,7 +26,6 @@ data class BugReport( val includeLogs: Boolean, val logs: String, val device: DeviceSnapshot, - val breadcrumbs: List, val schemaVersion: Int = DiagnosticsSchemaVersion, ) diff --git a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsSanitizer.kt b/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsSanitizer.kt index 6cef846..4b64a9e 100644 --- a/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsSanitizer.kt +++ b/shared/src/commonMain/kotlin/com/vnidrop/app/diagnostics/DiagnosticsSanitizer.kt @@ -1,30 +1,11 @@ package com.vnidrop.app.diagnostics -internal const val MaxDiagnosticProperties = 12 -internal const val MaxDiagnosticPropertyKeyBytes = 40 -internal const val MaxDiagnosticPropertyValueBytes = 128 -internal const val MaxDiagnosticNameBytes = 64 - internal fun sanitizeDiagnosticsInstallId(value: String): String { val trimmed = value.trim() if (trimmed.any { it.code < 0x20 || it.code == 0x7f }) return "" return trimmed.takeUtf8Bytes(DiagnosticsJson.MaxInstallIdBytes) } -internal fun sanitizeDiagnosticName(name: String): String = - name.takeUtf8Bytes(MaxDiagnosticNameBytes) - -internal fun sanitizeDiagnosticProperties(properties: Map): Map { - val sanitized = LinkedHashMap(minOf(properties.size, MaxDiagnosticProperties)) - for ((rawKey, rawValue) in properties) { - val key = rawKey.takeUtf8Bytes(MaxDiagnosticPropertyKeyBytes) - if (key.isEmpty() || key in sanitized) continue - sanitized[key] = LogRedactor.redact(rawValue).takeUtf8Bytes(MaxDiagnosticPropertyValueBytes) - if (sanitized.size == MaxDiagnosticProperties) break - } - return sanitized -} - internal fun String.takeUtf8Bytes(maxBytes: Int): String { require(maxBytes >= 0) { "maxBytes must not be negative" } val encoded = encodeToByteArray() diff --git a/shared/src/commonTest/kotlin/com/vnidrop/app/diagnostics/DiagnosticsTest.kt b/shared/src/commonTest/kotlin/com/vnidrop/app/diagnostics/DiagnosticsTest.kt index 0f31727..3fa13a6 100644 --- a/shared/src/commonTest/kotlin/com/vnidrop/app/diagnostics/DiagnosticsTest.kt +++ b/shared/src/commonTest/kotlin/com/vnidrop/app/diagnostics/DiagnosticsTest.kt @@ -204,22 +204,11 @@ class DiagnosticsTest { assertTrue(redacted.contains("[redacted-endpoint]")) } - @Test - fun breadcrumbBufferKeepsOnlyLatestEntries() { - val buffer = BreadcrumbBuffer(capacity = 3) - buffer.add("a") - buffer.add("b") - buffer.add("c") - buffer.add("d") - assertEquals(listOf("b", "c", "d"), buffer.snapshot().map { it.name }) - } - @Test fun bugReportRequiresWhatAndExpected() = runTest { val service = BugReportService( preferencesRepository = fakePrefs(), transport = RecordingDiagnosticsTransport(), - breadcrumbs = BreadcrumbBuffer(), appVersion = "1.0", platform = "Test", logReader = { "logs" }, @@ -238,7 +227,6 @@ class DiagnosticsTest { val service = BugReportService( preferencesRepository = fakePrefs(), transport = throwingTransport, - breadcrumbs = BreadcrumbBuffer(), appVersion = "1.0", platform = "Test", ) @@ -255,7 +243,6 @@ class DiagnosticsTest { val service = BugReportService( preferencesRepository = fakePrefs(), transport = transport, - breadcrumbs = BreadcrumbBuffer(), appVersion = "1.0", platform = "Test", logReader = { LogRedactor.redact("ticket=abcdefghijklmnopqrstuvwxyz012345 plain") }, @@ -286,7 +273,6 @@ class DiagnosticsTest { val service = BugReportService( preferencesRepository = fakePrefs(), transport = RecordingDiagnosticsTransport(), - breadcrumbs = BreadcrumbBuffer(), appVersion = "1.0", platform = "Test", logReader = { rawLogs }, @@ -304,7 +290,6 @@ class DiagnosticsTest { val service = BugReportService( preferencesRepository = fakePrefs(), transport = RecordingDiagnosticsTransport(), - breadcrumbs = BreadcrumbBuffer(), appVersion = "1.0", platform = "Test", logReader = { "🙂".repeat(60_000) }, @@ -333,7 +318,6 @@ class DiagnosticsTest { includeLogs = includeLogs, logs = logs, device = DeviceSnapshot(null, null, "OS", null, null), - breadcrumbs = emptyList(), ) private fun fakePrefs() = FakePreferencesRepository( diff --git a/shared/src/commonTest/kotlin/com/vnidrop/app/feature/ViewModelsTest.kt b/shared/src/commonTest/kotlin/com/vnidrop/app/feature/ViewModelsTest.kt index 7e5c5f3..f95b342 100644 --- a/shared/src/commonTest/kotlin/com/vnidrop/app/feature/ViewModelsTest.kt +++ b/shared/src/commonTest/kotlin/com/vnidrop/app/feature/ViewModelsTest.kt @@ -18,7 +18,6 @@ import com.vnidrop.app.core.ShareAccessPolicy import com.vnidrop.app.core.Transfer import com.vnidrop.app.core.TransferDirection import com.vnidrop.app.core.TransferStatus -import com.vnidrop.app.diagnostics.BreadcrumbBuffer import com.vnidrop.app.diagnostics.BugReportService import com.vnidrop.app.diagnostics.DiagnosticsTransport import com.vnidrop.app.diagnostics.NoOpDiagnosticsTransport @@ -989,7 +988,6 @@ class ViewModelsTest { BugReportService( preferencesRepository = preferences, transport = transport, - breadcrumbs = BreadcrumbBuffer(), appVersion = "1.0", platform = "Test", logReader = { "sample log line" },