fix(stacktrace): Skip serializing some null values in frames interface #944
Conversation
There was a problem hiding this comment.
Before merging this, please do a careful audit of Sentry code to ensure that no code expects these attributes to be present. This is effectively a schema change, and thus has the potential to introduce regressions in Sentry, which we do not have E2E tests here.
| @@ -93,6 +95,7 @@ pub struct Frame { | |||
| pub pre_context: Annotated<Array<String>>, | |||
|
|
|||
| /// Source code of the current line (`lineno`). | |||
| #[metastructure(skip_serialization = "empty")] | |||
There was a problem hiding this comment.
This should be null. We use context_line in Sentry to check if there is source context, and in theory the context line can be an empty string.
| @@ -104,6 +107,7 @@ pub struct Frame { | |||
| /// | |||
| /// Setting this attribute to `false` causes the frame to be hidden/collapsed by default and | |||
| /// mostly ignored during issue grouping. | |||
| #[metastructure(skip_serialization = "empty")] | |||
There was a problem hiding this comment.
Empty for booleans and numbers is always false, so let's use null in those instances, too.
|
I've verified that none of those values are required by the frames interface in Sentry nor the Snuba errors processor. |
untitaker
changed the title
untitaker
changed the title
untitaker
force-pushed
the
fix/skip-null-values
branch
from
1bf7239
to
e0d040e
Compare
| pub context_line: Annotated<String>, | ||
|
|
||
| /// Source code of the lines after `lineno`. | ||
| #[metastructure(skip_serialization = "empty")] | ||
| #[metastructure(skip_serialization = "null")] |
There was a problem hiding this comment.
empty is fine for the arrays, especially since you only changed it for post_context but not pre_context. Let's stick with empty.
| @@ -124,15 +130,18 @@ pub struct Frame { | |||
| /// If this is set and a known image is defined in the | |||
| /// [Debug Meta Interface]({%- link _documentation/development/sdk-dev/event-payloads/debugmeta.md -%}), | |||
| /// then symbolication can take place. | |||
| #[metastructure(skip_serialization = "empty")] | |||
There was a problem hiding this comment.
Since these are scalar values, let's use null here.
Noticed this while reviewing events: We serialize a lot of
nullkeys for every frame which are not necessary on all platforms.