blackfireio / php-sdk

The Blackfire PHP SDK
https://blackfire.io
MIT License
150 stars 22 forks source link

Inconsistency between guzzle middleware docs and code #28

Closed alcaeus closed 7 years ago

alcaeus commented 7 years ago

From the Profiling Microservices documentation:

Now, everytime you profile your application without aggregation, sub-requests handled by Guzzle will be profiled.

However, looking at the code, it requires explicitly setting a blackfire option when making the request (or via a second middleware that checks the HTTP_X_BLACKFIRE_QUERY server variable): https://github.com/blackfireio/php-sdk/blob/v1.9.0/src/Blackfire/Bridge/Guzzle/Middleware.php#L51

The question is: what requires fixing? The code or the docs?

romainneutron commented 7 years ago

Hello,

You're mixing two things: profiling using Guzzle (using the blackfire option) and profiling HTTP sub requests done with Guzzle automatically.

There's no issue in the documentation. However, reading the code just makes me realize there's an issue with the "Profiling Microservices" use case, it can not work since https://github.com/blackfireio/php-sdk/commit/7374a02be1d8d0368849767495045169c3099a39 that overwrote some part of the feature. I'm gonna fix this.

alcaeus commented 7 years ago

@romainneutron thanks for looking into it. Wordings aside, that commit looks like the cause of the issue. Thanks for fixing it!

romainneutron commented 7 years ago

It's been fixed in 1e9bf90676b773812f80b5a68bbdf02057adb116 and released as of https://github.com/blackfireio/php-sdk/releases/tag/v1.9.1

alcaeus commented 7 years ago

Awesome, thank you very much!

alcaeus commented 7 years ago

@romainneutron Could you check the packagist integration? The 1.9.1 release is still not available there: https://packagist.org/packages/blackfire/php-sdk. Thanks!

romainneutron commented 7 years ago

It's been fixed @alcaeus . Thanks for reporting