neeckeloo / NewRelic

NewRelic module for Laminas
MIT License
33 stars 13 forks source link

Setting application_name to null disables calling newrelic_set_appname #4

Closed bnkr closed 10 years ago

bnkr commented 10 years ago

This patch means that when ! (bool) newrelic.application_name, the RequestListener will not call client.setAppName.

On our newrelic deployment we found that setting the appname in a request event sometimes got called after the controller ran, which meat that all the metrics for that transaction would be discarded; the request time would be out by a factor of 10 (and it seemed to mess up the browser metrics to some extent). It also meant we lost any errors or warnings that were reported. (Further, I guess that setting in an event listener at all would always result in some lost profiling time and I'd like to see the full time of the framework as well as the controller).

I didn't really work out exactly why this happened. I suspect it could be do with other events I bound to and that I don't use layouts everywhere. The version of php running also seemed to make a difference.

It seems more simple to set the appname in the php or virtualhost configuration for my purposes anyway since this is more likely to compare properly against non-zend scripts.

Note that I am testing this principle on an affected production deployment by replacing the Client factory to return a Client which ignores an empty application name so this exact patch is not in use. If you'd like me to do more testing I'd be happy to do so, but I'm limited in what I can change on that particular server.

neeckeloo commented 10 years ago

I just make two fixes that might solve your problems. Now, I call newrelic_set_appname much earlier to be sure it is called first. And I fixed exceptions logging in NewRelicModule class.

bnkr commented 10 years ago

To be clear, when I was talking about errors being lost I meant PHP errors, such as those caused by trigger_error. I'm not sure if you detected some other problem here but I am successfully using code as you had originally written to report exceptions in other parts of my app, although I do not send exceptions to newrelic yet.

Thanks for such a quick merge.