From a9f5e2f72392cc7563f4c706cf7b81877f71f639 Mon Sep 17 00:00:00 2001 From: Oleksandr <1931331+olksdr@users.noreply.github.com> Date: Wed, 22 Feb 2023 16:08:02 +0100 Subject: [PATCH 1/4] fix(panic): revert sentry-types to 0.20.1 (#1872) Revert `sentry-types`. --- Cargo.lock | 43 +++++++++++++++++---- relay-common/Cargo.toml | 2 +- relay-server/src/extractors/request_meta.rs | 8 +--- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 260942872b..0538d38b13 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1008,6 +1008,16 @@ version = "2.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23d8666cb01533c39dde32bcbab8e227b4ed6679b2c925eba05feabea39508fb" +[[package]] +name = "debugid" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6ee87af31d84ef885378aebca32be3d682b0e0dc119d5b4860a2c5bb5046730" +dependencies = [ + "serde", + "uuid 0.8.2", +] + [[package]] name = "debugid" version = "0.8.0" @@ -2281,7 +2291,7 @@ version = "0.15.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94e5cd2ca4f6b85c6c7fb41ae0aebe0b443a6c0558876f1d779c7236d42417cf" dependencies = [ - "debugid", + "debugid 0.8.0", "encoding", "memmap2", "minidump-common", @@ -2301,7 +2311,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "694717103b2c15f8c16ddfaec1333fe15673bc22b10ffa6164427415701974ba" dependencies = [ "bitflags", - "debugid", + "debugid 0.8.0", "enum-primitive-derive", "num-traits", "range-map", @@ -3269,7 +3279,7 @@ dependencies = [ "parking_lot 0.12.1", "regex", "schemars", - "sentry-types", + "sentry-types 0.20.1", "serde", "serde_test", "thiserror", @@ -3362,7 +3372,7 @@ dependencies = [ "chrono", "cookie 0.17.0", "criterion", - "debugid", + "debugid 0.8.0", "dynfmt", "enumset", "hmac 0.12.1", @@ -3976,7 +3986,7 @@ checksum = "b5acbd3da4255938cf0384b6b140e6c07ff65919c26e4d7a989d8d90ee88fa91" dependencies = [ "once_cell", "rand 0.8.5", - "sentry-types", + "sentry-types 0.29.3", "serde", "serde_json", ] @@ -4023,13 +4033,28 @@ dependencies = [ "serde", ] +[[package]] +name = "sentry-types" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8124f0e9bc1113ecbcc8c3746e0e590943cf23e7d09c70a088c116869bb12e3" +dependencies = [ + "chrono", + "debugid 0.7.3", + "serde", + "serde_json", + "thiserror", + "url 2.3.1", + "uuid 0.8.2", +] + [[package]] name = "sentry-types" version = "0.29.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "10d8587b12c0b8211bb3066979ee57af6e8657e23cf439dc6c8581fd86de24e8" dependencies = [ - "debugid", + "debugid 0.8.0", "getrandom", "hex", "serde", @@ -4357,7 +4382,7 @@ version = "11.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "46939659856f0595dbbdbe0c8271d11c6788c780c859bf9afd834a723dd78fa3" dependencies = [ - "debugid", + "debugid 0.8.0", "memmap2", "stable_deref_trait", "uuid 1.3.0", @@ -5145,6 +5170,10 @@ name = "uuid" version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bc5cf98d8186244414c848017f0e2676b3fcb46807f6668a97dfe67359a3c4b7" +dependencies = [ + "getrandom", + "serde", +] [[package]] name = "uuid" diff --git a/relay-common/Cargo.toml b/relay-common/Cargo.toml index ae073089c5..7801116569 100644 --- a/relay-common/Cargo.toml +++ b/relay-common/Cargo.toml @@ -17,7 +17,7 @@ once_cell = "1.13.1" parking_lot = "0.12.1" regex = "1.5.5" schemars = { version = "0.8.1", features = ["uuid1", "chrono"], optional = true } -sentry-types = "0.29.3" +sentry-types = "0.20.0" serde = { version = "1.0.114", features = ["derive"] } thiserror = "1.0.38" uuid = { version = "1.3.0", features = ["serde", "v4", "v5"] } diff --git a/relay-server/src/extractors/request_meta.rs b/relay-server/src/extractors/request_meta.rs index 68a5733f4a..a2e8acb0be 100644 --- a/relay-server/src/extractors/request_meta.rs +++ b/relay-server/src/extractors/request_meta.rs @@ -85,11 +85,7 @@ pub struct PartialDsn { impl PartialDsn { /// Ensures a valid public key and project ID in the DSN. fn from_dsn(dsn: Dsn) -> Result { - let project_id = dsn - .project_id() - .value() - .parse() - .map_err(|_| ParseDsnError::NoProjectId)?; + let project_id = dsn.project_id().value(); let public_key = dsn .public_key() @@ -102,7 +98,7 @@ impl PartialDsn { host: dsn.host().to_owned(), port: dsn.port(), path: dsn.path().to_owned(), - project_id: Some(project_id), + project_id: Some(ProjectId::new(project_id)), }) } From d465f2d5effc013039a4fca884bc663efcb46527 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Wed, 22 Feb 2023 10:34:55 -0500 Subject: [PATCH 2/4] feat(profiling): Add PHP support (#1871) --- CHANGELOG.md | 6 ++++++ relay-profiling/src/lib.rs | 1 + relay-profiling/src/sample.rs | 3 +-- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1443cb69e9..db424f9445 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +**Internal**: + +- Add PHP support. ([#1871](https://github.com/getsentry/relay/pull/1871)) + ## 23.2.0 **Features**: diff --git a/relay-profiling/src/lib.rs b/relay-profiling/src/lib.rs index 3aec7df66c..cd5a8c117e 100644 --- a/relay-profiling/src/lib.rs +++ b/relay-profiling/src/lib.rs @@ -120,6 +120,7 @@ enum Platform { Android, Cocoa, Node, + Php, Python, Rust, } diff --git a/relay-profiling/src/sample.rs b/relay-profiling/src/sample.rs index e15a348115..b085855376 100644 --- a/relay-profiling/src/sample.rs +++ b/relay-profiling/src/sample.rs @@ -175,8 +175,7 @@ impl SampleProfile { && self.device.manufacturer.is_some() && self.device.model.is_some() } - Platform::Python => self.runtime.is_some(), - Platform::Node => self.runtime.is_some(), + Platform::Python | Platform::Node | Platform::Php => self.runtime.is_some(), _ => true, } } From 09849aa397b996d3901a5145da5101de518a0b08 Mon Sep 17 00:00:00 2001 From: Iker Barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Thu, 23 Feb 2023 10:34:07 +0100 Subject: [PATCH 3/4] ref(statsd): Revert back the adition of metric names as tag on Sentry errors (#1873) A previous PR added metric names as tags on Sentry errors, as part of an experiment to provide more visibility on what metrics was relay dropping for the channel being full ([sentry issue](https://sentry.my.sentry.io/organizations/sentry/issues/316057/?project=9)). In the last 3 weeks (since this "experiment" started), there have been no errors; thus, I'm reverting the experiment and not getting any conclusions. If relay starts dropping metrics again, we'll revisit this problem. --- CHANGELOG.md | 1 + relay-statsd/src/lib.rs | 127 ++++++++++++++-------------------------- 2 files changed, 44 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db424f9445..a133f6c643 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ **Internal**: +- Revert back the addition of metric names as tag on Sentry errors when relay drops metrics. ([#1873](https://github.com/getsentry/relay/pull/1873)) - Add PHP support. ([#1871](https://github.com/getsentry/relay/pull/1871)) ## 23.2.0 diff --git a/relay-statsd/src/lib.rs b/relay-statsd/src/lib.rs index 4659858638..0bfb212f9b 100644 --- a/relay-statsd/src/lib.rs +++ b/relay-statsd/src/lib.rs @@ -492,102 +492,67 @@ pub trait GaugeMetric { macro_rules! metric { // counter increment (counter($id:expr) += $value:expr $(, $k:ident = $v:expr)* $(,)?) => { - relay_log::with_scope( - |scope| { - scope.set_tag("key", &$crate::CounterMetric::name(&$id)); - }, || { - $crate::with_client(|client| { - use $crate::_pred::*; - client.send_metric( - client.count_with_tags(&$crate::CounterMetric::name(&$id), $value) - $(.with_tag(stringify!($k), $v))* - ) - }) - } - ) + $crate::with_client(|client| { + use $crate::_pred::*; + client.send_metric( + client.count_with_tags(&$crate::CounterMetric::name(&$id), $value) + $(.with_tag(stringify!($k), $v))* + ) + }) }; // counter decrement (counter($id:expr) -= $value:expr $(, $k:ident = $v:expr)* $(,)?) => { - relay_log::with_scope( - |scope| { - scope.set_tag("key", &$crate::CounterMetric::name(&$id)); - }, || { - $crate::with_client(|client| { - use $crate::_pred::*; - client.send_metric( - client.count_with_tags(&$crate::CounterMetric::name(&$id), -$value) - $(.with_tag(stringify!($k), $v))* - ) - }) - } - ) + $crate::with_client(|client| { + use $crate::_pred::*; + client.send_metric( + client.count_with_tags(&$crate::CounterMetric::name(&$id), -$value) + $(.with_tag(stringify!($k), $v))* + ) + }) }; // gauge set (gauge($id:expr) = $value:expr $(, $k:ident = $v:expr)* $(,)?) => { $crate::with_client(|client| { use $crate::_pred::*; - relay_log::with_scope( - |scope| { - scope.set_tag("key", &$crate::GaugeMetric::name(&$id)); - }, || { - client.send_metric( - client.gauge_with_tags(&$crate::GaugeMetric::name(&$id), $value) - $(.with_tag(stringify!($k), $v))* - ) - } + client.send_metric( + client.gauge_with_tags(&$crate::GaugeMetric::name(&$id), $value) + $(.with_tag(stringify!($k), $v))* ) }) }; // histogram (histogram($id:expr) = $value:expr $(, $k:ident = $v:expr)* $(,)?) => { - use $crate::_pred::*; - relay_log::with_scope( - |scope| { - scope.set_tag("key", &$crate::HistogramMetric::name(&$id)); - }, || { - $crate::with_client(|client| { - client.send_metric( - client.histogram_with_tags(&$crate::HistogramMetric::name(&$id), $value) - $(.with_tag(stringify!($k), $v))* - ) - }) - } - ) + $crate::with_client(|client| { + use $crate::_pred::*; + client.send_metric( + client.histogram_with_tags(&$crate::HistogramMetric::name(&$id), $value) + $(.with_tag(stringify!($k), $v))* + ) + }) }; // sets (count unique occurrences of a value per time interval) (set($id:expr) = $value:expr $(, $k:ident = $v:expr)* $(,)?) => { - use $crate::_pred::*; - relay_log::with_scope( - |scope| { - scope.set_tag("key", &$crate::SetMetric::name(&$id)); - }, || { - $crate::with_client(|client| { - client.send_metric( - client.set_with_tags(&$crate::SetMetric::name(&$id), $value) - $(.with_tag(stringify!($k), $v))* - ) - }) - } - ) + $crate::with_client(|client| { + use $crate::_pred::*; + client.send_metric( + client.set_with_tags(&$crate::SetMetric::name(&$id), $value) + $(.with_tag(stringify!($k), $v))* + ) + }) }; // timer value (duration) (timer($id:expr) = $value:expr $(, $k:ident = $v:expr)* $(,)?) => { $crate::with_client(|client| { - relay_log::with_scope( - |scope| { - scope.set_tag("key", &$crate::TimerMetric::name(&$id)); - }, || { - use $crate::_pred::*; - client.send_metric( - client.time_with_tags(&$crate::TimerMetric::name(&$id), $value) - $(.with_tag(stringify!($k), $v))* - ) - }) + use $crate::_pred::*; + client.send_metric( + client.time_with_tags(&$crate::TimerMetric::name(&$id), $value) + $(.with_tag(stringify!($k), $v))* + ) }) }; @@ -595,19 +560,13 @@ macro_rules! metric { (timer($id:expr), $($k:ident = $v:expr,)* $block:block) => {{ let now = std::time::Instant::now(); let rv = {$block}; - relay_log::with_scope( - |scope| { - scope.set_tag("key", &$crate::TimerMetric::name(&$id)); - }, || { - $crate::with_client(|client| { - use $crate::_pred::*; - client.send_metric( - client.time_with_tags(&$crate::TimerMetric::name(&$id), now.elapsed()) - $(.with_tag(stringify!($k), $v))* - ) - }); - } - ); + $crate::with_client(|client| { + use $crate::_pred::*; + client.send_metric( + client.time_with_tags(&$crate::TimerMetric::name(&$id), now.elapsed()) + $(.with_tag(stringify!($k), $v))* + ) + }); rv }}; } From 7cb05e095a8d56acc17bf06a86b1f53b71a6a12c Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 23 Feb 2023 10:35:35 +0100 Subject: [PATCH 4/4] feat(protocol): Add sourcemap debug image type to protocol (#1869) This adds the new `sourcemap` image type to the protocol so that it's possible to associate entries in the stack trace with debug files. Like `abs_path` we are exempting this from PII stripping so that they are not destroyed in the process for mapping purposes. ```json { "code_file": "https://mycdn.invalid/foo.js.min", "debug_id": "971f98e5-ce60-41ff-b2d7-235bbeb34578", "debug_file": "https://mycdn.invalid/foo.js.map", "other": "value", "type": "sourcemap" } ``` --- CHANGELOG.md | 4 ++ relay-general/src/protocol/debugmeta.rs | 67 +++++++++++++++++++ .../test_fixtures__event_schema.snap | 45 ++++++++++++- 3 files changed, 115 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a133f6c643..f572c912b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +**Features**: + +- Protocol validation for source map image type. ([#1869](https://github.com/getsentry/relay/pull/1869)) + **Internal**: - Revert back the addition of metric names as tag on Sentry errors when relay drops metrics. ([#1873](https://github.com/getsentry/relay/pull/1873)) diff --git a/relay-general/src/protocol/debugmeta.rs b/relay-general/src/protocol/debugmeta.rs index 97bb928b82..d0163935be 100644 --- a/relay-general/src/protocol/debugmeta.rs +++ b/relay-general/src/protocol/debugmeta.rs @@ -412,6 +412,43 @@ pub struct NativeDebugImage { pub other: Object, } +/// A debug image pointing to a source map. +/// +/// Examples: +/// +/// ```json +/// { +/// "type": "sourcemap", +/// "code_file": "https://example.com/static/js/main.min.js", +/// "debug_id": "395835f4-03e0-4436-80d3-136f0749a893" +/// } +/// ``` +/// +/// **Note:** Stack frames and the correlating entries in the debug image here +/// for `code_file`/`abs_path` are not PII stripped as they need to line up +/// perfectly for source map processing. +#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] +#[cfg_attr(feature = "jsonschema", derive(JsonSchema))] +pub struct SourceMapDebugImage { + /// Path and name of the image file as URL. (required). + /// + /// The absolute path to the minified JavaScript file. This helps to correlate the file to the stack trace. + #[metastructure(required = "true")] + pub code_file: Annotated, + + /// Unique debug identifier of the source map. + #[metastructure(required = "true")] + pub debug_id: Annotated, + + /// Path and name of the associated source map. + #[metastructure(pii = "maybe")] + pub debug_file: Annotated, + + /// Additional arbitrary fields for forwards compatibility. + #[metastructure(additional_properties)] + pub other: Object, +} + /// Proguard mapping file. /// /// Proguard images refer to `mapping.txt` files generated when Proguard obfuscates function names. The Java SDK integrations assign this file a unique identifier, which has to be included in the list of images. @@ -449,6 +486,8 @@ pub enum DebugImage { Proguard(Box), /// WASM debug image. Wasm(Box), + /// Source map debug image. + SourceMap(Box), /// A debug image that is unknown to this protocol specification. #[metastructure(fallback_variant)] Other(Object), @@ -824,6 +863,34 @@ mod tests { assert_eq!(json, image.to_json_pretty().unwrap()); } + #[test] + fn test_source_map_image_roundtrip() { + let json = r#"{ + "code_file": "https://mycdn.invalid/foo.js.min", + "debug_id": "971f98e5-ce60-41ff-b2d7-235bbeb34578", + "debug_file": "https://mycdn.invalid/foo.js.map", + "other": "value", + "type": "sourcemap" +}"#; + + let image = Annotated::new(DebugImage::SourceMap(Box::new(SourceMapDebugImage { + code_file: Annotated::new("https://mycdn.invalid/foo.js.min".into()), + debug_file: Annotated::new("https://mycdn.invalid/foo.js.map".into()), + debug_id: Annotated::new("971f98e5-ce60-41ff-b2d7-235bbeb34578".parse().unwrap()), + other: { + let mut map = Object::new(); + map.insert( + "other".to_string(), + Annotated::new(Value::String("value".to_string())), + ); + map + }, + }))); + + assert_eq!(image, Annotated::from_json(json).unwrap()); + assert_eq!(json, image.to_json_pretty().unwrap()); + } + #[test] fn test_debug_image_other_roundtrip() { let json = r#"{"other":"value","type":"mytype"}"#; diff --git a/relay-general/tests/snapshots/test_fixtures__event_schema.snap b/relay-general/tests/snapshots/test_fixtures__event_schema.snap index dcab159c01..97f859bff0 100644 --- a/relay-general/tests/snapshots/test_fixtures__event_schema.snap +++ b/relay-general/tests/snapshots/test_fixtures__event_schema.snap @@ -1,6 +1,5 @@ --- source: relay-general/tests/test_fixtures.rs -assertion_line: 109 expression: "relay_general::protocol::event_json_schema()" --- { @@ -1013,6 +1012,9 @@ expression: "relay_general::protocol::event_json_schema()" { "$ref": "#/definitions/NativeDebugImage" }, + { + "$ref": "#/definitions/SourceMapDebugImage" + }, { "type": "object", "additionalProperties": true @@ -2905,6 +2907,47 @@ expression: "relay_general::protocol::event_json_schema()" } ] }, + "SourceMapDebugImage": { + "description": " A debug image pointing to a source map.\n\n Examples:\n\n ```json\n {\n \"type\": \"sourcemap\",\n \"code_file\": \"https://example.com/static/js/main.min.js\",\n \"debug_id\": \"395835f4-03e0-4436-80d3-136f0749a893\"\n }\n ```\n\n **Note:** Stack frames and the correlating entries in the debug image here\n for `code_file`/`abs_path` are not PII stripped as they need to line up\n perfectly for source map processing.", + "anyOf": [ + { + "type": "object", + "required": [ + "code_file", + "debug_id" + ], + "properties": { + "code_file": { + "description": " Path and name of the image file as URL. (required).\n\n The absolute path to the minified JavaScript file. This helps to correlate the file to the stack trace.", + "type": [ + "string", + "null" + ] + }, + "debug_file": { + "description": " Path and name of the associated source map.", + "default": null, + "type": [ + "string", + "null" + ] + }, + "debug_id": { + "description": " Unique debug identifier of the source map.", + "anyOf": [ + { + "$ref": "#/definitions/DebugId" + }, + { + "type": "null" + } + ] + } + }, + "additionalProperties": false + } + ] + }, "SpanId": { "description": " A 16-character hex string as described in the W3C trace context spec.", "anyOf": [