diff --git a/proto/cymbal/resolution/v1/resolution.proto b/proto/cymbal/resolution/v1/resolution.proto index 450e12c861a1..f6eb82c429ad 100644 --- a/proto/cymbal/resolution/v1/resolution.proto +++ b/proto/cymbal/resolution/v1/resolution.proto @@ -58,7 +58,9 @@ message Accepted {} message Done { // Serialized rust/cymbal Exception after frame resolution and exception // remapping. It replaces only the submitted exception item, not the full - // event or exception list. + // event or exception list. Resolved frames may carry a `release` object + // (the release bound to the symbol set that resolved them); older servers + // omit it and callers must treat a missing key as "no release". bytes resolved_exception_json = 1; } diff --git a/rust/cymbal-proto/tests/contract.rs b/rust/cymbal-proto/tests/contract.rs index 17aea85b8db3..c30d1791b9b0 100644 --- a/rust/cymbal-proto/tests/contract.rs +++ b/rust/cymbal-proto/tests/contract.rs @@ -68,8 +68,9 @@ fn resolve_outcome_echoes_id_and_carries_done_error_or_retry() { .collect(); assert!(matches!( - decoded[0].result, - Some(resolve_outcome::Result::Done(_)) + &decoded[0].result, + Some(resolve_outcome::Result::Done(done)) + if done.resolved_exception_json == br#"{"type":"ResolvedError"}"# )); assert!(matches!( decoded[1].result, diff --git a/rust/cymbal/.sqlx/query-7002c21150e6a6e42b57cb9ebf9e71e76ff7b6595c2953ed108da0bd0f2279b9.json b/rust/cymbal/.sqlx/query-7002c21150e6a6e42b57cb9ebf9e71e76ff7b6595c2953ed108da0bd0f2279b9.json new file mode 100644 index 000000000000..dbd7664b1090 --- /dev/null +++ b/rust/cymbal/.sqlx/query-7002c21150e6a6e42b57cb9ebf9e71e76ff7b6595c2953ed108da0bd0f2279b9.json @@ -0,0 +1,59 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT r.id, r.team_id, r.hash_id, r.created_at, r.version, r.project, r.metadata\n FROM posthog_errortrackingsymbolset ss\n INNER JOIN posthog_errortrackingrelease r ON ss.release_id = r.id\n WHERE ss.ref = $1 AND ss.team_id = $2\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Uuid" + }, + { + "ordinal": 1, + "name": "team_id", + "type_info": "Int4" + }, + { + "ordinal": 2, + "name": "hash_id", + "type_info": "Text" + }, + { + "ordinal": 3, + "name": "created_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 4, + "name": "version", + "type_info": "Text" + }, + { + "ordinal": 5, + "name": "project", + "type_info": "Text" + }, + { + "ordinal": 6, + "name": "metadata", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Text", + "Int4" + ] + }, + "nullable": [ + false, + false, + false, + false, + false, + false, + true + ] + }, + "hash": "7002c21150e6a6e42b57cb9ebf9e71e76ff7b6595c2953ed108da0bd0f2279b9" +} diff --git a/rust/cymbal/.sqlx/query-eb812dd93e8af10192d414fb1d55fcb2bbd74c80b293672c4579531521e905cd.json b/rust/cymbal/.sqlx/query-eb812dd93e8af10192d414fb1d55fcb2bbd74c80b293672c4579531521e905cd.json new file mode 100644 index 000000000000..9f769295c356 --- /dev/null +++ b/rust/cymbal/.sqlx/query-eb812dd93e8af10192d414fb1d55fcb2bbd74c80b293672c4579531521e905cd.json @@ -0,0 +1,59 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT r.id, r.team_id, r.hash_id, r.created_at, r.version, r.project, r.metadata\n FROM posthog_errortrackingsymbolset ss\n INNER JOIN posthog_errortrackingrelease r ON ss.release_id = r.id\n WHERE ss.id = $1 AND ss.team_id = $2\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Uuid" + }, + { + "ordinal": 1, + "name": "team_id", + "type_info": "Int4" + }, + { + "ordinal": 2, + "name": "hash_id", + "type_info": "Text" + }, + { + "ordinal": 3, + "name": "created_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 4, + "name": "version", + "type_info": "Text" + }, + { + "ordinal": 5, + "name": "project", + "type_info": "Text" + }, + { + "ordinal": 6, + "name": "metadata", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Uuid", + "Int4" + ] + }, + "nullable": [ + false, + false, + false, + false, + false, + false, + true + ] + }, + "hash": "eb812dd93e8af10192d414fb1d55fcb2bbd74c80b293672c4579531521e905cd" +} diff --git a/rust/cymbal/docs/compatibility.md b/rust/cymbal/docs/compatibility.md index b735e6ec8b48..2dc3967153db 100644 --- a/rust/cymbal/docs/compatibility.md +++ b/rust/cymbal/docs/compatibility.md @@ -27,6 +27,7 @@ No generated type changes are needed: - Events use the remote pool and do not silently fall back to inline resolution on failures. - Events are flattened into exception-level `ResolveItem`s. Items are grouped by their first symbol-set reference when available, with a per-team fallback, then submitted over per-endpoint bidirectional `Resolve` streams. - Resolver-specific context is carried in `ResolveItem.metadata` as JSON bytes. The native symbolication convention uses a `debug_images_json` key. +- Resolved frames in `Done.resolved_exception_json` may carry a `release` object: the release bound to the symbol set that resolved the frame. The key is omitted when a frame has none (also what older servers send for every frame), and callers must treat a missing key as "no release". The processing side strips `Frame.release` before any clickhouse-bound serialization. - Per-item `ResolveOutcome.Error.kind` is the control-flow surface. `ERROR_KIND_OVERLOADED` is result-only backpressure and triggers item reroute. Accepted items emit `ResolveOutcome.Accepted` before their terminal outcome; cymbal releases its routing permit on that acceptance signal. If `CYMBAL_REMOTE_RESOLUTION_OVERLOAD_EJECTION_MS` is non-zero, the overloaded endpoint is also temporarily excluded from new routing in that cymbal process. Repeated overloads double that cooldown up to `CYMBAL_REMOTE_RESOLUTION_OVERLOAD_EJECTION_MAX_MS`, and a quiet `CYMBAL_REMOTE_RESOLUTION_OVERLOAD_EJECTION_DECAY_MS` window resets it. `CYMBAL_REMOTE_RESOLUTION_ROUTING_JITTER` controls how much routing flattens across the load-adjusted rendezvous-ranked candidate list (`0.0` strict top load-adjusted candidate, `1.0` load-weighted across candidates). `LoadEvent` carries endpoint freshness/draining state plus `in_flight` / `max_in_flight` as a soft routing load signal. This means Node request chunking limits protect cymbal's public HTTP body size, while cymbal's private gRPC path owns exception-level routing, reroute depth, and overload handling. diff --git a/rust/cymbal/src/core/symbolication/symbol/local.rs b/rust/cymbal/src/core/symbolication/symbol/local.rs index 29a35487ae3c..029bec3eec94 100644 --- a/rust/cymbal/src/core/symbolication/symbol/local.rs +++ b/rust/cymbal/src/core/symbolication/symbol/local.rs @@ -16,7 +16,7 @@ use sqlx::PgPool; use crate::{ core::config::ResolverConfig, error::{JsResolveErr, ProguardError, ResolveError, UnhandledError}, - frames::{Frame, RawFrame}, + frames::{releases::ReleaseRecord, Frame, RawFrame}, langs::native::DebugImage, metric_consts::{ FRAME_CACHE_HITS, FRAME_CACHE_MISSES, FRAME_DB_HITS, FRAME_DB_MISSES, @@ -156,19 +156,28 @@ impl LocalSymbolResolver { assert!(!resolved.is_empty()); // If this ever happens, we've got a data-dropping bug, and want to crash - let set = if let Some(set_ref) = frame.symbol_set_ref(debug_images) { - let mut set = SymbolSetRecord::load(&self.pool, raw_id.team_id, &set_ref).await?; + let (set, release) = if let Some(set_ref) = frame.symbol_set_ref(debug_images) { + let set_fut = SymbolSetRecord::load(&self.pool, raw_id.team_id, &set_ref); + let release_fut = async { + ReleaseRecord::for_symbol_set_ref(&self.pool, &set_ref, raw_id.team_id) + .await + .map_err(UnhandledError::from) + }; + let (mut set, release) = tokio::try_join!(set_fut, release_fut)?; if let Some(s) = &mut set { s.set_last_used(&self.pool).await?; } - set + (set, release) } else { - None + (None, None) }; let mut records = Vec::new(); - for r_frame in &resolved { - // Save back to the DB + let mut resolved = resolved; + for r_frame in resolved.iter_mut() { + r_frame.release = release.clone(); // Enrich with release information + + // And save back to the DB let record = ErrorTrackingStackFrame::new( r_frame.frame_id.clone(), set.as_ref().map(|s| s.id), diff --git a/rust/cymbal/src/core/symbolication/symbol/records.rs b/rust/cymbal/src/core/symbolication/symbol/records.rs index e53608af9dc9..c9a7a72fd24e 100644 --- a/rust/cymbal/src/core/symbolication/symbol/records.rs +++ b/rust/cymbal/src/core/symbolication/symbol/records.rs @@ -6,7 +6,7 @@ use sqlx::Executor; use uuid::Uuid; use crate::error::UnhandledError; -use crate::frames::{Context, Frame}; +use crate::frames::{releases::ReleaseRecord, Context, Frame}; const FRAME_TTL_JITTER_PERCENT: u32 = 10; @@ -108,6 +108,12 @@ impl ErrorTrackingStackFrame { } else { None }; + // Stored contents never include the release: `load_all` re-joins it via the symbol set so + // a release (re)bind takes effect on the next load instead of going stale in cache rows. + let mut contents = serde_json::to_value(&self.contents)?; + if let Some(object) = contents.as_object_mut() { + object.remove("release"); + } sqlx::query!( r#" INSERT INTO posthog_errortrackingstackframe (raw_id, part, team_id, created_at, symbol_set_id, contents, resolved, id, context) @@ -124,7 +130,7 @@ impl ErrorTrackingStackFrame { self.id.team_id, self.created_at, self.symbol_set_id, - serde_json::to_value(&self.contents)?, + contents, self.resolved, Uuid::now_v7(), context, @@ -175,6 +181,11 @@ impl ErrorTrackingStackFrame { return Ok(Vec::new()); } + let mut release = None; + if let Some(ss_id) = &res[0].symbol_set_id { + release = ReleaseRecord::for_symbol_set_id(e, *ss_id, id.team_id).await?; + } + for found in res { // Frame ID's lose team_id when they're serialized, so we fix that up here when loading them let frame_id = FrameId::new(found.raw_id, found.team_id, found.part); @@ -192,6 +203,7 @@ impl ErrorTrackingStackFrame { None }; + frame.release = release.clone(); frame.context = context.clone(); results.push(Self { @@ -230,6 +242,7 @@ mod tests { junk_drawer: None, code_variables: None, context: None, + release: None, } } diff --git a/rust/cymbal/src/core/symbolication/symbol_store/saving.rs b/rust/cymbal/src/core/symbolication/symbol_store/saving.rs index 32ee989b62cb..ad782dc1681b 100644 --- a/rust/cymbal/src/core/symbolication/symbol_store/saving.rs +++ b/rust/cymbal/src/core/symbolication/symbol_store/saving.rs @@ -34,7 +34,7 @@ const NEGATIVE_CACHE_MAX_WEIGHT: u64 = 64 * 1024 * 1024; // We truncate the reference to resolve an issue with the maximum size in a BTRee index on Postgres // TODO: update model to use a hash of the reference instead -fn truncate_ref(s: &str) -> &str { +pub(crate) fn truncate_ref(s: &str) -> &str { if s.len() <= MAX_REF_BYTES { return s; } diff --git a/rust/cymbal/src/core/types/frames/mod.rs b/rust/cymbal/src/core/types/frames/mod.rs index 5b06b43a60f4..22bc1968059e 100644 --- a/rust/cymbal/src/core/types/frames/mod.rs +++ b/rust/cymbal/src/core/types/frames/mod.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use common_types::error_tracking::{FrameData, FrameId, RawFrameId}; +use releases::ReleaseRecord; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -172,6 +173,12 @@ pub struct Frame { // use in the frontend #[serde(skip)] pub context: Option, + // The release bound to the symbol set that resolved this frame. Serializable so it crosses the + // resolution-service wire inside the frame JSON, but it must not reach the clickhouse-bound + // event JSON — `into_resolved` strips it after `$exception_release` selection — and the PG + // frame cache re-joins it at load instead of trusting a stored copy (see records.rs). + #[serde(skip_serializing_if = "Option::is_none", default)] + pub release: Option, } #[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)] diff --git a/rust/cymbal/src/core/types/frames/releases.rs b/rust/cymbal/src/core/types/frames/releases.rs index ae99af412d0e..77cd99c65297 100644 --- a/rust/cymbal/src/core/types/frames/releases.rs +++ b/rust/cymbal/src/core/types/frames/releases.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -5,6 +7,10 @@ use sha2::{Digest, Sha512}; use sqlx::Executor; use uuid::Uuid; +use crate::symbolication::symbol_store::saving::truncate_ref; + +use super::Frame; + /// The release API does not bound what a row can hold (`version`/`project`/`metadata` are /// unbounded TextField/JSONField columns), but every one of these fields is embedded into every /// matching exception event, so a single oversized row would be amplified across the whole event @@ -15,7 +21,9 @@ use uuid::Uuid; pub const MAX_RELEASE_METADATA_BYTES: usize = 8 * 1024; pub const MAX_RELEASE_TEXT_CHARS: usize = 255; -#[derive(Debug, Clone, Eq, PartialEq)] +// Serialized only on the internal resolution-service wire, inside each resolved frame's JSON — +// never into the clickhouse-bound event JSON, which `into_resolved` strips `Frame.release` from. +#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)] pub struct ReleaseRecord { pub id: Uuid, pub team_id: i32, @@ -84,6 +92,59 @@ impl ReleaseRecord { Ok(row.map(Self::clamped)) } + pub async fn for_symbol_set_ref<'c, E>( + e: E, + symbol_set_ref: &str, + team_id: i32, + ) -> Result, sqlx::Error> + where + E: Executor<'c, Database = sqlx::Postgres>, + { + // Stored refs are truncated to MAX_REF_BYTES by SymbolSetRecord::load/save; match on the + // same truncated value or long refs (e.g. >2KB JS source URLs) never join. + let symbol_set_ref = truncate_ref(symbol_set_ref); + let row = sqlx::query_as!( + Self, + r#" + SELECT r.id, r.team_id, r.hash_id, r.created_at, r.version, r.project, r.metadata + FROM posthog_errortrackingsymbolset ss + INNER JOIN posthog_errortrackingrelease r ON ss.release_id = r.id + WHERE ss.ref = $1 AND ss.team_id = $2 + "#, + symbol_set_ref, + team_id + ) + .fetch_optional(e) + .await?; + + Ok(row.map(Self::clamped)) + } + + pub async fn for_symbol_set_id<'c, E>( + e: E, + symbol_set_id: Uuid, + team_id: i32, + ) -> Result, sqlx::Error> + where + E: Executor<'c, Database = sqlx::Postgres>, + { + let row = sqlx::query_as!( + Self, + r#" + SELECT r.id, r.team_id, r.hash_id, r.created_at, r.version, r.project, r.metadata + FROM posthog_errortrackingsymbolset ss + INNER JOIN posthog_errortrackingrelease r ON ss.release_id = r.id + WHERE ss.id = $1 AND ss.team_id = $2 + "#, + symbol_set_id, + team_id + ) + .fetch_optional(e) + .await?; + + Ok(row.map(Self::clamped)) + } + pub fn to_info(&self) -> ReleaseInfo { ReleaseInfo { id: self.id, @@ -94,9 +155,29 @@ impl ReleaseRecord { } } - /// Bounds every field this record can carry into an event: `metadata` over the cap the API - /// enforces on new writes is dropped, `version`/`project` are truncated. The `id` survives, - /// so consumers can still fetch the full release. + /// Distinct releases attached to the given frames, deduped by release id, in first-seen order. + pub fn collect_from_frames<'a>(frames: impl Iterator) -> Vec { + let mut seen = HashSet::new(); + let mut out = Vec::new(); + for release in frames.filter_map(|f| f.release.as_ref()) { + if seen.insert(release.id) { + out.push(release.clone()); + } + } + out + } + + /// The most recently created release, with ties broken by id so the pick is deterministic + /// regardless of frame order. + pub fn latest(releases: impl IntoIterator) -> Option { + releases + .into_iter() + .max_by_key(|release| (release.created_at, release.id)) + } + + /// Bounds every field this record can carry into an event: `metadata` over the cap is + /// dropped, `version`/`project` are truncated. The `id` survives, so consumers can still + /// fetch the full release. fn clamped(mut self) -> Self { let oversized = self.metadata.as_ref().is_some_and(|metadata| { serde_json::to_string(metadata).map_or(true, |s| s.len() > MAX_RELEASE_METADATA_BYTES) diff --git a/rust/cymbal/src/core/types/langs/apple.rs b/rust/cymbal/src/core/types/langs/apple.rs index be1b2a774338..ddd9a776d3a5 100644 --- a/rust/cymbal/src/core/types/langs/apple.rs +++ b/rust/cymbal/src/core/types/langs/apple.rs @@ -224,6 +224,7 @@ impl RawAppleFrame { resolve_failure: None, junk_drawer: None, + release: None, synthetic: self.meta.synthetic, context: None, suspicious: false, @@ -291,6 +292,7 @@ impl RawAppleFrame { resolved: false, resolve_failure: Some(err.to_string()), junk_drawer: None, + release: None, synthetic: self.meta.synthetic, context: None, suspicious: false, @@ -382,6 +384,7 @@ impl From<&RawAppleFrame> for Frame { resolve_failure: None, junk_drawer: None, + release: None, synthetic: raw.meta.synthetic, context: None, suspicious: false, diff --git a/rust/cymbal/src/core/types/langs/custom.rs b/rust/cymbal/src/core/types/langs/custom.rs index 2b91df43225a..c76908cc5eb0 100644 --- a/rust/cymbal/src/core/types/langs/custom.rs +++ b/rust/cymbal/src/core/types/langs/custom.rs @@ -101,6 +101,7 @@ impl From<&CustomFrame> for Frame { junk_drawer: None, context: value.get_context(), + release: None, synthetic: value.meta.synthetic, suspicious: false, module: value.module.clone(), diff --git a/rust/cymbal/src/core/types/langs/dart.rs b/rust/cymbal/src/core/types/langs/dart.rs index b3f308cee908..9bd190e5882d 100644 --- a/rust/cymbal/src/core/types/langs/dart.rs +++ b/rust/cymbal/src/core/types/langs/dart.rs @@ -50,6 +50,7 @@ impl From<&RawDartFrame> for Frame { resolve_failure: None, junk_drawer: None, + release: None, synthetic: raw.meta.synthetic, context: None, suspicious: false, diff --git a/rust/cymbal/src/core/types/langs/go.rs b/rust/cymbal/src/core/types/langs/go.rs index d848f29562a1..1e78055fa0bb 100644 --- a/rust/cymbal/src/core/types/langs/go.rs +++ b/rust/cymbal/src/core/types/langs/go.rs @@ -40,6 +40,7 @@ impl From<&RawGoFrame> for Frame { synthetic: frame.meta.synthetic, junk_drawer: None, context: None, + release: None, suspicious: false, module: None, code_variables: None, diff --git a/rust/cymbal/src/core/types/langs/hermes.rs b/rust/cymbal/src/core/types/langs/hermes.rs index 566b87f4c2e5..2a059a5271e9 100644 --- a/rust/cymbal/src/core/types/langs/hermes.rs +++ b/rust/cymbal/src/core/types/langs/hermes.rs @@ -156,6 +156,7 @@ impl From<(&RawHermesFrame, HermesError)> for Frame { junk_drawer: None, code_variables: None, context: None, + release: None, suspicious: false, module: None, }; @@ -197,6 +198,7 @@ impl From<(&RawHermesFrame, Token<'_>, Option, usize)> for Frame { junk_drawer: None, code_variables: None, context: get_token_context(&token, token.get_src_line() as usize, context_lines), + release: None, suspicious: false, module: None, }; @@ -231,6 +233,7 @@ impl From<&RawHermesFrame> for Frame { junk_drawer: None, code_variables: None, context: None, + release: None, synthetic: raw_frame.meta.synthetic, suspicious: false, module: None, diff --git a/rust/cymbal/src/core/types/langs/java.rs b/rust/cymbal/src/core/types/langs/java.rs index 4bda68721d40..0edee149c145 100644 --- a/rust/cymbal/src/core/types/langs/java.rs +++ b/rust/cymbal/src/core/types/langs/java.rs @@ -191,6 +191,7 @@ impl<'a> From<(&'a RawJavaFrame, StackFrame<'a>)> for Frame { junk_drawer: None, code_variables: None, + release: None, synthetic: raw.meta.synthetic, context: None, suspicious: false, @@ -222,6 +223,7 @@ impl From<(&RawJavaFrame, ProguardError)> for Frame { resolve_failure, junk_drawer: None, code_variables: None, + release: None, synthetic: raw.meta.synthetic, context: None, suspicious: false, diff --git a/rust/cymbal/src/core/types/langs/js.rs b/rust/cymbal/src/core/types/langs/js.rs index ee89dd9a85d2..bcfa57a9d189 100644 --- a/rust/cymbal/src/core/types/langs/js.rs +++ b/rust/cymbal/src/core/types/langs/js.rs @@ -235,6 +235,7 @@ impl From<(&RawJSFrame, SourceLocation<'_>, usize)> for Frame { junk_drawer: None, code_variables: None, context: get_sourcelocation_context(&token, context_lines), + release: None, synthetic: raw_frame.meta.synthetic, suspicious, module: None, @@ -293,6 +294,7 @@ impl From<(&RawJSFrame, JsResolveErr, &FrameLocation)> for Frame { junk_drawer: None, code_variables: None, context: None, + release: None, synthetic: raw_frame.meta.synthetic, suspicious: false, module: None, @@ -332,6 +334,7 @@ impl From<&RawJSFrame> for Frame { junk_drawer: None, code_variables: None, context: None, + release: None, synthetic: raw_frame.meta.synthetic, suspicious: false, module: None, diff --git a/rust/cymbal/src/core/types/langs/native.rs b/rust/cymbal/src/core/types/langs/native.rs index 8f71c70c7c14..7b060082a0f5 100644 --- a/rust/cymbal/src/core/types/langs/native.rs +++ b/rust/cymbal/src/core/types/langs/native.rs @@ -354,6 +354,7 @@ impl RawNativeFrame { resolve_failure: None, junk_drawer: None, + release: None, synthetic: self.meta.synthetic, context: None, suspicious: false, @@ -403,6 +404,7 @@ impl RawNativeFrame { resolved: false, resolve_failure: Some(err.to_string()), junk_drawer: None, + release: None, synthetic: self.meta.synthetic, context: None, suspicious: false, @@ -565,6 +567,7 @@ impl From<&RawNativeFrame> for Frame { resolve_failure: None, junk_drawer: None, + release: None, synthetic: raw.meta.synthetic, context: None, suspicious: false, diff --git a/rust/cymbal/src/core/types/langs/node.rs b/rust/cymbal/src/core/types/langs/node.rs index bb3b2a4a866f..6c27f912a387 100644 --- a/rust/cymbal/src/core/types/langs/node.rs +++ b/rust/cymbal/src/core/types/langs/node.rs @@ -189,6 +189,7 @@ impl From<&RawNodeFrame> for Frame { junk_drawer: None, context: raw.get_context(), + release: None, synthetic: raw.meta.synthetic, suspicious: false, module: raw.module.clone(), @@ -234,6 +235,7 @@ impl From<(&RawNodeFrame, SourceLocation<'_>, usize)> for Frame { junk_drawer: None, code_variables: None, context: get_sourcelocation_context(&location, context_lines), + release: None, synthetic: raw_frame.meta.synthetic, suspicious: false, module: raw_frame.module.clone(), @@ -291,6 +293,7 @@ impl From<(&RawNodeFrame, JsResolveErr)> for Frame { junk_drawer: None, code_variables: None, context: raw_frame.get_context(), + release: None, synthetic: raw_frame.meta.synthetic, suspicious: false, module: raw_frame.module.clone(), diff --git a/rust/cymbal/src/core/types/langs/php.rs b/rust/cymbal/src/core/types/langs/php.rs index 2a52e2fc3083..497d7969993b 100644 --- a/rust/cymbal/src/core/types/langs/php.rs +++ b/rust/cymbal/src/core/types/langs/php.rs @@ -94,6 +94,7 @@ impl From<&RawPHPFrame> for Frame { junk_drawer: None, context: raw.get_context(), + release: None, synthetic: raw.meta.synthetic, suspicious: false, module: None, diff --git a/rust/cymbal/src/core/types/langs/python.rs b/rust/cymbal/src/core/types/langs/python.rs index 45885b29a2d4..33ae57d09d5f 100644 --- a/rust/cymbal/src/core/types/langs/python.rs +++ b/rust/cymbal/src/core/types/langs/python.rs @@ -347,6 +347,7 @@ impl From<&RawPythonFrame> for Frame { junk_drawer: None, context: raw.get_context(), + release: None, synthetic: raw.meta.synthetic, suspicious: false, module: raw.module.clone(), diff --git a/rust/cymbal/src/core/types/langs/ruby.rs b/rust/cymbal/src/core/types/langs/ruby.rs index 7f2464ccfcb9..b41ba51ae7c7 100644 --- a/rust/cymbal/src/core/types/langs/ruby.rs +++ b/rust/cymbal/src/core/types/langs/ruby.rs @@ -90,6 +90,7 @@ impl From<&RawRubyFrame> for Frame { junk_drawer: None, context: raw.get_context(), + release: None, synthetic: raw.meta.synthetic, suspicious: false, module: None, diff --git a/rust/cymbal/src/modes/processing/fingerprinting/mod.rs b/rust/cymbal/src/modes/processing/fingerprinting/mod.rs index 0e19613e9f91..10e6d43a2793 100644 --- a/rust/cymbal/src/modes/processing/fingerprinting/mod.rs +++ b/rust/cymbal/src/modes/processing/fingerprinting/mod.rs @@ -489,6 +489,7 @@ mod test { junk_drawer: None, code_variables: None, context: None, + release: None, synthetic: false, suspicious: false, module: None, diff --git a/rust/cymbal/src/modes/processing/normalization.rs b/rust/cymbal/src/modes/processing/normalization.rs index 90190429a64c..f976d717cc76 100644 --- a/rust/cymbal/src/modes/processing/normalization.rs +++ b/rust/cymbal/src/modes/processing/normalization.rs @@ -661,6 +661,7 @@ mod test { junk_drawer: None, code_variables: None, context: None, + release: None, module: None, }; diff --git a/rust/cymbal/src/modes/processing/stages/resolution/remote/resolver/retry.rs b/rust/cymbal/src/modes/processing/stages/resolution/remote/resolver/retry.rs index 826b2d8a7e4d..dafa5ee073f3 100644 --- a/rust/cymbal/src/modes/processing/stages/resolution/remote/resolver/retry.rs +++ b/rust/cymbal/src/modes/processing/stages/resolution/remote/resolver/retry.rs @@ -426,6 +426,8 @@ fn record_reroute_depth(outcome: &'static str, attempts_used: u32) { mod tests { use cymbal_proto::cymbal::resolution::v1::{Done, Error, Retry}; + use crate::frames::releases::ReleaseRecord; + use super::*; #[test] @@ -440,7 +442,49 @@ mod tests { }; let decision = classify_outcome(&work_item, outcome).expect("done outcome"); - assert!(matches!(decision, ItemDecision::Done(exc) if exc.exception_type == "Resolved")); + assert!(matches!( + decision, + ItemDecision::Done(exception) if exception.exception_type == "Resolved" + )); + } + + // The frame-derived release crosses the wire inside the frame JSON; a frame without the + // key (an older server, or no release bound) must parse to `None`, and one with it must + // land on `Frame.release`. + #[test] + fn classify_outcome_parses_frame_releases_from_the_exception_json() { + let release = ReleaseRecord { + id: uuid::Uuid::now_v7(), + team_id: 42, + hash_id: "hash".to_string(), + created_at: chrono::Utc::now(), + version: "1.2.3".to_string(), + project: "my-app".to_string(), + metadata: None, + }; + let mut with_release = exception("Resolved"); + with_release.stack = Some(crate::types::Stacktrace::Resolved { + frames: vec![frame(Some(release.clone())), frame(None)], + }); + + let work_item = work_item(7); + let outcome = ResolveOutcome { + id: 7, + result: Some(resolve_outcome::Result::Done(Done { + resolved_exception_json: serde_json::to_vec(&with_release) + .expect("valid exception"), + })), + }; + + let decision = classify_outcome(&work_item, outcome).expect("done outcome"); + let ItemDecision::Done(parsed) = decision else { + panic!("expected done decision"); + }; + let Some(crate::types::Stacktrace::Resolved { frames }) = parsed.stack else { + panic!("expected resolved stack"); + }; + assert_eq!(frames[0].release, Some(release)); + assert_eq!(frames[1].release, None); } #[test] @@ -536,4 +580,26 @@ mod tests { stack: None, } } + + fn frame(release: Option) -> crate::frames::Frame { + crate::frames::Frame { + frame_id: common_types::error_tracking::FrameId::placeholder(), + mangled_name: "f".to_string(), + line: None, + column: None, + source: None, + module: None, + in_app: true, + resolved_name: None, + lang: "javascript".to_string(), + resolved: true, + resolve_failure: None, + synthetic: false, + suspicious: false, + junk_drawer: None, + code_variables: None, + context: None, + release, + } + } } diff --git a/rust/cymbal/src/modes/processing/types/exception_event.rs b/rust/cymbal/src/modes/processing/types/exception_event.rs index f1d494bf7fcf..6eda48a8cf78 100644 --- a/rust/cymbal/src/modes/processing/types/exception_event.rs +++ b/rust/cymbal/src/modes/processing/types/exception_event.rs @@ -48,13 +48,21 @@ impl ResolvedMetadata { exception_list: &ExceptionList, event_release: Option<&ReleaseRecord>, ) -> Self { + // The event-level release (`$release_id` / mobile app-metadata hash) is authoritative; + // frame-derived releases only fill in when it resolved nothing, picking the latest so an + // event whose stack mixes chunks from several releases reports the newest one. + let release = event_release + .cloned() + .or_else(|| ReleaseRecord::latest(exception_list.get_frame_releases())) + .map(|release| release.to_info()); + Self { sources: exception_list.get_unique_sources(), types: exception_list.get_unique_types(), messages: exception_list.get_unique_messages(), functions: exception_list.get_unique_functions(), handled: exception_list.get_is_handled(), - release: event_release.map(ReleaseRecord::to_info), + release, } } } @@ -217,11 +225,14 @@ impl ExceptionEvent { self.state.event_release = release; } - pub(crate) fn into_resolved(self) -> ExceptionEvent { + pub(crate) fn into_resolved(mut self) -> ExceptionEvent { let metadata = ResolvedMetadata::from_exception_list( &self.exception_list, self.state.event_release.as_ref(), ); + // `Frame.release` serializes on the resolution-service wire, but must never reach the + // clickhouse-bound serializations of the exception list; selection is done, so drop it. + self.exception_list.clear_frame_releases(); self.map_state(|state| Resolved { metadata, client_fingerprint: state.client_fingerprint, @@ -713,21 +724,57 @@ mod tests { } fn release_record(hash_id: &str) -> ReleaseRecord { + release_record_at(hash_id, 0) + } + + fn release_record_at(hash_id: &str, created_secs: i64) -> ReleaseRecord { ReleaseRecord { id: Uuid::now_v7(), team_id: 42, hash_id: hash_id.to_string(), - created_at: chrono::DateTime::from_timestamp(0, 0).unwrap(), - version: "1.2.3".to_string(), + created_at: chrono::DateTime::from_timestamp(created_secs, 0).unwrap(), + version: format!("1.2.{created_secs}"), project: "my-app".to_string(), metadata: None, } } + fn frame_with_release(release: Option) -> crate::frames::Frame { + crate::frames::Frame { + frame_id: common_types::error_tracking::FrameId::placeholder(), + mangled_name: "f".to_string(), + line: None, + column: None, + source: None, + module: None, + in_app: true, + resolved_name: None, + lang: "javascript".to_string(), + resolved: true, + resolve_failure: None, + synthetic: false, + suspicious: false, + junk_drawer: None, + code_variables: None, + context: None, + release, + } + } + + fn exception_list_with_frames(frames: Vec) -> ExceptionList { + ExceptionList(vec![crate::types::Exception { + exception_id: None, + exception_type: "Error".to_string(), + exception_message: "boom".to_string(), + mechanism: None, + module: None, + thread_id: None, + stack: Some(crate::types::Stacktrace::Resolved { frames }), + }]) + } + #[test] fn event_release_populates_the_singular_release() { - // The event-level release (from `$release_id` or mobile app metadata) is the sole source of - // `$exception_release`; it does not depend on any frame carrying a release. let metadata = ResolvedMetadata::from_exception_list( &ExceptionList::default(), Some(&release_record("hash-abc")), @@ -737,11 +784,78 @@ mod tests { #[test] fn missing_event_release_leaves_the_release_unset() { - // Without an event-level release there is nothing to emit; there is no per-frame fallback. + // Without an event-level release and without any frame-derived candidate there is nothing + // to emit. let metadata = ResolvedMetadata::from_exception_list(&ExceptionList::default(), None); assert!(metadata.release.is_none()); } + #[test] + fn event_release_takes_precedence_over_frame_releases() { + // `$release_id`/app-hash resolution is authoritative even when frame-derived releases are + // newer; the fallback only fills a gap, it never overrides. + let event_release = release_record_at("event-hash", 100); + let newer_frame_release = release_record_at("frame-hash", 5_000); + let exception_list = + exception_list_with_frames(vec![frame_with_release(Some(newer_frame_release))]); + + let metadata = ResolvedMetadata::from_exception_list(&exception_list, Some(&event_release)); + + let expected = serde_json::to_value(event_release.to_info()).unwrap(); + assert_eq!(serde_json::to_value(&metadata.release).unwrap(), expected); + } + + #[test] + fn into_resolved_strips_releases_from_frames_after_selection() { + // `Frame.release` serializes (for the resolution-service wire), so if `into_resolved` + // stopped stripping it, release payloads would leak into every clickhouse-bound + // serialization of the exception list. + let release = release_record_at("frame-hash", 5_000); + let parsed = ExceptionEvent { + uuid: Uuid::now_v7(), + team_id: 42, + timestamp: "2026-01-01T00:00:00Z".to_string(), + exception_list: exception_list_with_frames(vec![frame_with_release(Some(release))]), + debug_images: vec![], + props: HashMap::new(), + proposed_issue_name: None, + proposed_issue_description: None, + state: Parsed { + client_fingerprint: None, + legacy_order_exception_list: None, + legacy_order_resolved: None, + event_release: None, + }, + }; + + let resolved = parsed.into_resolved(); + + assert!(resolved.metadata().release.is_some(), "selection ran first"); + let list_json = serde_json::to_string(resolved.exception_list()).unwrap(); + assert!( + !list_json.contains("release"), + "clickhouse-bound exception list must not carry frame releases: {list_json}" + ); + } + + #[test] + fn frame_release_fallback_picks_the_latest() { + // Without an event-level release, the fallback picks the most recently created release + // across the frames, regardless of frame order. + let older_release = release_record_at("older-hash", 100); + let latest_release = release_record_at("latest-hash", 5_000); + let exception_list = exception_list_with_frames(vec![ + frame_with_release(Some(older_release)), + frame_with_release(None), + frame_with_release(Some(latest_release.clone())), + ]); + + let metadata = ResolvedMetadata::from_exception_list(&exception_list, None); + + let expected = serde_json::to_value(latest_release.to_info()).unwrap(); + assert_eq!(serde_json::to_value(&metadata.release).unwrap(), expected); + } + #[test] fn exception_release_emitted_only_when_a_release_resolves() { let issue = Issue { diff --git a/rust/cymbal/src/modes/processing/types/mod.rs b/rust/cymbal/src/modes/processing/types/mod.rs index 05a4caca9fe0..2934527b0b6d 100644 --- a/rust/cymbal/src/modes/processing/types/mod.rs +++ b/rust/cymbal/src/modes/processing/types/mod.rs @@ -7,7 +7,7 @@ use std::ops::{Deref, DerefMut}; use uuid::Uuid; use crate::fingerprinting::{FingerprintRecordPart, FingerprintVersion}; -use crate::frames::releases::ReleaseInfo; +use crate::frames::releases::{ReleaseInfo, ReleaseRecord}; use crate::frames::{Frame, RawFrame}; use crate::langs::native::DebugImage; use crate::metric_consts::POSTHOG_SDK_EXCEPTION_RESOLVED; @@ -93,6 +93,24 @@ impl ExceptionList { .and_then(|m| m.handled) .unwrap_or(false) } + + /// Releases attached to this list's frames — by local symbolication directly, or deserialized + /// from the remote resolution response (`Frame.release` serializes on that wire). + pub fn get_frame_releases(&self) -> Vec { + ReleaseRecord::collect_from_frames(self.get_frames_iter()) + } + + /// Drops `Frame.release` from every resolved frame. Called once `$exception_release` + /// selection is done, so clickhouse-bound serializations of this list never carry it. + pub fn clear_frame_releases(&mut self) { + for exception in self.0.iter_mut() { + if let Some(Stacktrace::Resolved { frames }) = exception.stack.as_mut() { + for frame in frames { + frame.release = None; + } + } + } + } } /// Untrusted exception properties accepted from ClickHouse and SDK event payloads. diff --git a/rust/cymbal/src/modes/resolution/README.md b/rust/cymbal/src/modes/resolution/README.md index 0bf1bdfd2caa..d8473ccda3ff 100644 --- a/rust/cymbal/src/modes/resolution/README.md +++ b/rust/cymbal/src/modes/resolution/README.md @@ -45,7 +45,7 @@ events ──HTTP─────▶│ cymbal The contract is intentionally split across two streams: -- **`Resolve`** is bidirectional work traffic. The caller sends independent `ResolveItem`s, each with a per-stream id, `team_id`, serialized exception JSON, JSON `metadata` bytes, and an item deadline. The server emits an `Accepted` outcome when it admits an item, then exactly one terminal `ResolveOutcome` with the same id: `Done`, `Retry`, or `Error`. +- **`Resolve`** is bidirectional work traffic. The caller sends independent `ResolveItem`s, each with a per-stream id, `team_id`, serialized exception JSON, JSON `metadata` bytes, and an item deadline. The server emits an `Accepted` outcome when it admits an item, then exactly one terminal `ResolveOutcome` with the same id: `Done`, `Retry`, or `Error`. A `Done` carries the resolved exception JSON; each resolved frame may carry a `release` object (the release bound to the symbol set that resolved it), with the key omitted when there is none — which is also what older servers send for every frame. - **`Subscribe`** is endpoint freshness, draining, and soft load state. The cymbal-side `EndpointPool` opens one long-lived stream per pod and treats the latest `LoadEvent` as a freshness snapshot plus an `in_flight` / `max_in_flight` routing bias. `LoadEvent` does not carry overload state or suggested batch sizing. `Error.kind` is the shared control-flow surface: diff --git a/rust/cymbal/src/modes/resolution/service/resolve.rs b/rust/cymbal/src/modes/resolution/service/resolve.rs index c9b341af054a..06787198f374 100644 --- a/rust/cymbal/src/modes/resolution/service/resolve.rs +++ b/rust/cymbal/src/modes/resolution/service/resolve.rs @@ -150,9 +150,9 @@ async fn process_item( let (result, outcome, kind) = match tokio::time::timeout(deadline, resolve_item(&stage, &item)).await { - Ok(Ok(resolved)) => ( + Ok(Ok(exception_json)) => ( resolve_outcome::Result::Done(Done { - resolved_exception_json: resolved, + resolved_exception_json: exception_json, }), "done", "ok", @@ -248,6 +248,8 @@ async fn resolve_item( ResolveOneError::Unhandled(err) => ItemFailure::Unhandled(err), })?; + // Resolved frames carry their symbol set's release inline (`Frame.release` serializes on + // this wire); the processing side strips it before anything clickhouse-bound. serde_json::to_vec(&resolved) .map_err(|e| ItemFailure::Unhandled(format!("serialize resolved exception: {e}"))) } diff --git a/rust/cymbal/tests/common/mod.rs b/rust/cymbal/tests/common/mod.rs index 6fa923d4740d..c0ffb610f305 100644 --- a/rust/cymbal/tests/common/mod.rs +++ b/rust/cymbal/tests/common/mod.rs @@ -456,6 +456,16 @@ pub fn build_event_with( team_id: i32, uuid: Uuid, debug_images: Vec, +) -> ExceptionEvent { + build_event_with_raw_frames(num_exceptions, team_id, uuid, debug_images, Vec::new()) +} + +pub fn build_event_with_raw_frames( + num_exceptions: usize, + team_id: i32, + uuid: Uuid, + debug_images: Vec, + frames: Vec, ) -> ExceptionEvent { let exceptions: Vec = (0..num_exceptions) .map(|i| Exception { @@ -465,7 +475,9 @@ pub fn build_event_with( mechanism: None, module: None, thread_id: None, - stack: Some(Stacktrace::Raw { frames: vec![] }), + stack: Some(Stacktrace::Raw { + frames: frames.clone(), + }), }) .collect(); AnyEvent { diff --git a/rust/cymbal/tests/event.rs b/rust/cymbal/tests/event.rs index 3267c46223f9..a86bb11caf7b 100644 --- a/rust/cymbal/tests/event.rs +++ b/rust/cymbal/tests/event.rs @@ -103,6 +103,7 @@ fn make_frame_js(name: &str) -> Frame { junk_drawer: None, code_variables: None, context: None, + release: None, } } @@ -125,6 +126,7 @@ fn make_frame_ts(name: &str) -> Frame { junk_drawer: None, code_variables: None, context: None, + release: None, } } diff --git a/rust/cymbal/tests/fingerprint_golden.rs b/rust/cymbal/tests/fingerprint_golden.rs index de0f33a798e6..8ce6bb2504b7 100644 --- a/rust/cymbal/tests/fingerprint_golden.rs +++ b/rust/cymbal/tests/fingerprint_golden.rs @@ -36,6 +36,7 @@ fn frame( junk_drawer: None, code_variables: None, context: None, + release: None, synthetic: false, suspicious: false, module: module.map(String::from), diff --git a/rust/cymbal/tests/resolution_service_tests.rs b/rust/cymbal/tests/resolution_service_tests.rs index c3c8f00c7074..924d7bd953c9 100644 --- a/rust/cymbal/tests/resolution_service_tests.rs +++ b/rust/cymbal/tests/resolution_service_tests.rs @@ -6,6 +6,7 @@ use std::time::Duration; use async_trait::async_trait; use cymbal::error::{ResolveError, UnhandledError}; +use cymbal::frames::releases::ReleaseRecord; use cymbal::frames::{Frame, RawFrame}; use cymbal::langs::native::DebugImage; use cymbal::modes::resolution::load_monitor::LoadMonitor; @@ -342,6 +343,58 @@ async fn raw_frames_are_resolved_into_done_payload() { assert_eq!(frames, vec![expected_wire_frame]); } +#[tokio::test] +async fn frame_releases_are_serialized_inside_the_resolved_frames() { + let release = ReleaseRecord { + id: uuid::Uuid::from_u128(7), + team_id: 123, + hash_id: "wire-hash".to_string(), + created_at: chrono::DateTime::from_timestamp(1_700_000_000, 0).unwrap(), + version: "9.9.9".to_string(), + project: "my-app".to_string(), + metadata: None, + }; + let raw_frame = sample_raw_frame(); + // One frame with a release, one without: the release must ride its own frame's JSON and a + // missing key (what an older server sends for every frame) must deserialize to `None`. + let mut first = sample_resolved_frame(&raw_frame); + first.frame_id = raw_frame.frame_id(123, 0, &[]); + first.release = Some(release.clone()); + let mut second = sample_resolved_frame(&raw_frame); + second.frame_id = raw_frame.frame_id(123, 1, &[]); + let service = make_service(FakeResolver { + fail_unhandled: false, + resolved_frames: vec![first, second], + }); + let mut exc = raw_exception("RuntimeError"); + exc.stack = Some(Stacktrace::Raw { + frames: vec![raw_frame], + }); + + let outcomes = resolve_items(service, vec![make_item(1, &exc)]).await; + assert_eq!(outcomes.len(), 1); + let resolve_outcome::Result::Done(done) = outcome_result(&outcomes[0]) else { + panic!("expected Done outcome, got {:?}", outcomes[0]); + }; + + let resolved: Exception = + serde_json::from_slice(&done.resolved_exception_json).expect("valid resolved exception"); + let Some(Stacktrace::Resolved { frames }) = resolved.stack else { + panic!("raw stack must be replaced with resolved frames"); + }; + assert_eq!(frames[0].release, Some(release)); + assert_eq!(frames[1].release, None); + + let raw: serde_json::Value = + serde_json::from_slice(&done.resolved_exception_json).expect("valid resolved exception"); + let frames_json = raw["stacktrace"]["frames"] + .as_array() + .expect("resolved frames present"); + assert!(frames_json[0].get("release").is_some()); + // `skip_serializing_if` keeps release-less frames byte-identical to the old wire shape. + assert!(frames_json[1].get("release").is_none()); +} + #[tokio::test] async fn bidi_resolve_stream_emits_outcomes_as_items_complete_out_of_order() { let active = Arc::new(AtomicUsize::new(0)); @@ -537,6 +590,7 @@ fn sample_resolved_frame(raw_frame: &RawFrame) -> Frame { junk_drawer: None, code_variables: None, context: None, + release: None, } }