8000 feature #30539 [Messenger] deprecate LoggingMiddleware in favor of pr… · symfony/symfony@a116347 · GitHub
[go: up one dir, main page]
Skip to content

Commit a116347

Browse files
nicolas-grekasnicolas-grekas
committed
feature #30539 [Messenger] deprecate LoggingMiddleware in favor of providing a logger to SendMessageMiddleware (nicolas-grekas)
This PR was merged into the 4.3-dev branch. Discussion ---------- [Messenger] deprecate LoggingMiddleware in favor of providing a logger to SendMessageMiddleware | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Discussing with @simensen, we figured out the currently logged messages are not precise enough. Logging is a cross-cutting concern: splitting it in a dedicated middleware means losing details - or building complexity. Let's make things simple and log the best messages depending on the internal situation. While the component is experimental, removing the `LoggingMiddleware` altogether would break FrameworkBundle 4.2 when it is used with Messenger 4.3. Not worth the trouble when it's two lines to deprecate IMHO. Commits ------- 2bff625 [Messenger] deprecate LoggingMiddleware in favor of providing a logger to SendMessageMiddleware
2 parents 9ee5ff7 + 2bff625 commit a116347

File tree

12 files changed

+75
-29
lines changed

12 files changed

+75
-29
lines changed

‎UPGRADE-4.3.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ Messenger
6161
---------
6262

6363
* `Amqp` transport does not throw `\AMQPException` anymore, catch `TransportException` instead.
64+
* Deprecated the `LoggingMiddleware` class, pass a logger to `SendMessageMiddleware` instead.
6465

6566
Routing
6667
-------

‎UPGRADE-5.0.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,11 @@ HttpKernel
207207
* Removed `ConfigDataCollector::getApplicationName()`
208208
* Removed `ConfigDataCollector::getApplicationVersion()`
209209

210+
Messenger
211+
---------
212+
213+
* The `LoggingMiddleware` class has been removed, pass a logger to `SendMessageMiddleware` instead.
214+
210215
Monolog
211216
-------
212217

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1598,7 +1598,7 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
15981598
}
15991599

