feat: Add support for vars attribute in SentryStackFrame#4686
feat: Add support for vars attribute in SentryStackFrame#4686
vars attribute in SentryStackFrame#4686Conversation
The vars field previously existed as a property but was never serialized. We use frame.vars to report Godot GDScript trace variables in constructed frames.
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ee747ae | 382.73 ms | 435.41 ms | 52.68 ms |
| ee747ae | 415.92 ms | 470.15 ms | 54.23 ms |
| ee747ae | 400.46 ms | 423.61 ms | 23.15 ms |
| ee747ae | 554.98 ms | 611.50 ms | 56.52 ms |
| ee747ae | 358.21 ms | 389.41 ms | 31.20 ms |
| 3699cd5 | 423.60 ms | 495.52 ms | 71.92 ms |
| 85d7417 | 347.21 ms | 394.35 ms | 47.15 ms |
| 7314dbe | 437.83 ms | 505.64 ms | 67.81 ms |
| ee747ae | 357.79 ms | 421.84 ms | 64.05 ms |
| ee747ae | 374.71 ms | 455.18 ms | 80.47 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| 3699cd5 | 1.58 MiB | 2.10 MiB | 533.45 KiB |
| 85d7417 | 1.58 MiB | 2.10 MiB | 533.44 KiB |
| 7314dbe | 1.58 MiB | 2.10 MiB | 533.45 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
Previous results on branch: feat/frame-vars
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 5355eb0 | 361.81 ms | 412.04 ms | 50.23 ms |
| 712d637 | 420.38 ms | 481.23 ms | 60.85 ms |
| 0da200a | 384.89 ms | 414.40 ms | 29.51 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 5355eb0 | 1.58 MiB | 2.10 MiB | 533.47 KiB |
| 712d637 | 1.58 MiB | 2.10 MiB | 533.46 KiB |
| 0da200a | 1.58 MiB | 2.10 MiB | 533.46 KiB |
vars attribute in SentryFrame
vars attribute in SentryFramevars attribute in SentryStackFrame
lcian
left a comment
There was a problem hiding this comment.
Changes look good to me, however this is technically a breaking change as we're changing the return/param types on a public class.
I'll let someone else from the team give their opinion on whether this is a reasonable change or if we should look for a workaround so that this is not breaking.
|
Technically, it would be a breaking change if it were in use and working as expected. Seeing that it's actually not serialized, I strongly suspect that it's not being used anywhere, or otherwise you'd have it at least reported. Not sure it's worth the trouble engineering a workaround for API contract that didn't work in the first place. |
romtsn
left a comment
There was a problem hiding this comment.
LGTM, let's just add a note to the changelog, but the impact should be minimal-to-none, so we should be good here.
Co-authored-by: Roman Zavarnitsyn <[email protected]>
📜 Description
This PR adds support for
varsattribute to theSentryStackFrameclass. While it existed previously, it wasn’t serialized and had a String->String mapping type.💡 Motivation and Context
These changes are necessary for the Godot SDK to capture script variables on Android. For GDScript errors, we construct a stack trace and attach variables for each frame. There is a similar setup with Sentry Native and Cocoa SDKs.
💚 How did you test it?
I've added serialization tests.
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps