-
Notifications
You must be signed in to change notification settings - Fork 50
Added web profiler and toolbar #19
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
<?php | ||
|
||
namespace Http\HttplugBundle\Collector; | ||
|
||
use Http\Client\Exception; | ||
use Http\Client\Plugin\Journal; | ||
use Http\Message\Formatter; | ||
use Http\Message\Formatter\SimpleFormatter; | ||
use Psr\Http\Message\RequestInterface; | ||
use Psr\Http\Message\ResponseInterface; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
use Symfony\Component\HttpKernel\DataCollector\DataCollector; | ||
|
||
/** | ||
* @author Tobias Nyholm <[email protected]> | ||
*/ | ||
class MessageJournal extends DataCollector implements Journal | ||
{ | ||
/** | ||
* @var Formatter | ||
*/ | ||
private $formatter; | ||
|
||
/** | ||
* @param Formatter $formatter | ||
*/ | ||
public function __construct(Formatter $formatter = null) | ||
{ | ||
$this->formatter = $formatter ?: new SimpleFormatter(); | ||
$this->data = ['success' => [], 'failure' => []]; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function addSuccess(RequestInterface $request, ResponseInterface $response) | ||
{ | ||
$this->data['success'][] = [ | ||
'request' => $this->formatter->formatRequest($request), | ||
'response' => $this->formatter->formatResponse($response), | ||
]; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function addFailure(RequestInterface $request, Exception $exception) | ||
{ | ||
if ($exception instanceof Exception\HttpException) { | ||
$formattedResponse = $this->formatter->formatResponse($exception->getResponse()); | ||
} elseif ($exception instanceof Exception\TransferException) { | ||
$formattedResponse = $exception->getMessage(); | ||
} else { | ||
$formattedResponse = sprintf('Unexpected exception of type "%s"', get_class($exception)); | ||
} | ||
|
||
$this->data['failure'][] = [ | ||
'request' => $this->formatter->formatRequest($request), | ||
'response' => $formattedResponse, | ||
]; | ||
} | ||
|
||
/** | ||
* Get the successful request-resonse pairs. | ||
* | ||
* @return array | ||
*/ | ||
public function getSucessfulRequests() | ||
{ | ||
return $this->data['success']; | ||
} | ||
|
||
/** | ||
* Get the failed request-resonse pairs. | ||
* | ||
* @return array | ||
*/ | ||
public function getFailedRequests() | ||
{ | ||
return $this->data['failure']; | ||
} | ||
|
||
/** | ||
* Get the total number of request made. | ||
* | ||
* @return int | ||
*/ | ||
public function getTotalRequests() | ||
{ | ||
return count($this->data['success']) + count($this->data['failure']); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function collect(Request $request, Response $response, \Exception $exception = null) | ||
{ | ||
// We do not need to collect any data form the Symfony Request and Response | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function getName() | ||
{ | ||
return 'httplug'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,8 +34,7 @@ public function getConfigTreeBuilder() | |
return !empty($v['classes']['client']) | ||
|| !empty($v['classes']['message_factory']) | ||
|| !empty($v['classes']['uri_factory']) | ||
|| !empty($v['classes']['stream_factory']) | ||
; | ||
|| !empty($v['classes']['stream_factory']); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this styleci? my idea was that adding another check only does a diff on one line, like trailing comma in arrays. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this is styleci |
||
}) | ||
->then(function ($v) { | ||
foreach ($v['classes'] as $key => $class) { | ||
|
@@ -72,8 +71,19 @@ public function getConfigTreeBuilder() | |
->scalarNode('stream_factory')->defaultNull()->end() | ||
->end() | ||
->end() | ||
->end() | ||
; | ||
->arrayNode('toolbar') | ||
->addDefaultsIfNotSet() | ||
->info('Extend the debug profiler with inforation about requests.') | ||
->children() | ||
->enumNode('enabled') | ||
->info('If "auto" (default), the toolbar is activated when kernel.debug is true. You can force the toolbar on and off by changing this option.') | ||
->values([true, false, 'auto']) | ||
->defaultValue('auto') | ||
->end() | ||
->scalarNode('formatter')->defaultNull()->end() | ||
->end() | ||
->end() | ||
->end(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same logic here, if we add another section, there will be an unnecessary diff on this line. |
||
|
||
return $treeBuilder; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,8 @@ | |
namespace Http\HttplugBundle\DependencyInjection; | ||
|
||
use Http\HttplugBundle\ClientFactory\DummyClient; | ||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
use Symfony\Component\Config\FileLocator; | ||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; | ||
use Symfony\Component\DependencyInjection\Reference; | ||
use Symfony\Component\HttpKernel\DependencyInjection\Extension; | ||
|
@@ -27,6 +27,18 @@ public function load(array $configs, ContainerBuilder $container) | |
$loader->load('services.xml'); | ||
$loader->load('plugins.xml'); | ||
$loader->load('discovery.xml'); | ||
|
||
$enabled = is_bool($config['toolbar']['enabled']) ? $config['toolbar']['enabled'] : $container->hasParameter('kernel.debug') && $container->getParameter('kernel.debug'); | ||
if ($enabled) { | ||
$loader->load('data-collector.xml'); | ||
$config['_inject_collector_plugin'] = true; | ||
|
||
if (!empty($config['toolbar']['formatter'])) { | ||
$container->getDefinition('httplug.collector.message_journal') | ||
->replaceArgument(0, new Reference($config['toolbar']['formatter'])); | ||
} | ||
} | ||
|
||
foreach ($config['classes'] as $service => $class) { | ||
if (!empty($class)) { | ||
$container->removeDefinition(sprintf('httplug.%s.default', $service)); | ||
|
@@ -54,6 +66,10 @@ protected function configureClients(ContainerBuilder $container, array $config) | |
$first = $name; | ||
} | ||
|
||
if (isset($config['_inject_collector_plugin'])) { | ||
array_unshift($arguments['plugins'], 'httplug.collector.history_plugin'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually, adds the plugin first. does that mean we log before anything else? that will mean that if we have the redirect plugin active, we will not see that a redirect happened. then again, if redirect was activated, we might not care about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you have the CachePlugin and put the HistoryPlugin last you will not see anything... But that might be okey, since no request was actually made. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. true. hm. it would be cool to see whether requests where served from
cache or the actual backend. we could try to have two history plugins
and see whether a request goes to the client or not, and also see
whether there is a redirect to serve one answer.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. created #26 for this idea. i guess what we have now is ok, but the other thing sure would be cool. |
||
} | ||
|
||
$def = $container->register('httplug.client.'.$name, DummyClient::class); | ||
|
||
if (empty($arguments['plugins'])) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,10 +51,11 @@ For information how to write applications with the services provided by this bun | |
| httplug.stream_factory | Service* that provides the `Http\Message\StreamFactory` | ||
| httplug.client.[name] | This is your Httpclient that you have configured. With the configuration below the name would be `acme_client`. | ||
| httplug.client | This is the first client configured or a client named `default`. | ||
| httplug.plugin.content_length <br> httplug.plugin.decoder<br> httplug.plugin.error<br> httplug.plugin.logger<br> httplug.plugin.redirect<br> httplug.plugin.retry | These are build in plugins that lives in the `php-http/plugins` package. These servcies are not public and may only be used when configure HttpClients or services. | ||
| httplug.plugin.content_length <br> httplug.plugin.decoder<br> httplug.plugin.error<br> httplug.plugin.logger<br> httplug.plugin.redirect<br> httplug.plugin.retry | These are built in plugins that live in the `php-http/plugins` package. These servcies are not public and may only be used when configure HttpClients or services. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets make an explicit note on the history plugin. that plugin needs to be set up with a journal, so you need to define your own service for it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've made it "internal". Configuration for plugins should be solved by #17 |
||
\* *These services are always an alias to another service. You can specify your own service or leave the default, which is the same name with `.default` appended. The default services in turn use the service discovery mechanism to provide the best available implementation. You can specify a class for each of the default services to use instead of discovery, as long as those classes can be instantiated without arguments.* | ||
|
||
|
||
If you need a more custom setup, define the services in your application configuration and specify your service in the `main_alias` section. For example, to add authentication headers, you could define a service that decorates the service `httplug.client.default` with a plugin that injects the authentication headers into the request and configure `httplug.main_alias.client` to the name of your service. | ||
|
||
```yaml | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<container xmlns="http://symfony.com/schema/dic/services" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> | ||
|
||
<services> | ||
<service id="httplug.collector.message_journal" class="Http\HttplugBundle\Collector\MessageJournal" public="false"> | ||
<tag name="data_collector" template="HttplugBundle::webprofiler.html.twig" priority="200" | ||
id="httplug"/> | ||
<argument>null</argument> | ||
</service> | ||
|
||
<service id="httplug.collector.history_plugin" class="Http\Client\Plugin\HistoryPlugin" public="false"> | ||
<argument type="service" id="httplug.collector.message_journal"/> | ||
</service> | ||
</services> | ||
</container> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
{% extends '@WebProfiler/Profiler/layout.html.twig' %} | ||
|
||
{% import _self as macro %} | ||
|
||
{% block toolbar %} | ||
{% if collector.totalRequests > 0 %} | ||
{% set icon %} | ||
{{ include('@WebProfiler/Icon/ajax.svg') }} | ||
<span class="sf-toolbar-status">{{ collector.totalRequests }}</span> | ||
{% endset %} | ||
|
||
{% set text %} | ||
<div class="sf-toolbar-info-piece"> | ||
<b>Successful requests</b> | ||
<span>{{ collector.sucessfulRequests|length }}</span> | ||
</div> | ||
<div class="sf-toolbar-info-piece"> | ||
<b>Faild requests</b> | ||
<span>{{ collector.failedRequests|length }}</span> | ||
</div> | ||
|
||
{% endset %} | ||
{% include 'WebProfilerBundle:Profiler:toolbar_item.html.twig' with { 'link': profiler_url } %} | ||
{% endif %} | ||
{% endblock %} | ||
|
||
{% block head %} | ||
{# Optional. Here you can link to or define your own CSS and JS contents. #} | ||
{{ parent() }} | ||
{% endblock %} | ||
|
||
{% block menu %} | ||
{# This left-hand menu appears when using the full-screen profiler. #} | ||
<span class="label {{ collector.totalRequests == 0 ? 'disabled' }}"> | ||
<span class="icon">{{ include('@WebProfiler/Icon/ajax.svg') }}</span> | ||
<strong>Httplug</strong> | ||
</span> | ||
{% endblock %} | ||
|
||
{% block panel %} | ||
<h2>HTTPlug</h2> | ||
{% if (collector.failedRequests|length > 0) %} | ||
<h3>Failed requests</h3> | ||
{{ macro.printMessages(collector.failedRequests) }} | ||
{% endif %} | ||
|
||
{% if (collector.sucessfulRequests|length > 0) %} | ||
<h3>Successful requests</h3> | ||
{{ macro.printMessages(collector.sucessfulRequests) }} | ||
{% endif %} | ||
|
||
{% if collector.totalRequests == 0 %} | ||
|
||
<div class="empty"> | ||
<p>No request were sent.</p> | ||
</div> | ||
{% endif %} | ||
|
||
{% endblock %} | ||
|
||
{% macro printMessages(messages) %} | ||
<table> | ||
<tr> | ||
<th>Request</th> | ||
<th>Response</th> | ||
</tr> | ||
|
||
{% for message in messages %} | ||
<tr> | ||
<td>{{ message['request'] }}</td> | ||
<td>{{ message['response'] }}</td> | ||
</tr> | ||
{% endfor %} | ||
</table> | ||
{% endmacro %} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
<?php | ||
|
||
$container->loadFromExtension('httplug', array()); | ||
$container->loadFromExtension('httplug', []); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,16 @@ | ||
<?php | ||
|
||
$container->loadFromExtension('httplug', array( | ||
'main_alias' => array( | ||
'client' => 'my_client', | ||
$container->loadFromExtension('httplug', [ | ||
'main_alias' => [ | ||
'client' => 'my_client', | ||
'message_factory' => 'my_message_factory', | ||
'uri_factory' => 'my_uri_factory', | ||
'stream_factory' => 'my_stream_factory', | ||
), | ||
'classes' => array( | ||
'client' => 'Http\Adapter\Guzzle6\Client', | ||
'uri_factory' => 'my_uri_factory', | ||
'stream_factory' => 'my_stream_factory', | ||
], | ||
'classes' => [ | ||
'client' => 'Http\Adapter\Guzzle6\Client', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this styleci as well? AFAIK symfony preset unaligns double arrows, doesn't it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests is ignored by styleci i think. not sure if we really want to ignore them. consistent CS in the tests sounds like a valuable thing to have. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see. Well, the reason why I ignored tests by default was that spec tests are snake cased and function names does not contain visibility. I agree it would make sense here, so let's just remove the test paths from exclusions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, That is styleci. |
||
'message_factory' => 'Http\Message\MessageFactory\GuzzleMessageFactory', | ||
'uri_factory' => 'Http\Message\UriFactory\GuzzleUriFactory', | ||
'stream_factory' => 'Http\Message\StreamFactory\GuzzleStreamFactory', | ||
), | ||
)); | ||
'uri_factory' => 'Http\Message\UriFactory\GuzzleUriFactory', | ||
'stream_factory' => 'Http\Message\StreamFactory\GuzzleStreamFactory', | ||
], | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this intentionally empty? if so, please add a comment why it is there and empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okey, thank you.
A upcoming PR maybe want to log which Symfony Request that made which Httplug Request. But it should be empty for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