Skip to content

Commit

Permalink
feat(err): store frame context for use in the frontend (#26091)
Browse files Browse the repository at this point in the history
  • Loading branch information
oliverb123 authored Nov 11, 2024
1 parent 9bd920a commit 4ef61ac
Show file tree
Hide file tree
Showing 15 changed files with 153 additions and 40 deletions.
17 changes: 17 additions & 0 deletions posthog/migrations/0514_errortrackingstackframe_context.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 4.2.15 on 2024-11-08 12:20

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("posthog", "0513_alter_dashboardtemplate_github_url"),
]

operations = [
migrations.AddField(
model_name="errortrackingstackframe",
name="context",
field=models.TextField(blank=True, null=True),
),
]
2 changes: 1 addition & 1 deletion posthog/migrations/max_migration.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0513_alter_dashboardtemplate_github_url
0514_errortrackingstackframe_context
2 changes: 2 additions & 0 deletions posthog/models/error_tracking/error_tracking.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ class ErrorTrackingStackFrame(UUIDModel):
symbol_set = models.ForeignKey("ErrorTrackingSymbolSet", on_delete=models.CASCADE, null=True)
contents = models.JSONField(null=False, blank=False)
resolved = models.BooleanField(null=False, blank=False)
# The context around the frame, +/- a few lines, if we can get it
context = models.TextField(null=True, blank=True)

