google-cloud-php
google-cloud-php copied to clipboard
feat: Add support for Monolog 3.x
Hey everyone π,
this is a follow-up to #2302 and aims to add support for all currently available monolog/monolog versions.
-
AppEngineFlexHandlerV3
andAppEngineFlexFormatterV3
are the new implementations for Monolog 3.x. - The
formatPayload
method in theFormatterTrait
should support arrays and the newLogRecord
class
Prerequisites for this PR to be mergeable
- https://github.com/phpDocumentor/Reflection/pull/247 is merged but not released (this PR uses ^5.x-dev for now)
- ~https://github.com/googleapis/gax-php/pull/389 is not reviewed/merged/released (this PR uses my PR branch from over there for now)~
- https://github.com/GoogleCloudPlatform/grpc-gcp-php/pull/45 is not (yet) merged - if it's just because of the CLA, I can create a new PR there
Things to look into
- The ReflectionHandlerV5 class is a 90% copy of the ReflectionHandlerV4 - perhaps this can be deduplicated
Notes
- There are deprecation warnings that I was not yet able to tackle yet:
-
opis/closure
Deprecated: Opis\Closure\SerializableClosure implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in /Users/jg/Code/opensource/google-cloud-php/vendor/opis/closure/src/SerializableClosure.php on line 18
-
:octocat:
~I'll ignore the conventionalcommits.org tests for now, but it's good to see that the "Generate Documentation" job fails, this shows that https://github.com/googleapis/google-cloud-php/blob/main/Core/src/Testing/Reflection/ReflectionHandlerFactory.php needs updating to support phpdocumentor/reflection ^5~
I caved so that there's one less failing job π€·π
The reflection handlers should probably be refactored to avoid much-duplicated code, I will get to that once I'm back at my desk in a few hours
Alright, this is a rabbit hole - when I fix one problem, another one pops up, that's why I'm gradually working my way through the classes and force pushing all the time.
The good news is that we'll probably have PHP 8.1 compatibility when we're through (if that's even possible)
Update: Sorry for all the force pushes - I needed to make a bunch of preparations in order to be able to run the tests without throwing errors with PHP 8.1 before adding Monolog 3 to the mix.
There's currently a blocker with a ReadOnly
class because it collides with the reserved readonly
in PHP 8.1.
This has already been addressed and a fix is on its way with https://github.com/protocolbuffers/protobuf/pull/10041
Until then I think this PR is on hold, but I'm watching the protobuf repo and will continue as soon as it's through... perhaps google/gax und phpdocumentor/reflection will have released new versions as well by then π€
Whew! Alright, 20 action runs later, I think I'm getting somewhere - the test suites finally don't exit with errors anymore π₯³
Due to the two PRs not being merged/released yet, this one is not mergeable as well, but if someone wants to try this PR in their projects, that would be the best test π . And of course, nobody has reviewed the changes yet π¬.
(The Docs Generator Job is still failing, and I can't figure out why. If you have an idea, please let me know, otherwise I'll give it a closer look tomorrow with a rested brain πͺ)
I had to add the dev branch of the reflection library because its dependency of psr/log
1 prevented Monolog from installing, since it needs version 3. I hoped my PR over there would have been released, but removing that part of the doc generation altogether sounds even better!
I will rebase this PR with main later (in a few hours) to see if everything still works π€π».
Do you see a chance to merge and release https://github.com/GoogleCloudPlatform/grpc-gcp-php/pull/45 in the near future? This is another blocker π
Alright! I managed to green up all checks π₯³
Concerning the jump to a new major version: phpdocumentor/reflection
is exclusively used as a dev-dependency in the following places (I think you mentioned that already, @bshaffer):
-
Core
(only tests) - docs generation
Although it's unfortunate to have to rely on a not-yet-released dev dependency, we already hat one dev dependency before (which is now removed):

One out, one in? π
#5325 could still be applied afterward, and I have a notification on the next phpdocumentor/reflection
stable 5.x release.
To re-iterate the COMPOSER_TOKEN: ${{ secrets.GITHUB_TOKEN }}
topic: this doesn't expose a (new) secret. The token is available on every workflow run, and its only purpose is to avoid running into rate limits when fetching composer dependencies.
I also want to add that many changes seemingly are not related to Monolog, but without them, the tests failed on my machine with PHP 8.1.
Great work @jeromegamez. I'm relying on this PR to make one of the APIs work with Symfony 6, so many thanks for your efforts. Is there anything you would like support with?
Good news! phpdocumentor/reflection
5.3 was just released, and I have reverted the ${GITHUB_TOKEN}
changes from the workflows.
The package test is failing in ErrorReporting
with the following:
PHP Fatal error: Declaration of Google\Cloud\Logging\PsrLogger::emergency($message, array $context = []) must be compatible with Psr\Log\LoggerInterface::emergency(Stringable|string $message, array $context = []): void in /home/runner/work/google-cloud-php/google-cloud-php/Logging/src/PsrLogger.php on line 172
Edit: After some investigation, the newer psr/log: 3.0
is being pulled in, which our Google\Cloud\Logging\PsrLogger
is not compatible with. So we'll need to add a dependency on "psr/log": "^1.0||^2.0"
in Logging/composer.json
or wait until we drop support for PHP 7.0 and add the void
return types to this logger.
I have a solution for making it compatible with all three psr/log
versions 90% working locally because I had a feeling people would demand it. I will propose this in a separate PR based on the changes in this one π€. I hope the latest push will finally do the trick βΊοΈ
I have another question - at the moment, run-package-tests.sh
doesn't output the installed dependencies so that we won't see which Monolog version was actually installed (same with Guzzle). What would you think about adding composer show
to the output or removing the -q
parameter on installation? Or would that blow the output up too much?
It went through! π€― π₯³
@bshaffer With the changed testing process, would you like me to try reverting the changes related to components other than Core and Logging? That would lead to new PRs though, I don't know how exhausted you guys are of me already π¬.
I removed almost all commits that are not strictly related to the feature and renamed/reordered the others to read better and to have a fix/chore/feat order. πͺ
This PR has had quite a journey! βΊοΈ
@bshaffer I've seen that you merged main into this branch after #5442 but I rebased anyway to reduce the number of commits - just in case you didn't want to squash-merge later.
We missed Debugger
: https://github.com/googleapis/google-cloud-php/pull/5711
We are going to wait until after the new year to release this, as our team is currently in a release freeze for the holidays!
Super glad to see it released now! I enjoyed the process! We'll meet again at latest when Monolog 4 is released π