feat(blackfire): Add bridge for upscale/swoole-blackfire

[luca-nardelli]

Hi again!

I noticed that I was unable to profile requests with blackfire (before swoole I was using the standard nginx + php-fpm setup), as blackfire curl only told me that the probe was not initialized. After a while I stumbled upon this library https://github.com/upscalesoftware/swoole-blackfire and I tried to connect it to this bundle.

I've managed to make it work (I had to add the WithProfiler configurator after WithRequestHandler otherwise the instrumentation call would fail), however I didn't write any tests at the moment so I'd say the PR cannot be merged yet. I'm opening this PR to discuss this feature (with the proposed implementation)

Question 1: Is this a relevant addition to the bundle?
Question 2: Could you tell me which test I should look at if I wanted to write one for this new feature?

Thanks!

[k911]

This feature seems to be useful, so yeah feel free to contribute :)
Edit: A few lines of documentation how to use it could also be handy

[codecov]

Codecov Report

Merging #221 into develop will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@              Coverage Diff              @@
##             develop     #221      +/-   ##
=============================================
+ Coverage      87.09%   87.23%   +0.13%     
- Complexity       612      617       +5     
=============================================
  Files             84       85       +1     
  Lines           1883     1903      +20     
=============================================
+ Hits            1640     1660      +20     
  Misses           243      243              

Impacted Files	Coverage Δ	Complexity Δ
...mfony/Bundle/DependencyInjection/Configuration.php	97.90% <100.00%> (+0.03%)	7.00 <0.00> (ø)
...ony/Bundle/DependencyInjection/SwooleExtension.php	88.88% <100.00%> (+0.96%)	48.00 <0.00> (+3.00)
src/Bridge/Upscale/Blackfire/WithProfiler.php	100.00% <100.00%> (ø)	2.00 <2.00> (?)

[luca-nardelli]

I've added the following:

• A documentation entry in the docs folder
• A Unit test with prophpecy that ensures instrument() is called
• A Feature test that ensures that the WithProfiler configurator is added after WithRequestHandler in the container. To do that, I had to make two services public for testing purposes though.
• An entry in the suggests section of composer.json

Let me know if something else should be changed!

[k911]

@luca-nardelli Thanks, really good job! I've left some comments, we'll have to address them and I think that's it.

[luca-nardelli]

Fixed the usage of the service container (thanks for that!) and removed the unused property!

[luca-nardelli]

Ok I should have applied all your suggestions

[codeclimate]

Code Climate has analyzed commit c102382 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 87.2% (0.2% change).

View more on Code Climate.

[k911]

@luca-nardelli Thanks again for your hard work, I'll leave this on develop branch for a bit, then release.

[luca-nardelli]

Great! Thank you for the bundle :) Il sab 23 mag 2020, 14:45 Konrad Obal <notifications@github.com> ha scritto:

…

@luca-nardelli <https://github.com/luca-nardelli> Thanks again for your hard work, I'll leave this on develop branch for a bit, then release. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#221 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABPNL6UPUHQJPWBMOF3TT5TRS7AOFANCNFSM4NGK4FCA> .