[REQ][PHP] depend on meta packages providing the HTTP implementation instead of directly on Guzzle

[dkarlovi]

Is your feature request related to a problem? Please describe.

Currently, the packages generated by php depend on Guzzle, like so

{
    "require": {
        "php": ">=7.2",
        "ext-curl": "*",
        "ext-json": "*",
        "ext-mbstring": "*",
        "guzzlehttp/guzzle": "^6.2"
    }
}

This is functional, but not ideal, making it harder to swap out HTTP clients, like for example Symfony HTTP Client if you're using Symfony.

Being able to choose your desired HTTP client is very desirable in modern PHP apps since you might have existing clients already, setup to log requests via telemetry, use caching, etc.

Describe the solution you'd like

Instead of relying on Guzzle directly, depend on a psr/http-client-implementation (AKA PSR 18). This is a metapackage which implementations can declare they provide, all the major ones already do (including Guzzle, Symfony, etc).

Describe alternatives you've considered

Alternatively, the client implementation to select might be php-http/async-client-implementation (since the client is expected to be async). The offered implementations are fewer, but the major ones are still there. Result is the same: developer being able to inject their desired client.

This could also be done both ways: depend on the regular client, check if it's one of the async ones at runtime and allow async ops if so.

[dkarlovi]

This became more important since the used features got deprecated, for example

  344    Call to deprecated function GuzzleHttp\Psr7\build_query():            
         build_query will be removed in guzzlehttp/psr7:2.0. Use Query::build  
         instead.

[grongor]

Hey @dkarlovi , just wanted to ask you if you started working on this or if this is just an issue for reference. (or maybe @ybelenko ? Saw your name in other php-related stuff). We at https://www.cdn77.com are now in need of this, so ... if you or somebody else isn't working on this, I'd look into it.

[ybelenko]

@grongor I'm definitely unaware of new PSR-18 and have no unfinished PRs about this issue 🤣

I prefer not to touch current PHP client generator, because it was developed long time ago before I joined community. The only exception is when I see obvious bugs like #8481

[dkarlovi]

@grongor I was thinking about doing it, but I opened an issue instead at the time. :) If you have the resources to do it, feel free to do so. 👍

[rauanmayemir]

It doesn't look like simply depending on psr/http-client-implementation will solve it.
Seeing how all the generated api methods are manually creating guzzle requests (no container or factory), you will also need to bring psr/http-factory to create the request objects.

I think psr/http-client-implementation might be too low-level to build on and PHP-HTTP would be a better alternative.

Depending on psr/http-factory and explicitly passing RequestFactoryInterface to the apiInstance is a good solution.

I would consider explicitly depending on any of the psr7 implementation packages less abstract, but also a good hassle-free solution. (for serializing query, payload, etc)

[rauanmayemir]

I'd like to take a stab at this.

How should this effort be organized? Will it work if I add a new generator php-psr and since it's going to be a non-breaking feature, target 5.2.x branch?

[dkarlovi]

I'd like to take a stab at this.

@rauanmayemir go for it. Since it will be BC, you can target the regular PHP generator.

[dkarlovi]

Here's an example of doing it for another lib:

           $body = $this->streamFactory->createStream(json_encode($convertedSpans));
            $request = $this->requestFactory
                ->createRequest('POST', $this->endpointUrl)
                ->withBody($body)
                ->withHeader('content-type', 'application/json');
            
            $response = $this->client->sendRequest($request);

See the full example here: https://gist.github.com/dkarlovi/ec38d1c4cc47b85609c6e2bffa2451e5

See the rationale here: open-telemetry/opentelemetry-php#223 (comment)

[rauanmayemir]

@dkarlovi It's great that we're on the same page. I also saw depending on http-factory-implementation as both correct way and lesser evil.
As for the use of guzzle's helper functions, I think it can be discarded in favor of PHP built-in functions. (guzzle mostly just wraps them anyway)

P.S: Planning to dig into it this weekend.

[dkarlovi]

@rauanmayemir were you able to makeany progress here?

[rauanmayemir]

Sorry, dropped the ball here. Feel free to start working on it yourself if it's urgent.

[dkarlovi]

@rauanmayemir didn't mean to imply you had to do it, I just wanted to verify if you made any progress so the work is not wasted. :) Thanks, I'll very likely tackle it soon-ish.

[olexiyk]

Hi @dkarlovi I made similar work updating one API so it does not depend on any particular http client following the ideas mentioned in the discussion.

I created a few custom templates and generated an API for myself. The API I was working on is pretty basic with a limited number of use cases. Maybe a broke a lot of things unknowingly, I will follow CONTRIBUTING guide and create a PR shortly, bringing custom templates to php generator