Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Blackfire agent and full support #2448

Merged
merged 13 commits into from Jan 14, 2021

Conversation

lolautruche
Copy link
Collaborator

The Problem/Issue/Bug:

Blackfire CLI makes it possible to profile CLI commands.
It needs to be configured with credentials with env vars as documented
on Blackfire website: https://blackfire.io/docs/configuration/cli

How this PR Solves The Problem:

It installs blackfire-agent package, which bundles blackfire CLI command.
It is much easier for the developer to have access to the blackfire command directly from the main container.
The agent is however disabled (prevented to run on startup), to keep using the dedicated image.

Manual Testing Instructions:

Automated Testing Overview:

Related Issue Link(s):

Release/Deployment notes:

@CLAassistant
Copy link

CLAassistant commented Aug 12, 2020

CLA assistant check
All committers have signed the CLA.

@rfay
Copy link
Member

rfay commented Aug 12, 2020

Traditionally people have done this with an additional service, as shown in https://github.com/drud/ddev-contrib/tree/master/docker-compose-services/blackfire - it seems to have worked OK for some time.

@lolautruche
Copy link
Collaborator Author

Yes, I noticed the additional service. However it be cumbersome to use the CLI within the agent container when one want to profile CLI commands, or use it for other purposes (e.g. bootstraping tests, triggering build webhooks...)

This is why I am proposing this change.

@rfay
Copy link
Member

rfay commented Aug 12, 2020

I guess I was assuming that most people would take the first option there and install the client on their host machine. I always use the Chrome extension when testing.

I do wish the CLI were available via separate download.

Also: If we wanted this to be added to the base image to be used by DDEV-Live (in the future), we'd want this PR in drud/ddev-images rather than here.

I added a commit to point to a newly-pushed docker image, and remove the init.d stuff, since the container doesn't run systemd or any init system (processes start with supervisord)

I note that this adds about 11MB to the compressed image size, which is certainly more than I wish it did. https://hub.docker.com/repository/docker/drud/ddev-webserver/tags

@lolautruche
Copy link
Collaborator Author

Thanks @rfay !
It is actually possible to get the CLI via separate download, but I was concerned about updates since it's a direct link.
If you prefer, I can refer to that in the Dockerfile instead.

Question: How are all these pieces of software been updated?

@lolautruche
Copy link
Collaborator Author

Also: If we wanted this to be added to the base image to be used by DDEV-Live (in the future), we'd want this PR in drud/ddev-images rather than here.

I can definitely move this PR to https://github.com/drud/ddev-images/ !
To be complete, I guess I should also move the installation of the PHP probe there.

@rfay
Copy link
Member

rfay commented Aug 13, 2020

I think we should consider moving this as is to drud/ddev-images, depending on the strategy being used in DDEV-Live, I'll check on that today. Maybe we'll just end up adding a command to start the agent on DDEV-Local and have the whole thing built in.

@lolautruche
Copy link
Collaborator Author

I think we should consider moving this as is to drud/ddev-images, depending on the strategy being used in DDEV-Live, I'll check on that today.

