-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[Unreal] Add transaction and span set_data documentation #14599
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
base: master
Are you sure you want to change the base?
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.09kB (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-client-array-pushAssets Changed:
view changes for bundle: sentry-docs-server-cjsAssets Changed:
|
transaction->SetData(TEXT("key"), {{TEXT("data_key1"), TEXT("value")}}); | ||
transaction->SetData(TEXT("key"), {{TEXT("data_key2"), 123}}); | ||
transaction->SetData(TEXT("key"), {{TEXT("data_key3"), 123.456f}}); | ||
transaction->SetData(TEXT("key"), {{TEXT("data_key4"), TArray<FSentryVariant>{{123, 456}}}}); | ||
transaction->SetData(TEXT("key"), {{TEXT("data_key5"), TMap<FString, FSentryVariant>{{TEXT("inner_key1"), 123}}}}); |
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.
Potential bug: The Unreal `SetData` documentation example incorrectly uses the same key for multiple calls, which will cause data to be overwritten, with only the last value being saved.
- Description: The code examples for
SetData
in the Unreal documentation repeatedly use the same key,TEXT("key")
, for multiple calls. Standard key-value map behavior, which Unreal'sTMap
follows, dictates that using the same key overwrites the previous value. Consequently, if a user implements this example, only the data from the finalSetData
call will be preserved on the transaction, and all previously set attributes will be lost. This contradicts the documentation's stated goal of demonstrating how to add multiple, queryable data attributes and is inconsistent with examples for all other Sentry SDKs. - Suggested fix: Update the code examples in the documentation to use a unique key for each
SetData
call. For instance, use keys likeTEXT("data_key1")
,TEXT("data_key2")
, etc., to align with the intended functionality and the pattern established in other Sentry SDK documentation.
severity: 0.45, confidence: 0.98
Did we get this right? 👍 / 👎 to inform future reviews.
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'd change the TEXT("key")
so the snippet isn't overwriting the data and to avoid copy-pasta user confusion. LGTM!
Preview: https://sentry-docs-git-unreal-attach-data.sentry.dev/platforms/unreal/tracing/instrumentation/custom-instrumentation/
DESCRIBE YOUR PR
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:
EXTRA RESOURCES