search

microsoft / ApplicationInsights-PHP

Azure Application Insights SDK for PHP
https://azure.microsoft.com/services/application-insights/
Other
108 stars 92 forks source link

Async channel #26

Closed SharePointRadi closed 6 years ago

SharePointRadi commented 7 years ago

Does the client send requests asynchronously?

I don't know much about PHP, but looking at the Guzzle library implementation (http://docs.guzzlephp.org/en/latest/faq.html) and the Channel class (https://github.com/Microsoft/ApplicationInsights-PHP/blob/master/ApplicationInsights/Channel/Telemetry_Channel.php#L173) I believe this is sending requests synchronously. Is this the case? Wouldn't this slow down the PHP application we are monitoring?

Would you add async support?

JakubOleksy commented 7 years ago

Yes, it is all synchronous in the context of the current request. I am not a PHP expert so perhaps there are ways to make the call to the library async? I haven't heard of any performance issues from customer however.

SharePointRadi commented 7 years ago

The Guzzle library supports async calls. Would you consider making it async? It doesn't make sense to inject 10 to 30 traces when probing a complex application, which is a valid business case, and slow it down with 10 to 30 synchronous HTTP calls to the app insights API. You will get inconsistent page load times depending on network load and the PHP application might not even be in the Azure network.

Due to this reason, we can't use this library in production.

Danthar commented 7 years ago

+1 on this. We are currently evaluating app-insight for a php application. However performance is simply atrocious. Simply adding page duration tracking adds between 200 and 3000ms to our server response.

kwilliams1987 commented 7 years ago

Would it at least be possible to dump the AI telemetry to disk (similar to the ASP.NET persistence channel) to have a background task upload it?

I have a client with one site using AI on .NET who was really keen to have it in their PHP site, but the performance hit of uploading SQL dependency data was too big.

SharePointRadi commented 7 years ago

We ditched App Insights altogether due to this reason. We're a .NET shop and PHP was out-of-the-ordinary.

jondmcelroy commented 6 years ago

It would be simple to update this SDK to use the async built into guzzle, but the SDK would need to only allow Guzzle 6.*. Then change the Telemetry_Channel to call $client->postAsync(

PR: #40

JakubOleksy commented 6 years ago

@SergeyKanzhelev I know you were looking at this repo recently, FYI to this issue.