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

Sentry uses a lot of walltime #878

Closed
OskarStark opened this issue Sep 10, 2019 · 11 comments
Closed

Sentry uses a lot of walltime #878

OskarStark opened this issue Sep 10, 2019 · 11 comments

Comments

@OskarStark
Copy link

Screenshot 2019-09-10 15 18 22

Maybe this wait() can be fixed by using the new Symfony HttpClient?

@Jean85
Copy link
Collaborator

Jean85 commented Sep 10, 2019

wait is just waiting for the requests to go to the Sentry server, that's not something that you can optimize at the code level. The Sentry client 2.0 uses HTTPlug, so you can switch your HTTP transport as you wish, I'm not sure if the SF client already has an adapter for it, but it can for sure, since it has a PSR-18 adapter.

Also, IIRC with the current behavior, this wait is called in the shutdown function, so even after that your response has been rendered to the client.

@OskarStark
Copy link
Author

Hmm you could be right. Or did I miss something @nicolas-grekas?

@nicolas-grekas
Copy link
Contributor

php-http/curl-client#55

@Jean85
Copy link
Collaborator

Jean85 commented Sep 10, 2019

Thanks @nicolas-grekas for pointing that out.

Closing since this seems to be a downstream issue with the Curl transport. As a workaround, you can switch out to any different transport supported by HTTPlug.

@Jean85 Jean85 closed this as completed Sep 10, 2019
@nicolas-grekas
Copy link
Contributor

I would recommend using the Symfony HttpClient by default. Version 4.4 will ship an HttplugClient btw.

@mfb
Copy link
Contributor

mfb commented Sep 10, 2019

@B-Galati
Copy link
Contributor

I ended up doing this:

class SentryHttpClient implements HttpAsyncClient
{
    private $client;

    public function __construct()
    {
        $factory = new Psr17Factory();

        // DO NOT use the http client from Symfony or you could turn into an infinite loop depending on your monolog config and the if logging of HTTP request is enabled or not
        $client = HttpClient::create(['timeout' => 2]);

        $this->client = new Psr18Client($client, $factory, $factory);
    }

    /**
     * {@inheritdoc}
     */
    public function sendAsyncRequest(RequestInterface $request)
    {
        try {
            $response = $this->client->sendRequest($request);
        } catch (RequestExceptionInterface $e) {
            return new HttpRejectedPromise(new RequestException($e->getMessage(), $request, $e));
        } catch (NetworkExceptionInterface $e) {
            return new HttpRejectedPromise(new NetworkException($e->getMessage(), $request, $e));
        }

        return new HttpFulfilledPromise($response);
    }
}

Works like a charm, thank you @nicolas-grekas!

I'll update my symfony guide ASAP :-)

@OskarStark
Copy link
Author

Thank you @B-Galati, let me please know when its done 😃

@B-Galati
Copy link
Contributor

@OskarStark Yes it's about to be merged in B-Galati/monolog-sentry-handler#8

Let me know for any feedback please :-)

@garak
Copy link

garak commented Sep 26, 2019

@B-Galati what about proposing your code to Httplug lib?
See the HttpClient issue mentioned above

@B-Galati
Copy link
Contributor

@garak the proposed implementation is a dumb one that aims at solving a specific problem. I am not even sure it respects HttpPlug specs.

In any case it is something that should be done in Symfony: see symfony/symfony#33710.

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

No branches or pull requests

6 participants