Skip to content

Commit 9124117

Browse files
hpouillothpouillot
authored
feat(cymbal): add typed frame resolution metrics (#52057)
1 parent 5a1938a commit 9124117

File tree

16 files changed

+200
-65
lines changed

16 files changed

+200
-65
lines changed

‎rust/common/symbol_data/src/error.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::string::FromUtf8Error;
33
use serde::{Deserialize, Serialize};
44
use thiserror::Error;
55

6-
#[derive(Debug, Error, Serialize, Deserialize)]
6+
#[derive(Debug, Clone, PartialEq, Eq, Error, Serialize, Deserialize)]
77
pub enum Error {
88
#[error("Wrong version: {0}, expected {1}")]
99
WrongVersion(u32, u32),

‎rust/cymbal/src/error.rs‎

Lines changed: 71 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ pub enum UnhandledError {
6464
// NOTE - these are serialized and deserialized, so that when we fail to get a symbol set from
6565
// some provider (e.g. we fail to look up a sourcemap), we can return the correct error in the future
6666
// without hitting their infra again (by storing it in PG).
67-
#[derive(Debug, Error, Serialize, Deserialize)]
67+
#[derive(Debug, Clone, PartialEq, Eq, Error, Serialize, Deserialize)]
6868
pub enum FrameError {
6969
#[error(transparent)]
7070
JavaScript(#[from] JsResolveErr),
@@ -78,7 +78,7 @@ pub enum FrameError {
7878
MissingChunkIdData(String),
7979
}
8080

81-
#[derive(Debug, Error, Serialize, Deserialize)]
81+
#[derive(Debug, Clone, PartialEq, Eq, Error, Serialize, Deserialize)]
8282
pub enum JsResolveErr {
8383
#[error("This frame had no source url or chunk id")]
8484
NoUrlOrChunkId,
@@ -130,7 +130,7 @@ pub enum JsResolveErr {
130130
NoSourcemapUploaded(String),
131131
}
132132

133-
#[derive(Debug, Error, Serialize, Deserialize)]
133+
#[derive(Debug, Clone, PartialEq, Eq, Error, Serialize, Deserialize)]
134134
pub enum HermesError {
135135
#[error("Data error: {0}")]
136136
DataError(#[from] SymbolDataError),
@@ -144,7 +144,7 @@ pub enum HermesError {
144144
NoTokenForColumn(u32, String),
145145
}
146146

147-
#[derive(Debug, Error, Serialize, Deserialize)]
147+
#[derive(Debug, Clone, PartialEq, Eq, Error, Serialize, Deserialize)]
148148
pub enum ProguardError {
149149
#[error("Data error: {0}")]
150150
DataError(#[from] SymbolDataError),
@@ -164,7 +164,7 @@ pub enum ProguardError {
164164
InvalidClass,
165165
}
166166

167-
#[derive(Debug, Error, Serialize, Deserialize)]
167+
#[derive(Debug, Clone, PartialEq, Eq, Error, Serialize, Deserialize)]
168168
pub enum AppleError {
169169
#[error("Data error: {0}")]
170170
DataError(#[from] SymbolDataError),
@@ -206,6 +206,72 @@ pub enum EventError {
206206
FilteredByTeamId,
207207
}
208208

209+
impl JsResolveErr {
210+
pub fn metric_reason(&self) -> &'static str {
211+
match self {
212+
Self::NoUrlOrChunkId | Self::NoSourceUrl => "no_reference",
213+
Self::NoSourcemap(_) | Self::NoSourcemapUploaded(_) => "no_symbol_set",
214+
Self::TokenNotFound(..) => "symbol_not_found",
215+
Self::Timeout(_)
216+
| Self::HttpStatus(..)
217+
| Self::NetworkError(_)
218+
| Self::RedirectError(_) => "network_error",
219+
Self::InvalidSourceMap(_)
220+
| Self::InvalidSourceUrl(_)
221+
| Self::InvalidSourceMapHeader(_)
222+
| Self::InvalidSourceMapUrl(_)
223+
| Self::InvalidDataUrl(..)
224+
| Self::JSDataError(_)
225+
| Self::InvalidSourceAndMap => "invalid_data",
226+
}
227+
}
228+
}
229+
230+
impl HermesError {
231+
pub fn metric_reason(&self) -> &'static str {
232+
match self {
233+
Self::NoChunkId => "no_reference",
234+
Self::NoSourcemapUploaded(_) => "no_symbol_set",
235+
Self::NoTokenForColumn(..) => "symbol_not_found",
236+
Self::DataError(_) | Self::InvalidMap(_) => "invalid_data",
237+
}
238+
}
239+
}
240+
241+
impl ProguardError {
242+
pub fn metric_reason(&self) -> &'static str {
243+
match self {
244+
Self::NoMapId | Self::NoModuleProvided => "no_reference",
245+
Self::MissingMap(_) => "no_symbol_set",
246+
Self::NoOriginalFrames | Self::MissingClass => "symbol_not_found",
247+
Self::DataError(_) | Self::InvalidMapping | Self::InvalidClass => "invalid_data",
248+
}
249+
}
250+
}
251+
252+
impl AppleError {
253+
pub fn metric_reason(&self) -> &'static str {
254+
match self {
255+
Self::NoDebugId | Self::NoMatchingDebugImage => "no_reference",
256+
Self::MissingDsym(_) => "no_symbol_set",
257+
Self::SymbolNotFound(_) => "symbol_not_found",
258+
Self::DataError(_) | Self::InvalidAddress(_) | Self::ParseError(_) => "invalid_data",
259+
}
260+
}
261+
}
262+
263+
impl FrameError {
264+
pub fn metric_reason(&self) -> &'static str {
265+
match self {
266+
Self::JavaScript(e) => e.metric_reason(),
267+
Self::Hermes(e) => e.metric_reason(),
268+
Self::Proguard(e) => e.metric_reason(),
269+
Self::Apple(e) => e.metric_reason(),
270+
Self::MissingChunkIdData(_) => "no_symbol_set",
271+
}
272+
}
273+
}
274+
209275
impl From<JsResolveErr> for ResolveError {
210276
fn from(e: JsResolveErr) -> Self {
211277
FrameError::JavaScript(e).into()

‎rust/cymbal/src/fingerprinting/mod.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ mod test {
144144
resolved_name: Some("bar".to_string()),
145145
resolved: true,
146146
resolve_failure: None,
147+
147148
lang: "javascript".to_string(),
148149
junk_drawer: None,
149150
code_variables: None,
@@ -163,6 +164,7 @@ mod test {
163164
resolved_name: Some("baz".to_string()),
164165
resolved: true,
165166
resolve_failure: None,
167+
166168
lang: "javascript".to_string(),
167169
junk_drawer: None,
168170
code_variables: None,
@@ -184,6 +186,7 @@ mod test {
184186
resolved_name: None,
185187
resolved: false,
186188
resolve_failure: None,
189+
187190
lang: "javascript".to_string(),
188191
junk_drawer: None,
189192
code_variables: None,
@@ -236,6 +239,7 @@ mod test {
236239
resolved_name: Some("bar".to_string()),
237240
resolved: false,
238241
resolve_failure: None,
242+
239243
lang: "javascript".to_string(),
240244
junk_drawer: None,
241245
code_variables: None,
@@ -255,6 +259,7 @@ mod test {
255259
resolved_name: Some("baz".to_string()),
256260
resolved: false,
257261
resolve_failure: None,
262+
258263
lang: "javascript".to_string(),
259264
junk_drawer: None,
260265
code_variables: None,
@@ -274,6 +279,7 @@ mod test {
274279
resolved_name: None,
275280
resolved: false,
276281
resolve_failure: None,
282+
277283
lang: "javascript".to_string(),
278284
junk_drawer: None,
279285
code_variables: None,
@@ -321,6 +327,7 @@ mod test {
321327
resolved_name: Some("bar".to_string()),
322328
resolved: false,
323329
resolve_failure: None,
330+
324331
lang: "javascript".to_string(),
325332
junk_drawer: None,
326333
code_variables: None,
@@ -341,6 +348,7 @@ mod test {
341348
resolved_name: Some("baz".to_string()),
342349
resolved: false,
343350
resolve_failure: None,
351+
344352
lang: "javascript".to_string(),
345353
junk_drawer: None,
346354
code_variables: None,

‎rust/cymbal/src/frames/mod.rs‎

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize};
66
use serde_json::Value;
77

88
use crate::{
9-
error::UnhandledError,
9+
error::{FrameError, UnhandledError},
1010
fingerprinting::{FingerprintBuilder, FingerprintComponent, FingerprintRecordPart},
1111
langs::{
1212
apple::{AppleDebugImage, RawAppleFrame},
@@ -20,7 +20,7 @@ use crate::{
2020
python::RawPythonFrame,
2121
ruby::RawRubyFrame,
2222
},
23-
metric_consts::{LEGACY_JS_FRAME_RESOLVED, PER_FRAME_TIME},
23+
metric_consts::{FRAME_NOT_RESOLVED, FRAME_RESOLVED, LEGACY_JS_FRAME_RESOLVED, PER_FRAME_TIME},
2424
sanitize_string,
2525
symbol_store::Catalog,
2626
};
@@ -108,6 +108,29 @@ impl RawFrame {
108108
.label("lang", lang_tag)
109109
.fin();
110110

111+
if let Ok(frames) = &res {
112+
for frame in frames {
113+
if frame.resolved {
114+
metrics::counter!(FRAME_RESOLVED, "lang" => lang_tag).increment(1);
115+
} else if let Some(err) = &frame.resolve_failure {
116+
let reason = err.metric_reason();
117+
match reason {
118+
"network_error" | "invalid_data" | "symbol_not_found" => {
119+
tracing::warn!(lang = lang_tag, reason = reason, error = %err, "frame resolution failed");
120+
}
121+
_ => {
122+
tracing::debug!(lang = lang_tag, reason = reason, error = %err, "frame resolution failed");
123+
}
124+
}
125+
metrics::counter!(FRAME_NOT_RESOLVED, "lang" => lang_tag, "reason" => reason)
126+
.increment(1);
127+
} else {
128+
metrics::counter!(FRAME_NOT_RESOLVED, "lang" => lang_tag, "reason" => "unknown")
129+
.increment(1);
130+
}
131+
}
132+
}
133+
111134
res
112135
}
113136

@@ -176,8 +199,13 @@ pub struct Frame {
176199
pub resolved_name: Option<String>, // The name of the function, after symbolification
177200
pub lang: String, // The language of the frame. Always known (I guess?)
178201
pub resolved: bool, // Did we manage to resolve the frame?
179-
#[serde(skip_serializing_if = "Option::is_none")]
180-
pub resolve_failure: Option<String>, // If we failed to resolve the frame, why?
202+
#[serde(
203+
serialize_with = "frame_error_serde::serialize",
204+
skip_deserializing,
205+
skip_serializing_if = "Option::is_none",
206+
default
207+
)]
208+
pub resolve_failure: Option<FrameError>, // If we failed to resolve the frame, why?
181209

182210
#[serde(default)] // Defaults to false
183211
pub synthetic: bool, // Some SDKs construct stack traces, or partially reconstruct them. This flag indicates whether the frame is synthetic or not.
@@ -344,7 +372,11 @@ impl std::fmt::Display for Frame {
344372
writeln!(
345373
f,
346374
" resolve_failure: {}",
347-
self.resolve_failure.as_deref().unwrap_or("no failure")
375+
self.resolve_failure
376+
.as_ref()
377+
.map(|e| e.to_string())
378+
.as_deref()
379+
.unwrap_or("no failure")
348380
)?;
349381

350382
// Context
@@ -397,6 +429,25 @@ impl From<Frame> for FrameData {
397429
}
398430
}
399431

432+
mod frame_error_serde {
433+
use super::FrameError;
434+
use serde::Serializer;
435+
436+
pub fn serialize<S>(value: &Option<FrameError>, serializer: S) -> Result<S::Ok, S::Error>
437+
where
438+
S: Serializer,
439+
{
440+
// skip_serializing_if = "Option::is_none" guarantees value is Some here,
441+
// but we match exhaustively to satisfy the type checker.
442+
match value {
443+
Some(err) => serializer.serialize_str(&err.to_string()),
444+
None => {
445+
unreachable!("skip_serializing_if = Option::is_none prevents None reaching here")
446+
}
447+
}
448+
}
449+
}
450+
400451
fn to_vec<T, E>(item: Result<T, E>) -> Result<Vec<T>, E> {
401452
item.map(|t| vec![t])
402453
}

‎rust/cymbal/src/langs/apple.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ impl RawAppleFrame {
301301
lang: lang_from_filename(symbol_info.filename.as_deref()).to_string(),
302302
resolved: true,
303303
resolve_failure: None,
304+
304305
junk_drawer: None,
305306
release: None,
306307
synthetic: self.meta.synthetic,
@@ -366,7 +367,7 @@ impl RawAppleFrame {
366367
resolved_name,
367368
lang: lang_from_filename(self.filename.as_deref()).to_string(),
368369
resolved: false,
369-
resolve_failure: Some(err.to_string()),
370+
resolve_failure: Some(FrameError::from(err)),
370371
junk_drawer: None,
371372
release: None,
372373
synthetic: self.meta.synthetic,
@@ -435,6 +436,7 @@ impl From<&RawAppleFrame> for Frame {
435436
lang: lang_from_filename(raw.filename.as_deref()).to_string(),
436437
resolved: raw.function.is_some(),
437438
resolve_failure: None,
439+
438440
junk_drawer: None,
439441
release: None,
440442
synthetic: raw.meta.synthetic,

‎rust/cymbal/src/langs/custom.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ impl From<&CustomFrame> for Frame {
9898
lang: value.lang.clone(),
9999
resolved: value.resolved,
100100
resolve_failure: None,
101+
101102
junk_drawer: None,
102103
context: value.get_context(),
103104
release: None,

‎rust/cymbal/src/langs/dart.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ impl From<&RawDartFrame> for Frame {
4848
lang: "dart".to_string(),
4949
resolved: true,
5050
resolve_failure: None,
51+
5152
junk_drawer: None,
5253
release: None,
5354
synthetic: raw.meta.synthetic,

‎rust/cymbal/src/langs/go.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ impl From<&RawGoFrame> for Frame {
3636
lang: "go".to_string(),
3737
resolved: true,
3838
resolve_failure: None,
39+
3940
synthetic: frame.meta.synthetic,
4041
junk_drawer: None,
4142
context: None,

‎rust/cymbal/src/langs/hermes.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use crate::{
1212
utils::{add_raw_to_junk, get_token_context},
1313
CommonFrameMetadata,
1414
},
15-
metric_consts::FRAME_NOT_RESOLVED,
1615
sanitize_string,
1716
symbol_store::{chunk_id::OrChunkId, hermesmap::ParsedHermesMap, SymbolCatalog},
1817
};
@@ -134,7 +133,7 @@ impl From<(&RawHermesFrame, HermesError)> for Frame {
134133
resolved_name: None,
135134
lang: "javascript".to_string(),
136135
resolved: false,
137-
resolve_failure: Some(err.to_string()),
136+
resolve_failure: Some(FrameError::from(err)),
138137
synthetic: frame.meta.synthetic,
139138
junk_drawer: None,
140139
code_variables: None,
@@ -169,6 +168,7 @@ impl From<(&RawHermesFrame, Token<'_>, Option<String>)> for Frame {
169168
lang: "javascript".to_string(),
170169
resolved: true,
171170
resolve_failure: None,
171+
172172
synthetic: frame.meta.synthetic,
173173
junk_drawer: None,
174174
code_variables: None,
@@ -188,8 +188,6 @@ impl From<(&RawHermesFrame, Token<'_>, Option<String>)> for Frame {
188188
// probably a native function or something else weird
189189
impl From<&RawHermesFrame> for Frame {
190190
fn from(raw_frame: &RawHermesFrame) -> Self {
191-
metrics::counter!(FRAME_NOT_RESOLVED, "lang" => "hermes").increment(1);
192-
193191
// If this is a source_url: <anonymous> frame, we always assume it's not in_app
194192
let is_anon = raw_frame.source.eq("<anonymous>");
195193

@@ -206,6 +204,7 @@ impl From<&RawHermesFrame> for Frame {
206204
lang: "javascript".to_string(),
207205
resolved: true, // Without location information, we're assuming this is not minified
208206
resolve_failure: None,
207+
209208
junk_drawer: None,
210209
code_variables: None,
211210
context: None,

0 commit comments

Comments
 (0)