Move exception group fields to mechanism

[ribice]

While updating develop for #1204, noticed that the new mechanism fields were at the wrong place.

RFC 0079 specifies these fields (is_exception_group, parent_id, exception_id) to be in the mechanism.

This is how I initially implemented it, but for some reason (UI being broken?) I moved them to Data which is wrong according to RFC and other SDKs.

This is how it looks like on the UI.

#skip-changelog

[ribice]

Shall we add omitempty to exception_id? If so, the 0 is not sent (unless it's a pointer), but otherwise it will always be included.

Also other SDK-s have special logic for IsExceptionGroup, while for Go the only logic is if it's wrapped - it's an exception group.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.08%. Comparing base (89924ea) to head (ae0b042).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #800      +/-   ##
==========================================
- Coverage   81.20%   81.08%   -0.13%     
==========================================
  Files          49       49              
  Lines        4087     4087              
==========================================
- Hits         3319     3314       -5     
- Misses        629      632       +3     
- Partials      139      141       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[vaind]

I'm not familiar with the grouping mechanism so to clarify the discrepancy between RFC and implementations in other SDKs and the UI, I suggest you reach out to the developers that landed this in SDKs and maybe find a related PR in https://github.com/getsentry/sentry as well

[vaind]

not a fan of the pointer type here. It's not referring to a memory address

[ribice]

The reason for using pointer is that for first exception, it expects 0 for exception_id and nothing for parent_id. The second exception is {"exception_id":1,"parent_id":0}. Since go defaults to 0 for zero int values, it wouldn't work for first exception.

It's either using pointers (simpler) or a nullable type with custom marshal function (something like sql.NullInt16).

[ribice]

I'm not familiar with the grouping mechanism so to clarify the discrepancy between RFC and implementations in other SDKs and the UI, I suggest you reach out to the developers that landed this in SDKs and maybe find a related PR in https://github.com/getsentry/sentry as well

They are definitely on Mechanism level, not data. This PR should fix that.

Example: getsentry/sentry@f4fa54e

[cleptric]

The source of truth is always the protocol implemented in Relay.
In case of Mechansim, see https://github.com/getsentry/relay/blob/master/relay-event-schema/src/protocol/mechanism.rs#L170-L191.

Changes look good to me, besides this Pointer() thing.

[ribice]

The reason for using the pointer type for parent_id (and that should be used for exception_id as well for similar reasons) is that we shouldn't send anything in case it's not set / first in the exception group, and for the second item we should send 0.

So it's either using a pointer to achieve this (what I did) or using a nullable value (something like sql.NullInt16).