-
-
Notifications
You must be signed in to change notification settings - Fork 52
Add category as attribute to structured logs #1199
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
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Internal Changes 🔧Deps
Release
Other
Other
🤖 This preview updates automatically when you update the PR. |
|
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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| FinalAttributes.Add(TEXT("category"), Category); | ||
| } | ||
|
|
||
| SubsystemNativeImpl->AddLog(FormattedMessage, Level, FinalAttributes); |
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.
Category attribute breaks global attribute preservation
Medium Severity
On Windows/Linux, when a user provides a category but no explicit per-log attributes, global attributes set via SetAttribute() are no longer included in the log. The automatic category attribute causes Attributes.Num() > 0 to be true in the platform implementation, which triggers creation of a non-null attributes object. Per the code comment, passing null preserves global attributes, but passing non-null does not. Before this change, LogDebug("msg", "cat") would preserve global attributes; now it loses them.
Additional Locations (1)
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.
Looks like an issue in sentry-native (getsentry/sentry-native#1485) which should probably be addressed before moving on with these changes in Unreal.
On Mac, global and per-log attributes are merged properly.
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.
This should be addressed via getsentry/sentry-native#1486
# Conflicts: # CHANGELOG.md
This PR adds an automatic category attribute to structured logs unless it is explicitly provided by the user. This allows more efficient log filtering and provides an experience similar to the Unreal Editor.
Key changes:
USentrySubsystem::AddLogto reduce code duplicationISentrySubsystem::AddLoginterface by removingCategoryparametercategoryattribute to structured logs unless explicitly provided by userRelated items: