ref(breakdowns): All fields in breakdown config should be camelCase, and rename the breakdown key name in project options. #1020
Conversation
| @@ -202,15 +202,19 @@ impl EmitBreakdowns for SpanOperationsConfig { | |||
|
|
|||
| /// Configuration to define breakdown to be generated based on properties and breakdown type. | |||
| #[derive(Debug, Clone, Serialize, Deserialize)] | |||
| #[serde(tag = "type", rename_all = "snake_case")] | |||
| #[serde(tag = "type", rename_all = "camelCase")] | |||
| pub enum BreakdownConfig { | |||
There was a problem hiding this comment.
@untitaker I couldn't really use enum ErrorBoundary<T> in the project config (
relay/relay-general/src/store/mod.rs
Line 55 in a231a2b
I'm assuming this is what you had in mind.
If so, we might need to consolidate enum ErrorBoundary<T> somewhere, and implement Default for enum ErrorBoundary<T>.
dashed
changed the title
Co-authored-by: Markus Unterwaditzer <markus-honeypot@unterwaditzer.net>
There was a problem hiding this comment.
Looks good. The "V2" looks a bit strange at first, but is a good and explicit solution.
One change requested below, since this PR touches more places than required.
Note that once you merge and deploy this, Relay will no longer compute breakdowns until the corresponding Sentry PR is merged.
relay-general/benches/benchmarks.rs
Outdated
| @@ -90,7 +90,7 @@ fn bench_store_processor(c: &mut Criterion) { | |||
| user_agent: None, | |||
| sent_at: None, | |||
| received_at: None, | |||
| breakdowns: None, | |||
| breakdowns_v2: None, | |||
There was a problem hiding this comment.
The store config does not have to be renamed, since this is just an internal type.
There was a problem hiding this comment.
You're right. I've renamed this back to breakdowns.
Yep, I'm aware. That should be fine as the computed breakdown are not critical to the product. |
There was a problem hiding this comment.
Thanks!
@untitaker can you please also confirm that you're +1 on keeping StoreConfig as-is?
|
I presume the places where we still have this as public API don't get breakdowns anyway |
* master: ref(breakdowns): All fields in breakdown config should be camelCase, and rename the breakdown key name in project options. (#1020)
Replacement for #993 with applied suggestions:
Hard rename the breakdown key in project state options from
breakdowntobreakdown_v2(Sentry will provide the config using its camel-case namebreakdownV2).I've added the serde attribute to catch for all unknown enum variants for
enum BreakdownConfig.