Skip to content

Conversation

@huangdijia
Copy link
Contributor

@huangdijia huangdijia commented Sep 29, 2025

Summary

• Extracted the attribute compilation logic from the handle() method into a dedicated compileAttributes() method
• Improves code readability and maintainability by separating concerns
• Makes the attribute compilation logic reusable for future enhancements

Changes

  • Added protected method compileAttributes(LogRecord $record): array
  • Refactored handle() method to use the new method instead of inline array_merge
  • No functional changes - this is purely a code organization improvement

Test plan

  • Existing tests should continue to pass as this is a refactoring with no functional changes
  • The Monolog handler should continue to work exactly as before
  • The sentry.origin attribute should still be properly added to log records

Note

Refactors LogsHandler to extract log attribute compilation into a reusable compileAttributes() and updates handle() to use it, with no behavior change.

  • Monolog LogsHandler:
    • Refactor: Extract attribute assembly into protected compileAttributes($record): array that merges context, extra, and adds sentry.origin.
    • Update handle() to use compileAttributes($record) instead of inline array_merge.
    • Add PHPDoc for the new helper method.

Written by Cursor Bugbot for commit 1790af4. This will update automatically on new commits. Configure here.

Extracted the array_merge logic for compiling log record attributes into a
dedicated protected method `compileAttributes()`. This improves code
readability and maintainability by separating concerns and making the
attribute compilation logic reusable.
cursor[bot]

This comment was marked as outdated.

@cleptric
Copy link
Member

Thanks, but the code is fine as is.

@cleptric cleptric closed this Sep 29, 2025
@huangdijia
Copy link
Contributor Author

Thanks, but the code is fine as is.

I hope to support custom attributes combination, like: https://github.com/friendsofhyperf/components/blob/9b223569b9ade662f3c811138deb635746f0538c/src/sentry/src/Monolog/LogsHandler.php#L53-L64

@huangdijia huangdijia deleted the refactor/monolog-handler-extract-attributes-method branch September 29, 2025 09:20
@cleptric
Copy link
Member

It might be worth adding such "important" information in the PR description then.

@huangdijia
Copy link
Contributor Author

It might be worth adding such "important" information in the PR description then.

That’s alright, I can handle it by extending a subclass.

@huangdijia
Copy link
Contributor Author

huangdijia commented Sep 29, 2025

In fact, the current design is risky. For example:

$record['context'] = ['foo' => 'foo'];
$record['extra'] = ['foo' => 'bar'];
$result = array_merge($record['context'], $record['extra']);

output: ['foo' => 'bar']

@cleptric
Copy link
Member

Would you mind restoring the PR? With the added context, it makes sense, and we can get this in.

@huangdijia
Copy link
Contributor Author

Would you mind restoring the PR? With the added context, it makes sense, and we can get this in.

But I have already deleted the branch. Can this PR be restored?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants