8p / EightPointsGuzzleBundle

⛽️ Integrates Guzzle 6.x, a PHP HTTP Client, into Symfony
MIT License
440 stars 71 forks source link

Improve Profiler Layout #74

Closed florianpreusner closed 6 years ago

florianpreusner commented 7 years ago

Some textareas are pretty small by default. This should be improved. Also some new screenshots should be created and inserted into README.md.

Neirda24 commented 6 years ago

Also it would be nice to have some time results in the profiler. Maybe using http://guzzle.readthedocs.io/en/latest/request-options.html#on-stats ?

gregurco commented 6 years ago

@Neirda24 yep, it's really good idea. But I'm not sure if we can use this option. This option is allowed only for requests, not for clients and probably we need to collect execution time in another way. I'm trying to find the solution :slightly_smiling_face:

Neirda24 commented 6 years ago

@gregurco : agreed. Another way would be to add a middleware at the beginning and another at the end. each of the same instance recording the start time and the end time per requests.

gregurco commented 6 years ago

@Neirda24 in this case you will not know the exact time. It will be very approximate. Here you can see how guzzle calculates time for on_stats callback:

For different handlers there are different methods. Yep, probably it should be middleware, but somehow in another way.

Neirda24 commented 6 years ago

@gregurco : I see... Why can't we use the on_stats method ? We can add it as default option on the client. (I didn't made any research on this subject as of now...)

gregurco commented 6 years ago

@Neirda24 the main problem is that default client option can be easy rewritten by request option and you will not be able to calculate time correctly :slightly_frowning_face:

Neirda24 commented 6 years ago

@gregurco : Fair enough. keep me posted if you have any ideas. I'll be glad to help.

andheiberg commented 6 years ago

@Neirda24 the main problem is that default client option can be easy rewritten by request option and you will not be able to calculate time correctly 🙁

Could you elaborate on this?

I'm not sure I understand why this wouldn't be trivial to implement?

gregurco commented 6 years ago

@andheiberg calculation of all requests time was done in PR #156 and released in v7.3.0

andheiberg commented 6 years ago

@gregurco thanks for your response.

See https://github.com/ludofleury/GuzzleBundle for the split between connection and response timing.

See https://github.com/e-moe/guzzle6-bundle specifically for how they have accurate time lines in the symfony profiler.

I don't believe either is currently functional in this package.

gregurco commented 6 years ago

@andheiberg no, for now we just calculate total time for each request and time for all requests. In future releases probably we will add more information related to request time.

florianpreusner commented 6 years ago

Some improvements are already implemented. Visualization of timings of each request were implemented in https://github.com/8p/EightPointsGuzzleBundle/pull/208 👏

To keep this topic (improvements for profiler) a little bit cleaner we created a new issue (https://github.com/8p/EightPointsGuzzleBundle/issues/210) and will close this one.