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.

Sign up for GitHub

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

Jump to bottom

Cleanup the Monolog handler from deprecated features #1068

Merged
merged 2 commits into from Aug 24, 2020
Merged

Cleanup the Monolog handler from deprecated features #1068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ab8b35
Cleanup the Monolog handler from deprecated features
ste93cry Aug 19, 2020
3f26aba
Fix CR issues
ste93cry Aug 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@
- [BC BREAK] The `Scope::setUser()` method now always merges the given data with the existing one instead of replacing it as a whole (#1047)
- [BC BREAK] Remove the `Context::CONTEXT_USER`, `Context::CONTEXT_RUNTIME`, `Context::CONTEXT_TAGS`, `Context::CONTEXT_EXTRA`, `Context::CONTEXT_SERVER_OS` constants (#1047)
- [BC BREAK] Use PSR-17 factories in place of the Httplug's ones and return a promise from the transport (#1066)
- [BC BREAK] The Monolog handler does not set anymore tags and extras on the event object (#1068)

### 2.4.1 (2020-07-03)

Expand Down
47 changes: 47 additions & 0 deletions UPGRADE-3.0.md
View file
Open in desktop
Expand Up @@ -37,3 +37,50 @@
- The signature of the `HttpClientFactory::__construct()` method changed to accept instances of the PSR-17 factories in place of Httplug's ones
- The signature of the `DefaultTransportFactory::__construct()` method changed to accept instances of the PSR-17 factories in place of Httplug's ones
- The signature of the `GzipEncoderPlugin::__construct()` method changed to accept an instance of the `Psr\Http\Message\StreamFactoryInterface` interface only
- The Monolog handler does not set anymore the tags and extras on the event by extracting automatically the data from the record payload. You can decorate the
class and set such data on the scope as shown below:

```php
use Monolog\Handler\HandlerInterface;
use Sentry\State\Scope;
use function Sentry\withScope;

final class MonologHandler implements HandlerInterface
{
private $decoratedHandler;

public function __construct(HandlerInterface $decoratedHandler)
{
$this->decoratedHandler = $decoratedHandler;
}

public function isHandling(array $record): bool
{
return $this->decoratedHandler->isHandling($record);
}

public function handle(array $record): bool
{
$result = false;

withScope(function (Scope $scope) use ($record, &$result): void {
$scope->setTags(...);
$scope->setExtras(...);

$result = $this->decoratedHandler->handle($record);
});

return $result;
}

public function handleBatch(array $records): void
{
$this->decoratedHandler->handleBatch($records);
}

public function close(): void
{
$this->decoratedHandler->close();
}
}
```
17 changes: 2 additions & 15 deletions src/Monolog/Handler.php
View file
Open in desktop
Expand Up @@ -58,18 +58,6 @@ protected function write(array $record): void
$scope->setExtra('monolog.channel', $record['channel']);
$scope->setExtra('monolog.level', $record['level_name']);

if (isset($record['context']['extra']) && \is_array($record['context']['extra'])) {
foreach ($record['context']['extra'] as $key => $value) {
$scope->setExtra((string) $key, $value);
}
}

if (isset($record['context']['tags']) && \is_array($record['context']['tags'])) {
foreach ($record['context']['tags'] as $key => $value) {
$scope->setTag($key, $value);
}
}

$this->hub->captureEvent($payload);
});
}
Expand All @@ -84,9 +72,6 @@ private static function getSeverityFromLevel(int $level): Severity
switch ($level) {
case Logger::DEBUG:
return Severity::debug();
case Logger::INFO:
case Logger::NOTICE:
return Severity::info();
case Logger::WARNING:
return Severity::warning();
case Logger::ERROR:
Expand All @@ -95,6 +80,8 @@ private static function getSeverityFromLevel(int $level): Severity
case Logger::ALERT:
case Logger::EMERGENCY:
return Severity::fatal();
case Logger::INFO:
case Logger::NOTICE:
default:
return Severity::info();
}
Expand Down
95 changes: 4 additions & 91 deletions tests/Monolog/HandlerTest.php
View file
Open in desktop
Expand Up @@ -19,29 +19,26 @@ final class HandlerTest extends TestCase
/**
* @dataProvider handleDataProvider
*/
public function testHandle(array $record, array $expectedPayload, array $expectedExtra, array $expectedTags): void
public function testHandle(array $record, array $expectedPayload, array $expectedExtra): void
{
$scope = new Scope();

/** @var ClientInterface&MockObject $client */
$client = $this->createMock(ClientInterface::class);
$client->expects($this->once())
->method('captureEvent')
->with($expectedPayload, $this->callback(function (Scope $scopeArg) use ($expectedExtra, $expectedTags): bool {
->with($expectedPayload, $this->callback(function (Scope $scopeArg) use ($expectedExtra): bool {
$event = $scopeArg->applyToEvent(new Event(), []);

$this->assertNotNull($event);
$this->assertSame($expectedExtra, $event->getExtraContext()->toArray());
$this->assertSame($expectedTags, $event->getTagsContext()->toArray());

return true;
}));

$handler = new Handler(new Hub($client, $scope));
$handler = new Handler(new Hub($client, new Scope()));
$handler->handle($record);
}

public function handleDataProvider(): \Generator
public function handleDataProvider(): iterable
{
yield [
[
Expand All @@ -61,7 +58,6 @@ public function handleDataProvider(): \Generator
'monolog.channel' => 'channel.foo',
'monolog.level' => Logger::getLevelName(Logger::DEBUG),
],
[],
];

yield [
Expand All @@ -82,7 +78,6 @@ public function handleDataProvider(): \Generator
'monolog.channel' => 'channel.foo',
'monolog.level' => Logger::getLevelName(Logger::INFO),
],
[],
];

yield [
Expand All @@ -103,7 +98,6 @@ public function handleDataProvider(): \Generator
'monolog.channel' => 'channel.foo',
'monolog.level' => Logger::getLevelName(Logger::NOTICE),
],
[],
];

yield [
Expand All @@ -124,7 +118,6 @@ public function handleDataProvider(): \Generator
'monolog.channel' => 'channel.foo',
'monolog.level' => Logger::getLevelName(Logger::WARNING),
],
[],
];

yield [
Expand All @@ -145,7 +138,6 @@ public function handleDataProvider(): \Generator
'monolog.channel' => 'channel.foo',
'monolog.level' => Logger::getLevelName(Logger::ERROR),
],
[],
];

