search

philkra / elastic-apm-php-agent

PHP Agent for Elastic APM
MIT License
285 stars 95 forks source link

Not flush events on agent destruct #134

Closed tsantos84 closed 2 years ago

tsantos84 commented 4 years ago

Hi, first of all, this agent is great and helped me a lot integrating my apps to Elastic APM Server. Many thanks for all contributors.

I'm creating an internal Symfony Bundle to create integrate this library to APM server without duplicating code and etc. The question is that when we run cache:clear, for example, the Agent::__destruct is called causing the commit of events unnecessary. This behaviour can be useful in some way but I think it'd be optional. What if we introduce an auto-(flush|commit) entry in config options? WDYT?

science695 commented 4 years ago

@tsantos84 Are you trying to work on something similar to this? https://packagist.org/packages/goksagun/elastic-apm-bundle

When I was trying to set it up, I found it a bit inflexible for my needs, but it could be useful for you.

tsantos84 commented 4 years ago

Yes, it's a bit inflexible and I decided to write a new bundle for my company purposes (fit php versions, new spans and etc). After this experience, I'm planning to write a public bundle or contribute with goksagun/elastic-apm-bundle.

Anyway, the question here is about flushing the agent automatically. I think it'd be an optional process given the problems I've mentioned on the initial thread.

science695 commented 4 years ago

Perhaps what you need is something where if it is running in the symfony console, set the active config to false? That could be done in your symfony config, and not require a change in the agent code.

tsantos84 commented 4 years ago

I've thought about this approach, but didn't saw an easy way to do that. And thinking better, unit testing could cause the same side effect depending how they were written. In a bundle, it's better to let it flush the events instead of on __destruct. There is another side effect on it. If some error happens (e.g request failure), we can't see the error on __destruct.

science695 commented 4 years ago

You may be able to do it through native php functions, so if you had a php config file, it could override the active config for the agent on runtime.

https://electrictoolbox.com/determine-php-run-via-http-or-cli/ https://www.php.net/manual/en/function.php-sapi-name.php

I will say, there is a lot of power in using php configuration files, but they are only documented in some circumstances.

phoenixgao commented 4 years ago

If the apm server is not running or there is some network issue, then destruct function would throw an uncaught exception, would it be better to add a try catch here?