Skip to content
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

Added support for monolog/monolog v3 #602

Merged
merged 1 commit into from
Feb 16, 2023
Merged

Conversation

danielmorell
Copy link
Collaborator

@danielmorell danielmorell commented Feb 16, 2023

This PR adds support for Monolog v3. This is required to be compatible with Laravel v10. This was done without removing support for Monolog v2. I believe it is important to provide compatibility with existing packages that don't yet support Monolog v3.

Description of the change

The biggest changes are in the Rollbar\Config class. Monolog v3 changes the log level to an enum. Enums are new in PHP 8.1. Previously, the log level was an integer. This may seem like a small issue. However, it has a number of implications.

  1. To be compatible with PHP 8.0 we must support Monolog v2.
  2. In Monolog v3 our custom none verbose log level with an integer value of 1000 will not work.

To resolve the first issue a number of checks were added so that Monolog v2 and v3 return types are both supported.

To resolve the second issue if the 'verbose' config option is set to 'none' we set the verbose log hander to the NoopHandler instead of trying to set the log level as 1000 like we did previously.

Lastly, we bumped the version of vimeo/psalm to v5 to support some of the new PHP 8.1 features from Monolog.

Test Changes

There were some minor updates to the ConfigTest and `RollbarLoggerTest classes to support the above changes.

The biggest changes in the tests were in the VerbosityTest class. It previously used a sophisticated series of mocks and lambda functions to validate that the right verbose messages were logged. The changes to support Monolog v3 caused all of these tests to fail. I believe it had something to do with the NoopHandler in some of the cases, but I really don't know. After trying to debug the VerbosityTest test suite for way too long (like an hour) I decided to remove the mocks and the anonymous functions and write more declarative test with the existing ArrayLogger test helper as the verbose logger. I added a few new methods to ArrayLogger and updated all the test. The VerbosityTest class is now about half the size, retains all the previous test cases, and last but most important to me I can read and understand it.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Maintenance
  • New release

Related issues

Checklists

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests
  • All tests related to the changed code pass in development

Code review

  • This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
  • "Ready for review" label attached to the PR and reviewers assigned
  • Issue from task tracker has a link to this pull request
  • Changes have been reviewed by at least one other engineer

@danielmorell danielmorell added this to the v4.0.0 milestone Feb 16, 2023
@danielmorell danielmorell added the Type: Maintenance General up-keep, or changes that tidy an existing component or process. label Feb 16, 2023
This was referenced Feb 16, 2023
@michaelscola
Copy link

michaelscola commented Feb 16, 2023

This looks great! I wasn't comfortable enough with the codebase (just started using Rollbar) to perform such a refactor. I went ahead and closed my PR.

Copy link
Contributor

@waltjones waltjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@danielmorell danielmorell merged commit bc79ff3 into master Feb 16, 2023
@danielmorell danielmorell deleted the added/monolog-v3-support branch February 16, 2023 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Maintenance General up-keep, or changes that tidy an existing component or process.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Monolog 3 support
3 participants