That's my guess too.
My only concern is to be able to get the probe installed for every versions of PHP, because using the APT repository only installs the probe for the current version (default one if I'm guessing right).

Maybe we'll just end up adding a command to start the agent on DDEV-Local and have the whole thing built in.

That will work for DDEV-Local, but for DDEV-Live you'll most likely want to have a separate container.

@rfay
Copy link
Member

rfay commented Aug 13, 2020

Interesting,

using the APT repository only installs the probe for the current version

We'll have to know more about that, and it could be a blocker for this approach. I'm not sure the Blackfire package would know how to determine the default version. And of course, it's not actually determined until start time. Could you take another look at how that works before we move forward please?

@lolautruche
Copy link
Collaborator Author

OK I just checked and we're actually fine 😃 . Our installer script can find its way to every version of PHP installed.

Regarding the agent, we could install it into the app container, but how DDEV-Live work with these containers? There should be only one agent per app.

@rfay
Copy link
Member

rfay commented Aug 13, 2020

So @lolautruche it seems that the goal in DDEV-Live is to put the agent in a separate container (as is done in the ddev-contrib recipe) and so it doesn't make any sense for us to fiddle with ddev-images at this point.

So that sounds to me like your objective will be met by getting just the blackfire CLI into ddev-webserver, and perhaps as just the CLI, not the full package.

Question: How are all these pieces of software been updated?

So a new version of ddev-webserver is tagged for each major release and sometimes for minor. But they don't get updated unless people update DDEV-Local.

@rfay
Copy link
Member

rfay commented Aug 13, 2020

@josefkarasek bringing you into this issue. @lolautruche @josefkarasek is working on the DDEV-Live Blackfire integration.

@rfay
Copy link
Member

rfay commented Aug 13, 2020

Oh, I forgot to mention:

The blackfire CLI is already in the blackfire container, of course, so one can use it with just ddev ssh -s blackfire when using the ddev-contrib recipe. Does that make this whole thread irrelevant?

@lolautruche
Copy link
Collaborator Author

So that sounds to me like your objective will be met by getting just the blackfire CLI into ddev-webserver, and perhaps as just the CLI, not the full package.

OK. I can update the Dockerfile to only install the CLI. However it won't be easy to update it (there is no selfupdate command).

So a new version of ddev-webserver is tagged for each major release and sometimes for minor. But they don't get updated unless people update DDEV-Local.

Is it possible for users to update their packages? Because we roll out updates of the probe, agent and CLI tool quite often.

The blackfire CLI is already in the blackfire container, of course, so one can use it with just ddev ssh -s blackfire when using the ddev-contrib recipe. Does that make this whole thread irrelevant?

Yes, I noticed the ddev-contrib recipe. However, while the CLI tool is usable from there to do blackfire curl, it is not when one want to profile console commands (blackfire run), as the container has no knowledge of the source code and does not have PHP installed.

@rfay
Copy link
Member

rfay commented Aug 13, 2020

It's unusual for users to update packages; DDEV-Local versions come out every 2-3 months, so that's usually the path for most people to get new packages.

They could update by putting webimage_extra_packages: [blackfire-agent] with your existing setup (it would force latest).

Also, we support custom Dockerfile add-ons for things like this, and post-start hooks. But my bet is that people wouldn't be updating that often. If they do need an update, there are easy techniques.

I note that the blackfire CLI is about 13MB uncompressed, as is the agent. So perhaps we're winning half the disk space back with this approach.

@rfay
Copy link
Member

rfay commented Aug 13, 2020

One note though, as mentioned above, any user can already add webimage_extra_packages: [blackfire-agent] to the project .ddev/config.yaml and get the CLI. And nobody has ever asked for it.

@lolautruche
Copy link
Collaborator Author

Right, I wanted to improve the developer experience as the blackfire CLI tool can be handy. I can still fix this with documentation, I guess.

@rfay
Copy link
Member

rfay commented Aug 13, 2020

It may be fine to just add a little doc to the ddev-contrib recipe. OTOH, we certainly want people to enjoy the ddev/blackfire experience as much as possible, and they have in fact enjoyed it (blackfire is awesome)

@lolautruche
Copy link
Collaborator Author

There are already a few lines about getting the CLI client by adding blackfire-agent to webimage_extra_packages.
I'm currently writing the documentation on the Blackfire side as DDEV being an official integration. Having the opportunity to use Blackfire with DDEV-Live will be even more awesome 😉

@rfay
Copy link
Member

rfay commented Aug 19, 2020

I sure appreciate your initiative on this @lolautruche - I'm thinking the best path forward is just to put the package into ddev-images, but haven't completely decided. Also, if we decide to do that, we could possibly add a command to ddev to enable the agent when needed, and simplify everything for blackfire users.

@lolautruche
Copy link
Collaborator Author

Also, if we decide to do that, we could possibly add a command to ddev to enable the agent when needed, and simplify everything for blackfire users.

That would really rock!

Let me know about your decision, I can surely adapt this PR in order to only install the CLI.
However, as we often roll out updates on the probe, the agent and the CLI, it would be nice for a user to easily force an update. Does it sound feasible?

@lolautruche
Copy link
Collaborator Author

Oh, and here is the official documentation on Blackfire website: https://blackfire.io/docs/integrations/paas/ddev

@rfay rfay added this to the v1.17 milestone Oct 29, 2020
@rfay
Copy link
Member

rfay commented Jan 7, 2021

@lolautruche I think we should try to get this into v1.17, with the agent and the probe in the web image.

So what we'd do is

  1. in ddev-images, add the proper package, but leave the probe disabled.
  2. In ddev add a new script command ddev blackfire just like ddev xdebug that would enable the blackfire probe.
  3. I think we'd want blackfire probe to look in /mnt/ddev_config/blackfire for config (that would be mounted from .ddev/config/blackfire

What do you think?

@lolautruche
Copy link
Collaborator Author

I think it's great @rfay!
However, the agent really should run in a separate container as having a daemon running in the background of the main image can lead to unexpected errors and developers may not understand why it is not working. However istalling the agent package is fine to get the CLI command.

@rfay
Copy link
Member

rfay commented Jan 11, 2021

I rebased this and made a couple of changes:

  • Installation doesn't require --allow-unauthenticated any more. Congratulations!
  • ARM64 packages are now supported. Congratulations!

What I think I'll do is wait until Environment variables in config.yaml #2742 goes in, which would make it super easy to add the environment variables. Then we may just be able to use the agent at will, we'll see.

One question @lolautruche: Where are you on PHP8.0 support? I do see it whining during the build about PHP8.0 config files.

@lolautruche
Copy link
Collaborator Author

I rebased this and made a couple of changes:

  • Installation doesn't require --allow-unauthenticated any more. Congratulations!
  • ARM64 packages are now supported. Congratulations!

Awesome!

What I think I'll do is wait until Environment variables in config.yaml #2742 goes in, which would make it super easy to add the environment variables. Then we may just be able to use the agent at will, we'll see.

👍

One question @lolautruche: Where are you on PHP8.0 support? I do see it whining during the build about PHP8.0 config files.

PHP 8.0 is officially supported, with a dependency on curl extension (for now). What is the problem with the build exactly? I couldn't find out.

@rfay
Copy link
Member

rfay commented Jan 12, 2021

@leymannx the PHP8.0 thing I see is in the container build:

Setting up blackfire-php (1.48.1) ...
#21 8.681 WARNING: Module blackfire ini file doesn't exist under /etc/php/8.0/mods-available
#21 8.681 WARNING: Module blackfire ini file doesn't exist under /etc/php/8.0/mods-available

This is in https://github.com/drud/ddev/pull/2448/checks?check_run_id=1681814057

And of course with php8.0,

 phpenmod blackfire
WARNING: Module blackfire ini file doesn't exist under /etc/php/8.0/mods-available
WARNING: Module blackfire ini file doesn't exist under /etc/php/8.0/mods-available

It looks like there may be an improvement needing to be made in the blackfire-php debian package to make sure the file gets created? (It does fine in php7.4 and below)

@lolautruche
Copy link
Collaborator Author

@rfay We indeed have an issue with debian packages. A fix has been implemented and should be rolled out shortly

@rfay
Copy link
Member

rfay commented Jan 13, 2021

I should mention that since ddev doesn't use systemd or init inside the containers, you don't run the agent by running /etc/init.d/blackfire or whatever, just run blackfire-agent, and it doesn't need extra privileges, although sudo is available.

@lolautruche
Copy link
Collaborator Author

@rfay The logs are the agent's. It would be useful to get the ones from the agent and the probe. Furthermore, the logs mentions an outdated version of blackfire-agent: v1.45.1

@lolautruche
Copy link
Collaborator Author

I'll check it out using the Linux artifact. I just need to run ddev ssh from my PHP application root, right?

@rfay
Copy link
Member

rfay commented Jan 13, 2021

blackfire-agent 1.45.1 is the one currently being served up from your repository:

(apt-get update done)
$ sudo apt-get install blackfire-agent
Reading package lists... Done
Building dependency tree
Reading state information... Done
blackfire-agent is already the newest version (1.45.1).
0 upgraded, 0 newly installed, 0 to remove and 162 not upgraded.

I'll be out for the rest of your workday, but thanks so much for taking a look. Yes, just run ddev ssh from your project root.

@lolautruche
Copy link
Collaborator Author

Sorry I mixed up with the probe version

@lolautruche
Copy link
Collaborator Author

So, I did some tests and there are few issues:

  • Using ddev blackfire on only enables for PHP CLI, not for FPM. I had to run phpenmod blackfire to enable it, but restarting the container did reset to the previous state. This is most likely why you had this "Are you authorized" issue.
  • I tried to profile using blackfire run and I had the following error:

$ blackfire run public/test.php
Error while running command: fork/exec public/test.php: permission denied
Use the option '--ignore-exit-status' to ignore command exit status

In any case, as I already stated before, it is generally not a good idea to have blackfire-agent running in the background in a container. We really should use the Blackfire image. The agent package can be used to install the CLI command, even though this is going to change with the CLI v2 (but it's still in alpha version so we've got time).

@rfay
Copy link
Member

rfay commented Jan 13, 2021

Nice work - you pointed out the problem. Although the script was doing the phpenmod, I forgot to kill -HUP the php-fpm. I must have been remembering to manually do that earlier and then it fell out of my mind when I started scripting the process.

Works perfectly now.

I know you're reluctant to have this in the same container, but since it's an on/off that people can easily use, I'm thinking it will make blackfire super accessible to people, with almost no config required. It's not "running in the background" as much as "being turned on when needed".

@rfay
Copy link
Member

rfay commented Jan 13, 2021

blackfire run php junk.php seems to work fine with a trivial junk.php (inside container)

blackfire run drush st also seems to work fine, as does ddev exec blackfire run drush st and ddev exec blackfire curl https://d9.ddev.site

@rfay
Copy link
Member

rfay commented Jan 13, 2021

### Basic Blackfire Usage

1. Create an account on [blackfire.io](https://blackfire.io)
2. Get the Server ID and the Server Token from your Account->Credentials on blackfire.io
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server Credentials may come from an environment the user is being part of.
Client credentials should be retrieved as well (Account/Credentials)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don' t really understand "may come from an environment the user is being part of" - but we can link to docs if you can point me to it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A Blackfire user may be part of an organization, and the latter may have one or several collaborative environments.
The easiest way is to use the personal credentials (the user inherit most of the org permissions), but doing so prevents the profiles to be shared within the team working in an environment.

See the documentation here: https://blackfire.io/docs/up-and-running/configuration/agent#configuring-the-agent-via-environment-variables. A user may have a dropdown menu like this:
image


1. Create an account on [blackfire.io](https://blackfire.io)
2. Get the Server ID and the Server Token from your Account->Credentials on blackfire.io
3. Configure ddev with the credentials, `ddev config global --web-environment="BLACKFIRE_SERVER_ID=<id>,BLACKFIRE_SERVER_TOKEN=<token>"`. It's easiest to do this globally, but you can do the same thing with project-level `ddev config`.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that this command doesn't append but replaces the env vars to the global web_environment setting.
Is there a way to append variables instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The environment variable feature was just added in the previous PR (prompted by the need here, you used to have to do a docker-compose override), so I'm sure it will have some maturing to do. Most people just edit the appropriate configuration, the project .ddev/config.yaml or the global ~/.ddev/global_config.yaml - so that's the easiest way to just "edit" a set of env vars.

docs/users/blackfire-profiling.md Outdated Show resolved Hide resolved
docs/users/blackfire-profiling.md Outdated Show resolved Hide resolved
@lolautruche
Copy link
Collaborator Author

I know you're reluctant to have this in the same container, but since it's an on/off that people can easily use, I'm thinking it will make blackfire super accessible to people, with almost no config required.

You're right, I find it very accessible myself and I'm happy with it 😃 .
I tested with PHP 7.4 and PHP 8.0 and it's working fine, good job 👍

Co-authored-by: Jérôme Vieilledent <lolautruche@gmail.com>
Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review and careful thought. I've made a number of updates to the script and to the docs.

### Basic Blackfire Usage

1. Create an account on [blackfire.io](https://blackfire.io)
2. Get the Server ID and the Server Token from your Account->Credentials on blackfire.io
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don' t really understand "may come from an environment the user is being part of" - but we can link to docs if you can point me to it.


1. Create an account on [blackfire.io](https://blackfire.io)
2. Get the Server ID and the Server Token from your Account->Credentials on blackfire.io
3. Configure ddev with the credentials, `ddev config global --web-environment="BLACKFIRE_SERVER_ID=<id>,BLACKFIRE_SERVER_TOKEN=<token>"`. It's easiest to do this globally, but you can do the same thing with project-level `ddev config`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The environment variable feature was just added in the previous PR (prompted by the need here, you used to have to do a docker-compose override), so I'm sure it will have some maturing to do. Most people just edit the appropriate configuration, the project .ddev/config.yaml or the global ~/.ddev/global_config.yaml - so that's the easiest way to just "edit" a set of env vars.

@rfay
Copy link
Member

rfay commented Jan 14, 2021

Thanks for all the work on this! Let's get it into a prerelease and see if we can get some feedback from people.

@rfay rfay merged commit 10b71d9 into ddev:master Jan 14, 2021
@lolautruche lolautruche deleted the improveBlackfireSupport branch January 15, 2021 08:22
@lolautruche
Copy link
Collaborator Author

Awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants