Skip to content

Commit

Permalink
feat(err): Rewrite main, improve error handling, more metrics (#26155)
Browse files Browse the repository at this point in the history
  • Loading branch information
oliverb123 authored Nov 13, 2024
1 parent 224e9df commit 030866c
Show file tree
Hide file tree
Showing 18 changed files with 371 additions and 193 deletions.
1 change: 1 addition & 0 deletions rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion rust/common/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ workspace = true

[dependencies]
serde = { workspace = true }
serde_json = { workspace = true }
uuid = { workspace = true }
time = { workspace = true }
sqlx = { workspace = true }
sqlx = { workspace = true }
22 changes: 22 additions & 0 deletions rust/common/types/src/event.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::collections::HashMap;

use serde::{Deserialize, Serialize};
use serde_json::Value;
use time::OffsetDateTime;
use uuid::Uuid;

Expand Down Expand Up @@ -79,3 +82,22 @@ pub struct ClickHouseEvent {
pub group4_created_at: Option<String>,
pub person_mode: PersonMode,
}

impl ClickHouseEvent {
pub fn take_raw_properties(&mut self) -> Result<HashMap<String, Value>, serde_json::Error> {
// Sometimes properties are REALLY big, so we may as well do this.
let props = self.properties.take();
match props {
Some(properties) => serde_json::from_str(&properties),
None => Ok(HashMap::new()),
}
}

pub fn set_raw_properties(
&mut self,
properties: HashMap<String, Value>,
) -> Result<(), serde_json::Error> {
self.properties = Some(serde_json::to_string(&properties)?);
Ok(())
}
}
4 changes: 2 additions & 2 deletions rust/cymbal/src/app_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use tracing::info;

use crate::{
config::Config,
error::Error,
error::UnhandledError,
frames::resolver::Resolver,
symbol_store::{
caching::{Caching, SymbolSetCache},
Expand All @@ -33,7 +33,7 @@ pub struct AppContext {
}

impl AppContext {
pub async fn new(config: &Config) -> Result<Self, Error> {
pub async fn new(config: &Config) -> Result<Self, UnhandledError> {
let health_registry = HealthRegistry::new("liveness");
let worker_liveness = health_registry
.register("worker".to_string(), Duration::from_secs(60))
Expand Down
41 changes: 34 additions & 7 deletions rust/cymbal/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,30 @@ use aws_sdk_s3::primitives::ByteStreamError;
use rdkafka::error::KafkaError;
use serde::{Deserialize, Serialize};
use thiserror::Error;
use uuid::Uuid;

#[derive(Debug, Error)]
pub enum Error {
#[error("Serde error: {0}")]
SerdeError(#[from] serde_json::Error),
#[error(transparent)]
UnhandledError(#[from] UnhandledError),
#[error(transparent)]
ResolutionError(#[from] FrameError),
}

#[derive(Debug, Error)]
pub enum UnhandledError {
#[error("Config error: {0}")]
ConfigError(#[from] envconfig::Error),
#[error("Kafka error: {0}")]
KafkaError(#[from] KafkaError),
#[error("Sqlx error: {0}")]
SqlxError(#[from] sqlx::Error),
#[error(transparent)]
ResolutionError(#[from] ResolutionError),
#[error(transparent)]
S3Error(#[from] aws_sdk_s3::Error),
S3Error(#[from] Box<aws_sdk_s3::Error>),
#[error(transparent)]
ByteStreamError(#[from] ByteStreamError), // AWS specific bytestream error. Idk
#[error("Unhandled serde error: {0}")]
SerdeError(#[from] serde_json::Error),
}

// These are errors that occur during frame resolution. This excludes e.g. network errors,
Expand All @@ -28,7 +35,7 @@ pub enum Error {
// some provider (e.g. we fail to look up a sourcemap), we can return the correct error in the future
// without hitting their infra again (by storing it in PG).
#[derive(Debug, Error, Serialize, Deserialize)]
pub enum ResolutionError {
pub enum FrameError {
#[error(transparent)]
JavaScript(#[from] JsResolveErr),
}
Expand Down Expand Up @@ -75,9 +82,23 @@ pub enum JsResolveErr {
RedirectError(String),
}

#[derive(Debug, Error)]
pub enum EventError {
#[error("Wrong event type: {0} for event {1}")]
WrongEventType(String, Uuid),
#[error("No properties: {0}")]
NoProperties(Uuid),
#[error("Invalid properties: {0}, serde error: {1}")]
InvalidProperties(Uuid, String),
#[error("No exception list: {0}")]
NoExceptionList(Uuid),
#[error("Empty exception list: {0}")]
EmptyExceptionList(Uuid),
}

impl From<JsResolveErr> for Error {
fn from(e: JsResolveErr) -> Self {
ResolutionError::JavaScript(e).into()
FrameError::JavaScript(e).into()
}
}

Expand Down Expand Up @@ -111,3 +132,9 @@ impl From<reqwest::Error> for JsResolveErr {
JsResolveErr::NetworkError(e.to_string())
}
}

impl From<aws_sdk_s3::Error> for UnhandledError {
fn from(e: aws_sdk_s3::Error) -> Self {
UnhandledError::S3Error(Box::new(e))
}
}
82 changes: 9 additions & 73 deletions rust/cymbal/src/fingerprinting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,83 +16,11 @@ pub fn generate_fingerprint(exception: &[Exception]) -> String {

#[cfg(test)]
mod test {

use crate::{frames::Frame, types::Stacktrace};

use super::*;

#[test]
fn test_fingerprint_generation() {
let mut exception = Exception {
exception_type: "TypeError".to_string(),
exception_message: "Cannot read property 'foo' of undefined".to_string(),
mechanism: Default::default(),
module: Default::default(),
thread_id: None,
stack: Default::default(),
};

let resolved_frames = vec![
Frame {
mangled_name: "foo".to_string(),
line: Some(10),
column: Some(5),
source: Some("http://example.com/alpha/foo.js".to_string()),
in_app: true,
resolved_name: Some("bar".to_string()),
resolved: true,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
Frame {
mangled_name: "bar".to_string(),
line: Some(20),
column: Some(15),
source: Some("http://example.com/bar.js".to_string()),
in_app: true,
resolved_name: Some("baz".to_string()),
resolved: true,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
Frame {
mangled_name: "xyz".to_string(),
line: Some(30),
column: Some(25),
source: None,
in_app: true,
resolved_name: None,
resolved: true,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
Frame {
mangled_name: "<anonymous>".to_string(),
line: None,
column: None,
source: None,
in_app: false,
resolved_name: None,
resolved: true,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
];

exception.stack = Some(Stacktrace::Resolved {
frames: resolved_frames,
});

let fingerprint = super::generate_fingerprint(&[exception]);
assert_eq!(
fingerprint,
"7f5c327cd3941f2da655d852eb4661b411440c080c7ff014feb920afde68beaffe663908d4ab5fb7b7f1e7ab7f1f7cd17949139e8f812b1c3ff0911fc5b68f37"
);
}

#[test]
fn test_some_resolved_frames() {
let mut exception = Exception {
Expand All @@ -106,6 +34,7 @@ mod test {

let mut resolved_frames = vec![
Frame {
raw_id: String::new(),
mangled_name: "foo".to_string(),
line: Some(10),
column: Some(5),
Expand All @@ -118,6 +47,7 @@ mod test {
context: None,
},
Frame {
raw_id: String::new(),
mangled_name: "bar".to_string(),
line: Some(20),
column: Some(15),
Expand All @@ -132,6 +62,7 @@ mod test {
];

let unresolved_frame = Frame {
raw_id: String::new(),
mangled_name: "xyz".to_string(),
line: Some(30),
column: Some(25),
Expand Down Expand Up @@ -175,6 +106,7 @@ mod test {

let resolved_frames = vec![
Frame {
raw_id: String::new(),
mangled_name: "foo".to_string(),
line: Some(10),
column: Some(5),
Expand All @@ -187,6 +119,7 @@ mod test {
context: None,
},
Frame {
raw_id: String::new(),
mangled_name: "bar".to_string(),
line: Some(20),
column: Some(15),
Expand All @@ -199,6 +132,7 @@ mod test {
context: None,
},
Frame {
raw_id: String::new(),
mangled_name: "xyz".to_string(),
line: Some(30),
column: Some(25),
Expand Down Expand Up @@ -236,6 +170,7 @@ mod test {
};

let mut resolved_frames = vec![Frame {
raw_id: String::new(),
mangled_name: "foo".to_string(),
line: Some(10),
column: Some(5),
Expand All @@ -249,6 +184,7 @@ mod test {
}];

let non_app_frame = Frame {
raw_id: String::new(),
mangled_name: "bar".to_string(),
line: Some(20),
column: Some(15),
Expand Down
13 changes: 11 additions & 2 deletions rust/cymbal/src/frames/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha512};

use crate::{
error::Error, langs::js::RawJSFrame, metric_consts::PER_FRAME_TIME, symbol_store::Catalog,
error::UnhandledError, langs::js::RawJSFrame, metric_consts::PER_FRAME_TIME,
symbol_store::Catalog,
};

pub mod records;
Expand All @@ -17,11 +18,18 @@ pub enum RawFrame {
}

impl RawFrame {
pub async fn resolve(&self, team_id: i32, catalog: &Catalog) -> Result<Frame, Error> {
pub async fn resolve(&self, team_id: i32, catalog: &Catalog) -> Result<Frame, UnhandledError> {
let RawFrame::JavaScript(raw) = self;

let frame_resolve_time = common_metrics::timing_guard(PER_FRAME_TIME, &[]);
let res = raw.resolve(team_id, catalog).await;

// The raw id of the frame is set after it's resolved
let res = res.map(|mut f| {
f.raw_id = self.frame_id();
f
});

if res.is_err() {
frame_resolve_time.label("outcome", "failed")
} else {
Expand All @@ -46,6 +54,7 @@ impl RawFrame {
// We emit a single, unified representation of a frame, which is what we pass on to users.
#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)]
pub struct Frame {
pub raw_id: String, // The raw frame id this was resolved from
pub mangled_name: String, // Mangled name of the function
pub line: Option<u32>, // Line the function is define on, if known
pub column: Option<u32>, // Column the function is defined on, if known
Expand Down
10 changes: 7 additions & 3 deletions rust/cymbal/src/frames/records.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use serde_json::Value;
use sqlx::Executor;
use uuid::Uuid;

use crate::error::Error;
use crate::error::UnhandledError;

use super::Frame;

Expand Down Expand Up @@ -39,7 +39,7 @@ impl ErrorTrackingStackFrame {
}
}

pub async fn save<'c, E>(&self, e: E) -> Result<(), Error>
pub async fn save<'c, E>(&self, e: E) -> Result<(), UnhandledError>
where
E: Executor<'c, Database = sqlx::Postgres>,
{
Expand All @@ -66,7 +66,11 @@ impl ErrorTrackingStackFrame {
Ok(())
}

pub async fn load<'c, E>(e: E, team_id: i32, raw_id: &str) -> Result<Option<Self>, Error>
pub async fn load<'c, E>(
e: E,
team_id: i32,
raw_id: &str,
) -> Result<Option<Self>, UnhandledError>
where
E: Executor<'c, Database = sqlx::Postgres>,
{
Expand Down
4 changes: 2 additions & 2 deletions rust/cymbal/src/frames/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use sqlx::PgPool;

use crate::{
config::Config,
error::Error,
error::UnhandledError,
symbol_store::{saving::SymbolSetRecord, Catalog},
};

Expand All @@ -30,7 +30,7 @@ impl Resolver {
team_id: i32,
pool: &PgPool,
catalog: &Catalog,
) -> Result<Frame, Error> {
) -> Result<Frame, UnhandledError> {
if let Some(result) = self.cache.get(&frame.frame_id()) {
return Ok(result.contents);
}
Expand Down
Loading

0 comments on commit 030866c

Please sign in to comment.