Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(normalization): Use exposed device-class-synthesis feature flag to gate device.class synthesis in light normalization #1974

Merged
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Metrics:

- Upgrade the web framework and related dependencies. ([#1938](https://github.com/getsentry/relay/pull/1938))
- Apply transaction clustering rules before UUID scrubbing rules. ([#1964](https://github.com/getsentry/relay/pull/1964))
- Use exposed device-class-synthesis feature flag to gate device.class synthesis in light normalization.. ([#1974](https://github.com/getsentry/relay/pull/1974))
edwardgou-sentry marked this conversation as resolved.
Show resolved Hide resolved

## 23.3.1

Expand Down
1 change: 1 addition & 0 deletions relay-cabi/src/processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ pub unsafe extern "C" fn relay_store_normalizer_normalize_event(
normalize_user_agent: config.normalize_user_agent,
transaction_name_config: Default::default(), // only supported in relay
is_renormalize: config.is_renormalize.unwrap_or(false),
device_class_synthesis_config: false, // only supported in relay
};
light_normalize_event(&mut event, light_normalization_config)?;
process_value(&mut event, &mut *processor, ProcessingState::root())?;
Expand Down
5 changes: 5 additions & 0 deletions relay-dynamic-config/src/feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ pub enum Feature {
/// Transaction names modified by clusterer rules are always marked as such.
#[serde(rename = "organizations:transaction-name-mark-scrubbed-as-sanitized")]
TransactionNameMarkScrubbedAsSanitized,
/// Enables device.class synthesis
///
/// Enables device.class tag synthesis on mobile events.
#[serde(rename = "organizations:device-class-synthesis")]
DeviceClassSynthesis,
/// Unused.
///
/// This used to control the initial experimental metrics extraction for sessions and has been
Expand Down
7 changes: 6 additions & 1 deletion relay-general/src/store/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,7 @@ pub struct LightNormalizationConfig<'a> {
pub normalize_user_agent: Option<bool>,
pub transaction_name_config: TransactionNameConfig<'a>,
pub is_renormalize: bool,
pub device_class_synthesis_config: bool,
}

pub fn light_normalize_event(
Expand Down Expand Up @@ -749,7 +750,11 @@ pub fn light_normalize_event(
config.max_secs_in_future,
)?; // Timestamps are core in the metrics extraction
normalize_event_tags(event)?; // Tags are added to every metric
normalize_device_class(event);

// TODO: Consider moving to store normalization
if config.device_class_synthesis_config {
normalize_device_class(event);
}
Comment on lines +755 to +757
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this PR is related to #1970. This will only normalize device.class when the feature flag is enabled for the project, so the functionality your other PR does won't run for organizations with the feature flag disabled. It will also skip other functionality, so no device.class tags will exist on events. Is this what you want?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @iker-barriocanal , yup this is the effect we're looking for. We're starting to rollout device class to customers and we want to be able to turn off this functionality in case the classifications are wrong or something breaks.

edwardgou-sentry marked this conversation as resolved.
Show resolved Hide resolved
light_normalize_stacktraces(event)?;
normalize_exceptions(event)?; // Browser extension filters look at the stacktrace
normalize_user_agent(event, config.normalize_user_agent); // Legacy browsers filter
Expand Down
3 changes: 3 additions & 0 deletions relay-server/src/actors/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2221,6 +2221,9 @@ impl EnvelopeProcessorService {
.has_feature(Feature::TransactionNameMarkScrubbedAsSanitized),
rules: &state.project_state.config.tx_name_rules,
},
device_class_synthesis_config: state
.project_state
.has_feature(Feature::DeviceClassSynthesis),

is_renormalize: false,
};
Expand Down