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.

Sign up for GitHub

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

Extra check for the newrelic extension #221

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+1 −1
Open

Extra check for the newrelic extension #221

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DependencyInjection/EkinoNewRelicExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public function load(array $configs, ContainerBuilder $container): void
$loader->load('twig.xml');
}

if ($config['enabled'] && $config['monolog']['enabled']) {
if (\extension_loaded('newrelic') && $config['enabled'] && $config['monolog']['enabled']) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is done in Configuration.php already, isnt it?
Or that is only if you use: enable: auto?

Copy link
Author

Choose a reason for hiding this comment

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

enabled needs to be a boolean or you get this error:

Invalid type for path "ekino_new_relic.enabled". Expected boolean, but got string.

Copy link
Author

Choose a reason for hiding this comment

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

When the value is set to true, but the newrelic bundle is not installed you get this errors:

The newrelic PHP extension is required to use the NewRelicHandler

Which is thrown within the Monolog NewRelicHandler:

    protected function write(array $record)
    {
        if (!$this->isNewRelicEnabled()) {
            throw new MissingExtensionException('The newrelic PHP extension is required to use the NewRelicHandler');
        }

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds like an expected behavior. If you enable it, but don't have the extension, it'll faill.

Like using doctrine with pgsql DSN without the extension.

Copy link
Author

Choose a reason for hiding this comment

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

That's a fair point, I guess the issue is having the same yaml file for machines with the extension installed and then local machines without it.

What I should do it use an .env value to default to false locally and then true on deployed installs.

if (!\class_exists(\Monolog\Handler\NewRelicHandler::class)) {
throw new \LogicException('The "symfony/monolog-bundle" package must be installed in order to use "monolog" option.');
}
Expand Down