feat(error-tracking): return frame releases over the resolution wire

[ablaszkiewicz]

Changes

This PR brings back the old frame-release concept, and this time it survives the service split. Symbolication attaches the symbol set's release to each frame it resolves (the pre-#75456 join, restored), and Frame.release is now serializable, so it crosses the cymbal <-> cymbal-resolution wire inside each resolved frame's JSON:

Done {
  resolved_exception_json: {
    ...
    "stacktrace": {
      "frames": [
        { ..., "release": {...} } // NEW
      ]
    }
  }
}

No new proto fields. Skew-safe in both directions: an older server just doesn't send the key (parses to None), an older client ignores the unknown key.

Then we do release resolution in this order:

1. Has $release_id? Try to resolve release from it
2. Has app-metadata? Hash it and try to resolve release from it
3. Has releases attached to frames? Pick the latest one by created_at

Since Frame.release serializes now, two guards keep it out of everything clickhouse-bound:

• into_resolved strips it from all frames right after $exception_release selection, before any output serialization
• the PG frame cache strips it on save and keeps re-joining it on load (so a release re-bind never goes stale in cache rows)

Tests

• proto contract round trip for the single-field Done
• server test: release serialized inside the resolved frame JSON, key omitted for release-less frames
• client test: release parsed back onto Frame.release, missing key parses to None (old-server skew)
• parity test: $exception_release identical on the local and remote paths
• strip test: the resolved exception list never contains a release key
• golden fingerprint tests unchanged

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Written with Claude Code. First iteration shipped releases as a Done.releases_json sidecar (kept Frame.release serde-skipped); we reworked it to per-frame serialization because the flattening lost the frame-release association and the sidecar was an extra moving part. The strip-at-into_resolved choke point plus the PG save-path strip replace the type-level #[serde(skip)] guarantee.

[posthog]

🦔 ReviewHog reviewed this pull request

Found 0 must fix, 1 should fix, 1 consider.

Published 2 findings (view the review).

[posthog]

ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

[posthog]

ReviewHog Report

Business logic

Issues: 2 issues

Files (20)

• rust/cymbal/src/core/types/frames/releases.rs
• rust/cymbal/src/core/types/frames/mod.rs
• rust/cymbal/src/core/symbolication/symbol/local.rs
• rust/cymbal/src/core/symbolication/symbol/records.rs
• rust/cymbal/.sqlx/query-7002c21150e6a6e42b57cb9ebf9e71e76ff7b6595c2953ed108da0bd0f2279b9.json
• rust/cymbal/.sqlx/query-eb812dd93e8af10192d414fb1d55fcb2bbd74c80b293672c4579531521e905cd.json
• rust/cymbal/src/core/types/langs/apple.rs
• rust/cymbal/src/core/types/langs/custom.rs
• rust/cymbal/src/core/types/langs/dart.rs
• rust/cymbal/src/core/types/langs/go.rs
• rust/cymbal/src/core/types/langs/hermes.rs
• rust/cymbal/src/core/types/langs/java.rs
• rust/cymbal/src/core/types/langs/js.rs
• rust/cymbal/src/core/types/langs/native.rs
• rust/cymbal/src/core/types/langs/node.rs
• rust/cymbal/src/core/types/langs/php.rs
• rust/cymbal/src/core/types/langs/python.rs
• rust/cymbal/src/core/types/langs/ruby.rs
• rust/cymbal/src/modes/processing/fingerprinting/mod.rs
• rust/cymbal/src/modes/processing/normalization.rs

What were the main changes