16001600
$defaultMiddleware = [
1601-
'before' => [['id' => 'logging']],
1601+
'before' => [],
16021602
'after' => [['id' => 'send_message'], ['id' => 'handle_message']],
16031603
];
16041604
foreach ($config['buses'] as $busId => $bus) {

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@
1313
<argument type="collection" /> <!-- Messages to send and handle -->
1414
</service>
1515
<service id="messenger.middleware.send_message" class="Symfony\Component\Messenger\Middleware\SendMessageMiddleware">
16+
<tag name="monolog.logger" channel="messenger" />
1617
<argument type="service" id="messenger.senders_locator" />
18+
<call method="setLogger">
19+
<argument type="service" id="logger" on-invalid="ignore" />
20+
</call>
1721
</service>
1822

1923
<!-- Message encoding/decoding -->
@@ -28,7 +32,11 @@
2832

2933
<!-- Middleware -->
3034
<service id="messenger.middleware.handle_message" class="Symfony\Component\Messenger\Middleware\HandleMessageMiddleware" abstract="true">
35+
<tag name="monolog.logger" channel="messenger" />
3136
<argument /> <!-- Bus handler resolver -->
37+
<call method="setLogger">
38+
<argument type="service" id="logger" on-invalid="ignore" />
39+
</call>
3240
</service>
3341

3442
<service id="messenger.middleware.validation" class="Symfony\Component\Messenger\Middleware\ValidationMiddleware">
@@ -39,12 +47,6 @@
3947
<argument type="service" id="debug.stopwatch" />
4048
</service>
4149

42-
<!-- Logging -->
43-
<service id="messenger.middleware.logging" class="Symfony\Component\Messenger\Middleware\LoggingMiddleware">
44-
<tag name="monolog.logger" channel="messenger" />
45-
<argument type="service" id="logger" />
46-
</service>
47-
4850
<!-- Discovery -->
4951
<service id="messenger.receiver_locator">
5052
<tag name="container.service_locator" />

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,14 +701,12 @@ public function testMessengerWithMultipleBuses()
701701
$this->assertTrue($container->has('messenger.bus.commands'));
702702
$this->assertSame([], $container->getDefinition('messenger.bus.commands')->getArgument(0));
703703
$this->assertEquals([
704-
['id' => 'logging'],
705704
['id' => 'send_message'],
706705
['id' => 'handle_message'],
707706
], $container->getParameter('messenger.bus.commands.middleware'));
708707
$this->assertTrue($container->has('messenger.bus.events'));
709708
$this->assertSame([], $container->getDefinition('messenger.bus.events')->getArgument(0));
710709
$this->assertEquals([
711-
['id' => 'logging'],
712710
['id' => 'with_factory', 'arguments' => ['foo', true, ['bar' => 'baz']]],
713711
['id' => 'send_message'],
714712
['id' => 'handle_message'],

‎src/Symfony/Component/Messenger/CHANGELOG.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,13 @@ CHANGELOG
66

77
* Added `PhpSerializer` which uses PHP's native `serialize()` and
88
`unserialize()` to serialize messages to a transport
9-
109
* [BC BREAK] If no serializer were passed, the default serializer
1110
changed from `Serializer` to `PhpSerializer` inside `AmqpReceiver`,
1211
`AmqpSender`, `AmqpTransport` and `AmqpTransportFactory`.
13-
1412
* Added `TransportException` to mark an exception transport-related
15-
1613
* [BC BREAK] If listening to exceptions while using `AmqpSender` or `AmqpReceiver`, `\AMQPException` is
1714
no longer thrown in favor of `TransportException`.
15+
* Deprecated `LoggingMiddleware`, pass a logger to `SendMessageMiddleware` instead.
1816

1917
4.2.0
2018
-----

‎src/Symfony/Component/Messenger/Command/ConsumeMessagesCommand.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,8 @@ protected function execute(InputInterface $input, OutputInterface $output): void
170170

171171
$io->comment('Quit the worker with CONTROL-C.');
172172

173-
if (!$output->isDebug()) {
174-
$io->comment('Re-run the command with a -vvv option to see logs about consumed messages.');
173+
if (OutputInterface::VERBOSITY_VERBOSE > $output->getVerbosity()) {
174+
$io->comment('Re-run the command with a -vv option to see logs about consumed messages.');
175175
}
176176

177177
$worker = new Worker($receiver, $bus);

‎src/Symfony/Component/Messenger/Middleware/HandleMessageMiddleware.php

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Messenger\Middleware;
1313

14+
use Psr\Log\LoggerAwareTrait;
15+
use Psr\Log\NullLogger;
1416
use Symfony\Component\Messenger\Envelope;
1517
use Symfony\Component\Messenger\Exception\NoHandlerForMessageException;
1618
use Symfony\Component\Messenger\Handler\HandlersLocatorInterface;
@@ -23,13 +25,16 @@
2325
*/
2426
class HandleMessageMiddleware implements MiddlewareInterface
2527
{
28+
use LoggerAwareTrait;
29+
2630
private $handlersLocator;
2731
private $allowNoHandlers;
2832

2933
public function __construct(HandlersLocatorInterface $handlersLocator, bool $allowNoHandlers = false)
3034
{
3135
$this->handlersLocator = $handlersLocator;
3236
$this->allowNoHandlers = $allowNoHandlers;
37+
$this->logger = new NullLogger();
3338
}
3439

3540
/**
@@ -41,11 +46,24 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope
4146
{
4247
$handler = null;
4348
$message = $envelope->getMessage();
49+
50+
$context = [
51+
'message' => $message,
52+
'class' => \get_class($message),
53+
];
54+
4455
foreach ($this->handlersLocator->getHandlers($envelope) as $alias => $handler) {
45-
$envelope = $envelope->with(HandledStamp::fromCallable($handler, $handler($message), \is_string($alias) ? $alias : null));
56+
$handledStamp = HandledStamp::fromCallable($handler, $handler($message), \is_string($alias) ? $alias : null);
57+
$envelope = $envelope->with($handledStamp);
58+
$this->logger->info('Message "{class}" handled by "{handler}"', $context + ['handler' => $handledStamp->getCallableName()]);
4659
}
47-
if (null === $handler && !$this->allowNoHandlers) {
48-
throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', \get_class($envelope->getMessage())));
60+
61+
if (null === $handler) {
62+
if (!$this->allowNoHandlers) {
63+
throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', $context['class']));
64+
}
65+
66+
$this->logger->info('No handler for message "{class}"', $context);
4967
}
5068

5169
return $stack->next()->handle($envelope, $stack);

‎src/Symfony/Component/Messenger/Middleware/LoggingMiddleware.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@
1111

1212
namespace Symfony\Component\Messenger\Middleware;
1313

14+
@trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.3, pass a logger to SendMessageMiddleware instead.', LoggingMiddleware::class), E_USER_DEPRECATED);
15+
1416
use Psr\Log\LoggerInterface;
1517
use Symfony\Component\Messenger\Envelope;
1618

< 10000 div aria-hidden="true" style="left:-2px" class="position-absolute top-0 d-flex user-select-none DiffLineTableCellParts-module__in-progress-comment-indicator--hx3m3">
1719
/**
1820
* @author Samuel Roze <samuel.roze@gmail.com>
1921
*
20-
* @experimental in 4.2
22+
* @deprecated since 4.3, pass a logger to SendMessageMiddleware instead
2123
*/
2224
class LoggingMiddleware implements MiddlewareInterface
2325
{

‎src/Symfony/Component/Messenger/Middleware/SendMessageMiddleware.php

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Messenger\Middleware;
1313

14+
use Psr\Log\LoggerAwareTrait;
15+
use Psr\Log\NullLogger;
1416
use Symfony\Component\Messenger\Envelope;
1517
use Symfony\Component\Messenger\Stamp\ReceivedStamp;
1618
use Symfony\Component\Messenger\Stamp\SentStamp;
@@ -24,31 +26,48 @@
2426
*/
2527
class SendMessageMiddleware implements MiddlewareInterface
2628
{
29+
use LoggerAwareTrait;
30+
2731
private $sendersLocator;
2832

2933
public function __construct(SendersLocatorInterface $sendersLocator)
3034
{
3135
$this->sendersLocator = $sendersLocator;
36+
$this->logger = new NullLogger();
3237
}
3338

3439
/**
3540
* {@inheritdoc}
3641
*/
3742
public function handle(Envelope $envelope, StackInterface $stack): Envelope
3843
{
39-
if ($envelope->all(ReceivedStamp::class)) {
40-
// it's a received message, do not send it back
41-
return $stack->next()->handle($envelope, $stack);
42-
}
44+
$context = [
45+
'message' => $envelope->getMessage(),
46+
'class' => \get_class($envelope->getMessage()),
47+
];
48+
4349
$handle = false;
4450
$sender = null;
4551

46-
foreach ($this->sendersLocator->getSenders($envelope, $handle) as $alias => $sender) {
47-
$envelope = $sender->send($envelope)->with(new SentStamp(\get_class($sender), \is_string($alias) ? $alias : null));
48-
}
52+
try {
53+
if ($envelope->all(ReceivedStamp::class)) {
54+
// it's a received message, do not send it back
55+
$this->logger->info('Received message "{class}"', $context);
56+
} else {
57+
foreach ($this->sendersLocator->getSenders($envelope, $handle) as $alias => $sender) {
58+
$this->logger->info('Sending message "{class}" with "{sender}"', $context + ['sender' => \get_class($sender)]);
59+
$envelope = $sender->send($envelope)->with(new SentStamp(\get_class($sender), \is_string($alias) ? $alias : null));
60+
}
61+
}
62+
63+
if (null === $sender || $handle) {
64+
return $stack->next()->handle($envelope, $stack);
65+
}
66+
} catch (\Throwable $e) {
67+
$context['exception'] = $e;
68+
$this->logger->warning('An exception occurred while handling message "{class}"', $context);
4969

50-
if (null === $sender || $handle) {
51-
return $stack->next()->handle($envelope, $stack);
70+
throw $e;
5271
}
5372

5473
// message should only be sent and not be handled by the next middleware

‎src/Symfony/Component/Messenger/Tests/Middleware/LoggingMiddlewareTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
use Symfony\Component\Messenger\Test\Middleware\MiddlewareTestCase;
1818
use Symfony\Component\Messenger\Tests\Fixtures\DummyMessage;
1919

20+
/**
21+
* @group legacy
22+
*/
2023
class LoggingMiddlewareTest extends MiddlewareTestCase
2124
{
2225
public function testDebugLogAndNextMiddleware()

‎src/Symfony/Component/Messenger/composer.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@
1616
}
1717
],
1818
"require": {
19-
"php": "^7.1.3"
19+
"php": "^7.1.3",
20+
"psr/log": "~1.0"
2021
},
2122
"require-dev": {
22-
"psr/log": "~1.0",
2323
"symfony/console": "~3.4|~4.0",
2424
"symfony/dependency-injection": "~3.4.19|^4.1.8",
2525
"symfony/http-kernel": "~3.4|~4.0",

0 commit comments

Comments
 (0)
0