class Meta:
indexes = [
Expand Down

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

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

This file was deleted.

19 changes: 15 additions & 4 deletions rust/cymbal/src/app_context.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use aws_config::Region;
use common_kafka::{
kafka_consumer::SingleTopicConsumer, kafka_producer::create_kafka_producer,
kafka_producer::KafkaContext,
Expand Down Expand Up @@ -50,9 +51,19 @@ impl AppContext {
let options = PgPoolOptions::new().max_connections(config.max_pg_connections);
let pool = options.connect(&config.database_url).await?;

// TODO - this isn't really correct, we should instead specify the relevant vals
// in `Config`
let s3_client = aws_sdk_s3::Client::new(&aws_config::from_env().load().await);
let aws_credentials = aws_sdk_s3::config::Credentials::new(
&config.object_storage_access_key_id,
&config.object_storage_secret_access_key,
None,
None,
"environment",
);
let aws_conf = aws_sdk_s3::config::Builder::new()
.region(Region::new(config.object_storage_region.clone()))
.endpoint_url(&config.object_storage_endpoint)
.credentials_provider(aws_credentials)
.build();
let s3_client = aws_sdk_s3::Client::from_conf(aws_conf);
let s3_client = S3Client::new(s3_client);

let ss_cache = Arc::new(Mutex::new(SymbolSetCache::new(
Expand All @@ -64,7 +75,7 @@ impl AppContext {
smp,
pool.clone(),
s3_client,
config.ss_bucket.clone(),
config.object_storage_bucket.clone(),
config.ss_prefix.clone(),
);
let caching_smp = Caching::new(saving_smp, ss_cache);
Expand Down
21 changes: 19 additions & 2 deletions rust/cymbal/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,22 @@ pub struct Config {
#[envconfig(default = "100000000")] // 100MB - in prod, we should use closer to 1-10GB
pub symbol_store_cache_max_bytes: usize,

#[envconfig(default = "http://localhost:19000")] // minio
pub object_storage_endpoint: String,

#[envconfig(default = "symbol_sets")]
pub ss_bucket: String,
pub object_storage_bucket: String,

#[envconfig(default = "us-east-1")]
pub object_storage_region: String,

#[envconfig(default = "object_storage_root_user")]
pub object_storage_access_key_id: String,

#[envconfig(default = "sets")]
#[envconfig(default = "object_storage_root_password")]
pub object_storage_secret_access_key: String,

#[envconfig(default = "symbolsets")]
pub ss_prefix: String,

#[envconfig(default = "100000")]
Expand All @@ -52,6 +64,11 @@ pub struct Config {
pub frame_cache_ttl_seconds: u64,
}

pub enum AwsRegion {
USEast1,
USWest1,
}

impl Config {
pub fn init_with_defaults() -> Result<Self, envconfig::Error> {
ConsumerConfig::set_defaults("error-tracking-rs", "exception_symbolification_events");
Expand Down
12 changes: 12 additions & 0 deletions rust/cymbal/src/fingerprinting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ mod test {
resolved: true,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
Frame {
mangled_name: "bar".to_string(),
Expand All @@ -53,6 +54,7 @@ mod test {
resolved: true,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
Frame {
mangled_name: "xyz".to_string(),
Expand All @@ -64,6 +66,7 @@ mod test {
resolved: true,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
Frame {
mangled_name: "<anonymous>".to_string(),
Expand All @@ -75,6 +78,7 @@ mod test {
resolved: true,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
];

Expand Down Expand Up @@ -111,6 +115,7 @@ mod test {
resolved: true,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
Frame {
mangled_name: "bar".to_string(),
Expand All @@ -122,6 +127,7 @@ mod test {
resolved: true,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
];

Expand All @@ -135,6 +141,7 @@ mod test {
resolved: false,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
};

exception.stack = Some(Stacktrace::Resolved {
Expand Down Expand Up @@ -177,6 +184,7 @@ mod test {
resolved: false,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
Frame {
mangled_name: "bar".to_string(),
Expand All @@ -188,6 +196,7 @@ mod test {
resolved: false,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
Frame {
mangled_name: "xyz".to_string(),
Expand All @@ -199,6 +208,7 @@ mod test {
resolved: false,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
},
];

Expand Down Expand Up @@ -235,6 +245,7 @@ mod test {
resolved: false,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
}];

let non_app_frame = Frame {
Expand All @@ -247,6 +258,7 @@ mod test {
resolved: false,
resolve_failure: None,
lang: "javascript".to_string(),
context: None,
};

exception.stack = Some(Stacktrace::Resolved {
Expand Down
5 changes: 5 additions & 0 deletions rust/cymbal/src/frames/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ pub struct Frame {
pub lang: String, // The language of the frame. Always known (I guess?)
pub resolved: bool, // Did we manage to resolve the frame?
pub resolve_failure: Option<String>, // If we failed to resolve the frame, why?
// The lines of code surrounding the frame ptr, if known. We skip serialising this because
// it should never go in clickhouse / be queried over, but we do store it in PG for
// use in the frontend
#[serde(skip)]
pub context: Option<String>,
}

impl Frame {
Expand Down
26 changes: 20 additions & 6 deletions rust/cymbal/src/frames/records.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub struct ErrorTrackingStackFrame {
pub symbol_set_id: Option<Uuid>,
pub contents: Frame,
pub resolved: bool,
pub context: Option<String>,
}

impl ErrorTrackingStackFrame {
Expand All @@ -25,6 +26,7 @@ impl ErrorTrackingStackFrame {
symbol_set_id: Option<Uuid>,
contents: Frame,
resolved: bool,
context: Option<String>,
) -> Self {
Self {
raw_id,
Expand All @@ -33,6 +35,7 @@ impl ErrorTrackingStackFrame {
contents,
resolved,
created_at: Utc::now(),
context,
}
}

Expand All @@ -42,21 +45,23 @@ impl ErrorTrackingStackFrame {
{
sqlx::query!(
r#"
INSERT INTO posthog_errortrackingstackframe (raw_id, team_id, created_at, symbol_set_id, contents, resolved, id)
VALUES ($1, $2, $3, $4, $5, $6, $7)
INSERT INTO posthog_errortrackingstackframe (raw_id, team_id, created_at, symbol_set_id, contents, resolved, id, context)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
ON CONFLICT (raw_id, team_id) DO UPDATE SET
created_at = $3,
symbol_set_id = $4,
contents = $5,
resolved = $6
resolved = $6,
context = $8
"#,
self.raw_id,
self.team_id,
self.created_at,
self.symbol_set_id,
serde_json::to_value(&self.contents)?,
self.resolved,
Uuid::now_v7()
Uuid::now_v7(),
self.context
).execute(e).await?;
Ok(())
}
Expand All @@ -72,11 +77,12 @@ impl ErrorTrackingStackFrame {
symbol_set_id: Option<Uuid>,
contents: Value,
resolved: bool,
context: Option<String>,
}
let res = sqlx::query_as!(
Returned,
r#"
SELECT raw_id, team_id, created_at, symbol_set_id, contents, resolved
SELECT raw_id, team_id, created_at, symbol_set_id, contents, resolved, context
FROM posthog_errortrackingstackframe
WHERE raw_id = $1 AND team_id = $2
"#,
Expand All @@ -90,13 +96,21 @@ impl ErrorTrackingStackFrame {
return Ok(None);
};

// We don't serialise frame contexts on the Frame itself, but save it on the frame record,
// and so when we load a frame record we need to patch back up the context onto the frame,
// since we dropped it when we serialised the frame during saving.

let mut frame: Frame = serde_json::from_value(found.contents)?;
frame.context = found.context.clone();

Ok(Some(Self {
raw_id: found.raw_id,
team_id: found.team_id,
created_at: found.created_at,
symbol_set_id: found.symbol_set_id,
contents: serde_json::from_value(found.contents)?,
contents: frame,
resolved: found.resolved,
context: found.context,
}))
}
}
9 changes: 5 additions & 4 deletions rust/cymbal/src/frames/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ impl Resolver {
set.map(|s| s.id),
resolved.clone(),
resolved.resolved,
resolved.context.clone(),
);

record.save(pool).await?;
Expand Down Expand Up @@ -94,7 +95,7 @@ mod test {
S: FnOnce(&Config, S3Client) -> S3Client,
{
let mut config = Config::init_with_defaults().unwrap();
config.ss_bucket = "test-bucket".to_string();
config.object_storage_bucket = "test-bucket".to_string();
config.ss_prefix = "test-prefix".to_string();
config.allow_internal_ips = true; // Gonna be hitting the sourcemap mocks

Expand All @@ -119,7 +120,7 @@ mod test {
smp,
pool,
client,
config.ss_bucket.clone(),
config.object_storage_bucket.clone(),
config.ss_prefix.clone(),
);

Expand Down Expand Up @@ -165,7 +166,7 @@ mod test {
client
.expect_put()
.with(
predicate::eq(config.ss_bucket.clone()),
predicate::eq(config.object_storage_bucket.clone()),
predicate::str::starts_with(config.ss_prefix.clone()),
predicate::eq(Vec::from(MAP)),
)
Expand All @@ -175,7 +176,7 @@ mod test {
client
.expect_get()
.with(
predicate::eq(config.ss_bucket.clone()),
predicate::eq(config.object_storage_bucket.clone()),
predicate::str::starts_with(config.ss_prefix.clone()),
)
.returning(|_, _| Ok(Vec::from(MAP)))
Expand Down
Loading

0 comments on commit 4ef61ac

Please sign in to comment.