diff --git a/src/Contracts/SignatureGenerator.php b/src/Contracts/SignatureGenerator.php new file mode 100644 index 0000000..7f43a2b --- /dev/null +++ b/src/Contracts/SignatureGenerator.php @@ -0,0 +1,13 @@ +context['exception'] ?? null; + + if (!$exception instanceof Throwable) { + return $this->generateFromMessage($record); + } + + return $this->generateFromException($exception); + } + + /** + * Generate a signature from a message and context + */ + private function generateFromMessage(LogRecord $record): string + { + return md5($record->message . json_encode($record->context)); + } + + /** + * Generate a signature from an exception + */ + private function generateFromException(Throwable $exception): string + { + $trace = $exception->getTrace(); + $firstFrame = !empty($trace) ? $trace[0] : null; + + return md5(implode(':', [ + $exception::class, + $exception->getFile(), + $exception->getLine(), + $firstFrame ? ($firstFrame['file'] ?? '') . ':' . ($firstFrame['line'] ?? '') : '', + ])); + } +} diff --git a/src/GithubIssueFormatted.php b/src/Formatters/GithubIssueFormatted.php similarity index 82% rename from src/GithubIssueFormatted.php rename to src/Formatters/GithubIssueFormatted.php index 286aeb3..3b4d6c1 100644 --- a/src/GithubIssueFormatted.php +++ b/src/Formatters/GithubIssueFormatted.php @@ -1,6 +1,6 @@ message.json_encode($record->context)); + return md5($record->message . json_encode($record->context)); } $trace = $exception->getTrace(); @@ -64,7 +64,7 @@ private function generateSignature(LogRecord $record, ?Throwable $exception): st $exception::class, $exception->getFile(), $exception->getLine(), - $firstFrame ? ($firstFrame['file'] ?? '').':'.($firstFrame['line'] ?? '') : '', + $firstFrame ? ($firstFrame['file'] ?? '') . ':' . ($firstFrame['line'] ?? '') : '', ])); } @@ -110,7 +110,7 @@ private function formatTitle(LogRecord $record, ?Throwable $exception = null): s private function formatContent(LogRecord $record, ?Throwable $exception): string { return Str::of('') - ->when($record->message, fn ($str, $message) => $str->append("**Message:**\n{$message}\n\n")) + ->when($record->message, fn($str, $message) => $str->append("**Message:**\n{$message}\n\n")) ->when( $exception, function (Stringable $str, Throwable $exception) { @@ -120,8 +120,8 @@ function (Stringable $str, Throwable $exception) { ); } ) - ->when(! $exception && ! empty($record->context), fn ($str, $context) => $str->append("**Context:**\n```json\n".json_encode($record->context, JSON_PRETTY_PRINT)."\n```\n\n")) - ->when(! empty($record->extra), fn ($str, $extra) => $str->append("**Extra Data:**\n```json\n".json_encode($record->extra, JSON_PRETTY_PRINT)."\n```\n")) + ->when(! $exception && ! empty($record->context), fn($str, $context) => $str->append("**Context:**\n```json\n" . json_encode($record->context, JSON_PRETTY_PRINT) . "\n```\n\n")) + ->when(! empty($record->extra), fn($str, $extra) => $str->append("**Extra Data:**\n```json\n" . json_encode($record->extra, JSON_PRETTY_PRINT) . "\n```\n")) ->toString(); } @@ -141,7 +141,7 @@ private function formatBody(LogRecord $record, string $signature, ?Throwable $ex private function cleanStackTrace(string $stackTrace): string { return collect(explode("\n", $stackTrace)) - ->filter(fn ($line) => ! empty(trim($line))) + ->filter(fn($line) => ! empty(trim($line))) ->map(function ($line) { if (trim($line) === '"}') { return ''; @@ -217,8 +217,8 @@ private function formatExceptionDetails(Throwable $exception): array return [ 'message' => $exception->getMessage(), - 'stack_trace' => $header."\n[stacktrace]\n".$this->cleanStackTrace($exception->getTraceAsString()), - 'full_stack_trace' => $header."\n[stacktrace]\n".$exception->getTraceAsString(), + 'stack_trace' => $header . "\n[stacktrace]\n" . $this->cleanStackTrace($exception->getTraceAsString()), + 'full_stack_trace' => $header . "\n[stacktrace]\n" . $exception->getTraceAsString(), ]; } diff --git a/src/GithubIssueHandlerFactory.php b/src/GithubIssueHandlerFactory.php index 4a47c9a..149e025 100644 --- a/src/GithubIssueHandlerFactory.php +++ b/src/GithubIssueHandlerFactory.php @@ -5,9 +5,11 @@ use Illuminate\Support\Arr; use Illuminate\Support\Facades\File; use InvalidArgumentException; -use Monolog\Handler\DeduplicationHandler; use Monolog\Level; use Monolog\Logger; +use Naoray\LaravelGithubMonolog\Formatters\GithubIssueFormatter; +use Naoray\LaravelGithubMonolog\Handlers\SignatureDeduplicationHandler; +use Naoray\LaravelGithubMonolog\Handlers\IssueLogHandler; class GithubIssueHandlerFactory { @@ -32,9 +34,9 @@ protected function validateConfig(array $config): void } } - protected function createBaseHandler(array $config): GithubIssueLoggerHandler + protected function createBaseHandler(array $config): IssueLogHandler { - $handler = new GithubIssueLoggerHandler( + $handler = new IssueLogHandler( repo: $config['repo'], token: $config['token'], labels: $config['labels'] ?? [], @@ -47,9 +49,9 @@ protected function createBaseHandler(array $config): GithubIssueLoggerHandler return $handler; } - protected function wrapWithDeduplication(GithubIssueLoggerHandler $handler, array $config): DeduplicationHandler + protected function wrapWithDeduplication(IssueLogHandler $handler, array $config): SignatureDeduplicationHandler { - return new DeduplicationHandler( + return new SignatureDeduplicationHandler( $handler, deduplicationStore: $this->getDeduplicationStore($config), deduplicationLevel: Arr::get($config, 'level', Level::Error), diff --git a/src/GithubIssueLoggerHandler.php b/src/Handlers/IssueLogHandler.php similarity index 90% rename from src/GithubIssueLoggerHandler.php rename to src/Handlers/IssueLogHandler.php index 7b5a5fe..5687ec3 100644 --- a/src/GithubIssueLoggerHandler.php +++ b/src/Handlers/IssueLogHandler.php @@ -1,20 +1,18 @@ token) ->get('https://api.github.com/search/issues', [ - 'q' => "repo:{$this->repo} is:issue is:open label:".self::DEFAULT_LABEL." \"Signature: {$signature}\"", + 'q' => "repo:{$this->repo} is:issue is:open label:" . self::DEFAULT_LABEL . " \"Signature: {$signature}\"", ]); if ($response->failed()) { - throw new \RuntimeException('Failed to search GitHub issues: '.$response->body()); + throw new \RuntimeException('Failed to search GitHub issues: ' . $response->body()); } return $response->json('items.0', null); @@ -87,7 +85,7 @@ private function commentOnIssue(int $issueNumber, GithubIssueFormatted $formatte ]); if ($response->failed()) { - throw new \RuntimeException('Failed to comment on GitHub issue: '.$response->body()); + throw new \RuntimeException('Failed to comment on GitHub issue: ' . $response->body()); } } @@ -104,7 +102,7 @@ private function createIssue(GithubIssueFormatted $formatted): void ]); if ($response->failed()) { - throw new \RuntimeException('Failed to create GitHub issue: '.$response->body()); + throw new \RuntimeException('Failed to create GitHub issue: ' . $response->body()); } } } diff --git a/src/Handlers/SignatureDeduplicationHandler.php b/src/Handlers/SignatureDeduplicationHandler.php new file mode 100644 index 0000000..1cea8e0 --- /dev/null +++ b/src/Handlers/SignatureDeduplicationHandler.php @@ -0,0 +1,54 @@ +signatureGenerator = $signatureGenerator ?? new DefaultSignatureGenerator(); + } + + /** + * Override isDuplicate to use our signature-based deduplication + */ + protected function isDuplicate(array $store, LogRecord $record): bool + { + $timestampValidity = $record->datetime->getTimestamp() - $this->time; + $signature = $this->signatureGenerator->generate($record); + + foreach ($store as $entry) { + [$timestamp, $storedSignature] = explode(':', $entry, 2); + + if ($storedSignature === $signature && $timestamp > $timestampValidity) { + return true; + } + } + + return false; + } + + /** + * Override store entry format to use our signature + */ + protected function buildDeduplicationStoreEntry(LogRecord $record): string + { + return $record->datetime->getTimestamp() . ':' . $this->signatureGenerator->generate($record); + } +} diff --git a/tests/DefaultSignatureGeneratorTest.php b/tests/DefaultSignatureGeneratorTest.php new file mode 100644 index 0000000..1f9851c --- /dev/null +++ b/tests/DefaultSignatureGeneratorTest.php @@ -0,0 +1,87 @@ +generator = new DefaultSignatureGenerator(); +}); + +test('generates signature from message', function () { + $record = new LogRecord( + datetime: new \DateTimeImmutable(), + channel: 'test', + level: Level::Error, + message: 'Test message', + context: ['foo' => 'bar'], + extra: [], + ); + + $signature1 = $this->generator->generate($record); + expect($signature1)->toBeString(); + + // Same message and context should generate same signature + $record2 = new LogRecord( + datetime: new \DateTimeImmutable(), + channel: 'different-channel', + level: Level::Warning, + message: 'Test message', + context: ['foo' => 'bar'], + extra: ['something' => 'else'], + ); + $signature2 = $this->generator->generate($record2); + expect($signature2)->toBe($signature1); + + // Different message should generate different signature + $record3 = new LogRecord( + datetime: new \DateTimeImmutable(), + channel: 'test', + level: Level::Error, + message: 'Different message', + context: ['foo' => 'bar'], + extra: [], + ); + $signature3 = $this->generator->generate($record3); + expect($signature3)->not->toBe($signature1); +}); + +test('generates signature from exception', function () { + $exception = new \Exception('Test exception'); + $record = new LogRecord( + datetime: new \DateTimeImmutable(), + channel: 'test', + level: Level::Error, + message: 'Test message', + context: ['exception' => $exception], + extra: [], + ); + + $signature1 = $this->generator->generate($record); + expect($signature1)->toBeString(); + + // Same exception should generate same signature + $record2 = new LogRecord( + datetime: new \DateTimeImmutable(), + channel: 'different-channel', + level: Level::Warning, + message: 'Different message', + context: ['exception' => $exception], + extra: ['something' => 'else'], + ); + $signature2 = $this->generator->generate($record2); + expect($signature2)->toBe($signature1); + + // Different exception should generate different signature + $differentException = new \Exception('Different exception'); + $record3 = new LogRecord( + datetime: new \DateTimeImmutable(), + channel: 'test', + level: Level::Error, + message: 'Test message', + context: ['exception' => $differentException], + extra: [], + ); + $signature3 = $this->generator->generate($record3); + expect($signature3)->not->toBe($signature1); +}); diff --git a/tests/GithubIssueFormatterTest.php b/tests/Formatters/GithubIssueFormatterTest.php similarity index 97% rename from tests/GithubIssueFormatterTest.php rename to tests/Formatters/GithubIssueFormatterTest.php index 45c0a6b..e9b7483 100644 --- a/tests/GithubIssueFormatterTest.php +++ b/tests/Formatters/GithubIssueFormatterTest.php @@ -2,8 +2,8 @@ use Monolog\Level; use Monolog\LogRecord; -use Naoray\LaravelGithubMonolog\GithubIssueFormatted; -use Naoray\LaravelGithubMonolog\GithubIssueFormatter; +use Naoray\LaravelGithubMonolog\Formatters\GithubIssueFormatted; +use Naoray\LaravelGithubMonolog\Formatters\GithubIssueFormatter; test('it formats basic log records', function () { $formatter = new GithubIssueFormatter; diff --git a/tests/GithubIssueHandlerFactoryTest.php b/tests/GithubIssueHandlerFactoryTest.php index 2dc4679..3e5fbb6 100644 --- a/tests/GithubIssueHandlerFactoryTest.php +++ b/tests/GithubIssueHandlerFactoryTest.php @@ -3,12 +3,11 @@ use Monolog\Handler\DeduplicationHandler; use Monolog\Level; use Monolog\Logger; -use Naoray\LaravelGithubMonolog\GithubIssueFormatter; +use Naoray\LaravelGithubMonolog\Formatters\GithubIssueFormatter; use Naoray\LaravelGithubMonolog\GithubIssueHandlerFactory; -use Naoray\LaravelGithubMonolog\GithubIssueLoggerHandler; -use ReflectionProperty; +use Naoray\LaravelGithubMonolog\Handlers\IssueLogHandler; -function getWrappedHandler(DeduplicationHandler $handler): GithubIssueLoggerHandler +function getWrappedHandler(DeduplicationHandler $handler): IssueLogHandler { $reflection = new ReflectionProperty($handler, 'handler'); $reflection->setAccessible(true); @@ -45,16 +44,16 @@ function getDeduplicationStore(DeduplicationHandler $handler): string $deduplicationHandler = $logger->getHandlers()[0]; $handler = getWrappedHandler($deduplicationHandler); - expect($handler)->toBeInstanceOf(GithubIssueLoggerHandler::class); + expect($handler)->toBeInstanceOf(IssueLogHandler::class); }); test('it throws exception for missing required config', function () { $factory = new GithubIssueHandlerFactory; - expect(fn () => $factory([])) + expect(fn() => $factory([])) ->toThrow(InvalidArgumentException::class, 'GitHub repository is required'); - expect(fn () => $factory(['repo' => 'test/repo'])) + expect(fn() => $factory(['repo' => 'test/repo'])) ->toThrow(InvalidArgumentException::class, 'GitHub token is required'); }); @@ -72,7 +71,7 @@ function getDeduplicationStore(DeduplicationHandler $handler): string $handler = getWrappedHandler($deduplicationHandler); expect($handler) - ->toBeInstanceOf(GithubIssueLoggerHandler::class) + ->toBeInstanceOf(IssueLogHandler::class) ->and($handler->getLevel())->toBe(Level::Error) ->and($handler->getBubble())->toBeTrue() ->and($handler->getFormatter())->toBeInstanceOf(GithubIssueFormatter::class); diff --git a/tests/GithubIssueLoggerHandlerTest.php b/tests/Handlers/IssueLogHandlerTest.php similarity index 94% rename from tests/GithubIssueLoggerHandlerTest.php rename to tests/Handlers/IssueLogHandlerTest.php index d195bc5..114e576 100644 --- a/tests/GithubIssueLoggerHandlerTest.php +++ b/tests/Handlers/IssueLogHandlerTest.php @@ -4,11 +4,11 @@ use Illuminate\Support\Facades\Http; use Monolog\Level; use Monolog\LogRecord; -use Naoray\LaravelGithubMonolog\GithubIssueFormatter; -use Naoray\LaravelGithubMonolog\GithubIssueLoggerHandler; +use Naoray\LaravelGithubMonolog\Formatters\GithubIssueFormatter; +use Naoray\LaravelGithubMonolog\Handlers\IssueLogHandler; beforeEach(function () { - $this->handler = (new GithubIssueLoggerHandler( + $this->handler = (new IssueLogHandler( repo: 'test/repo', token: 'fake-token', labels: ['bug'], @@ -28,7 +28,7 @@ function createLogRecord(string $message = 'Test error', array $context = [], Le ); } -function createFormattedRecord(GithubIssueLoggerHandler $handler, string $message = 'Test error', array $context = [], Level $level = Level::Error): LogRecord +function createFormattedRecord(IssueLogHandler $handler, string $message = 'Test error', array $context = [], Level $level = Level::Error): LogRecord { $baseRecord = createLogRecord($message, $context, $level); @@ -81,7 +81,7 @@ function createFormattedRecord(GithubIssueLoggerHandler $handler, string $messag 'github.com/search/issues*' => Http::response(['message' => 'Bad credentials'], 401), ]); - expect(fn () => $this->handler->handle(createFormattedRecord($this->handler))) + expect(fn() => $this->handler->handle(createFormattedRecord($this->handler))) ->toThrow(\RuntimeException::class, 'Failed to search GitHub issues'); }); @@ -91,7 +91,7 @@ function createFormattedRecord(GithubIssueLoggerHandler $handler, string $messag 'github.com/repos/test/repo/issues' => Http::response(['number' => 1], 201), ]); - $handler = (new GithubIssueLoggerHandler( + $handler = (new IssueLogHandler( repo: 'test/repo', token: 'fake-token', labels: ['custom-label', 'another-label'], @@ -214,7 +214,7 @@ function createFormattedRecord(GithubIssueLoggerHandler $handler, string $messag $record = createFormattedRecord($this->handler); - expect(fn () => $this->handler->handle($record)) + expect(fn() => $this->handler->handle($record)) ->toThrow(\RuntimeException::class, 'Failed to create GitHub issue'); }); diff --git a/tests/Handlers/SignatureDeduplicationHandlerTest.php b/tests/Handlers/SignatureDeduplicationHandlerTest.php new file mode 100644 index 0000000..be0ae6a --- /dev/null +++ b/tests/Handlers/SignatureDeduplicationHandlerTest.php @@ -0,0 +1,109 @@ +deduplicationStore = sys_get_temp_dir() . '/test-dedup-' . uniqid() . '.log'; + $this->signatureGenerator = new DefaultSignatureGenerator(); + $this->handler = new SignatureDeduplicationHandler( + new NullHandler(), + $this->deduplicationStore, + Level::Debug, + time: 60, + signatureGenerator: $this->signatureGenerator + ); +}); + +afterEach(function () { + if (file_exists($this->deduplicationStore)) { + unlink($this->deduplicationStore); + } +}); + +test('deduplicates records with same signature', function () { + $record1 = new LogRecord( + datetime: new \DateTimeImmutable(), + channel: 'test', + level: Level::Error, + message: 'Test message', + context: ['foo' => 'bar'], + extra: [], + ); + + // First record should be handled + $this->handler->handle($record1); + $this->handler->flush(); + expect(file_exists($this->deduplicationStore))->toBeTrue(); + + // Same signature should be deduplicated + $record2 = new LogRecord( + datetime: new \DateTimeImmutable(), + channel: 'different-channel', + level: Level::Error, + message: 'Test message', + context: ['foo' => 'bar'], + extra: ['something' => 'else'], + ); + $this->handler->handle($record2); + $this->handler->flush(); + + // Different signature should not be deduplicated + $record3 = new LogRecord( + datetime: new \DateTimeImmutable(), + channel: 'test', + level: Level::Error, + message: 'Different message', + context: ['foo' => 'bar'], + extra: [], + ); + $this->handler->handle($record3); + + // Verify deduplication store contains both unique signatures + $this->handler->close(); + $contents = file_get_contents($this->deduplicationStore); + expect($contents) + ->toContain($this->signatureGenerator->generate($record1)) + ->toContain($this->signatureGenerator->generate($record3)); +}); + +test('deduplication respects time window', function () { + $record = new LogRecord( + datetime: new \DateTimeImmutable(), + channel: 'test', + level: Level::Error, + message: 'Test message', + context: ['foo' => 'bar'], + extra: [], + ); + + // Create handler with 1 second time window + $handler = new SignatureDeduplicationHandler( + new NullHandler(), + $this->deduplicationStore, + Level::Debug, + time: 1, + signatureGenerator: $this->signatureGenerator + ); + + // First record should be handled + $handler->handle($record); + $handler->flush(); + expect(file_exists($this->deduplicationStore))->toBeTrue(); + + // Wait for time window to expire + sleep(2); + + // Same record should be handled again after time window + $handler->handle($record); + $handler->flush(); + $handler->close(); + + // Verify deduplication store contains only the most recent entry + $contents = file_get_contents($this->deduplicationStore); + $signature = $this->signatureGenerator->generate($record); + expect(substr_count($contents, $signature))->toBe(1); +});