yield [
Expand All @@ -166,7 +158,6 @@ public function handleDataProvider(): \Generator
'monolog.channel' => 'channel.foo',
'monolog.level' => Logger::getLevelName(Logger::CRITICAL),
],
[],
];

yield [
Expand All @@ -187,7 +178,6 @@ public function handleDataProvider(): \Generator
'monolog.channel' => 'channel.foo',
'monolog.level' => Logger::getLevelName(Logger::ALERT),
],
[],
];

yield [
Expand All @@ -208,43 +198,6 @@ public function handleDataProvider(): \Generator
'monolog.channel' => 'channel.foo',
'monolog.level' => Logger::getLevelName(Logger::EMERGENCY),
],
[],
];

yield [
[
'message' => 'foo bar',
'level' => Logger::WARNING,
'level_name' => Logger::getLevelName(Logger::WARNING),
'context' => [
'extra' => [
'foo.extra' => 'foo extra value',
'bar.extra' => 'bar extra value',
],
'tags' => [
'foo.tag' => 'foo tag value',
'bar.tag' => 'bar tag value',
],
],
'channel' => 'channel.foo',
'datetime' => new \DateTimeImmutable(),
'extra' => [],
],
[
'level' => Severity::warning(),
'message' => 'foo bar',
'logger' => 'monolog.channel.foo',
],
[
'monolog.channel' => 'channel.foo',
'monolog.level' => Logger::getLevelName(Logger::WARNING),
'foo.extra' => 'foo extra value',
'bar.extra' => 'bar extra value',
],
[
'foo.tag' => 'foo tag value',
'bar.tag' => 'bar tag value',
],
];

yield [
Expand All @@ -254,14 +207,6 @@ public function handleDataProvider(): \Generator
'level_name' => Logger::getLevelName(Logger::WARNING),
'context' => [
'exception' => new \Exception('exception message'),
'extra' => [
'foo.extra' => 'foo extra value',
'bar.extra' => 'bar extra value',
],
'tags' => [
'foo.tag' => 'foo tag value',
'bar.tag' => 'bar tag value',
],
],
'channel' => 'channel.foo',
'datetime' => new \DateTimeImmutable(),
Expand All @@ -276,39 +221,7 @@ public function handleDataProvider(): \Generator
[
'monolog.channel' => 'channel.foo',
'monolog.level' => Logger::getLevelName(Logger::WARNING),
'foo.extra' => 'foo extra value',
'bar.extra' => 'bar extra value',
],
[
'foo.tag' => 'foo tag value',
'bar.tag' => 'bar tag value',
],
];

yield [
[
'message' => 'foo bar',
'level' => Logger::INFO,
'level_name' => Logger::getLevelName(Logger::INFO),
'channel' => 'channel.foo',
'context' => [
'extra' => [
1 => 'numeric key',
],
],
'extra' => [],
],
[
'level' => Severity::info(),
'message' => 'foo bar',
'logger' => 'monolog.channel.foo',
],
[
'monolog.channel' => 'channel.foo',
'monolog.level' => Logger::getLevelName(Logger::INFO),
'0' => 'numeric key',
],
[],
];
}
}