heroku / heroku-buildpack-php

Heroku's buildpack for PHP applications.
https://devcenter.heroku.com/categories/php
MIT License
805 stars 1.59k forks source link

Evolution of Blackfire support #419

Open lolautruche opened 4 years ago

lolautruche commented 4 years ago

Hello there πŸ‘‹ ,

Blackfire recently published an official buildpack, providing the agent and the CLI tool (doesn't appear on Heroku elements yet though). It is partially inspired by your amazing work in the official buildpack for PHP, and is needed for Python and Go, which we now support officially.

It would be nice to harmonize the way developers can install and use Blackfire on Heroku, which is why I'm creating this issue.

My suggestion is to:

WDYT? Of course, I'd be glad to contribute to thes changes πŸ˜‰ .

dzuelke commented 4 years ago

Cool.

Yeah, so, having the agent (I don't think the CLI tool is installed by the buildpack, but could be wrong) as a separate package is on my todo for the next few weeks.

The goal there is/was to allow independent updates of the two.

I can then change the buildpack to detect an agent from that buildpack and provide that package for our platform installs, then the buildpack won't pull in its own package.

Otherwise, it'll continue to be installed by the PHP buildpack, so that users do not have to also install a buildpack just to use the extension.

How does that sound?

P.S. there is obviously a bit of a concern around backwards compatibility - how will we know, in a far future, that an old agent version a user is somehow using (if that's possible?) works with a newer extension version, or vice versa?

lolautruche commented 4 years ago

Hello @dzuelke

Ah, so that's a great opportunity then πŸ˜„ . The PHP buildpack does install the CLI tool as well πŸ˜‰ .

I can then change the buildpack to detect an agent from that buildpack and provide that package for our platform installs, then the buildpack won't pull in its own package.

So if the Blackfire buildpack is present, you'd activate the installation of the probe, right? What do you mean by "provide that package for our platform install"?

Otherwise, it'll continue to be installed by the PHP buildpack, so that users do not have to also install a buildpack just to use the extension.

While I'm not a fan of having 2 different ways of installing the agent, I think it's probably the best for BC. Would it be possible to display a deprecation warning somehow?

P.S. there is obviously a bit of a concern around backwards compatibility - how will we know, in a far future, that an old agent version a user is somehow using (if that's possible?) works with a newer extension version, or vice versa?

A new version of the agent will be installed for every deploys, so that shouldn't be a problem. As for the probe version, there is a warning displayed in profiles when an old version is used. There might be some glitches in the far future if the probe and agent versions are not compatible any more, but that will most likely go through our support channel, and it is quite easy to spot IMHO.

dzuelke commented 4 years ago

I can then change the buildpack to detect an agent from that buildpack and provide that package for our platform installs, then the buildpack won't pull in its own package.

So if the Blackfire buildpack is present, you'd activate the installation of the probe, right? What do you mean by "provide that package for our platform install"?

No, I meant I'd check if the blackfire-agent is there (from your agent buildpack), and if so, I produce {"provide": {"heroku-sys/blackfire-agent":"1.2.3"}} in the "system" composer.json that we use for installation of PHP, extensions, and so forth (we take the app's composer.lock and generate something like {"require":{"heroku-sys/php":"~7.3.0"}} from it, and run that against a custom repository with a custom installer that then installs the PHP runtime using Composer).

I hadn't considered auto-installing the extension in case the buildpack is there... it currently already gets auto-installed if BLACKFIRE_LICENSE_KEY is set, so that wouldn't change, I guess.

While I'm not a fan of having 2 different ways of installing the agent, I think it's probably the best for BC. Would it be possible to display a deprecation warning somehow?

Yeah, no problem.

lolautruche commented 4 years ago

Alright then :-)

tristanbes commented 3 years ago

Hey πŸ‘‹

We've noticed Heroku runs an outdated version of Blackfire as reported by the latter:

We noticed that you are using an outdated agent version for the β€œProduction UK” environment. The v1 is deprecated and will sunset on Sept, 1st 2021. Consider upgrading to v2 soon. Read more at https://blackfire.io/docs/up-and-running/agent-upgrade

Is this issue involves this ?

dzuelke commented 3 years ago

https://github.com/heroku/heroku-buildpack-php/pull/488 addresses most of what we discussed.

It now only installs the agent if the user doesn't have the blackfire buildpack set on their app.