-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix(native): update sentry_value_t
example with 64-bit integer types
#14509
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Bundle ReportChanges will increase total bundle size by 1.02kB (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-server-cjsAssets Changed:
view changes for bundle: sentry-docs-client-array-pushAssets Changed:
|
You can add Data Attributes to both Spans and Transactions. This data is visible in the trace explorer in Sentry. | ||
The data must be of type `sentry_value_t`, which can store: | ||
* 32-bit signed integers, | ||
* 64-bit signed and unsigned integers, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention that 64-bit unsigned values get sent as Strings until further notice? For context, see getsentry/sentry-native#1326 (comment) . Not sure how relevant this is for most users.
e.g. add something like
<Alert level="warning">
Currently, 64-bit unsigned integers are sent as strings, since they can't be processed as numerical values yet.
</Alert>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's a good idea to mention it (see below why), but I'd probably rephrase it:
* 64-bit signed integers,
* 64-bit unsigned integers (due to a current limitation, these will be converted to strings before sending),
wdyt?
why mention it at all: as we just discussed offline, data properties can be searched, and (un)signed int64 values will show up as both number (int64) and string (uint64) keys, this means that the actual user experience differs for unsigned int64 values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
DESCRIBE YOUR PR
Related to the changes in getsentry/sentry-native#1326 , we update our docs where they explicitly mention the available
sentry_value_t
types.IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes: