-
Notifications
You must be signed in to change notification settings - Fork 104
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
Extra check for the newrelic extension #221
base: master
Are you sure you want to change the base?
Conversation
I've discovered that I cannot have `monolog.enabled = true` without the newrelic extension installed because monolog's NewRelicHandler will throw a `MissingExtensionException`. Is there a better way of achieving this? Locally this means I can use the same yaml file from a machine with the extension installed and not see the above exception
@@ -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']) { |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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');
}
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Is there any update on this @Nyholm |
I vote for closing this issue and deprecating the “enabled” option. Here is the config I was referring to: https://github.com/ekino/EkinoNewRelicBundle/blob/master/README.md#interactor-services |
I agree, that maybe this |
You should set the enable: true and use the adaptiveInteractor. That should work for you for all your machines. |
Hi, unfortunately I have not found the way how to make it work either, our first staging environment is using dev mode, not prod due to need of profiler bar, we have working set up for this instance within new-relic so we could find errors (from minimum configuration to start failing locally (without extension loaded):
Interactor is set to the adaptiveInteractor as default obviously. Then:
would throw Maybe the same logic should be applied to monolog setup as interactive adapter has currently, to not set monolog handler if extension is not present within
? |
I've discovered that I cannot have
monolog.enabled = true
without the newrelic extension installed because monolog's NewRelicHandler will throw aMissingExtensionException
.Is there a better way of achieving this?
Locally this means I can use the same yaml file from a machine with the extension installed and not see the above exception