Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the missing user attributes on log messages by ensuring that, if a user is set on the event, the log payload now includes the user's id, email, and username. Furthermore, the change refactors the serializer to rely on event properties (such as environment, release, and server name) instead of duplicating these values in the log aggregator.
- Updated test case to validate presence of new user attributes.
- Enhanced the envelope serializer to attach environment, release, server, and user attributes.
- Removed redundant attribute assignments from the LogsAggregator, relying on the event properties.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/Serializer/PayloadSerializerTest.php | Updated test expectations to include the new user attributes in the log payload. |
| src/Serializer/EnvelopItems/LogsItem.php | Refactored to map additional event properties (environment, release, server name) and user attributes. |
| src/Logs/LogsAggregator.php | Removed explicit setting of several attributes now handled in the envelope serializer. |
Comments suppressed due to low confidence (2)
src/Logs/LogsAggregator.php:75
- Removal of explicit log attributes (release, environment, and server address) is intentional since these are now sourced from event properties in the serializer. Please confirm that omitting these in the aggregator does not remove any necessary context when event properties are not set.
->setAttribute('sentry.release', $options->getRelease())
src/Serializer/EnvelopItems/LogsItem.php:58
- [nitpick] The user attribute key is set as 'user.name' based on getUsername(), which is acceptable; please ensure this naming convention is consistent with other parts of the codebase and documentation.
if ($user->getUsername() !== null) {
cleptric
force-pushed
the
user-log-attributes
branch
from
73a9c45 to
d654cdd
Compare
stayallive
approved these changes
Jun 20, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We didn't include various user attributes on log messages. If a user was set on the scope, the
id,usernameandemailare now being attached.