• Restores the pre-#75456 symbol-set→release join, verbatim: ReleaseRecord::for_symbol_set_ref on fresh resolves (local.rs) and for_symbol_set_id on PG frame-cache loads (records.rs)
• Adds Frame.release as an in-memory-only carrier (#[serde(skip)]) so it never leaks into the ClickHouse-bound event JSON
• Adds ReleaseRecord::collect_from_frames (dedup by id) and ReleaseRecord::latest (deterministic tie-break) helpers used by both the wire sidecar and the fallback selector
• Regenerated .sqlx offline query metadata for the restored join queries (byte-identical to pre-#75456)
• Mechanical fallout: every language's Frame struct-literal construction sites now set the new release field to None

[posthog]

for_symbol_set_ref queries with the untruncated ref, but stored refs are truncated to MAX_REF_BYTES

Why we think it's a valid issue

• Checked: Fetched the actual PR-head code (releases.rs and local.rs at 0ba8f76) since the local checkout is a post-refactor(error-tracking): remove the broken $exception_releases enrichment #75456 state without the restored join; cross-read saving.rs truncation and js.rs ref derivation.
• Found: Every SymbolSetRecord DB path truncates the ref before hitting posthog_errortrackingsymbolset.ref — load at saving.rs:466 (let truncated_ref = truncate_ref(set_ref) → WHERE ... ref = $2), plus save/save_data_if_missing/save_failure at saving.rs:519/548/588 — with MAX_REF_BYTES = 2048 (saving.rs:26-47). The stored ref is therefore never longer than 2048 bytes.
• Found: The PR's ReleaseRecord::for_symbol_set_ref runs ... INNER JOIN ... WHERE ss.ref = $1 AND ss.team_id = $2 bound to the raw, untruncated symbol_set_ref — no truncate_ref. The PR call site in local.rs passes the identical &set_ref (from frame.symbol_set_ref(debug_images)) to both SymbolSetRecord::load(...) and ReleaseRecord::for_symbol_set_ref(...) in one tokio::try_join!.
• Found: For JS web frames symbol_set_ref is derived from the client-controlled source_url (js.rs:121-124 → get_ref/source_url), which genuinely can exceed 2048 bytes — the exact case truncate_ref was added for (its comment cites the Postgres BTree index size limit) and that the negative cache is byte-weighted against.
• Impact: For any symbol set whose ref exceeds 2048 bytes and has a release bound, the fresh-resolve path silently drops the release: load truncates and matches the set, while for_symbol_set_ref compares the truncated stored ss.ref against the full $1 and returns zero rows. The release exists but never reaches Frame.release, degrading the $exception_release fallback. Reachable, named trigger + consequence, trivial one-line fix; not speculative (long refs are a known, already-handled input). The for_symbol_set_id UUID path is correctly noted as unaffected.

Issue description

The restored fresh-resolve join looks up a symbol set's release by matching the raw, untruncated symbol_set_ref string against posthog_errortrackingsymbolset.ref (ReleaseRecord::for_symbol_set_ref, releases.rs:80-103, called from local.rs:159-166 with &set_ref as received from frame.symbol_set_ref(debug_images)). But posthog_errortrackingsymbolset.ref is never stored verbatim for long refs: both SymbolSetRecord::load and SymbolSetRecord::save (symbol_store/saving.rs) run the ref through truncate_ref(..) (capped at MAX_REF_BYTES = 2048) before querying/writing. So for any symbol set whose ref exceeds 2048 bytes, SymbolSetRecord::load(&self.pool, team_id, &set_ref) still succeeds (it truncates internally and matches the stored, truncated value), while the concurrently-run ReleaseRecord::for_symbol_set_ref(&self.pool, &set_ref, team_id) compares the DB's truncated ss.ref against the full untruncated $1 and returns zero rows — silently dropping the release even though the symbol set (and its bound release) genuinely exists. This is a real, reachable case: JS web frames derive their symbol_set_ref from the resolved source URL (RawJSFrame::get_ref/symbol_set_ref, langs/js.rs), which can plausibly exceed 2048 bytes (long query strings, signed CDN URLs, etc.) — exactly the scenario truncate_ref and its dedicated tests in saving.rs were written to handle. Note the PG-frame-cache load path (ReleaseRecord::for_symbol_set_id in records.rs, matching by the stored symbol_set_id UUID) is NOT affected, since it doesn't depend on ref truncation at all — only the fresh-resolve path in local.rs is impacted.

Suggested fix

Truncate set_ref the same way before calling ReleaseRecord::for_symbol_set_ref, e.g. reuse (or expose) the existing truncate_ref helper from symbol_store::saving at the call site in local.rs, or have for_symbol_set_ref truncate internally the same way SymbolSetRecord::load/save do, so both queries key off the identical truncated value that's actually persisted in posthog_errortrackingsymbolset.ref.

Prompt to fix with AI (copy-paste)

## Context
@rust/cymbal/src/core/types/frames/releases.rs#L80-103
@rust/cymbal/src/core/types/frames/releases.rs#L159-166

<issue_description>
The restored fresh-resolve join looks up a symbol set's release by matching the raw, untruncated `symbol_set_ref` string against `posthog_errortrackingsymbolset.ref` (`ReleaseRecord::for_symbol_set_ref`, releases.rs:80-103, called from local.rs:159-166 with `&set_ref` as received from `frame.symbol_set_ref(debug_images)`). But `posthog_errortrackingsymbolset.ref` is never stored verbatim for long refs: both `SymbolSetRecord::load` and `SymbolSetRecord::save` (symbol_store/saving.rs) run the ref through `truncate_ref(..)` (capped at `MAX_REF_BYTES = 2048`) before querying/writing. So for any symbol set whose ref exceeds 2048 bytes, `SymbolSetRecord::load(&self.pool, team_id, &set_ref)` still succeeds (it truncates internally and matches the stored, truncated value), while the concurrently-run `ReleaseRecord::for_symbol_set_ref(&self.pool, &set_ref, team_id)` compares the DB's truncated `ss.ref` against the full untruncated `$1` and returns zero rows — silently dropping the release even though the symbol set (and its bound release) genuinely exists. This is a real, reachable case: JS web frames derive their symbol_set_ref from the resolved source URL (`RawJSFrame::get_ref`/`symbol_set_ref`, langs/js.rs), which can plausibly exceed 2048 bytes (long query strings, signed CDN URLs, etc.) — exactly the scenario `truncate_ref` and its dedicated tests in saving.rs were written to handle. Note the PG-frame-cache load path (`ReleaseRecord::for_symbol_set_id` in records.rs, matching by the stored `symbol_set_id` UUID) is NOT affected, since it doesn't depend on ref truncation at all — only the fresh-resolve path in local.rs is impacted.
</issue_description>

<issue_validation>
- **Checked:** Fetched the actual PR-head code (releases.rs and local.rs at 0ba8f76) since the local checkout is a post-#75456 state without the restored join; cross-read saving.rs truncation and js.rs ref derivation.
- **Found:** Every `SymbolSetRecord` DB path truncates the ref before hitting `posthog_errortrackingsymbolset.ref` — `load` at saving.rs:466 (`let truncated_ref = truncate_ref(set_ref)` → `WHERE ... ref = $2`), plus `save`/`save_data_if_missing`/`save_failure` at saving.rs:519/548/588 — with `MAX_REF_BYTES = 2048` (saving.rs:26-47). The stored `ref` is therefore never longer than 2048 bytes.
- **Found:** The PR's `ReleaseRecord::for_symbol_set_ref` runs `... INNER JOIN ... WHERE ss.ref = $1 AND ss.team_id = $2` bound to the raw, untruncated `symbol_set_ref` — no `truncate_ref`. The PR call site in local.rs passes the identical `&set_ref` (from `frame.symbol_set_ref(debug_images)`) to both `SymbolSetRecord::load(...)` and `ReleaseRecord::for_symbol_set_ref(...)` in one `tokio::try_join!`.
- **Found:** For JS web frames `symbol_set_ref` is derived from the client-controlled `source_url` (js.rs:121-124 → `get_ref`/`source_url`), which genuinely can exceed 2048 bytes — the exact case `truncate_ref` was added for (its comment cites the Postgres BTree index size limit) and that the negative cache is byte-weighted against.
- **Impact:** For any symbol set whose ref exceeds 2048 bytes and has a release bound, the fresh-resolve path silently drops the release: `load` truncates and matches the set, while `for_symbol_set_ref` compares the truncated stored `ss.ref` against the full `$1` and returns zero rows. The release exists but never reaches `Frame.release`, degrading the `$exception_release` fallback. Reachable, named trigger + consequence, trivial one-line fix; not speculative (long refs are a known, already-handled input). The `for_symbol_set_id` UUID path is correctly noted as unaffected.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Truncate `set_ref` the same way before calling `ReleaseRecord::for_symbol_set_ref`, e.g. reuse (or expose) the existing `truncate_ref` helper from `symbol_store::saving` at the call site in local.rs, or have `for_symbol_set_ref` truncate internally the same way `SymbolSetRecord::load`/`save` do, so both queries key off the identical truncated value that's actually persisted in `posthog_errortrackingsymbolset.ref`.
</potential_solution>

[posthog]

Release metadata is deep-cloned once per resolved sub-frame into an entry-count-bounded cache

Why we think it's a valid issue

• Checked: PR-head resolve_impl (local.rs) enrich/save loop, the frame cache construction, and ReleaseRecord/metadata typing in releases.rs.
• Found: In resolve_impl, r_frame.release = release.clone() runs once per resolved.iter_mut() element and is then re-cloned via r_frame.clone() into ErrorTrackingStackFrame::new(...); every record is pushed and the Vec is stored in self.cache, so N identical release copies (metadata included) are retained per raw frame.
• Found: The frame cache is CacheBuilder::new(config.frame_cache_size).expire_after(ttl_policy).build() (local.rs:91-93) with no .weigher(...) — bounded by entry count only, no byte-based eviction. Contrast the dedicated release cache added in this same PR, which is byte-weighed precisely because ReleaseRecord::approx_size_bytes's doc calls the client-controlled metadata: Option<Value> the dominant, effectively-unbounded term. The frame cache now carries that same payload without the byte bound.
• Found: The duplication is genuinely redundant — inlined sibling sub-frames share an identical release, whereas their contents/context legitimately differ per part, so release is the one field cloned needlessly N times.
• Impact: Real but minor: meaningful memory only when client metadata is large AND inlining expansion N is high, and Arc-wrapping only saves the intra-entry N-duplication (per-entry cost stays). The fix is a trivial, idiomatic Arc wrap (not an abstraction, not speculative — multi-part frames and large metadata both occur), consistent with the memory concern the author already engineered for. Fits the reviewer's consider (lowest, real-but-minor) rating, which I keep.

Issue description

r_frame.release = release.clone(); runs once per element of resolved.iter_mut(), so a single raw frame that expands into N resolved sub-frames (common for inlined JS/native/Java frames) now clones the same ReleaseRecord — including its free-form metadata: Value JSON column, which the existing estimated_size/weigher doc comment on ReleaseRecord explicitly calls out as "unbounded" and "the only reason" a byte-aware weigher exists for the release cache — N times into the Vec<ErrorTrackingStackFrame> that gets stored in self.cache (Cache<RawFrameId, Vec<ErrorTrackingStackFrame>>, built via CacheBuilder::new(config.frame_cache_size) with no .weigher(...), i.e. sized purely by entry count). The identical release payload is now redundantly duplicated in memory per part, in a cache that has no byte-based eviction to compensate. records.rs's load_all has the same per-found clone pattern.

Suggested fix

Wrap the shared release payload in an Arc<ReleaseRecord> (or at least Arc-wrap the metadata: Value field) so attaching the same release to multiple sibling frames is a cheap refcount bump instead of a deep JSON clone, especially since this cache has no byte-based sizing to absorb the extra memory.

Prompt to fix with AI (copy-paste)

## Context
@rust/cymbal/src/core/symbolication/symbol/local.rs#L176-180

<issue_description>
`r_frame.release = release.clone();` runs once per element of `resolved.iter_mut()`, so a single raw frame that expands into N resolved sub-frames (common for inlined JS/native/Java frames) now clones the same `ReleaseRecord` — including its free-form `metadata: Value` JSON column, which the existing `estimated_size`/weigher doc comment on `ReleaseRecord` explicitly calls out as "unbounded" and "the only reason" a byte-aware weigher exists for the release cache — N times into the `Vec<ErrorTrackingStackFrame>` that gets stored in `self.cache` (`Cache<RawFrameId, Vec<ErrorTrackingStackFrame>>`, built via `CacheBuilder::new(config.frame_cache_size)` with no `.weigher(...)`, i.e. sized purely by entry count). The identical release payload is now redundantly duplicated in memory per part, in a cache that has no byte-based eviction to compensate. `records.rs`'s `load_all` has the same per-`found` clone pattern.
</issue_description>

<issue_validation>
- **Checked:** PR-head `resolve_impl` (local.rs) enrich/save loop, the frame cache construction, and `ReleaseRecord`/`metadata` typing in releases.rs.
- **Found:** In `resolve_impl`, `r_frame.release = release.clone()` runs once per `resolved.iter_mut()` element and is then re-cloned via `r_frame.clone()` into `ErrorTrackingStackFrame::new(...)`; every `record` is pushed and the `Vec` is stored in `self.cache`, so N identical release copies (metadata included) are retained per raw frame.
- **Found:** The frame cache is `CacheBuilder::new(config.frame_cache_size).expire_after(ttl_policy).build()` (local.rs:91-93) with no `.weigher(...)` — bounded by entry count only, no byte-based eviction. Contrast the dedicated release cache added in this same PR, which is byte-weighed precisely because `ReleaseRecord::approx_size_bytes`'s doc calls the client-controlled `metadata: Option<Value>` the dominant, effectively-unbounded term. The frame cache now carries that same payload without the byte bound.
- **Found:** The duplication is genuinely redundant — inlined sibling sub-frames share an identical release, whereas their `contents`/`context` legitimately differ per part, so release is the one field cloned needlessly N times.
- **Impact:** Real but minor: meaningful memory only when client metadata is large AND inlining expansion N is high, and Arc-wrapping only saves the intra-entry N-duplication (per-entry cost stays). The fix is a trivial, idiomatic `Arc` wrap (not an abstraction, not speculative — multi-part frames and large metadata both occur), consistent with the memory concern the author already engineered for. Fits the reviewer's `consider` (lowest, real-but-minor) rating, which I keep.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Wrap the shared release payload in an `Arc<ReleaseRecord>` (or at least `Arc`-wrap the `metadata: Value` field) so attaching the same release to multiple sibling frames is a cheap refcount bump instead of a deep JSON clone, especially since this cache has no byte-based sizing to absorb the extra memory.
</potential_solution>

[trunk-io]

   

View Full Report ↗︎ ⋅ Docs