From 375ebf4383be8ef9c4b68967ffc7ba96e6e54485 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 3 Jul 2023 16:46:07 +0200 Subject: [PATCH 1/4] feat(metrics): Support access to structured release fields --- relay-general/src/protocol/event.rs | 22 +++++++++++++++++-- relay-general/src/protocol/mod.rs | 2 +- .../src/store/transactions/processor.rs | 2 +- relay-sampling/src/lib.rs | 17 ++++++++++++++ .../metrics_extraction/transactions/mod.rs | 2 +- relay-server/src/metrics_extraction/utils.rs | 2 +- 6 files changed, 41 insertions(+), 6 deletions(-) diff --git a/relay-general/src/protocol/event.rs b/relay-general/src/protocol/event.rs index 63dbe6ba68..005ded96f8 100644 --- a/relay-general/src/protocol/event.rs +++ b/relay-general/src/protocol/event.rs @@ -538,14 +538,21 @@ pub struct Event { } impl Event { - pub fn get_transaction_source(&self) -> &TransactionSource { + /// Returns the source of the transaction name if specified by the client. + /// + /// See the [type docs](TransactionSource) for more information. + pub fn transaction_source(&self) -> &TransactionSource { self.transaction_info .value() .and_then(|info| info.source.value()) .unwrap_or(&TransactionSource::Unknown) } - pub fn get_tag_value(&self, tag_key: &str) -> Option<&str> { + /// Returns the value of a tag with the given key. + /// + /// If tags are specified in a pair list and the tag is declared multiple times, this function + /// returns the first match. + pub fn tag_value(&self, tag_key: &str) -> Option<&str> { if let Some(tags) = self.tags.value() { tags.get(tag_key) } else { @@ -553,6 +560,7 @@ impl Event { } } + /// Returns `true` if [`modules`](Self::modules) contains the given module. pub fn has_module(&self, module_name: &str) -> bool { self.modules .value() @@ -560,6 +568,9 @@ impl Event { .unwrap_or(false) } + /// Return the identifier of the client SDK if available. + /// + /// Sentry's own SDKs use a naming schema prefixed with `sentry.`. Defaults to `"unknown"`. pub fn sdk_name(&self) -> &str { if let Some(client_sdk) = self.client_sdk.value() { if let Some(name) = client_sdk.name.as_str() { @@ -570,6 +581,9 @@ impl Event { "unknown" } + /// Return the version of the client SDK if available. + /// + /// Defaults to `"unknown"`. pub fn sdk_version(&self) -> &str { if let Some(client_sdk) = self.client_sdk.value() { if let Some(version) = client_sdk.version.as_str() { @@ -601,6 +615,10 @@ impl Event { Some(value) } + + pub fn parse_release(&self) -> Option { + sentry_release_parser::Release::parse(self.release.as_str()?).ok() + } } #[cfg(test)] diff --git a/relay-general/src/protocol/mod.rs b/relay-general/src/protocol/mod.rs index 07b5ba8848..306b871ea0 100644 --- a/relay-general/src/protocol/mod.rs +++ b/relay-general/src/protocol/mod.rs @@ -33,7 +33,7 @@ mod user; mod user_report; mod utils; -pub use sentry_release_parser::{validate_environment, validate_release}; +pub use sentry_release_parser::{validate_environment, validate_release, Release as ParsedRelease}; pub use self::breadcrumb::*; pub use self::breakdowns::*; diff --git a/relay-general/src/store/transactions/processor.rs b/relay-general/src/store/transactions/processor.rs index fa2c7605e5..78b2e70f47 100644 --- a/relay-general/src/store/transactions/processor.rs +++ b/relay-general/src/store/transactions/processor.rs @@ -354,7 +354,7 @@ pub fn is_high_cardinality_sdk(event: &Event) -> bool { return true; } - let is_http_status_404 = event.get_tag_value("http.status_code") == Some("404"); + let is_http_status_404 = event.tag_value("http.status_code") == Some("404"); if sdk_name == "sentry.python" && is_http_status_404 && client_sdk.has_integration("django") { return true; } diff --git a/relay-sampling/src/lib.rs b/relay-sampling/src/lib.rs index b4f4f50ca5..7921237b21 100644 --- a/relay-sampling/src/lib.rs +++ b/relay-sampling/src/lib.rs @@ -692,6 +692,23 @@ impl FieldValueProvider for Event { } _ => Value::Null, }, + "release.version.build" => self + .parse_release() + .as_ref() + .and_then(|r| r.version()) + .and_then(|v| v.build_code()) + .map_or(Value::Null, Value::from), + "release.package" => self + .parse_release() + .as_ref() + .and_then(|r| r.package()) + .map_or(Value::Null, Value::from), + "release.version.short" => self + .parse_release() + .as_ref() + .and_then(|r| r.version()) + .map(|v| v.raw_short()) + .map_or(Value::Null, Value::from), // Dynamic access to certain data bags _ => { diff --git a/relay-server/src/metrics_extraction/transactions/mod.rs b/relay-server/src/metrics_extraction/transactions/mod.rs index a09f16324b..1ed1874ad8 100644 --- a/relay-server/src/metrics_extraction/transactions/mod.rs +++ b/relay-server/src/metrics_extraction/transactions/mod.rs @@ -120,7 +120,7 @@ fn get_transaction_name( AcceptTransactionNames::ClientBased ) && !store::is_high_cardinality_sdk(event); - let source = event.get_transaction_source(); + let source = event.transaction_source(); let use_original_name = is_low_cardinality(source, treat_unknown_as_low_cardinality); let name_used; diff --git a/relay-server/src/metrics_extraction/utils.rs b/relay-server/src/metrics_extraction/utils.rs index 91bc541040..b4b23533ea 100644 --- a/relay-server/src/metrics_extraction/utils.rs +++ b/relay-server/src/metrics_extraction/utils.rs @@ -34,7 +34,7 @@ pub(crate) fn http_status_code_from_span(span: &Span) -> Option { /// Extracts the HTTP status code. pub(crate) fn extract_http_status_code(event: &Event) -> Option { // For SDKs which put the HTTP status code in the event tags. - if let Some(status_code) = event.get_tag_value("http.status_code") { + if let Some(status_code) = event.tag_value("http.status_code") { return Some(status_code.to_owned()); } From a82a0cedd88c601b68d3ce10f730d8a7a7426536 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 3 Jul 2023 16:49:51 +0200 Subject: [PATCH 2/4] meta: Changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c49c8396a..5cb7ce40f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ **Internal**: - Implement basic generic metrics extraction for transaction events. ([#2252](https://github.com/getsentry/relay/pull/2252), [#2257](https://github.com/getsentry/relay/pull/2257)) -- Support more fields in dynamic sampling, metric extraction, and conditional tagging. The added fields are `dist`, `user.{email,ip_address,name}`, `breakdowns.*`, and `extra.*`. ([#2259](https://github.com/getsentry/relay/pull/2259)) +- Support more fields in dynamic sampling, metric extraction, and conditional tagging. The added fields are `dist`, `release.*`, `user.{email,ip_address,name}`, `breakdowns.*`, and `extra.*`. ([#2259](https://github.com/getsentry/relay/pull/2259), [#2276](https://github.com/getsentry/relay/pull/2276)) ## 23.6.1 From 2e72a4e133e1af3c91ce6b8f040024675410566d Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 3 Jul 2023 17:23:33 +0200 Subject: [PATCH 3/4] fix: Apply review feedback --- relay-general/src/protocol/event.rs | 4 ++-- relay-sampling/src/lib.rs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/relay-general/src/protocol/event.rs b/relay-general/src/protocol/event.rs index 005ded96f8..c1475a8ad7 100644 --- a/relay-general/src/protocol/event.rs +++ b/relay-general/src/protocol/event.rs @@ -568,7 +568,7 @@ impl Event { .unwrap_or(false) } - /// Return the identifier of the client SDK if available. + /// Returns the identifier of the client SDK if available. /// /// Sentry's own SDKs use a naming schema prefixed with `sentry.`. Defaults to `"unknown"`. pub fn sdk_name(&self) -> &str { @@ -581,7 +581,7 @@ impl Event { "unknown" } - /// Return the version of the client SDK if available. + /// Returns the version of the client SDK if available. /// /// Defaults to `"unknown"`. pub fn sdk_version(&self) -> &str { diff --git a/relay-sampling/src/lib.rs b/relay-sampling/src/lib.rs index 7921237b21..0597461e14 100644 --- a/relay-sampling/src/lib.rs +++ b/relay-sampling/src/lib.rs @@ -692,16 +692,16 @@ impl FieldValueProvider for Event { } _ => Value::Null, }, - "release.version.build" => self + "release.package" => self .parse_release() .as_ref() - .and_then(|r| r.version()) - .and_then(|v| v.build_code()) + .and_then(|r| r.package()) .map_or(Value::Null, Value::from), - "release.package" => self + "release.version.build" => self .parse_release() .as_ref() - .and_then(|r| r.package()) + .and_then(|r| r.version()) + .and_then(|v| v.build_code()) .map_or(Value::Null, Value::from), "release.version.short" => self .parse_release() From 89f05beadfdffafe7af1e74133d8b13136364a21 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Mon, 3 Jul 2023 17:56:30 +0200 Subject: [PATCH 4/4] fix: Change access for `release.build` --- relay-sampling/src/lib.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/relay-sampling/src/lib.rs b/relay-sampling/src/lib.rs index 0597461e14..81ccd478af 100644 --- a/relay-sampling/src/lib.rs +++ b/relay-sampling/src/lib.rs @@ -692,16 +692,15 @@ impl FieldValueProvider for Event { } _ => Value::Null, }, - "release.package" => self + "release.build" => self .parse_release() .as_ref() - .and_then(|r| r.package()) + .and_then(|r| r.build_hash()) .map_or(Value::Null, Value::from), - "release.version.build" => self + "release.package" => self .parse_release() .as_ref() - .and_then(|r| r.version()) - .and_then(|v| v.build_code()) + .and_then(|r| r.package()) .map_or(Value::Null, Value::from), "release.version.short" => self .parse_release()